Git development
 help / color / mirror / Atom feed
* Re: git-svn with non-standard repository layout
From: Piotr Krukowiecki @ 2012-12-05 22:19 UTC (permalink / raw)
  To: Stephen Bash; +Cc: Git Mailing List, Carsten Fuchs
In-Reply-To: <1056577717.281976.1354725880396.JavaMail.root@genarts.com>

On Wed, Dec 5, 2012 at 5:44 PM, Stephen Bash <bash@genarts.com> wrote:
> ----- Original Message -----
>> From: "Piotr Krukowiecki" <piotr.krukowiecki@gmail.com>
>> Sent: Wednesday, December 5, 2012 11:26:54 AM
>> Subject: Re: git-svn with non-standard repository layout
>>
>> On Tue, Dec 4, 2012 at 10:19 PM, Carsten Fuchs
>> <carsten.fuchs@cafu.de> wrote:
>> > Hi Piotr,
>> >
>> > Am 2012-12-04 18:29, schrieb Piotr Krukowiecki:
>> >
>> >> Is there a way to handle svn repository with following layout?
>> >>
>> >> repo/trunk
>> >> repo/branches/branch1
>> >> repo/branches/branch2
>> >> repo/branches/work/developer1/branch3
>> >> repo/branches/work/developer1/branch4
>> >> repo/branches/work/developer2/branch5
>> >
>> > see my post at
>> >     http://www.cafu.de/forum/viewtopic.php?f=14&t=1092
>> > heading "Branches outside branches/".
>> >
>> > You may need something like
>> >     git config --add svn-remote.svn.fetch
>> > "path.../branchX:refs/remotes/branchX"
>> > for each of your branches.
>>
>> that works :)
>>
>> Although not an ideal solution - I have to manually configure all
>> branches + update them as they are created
>
> It's not a 100% solution, but you can use a limited glob-like syntax in the branches and tags lines of the svn-remote config block.  You still need to do some manual work (one entry for each developer), but the wildcards eliminate a lot of the grunt work as individual branches are created.  See the very end of the git-svn manpage for examples (section titled CONFIGURATION).  I use that technique to track a subdirectory of the Slimdevices SVN repo [1], which has a similarly complex layout:
>
> repo/7.1/trunk
> repo/7.1/branches/branchA
> repo/7.1/branches/branchB
> repo/7.1/tags/tag1
> repo/7.2/trunk
> repo/7.2/branches/branchC

Do you mean something like

   branches = branches/work/*/*:refs/remotes/work/*
   branches = branches/{branch1,branch2}:refs/remotes/branches/*

instead of (currently used)

   branches = branches/work/*/*:refs/remotes/work/*
   fetch = branches/branch1:refs/remotes/branches/branch1
   fetch = branches/branch2:refs/remotes/branches/branch2

I will try that tomorrow.

BTW what's the difference between "fetch" and "branches" keys? I could
only find one: "fetch" does not support glob arguments and "branches"
do.

--
Piotr Krukowiecki

^ permalink raw reply

* [PATCH] RFC Optionally handle symbolic links as copies
From: Robin Rosenberg @ 2012-12-05 22:46 UTC (permalink / raw)
  To: git; +Cc: Robin Rosenberg

If core.symlinks is set to copy then symbolic links in a git repository
will be checked out as copies of the file it points to. This allows repos
containing symbolic links to not only be checked out, but also that the
linked content may be used on OS:es and filesystems that do not support
symbolic links.

Plain files will be copied as hard links, directories will be replicated
with files as hard links. Stale links will not be copied and will appear
as missing files.

A git-ln utility whose usage is similar to the standard ln utilty. It will
create symbolic links unless core.symlinks is set to copy.

This patch still contains debug statements and open issues:
- git rm link-to-dir - remove without force or compare
- git status - when do we report a diff. 
	- After checkout we should probably not
	- if the "linked" files change?
	- if a change in the copied directory chsnges
	- if a file in the copied diretory is added/removed
	- update, should we update the copied structure automatically
	  when the link target changes
- git add - just ignore the stat diff for links
- git xxx -- gotchas
- ugly code
- debug statements
- few test cases

---
 .gitignore                          |   1 +
 Makefile                            |  10 +-
 builtin/checkout-index.c            |   1 +
 builtin/checkout.c                  |   2 +
 builtin/rm.c                        |  17 ++-
 cache.h                             |   3 +
 command-list.txt                    |   1 +
 config.c                            |   8 +-
 entry.c                             | 238 +++++++++++++++++++++++++++++++++++-
 environment.c                       |   1 +
 git-ln.sh                           | 132 ++++++++++++++++++++
 read-cache.c                        |  60 +++++----
 t/t2401-checkout-symlink-as-copy.sh | 142 +++++++++++++++++++++
 unpack-trees.c                      |  11 ++
 wrapper.c                           |  19 ++-
 15 files changed, 611 insertions(+), 35 deletions(-)
 create mode 100755 git-ln.sh
 create mode 100755 t/t2401-checkout-symlink-as-copy.sh

diff --git a/.gitignore b/.gitignore
index 5778ce1..0c82778 100644
--- a/.gitignore
+++ b/.gitignore
@@ -72,6 +72,7 @@
 /git-init
 /git-init-db
 /git-instaweb
+/git-ln
 /git-log
 /git-lost-found
 /git-ls-files
diff --git a/Makefile b/Makefile
index 1cff01e..3660597 100644
--- a/Makefile
+++ b/Makefile
@@ -341,10 +341,15 @@ endif
 
 # CFLAGS and LDFLAGS are for the users to override from the command line.
 
-CFLAGS = -g -O2 -Wall -Werror \
+CFLAGS = -g -Wall -Werror \
 	-Wno-pointer-to-int-cast \
 	-Wold-style-definition \
-	-Wdeclaration-after-statement
+	-Wdeclaration-after-statement \
+	-Wno-deprecated-declarations \
+	-Wstrict-prototypes \
+	-Wdeclaration-after-statement \
+	-Wno-pointer-to-int-cast \
+
 LDFLAGS =
 ALL_CFLAGS = $(CPPFLAGS) $(CFLAGS)
 ALL_LDFLAGS = $(LDFLAGS)
@@ -442,6 +447,7 @@ SCRIPT_SH += git-bisect.sh
 SCRIPT_SH += git-difftool--helper.sh
 SCRIPT_SH += git-filter-branch.sh
 SCRIPT_SH += git-lost-found.sh
+SCRIPT_SH += git-ln.sh
 SCRIPT_SH += git-merge-octopus.sh
 SCRIPT_SH += git-merge-one-file.sh
 SCRIPT_SH += git-merge-resolve.sh
diff --git a/builtin/checkout-index.c b/builtin/checkout-index.c
index b1feda7..aa840b7 100644
--- a/builtin/checkout-index.c
+++ b/builtin/checkout-index.c
@@ -113,6 +113,7 @@ static void checkout_all(const char *prefix, int prefix_length)
 			errs++;
 		last_ce = ce;
 	}
+	checkout_remaining_link_copies();
 	if (last_ce && to_tempfile)
 		write_tempfile_record(last_ce->name, prefix_length);
 	if (errs)
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 781295b..eafd3d5 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -328,6 +328,8 @@ static int checkout_paths(const struct checkout_opts *opts,
 		}
 	}
 
+	checkout_remaining_link_copies();
+
 	if (write_cache(newfd, active_cache, active_nr) ||
 	    commit_locked_index(lock_file))
 		die(_("unable to write new index file"));
diff --git a/builtin/rm.c b/builtin/rm.c
index b384c4c..14fb399 100644
--- a/builtin/rm.c
+++ b/builtin/rm.c
@@ -17,6 +17,8 @@ static const char * const builtin_rm_usage[] = {
 
 static struct {
 	int nr, alloc;
+	int nrmode, allocmode;
+	int *mode;
 	const char **name;
 } list;
 
@@ -174,7 +176,10 @@ int cmd_rm(int argc, const char **argv, const char *prefix)
 		if (!match_pathspec(pathspec, ce->name, ce_namelen(ce), 0, seen))
 			continue;
 		ALLOC_GROW(list.name, list.nr + 1, list.alloc);
-		list.name[list.nr++] = ce->name;
+		ALLOC_GROW(list.mode, list.nrmode + 1, list.allocmode); // ugly
+		list.name[list.nr] = ce->name;
+		list.mode[list.nr] = ce->ce_mode;
+		list.nr++;
 	}
 
 	if (pathspec) {
@@ -245,7 +250,13 @@ int cmd_rm(int argc, const char **argv, const char *prefix)
 		int removed = 0;
 		for (i = 0; i < list.nr; i++) {
 			const char *path = list.name[i];
-			if (!remove_path(path)) {
+			if (S_ISLNK(list.mode[i]) && copy_symlinks) {
+				struct strbuf sb = STRBUF_INIT;
+				strbuf_add(&sb, list.name[i], strlen(list.name[i]));
+				remove_dir_recursively(&sb, 0);
+				strbuf_release(&sb);
+				removed = 1;
+			} else if (!remove_path(path)) {
 				removed = 1;
 				continue;
 			}
diff --git a/cache.h b/cache.h
index fe0388c..125929d 100644
--- a/cache.h
+++ b/cache.h
@@ -532,6 +532,7 @@ extern int trust_executable_bit;
 extern int trust_ctime;
 extern int quote_path_fully;
 extern int has_symlinks;
+extern int copy_symlinks;
 extern int minimum_abbrev, default_abbrev;
 extern int ignore_case;
 extern int assume_unchanged;
@@ -954,6 +955,8 @@ extern int has_dirs_only_path(const char *name, int len, int prefix_len);
 extern void schedule_dir_for_removal(const char *name, int len);
 extern void remove_scheduled_dirs(void);
 
+extern int checkout_remaining_link_copies(void);
+
 extern struct alternate_object_database {
 	struct alternate_object_database *next;
 	char *name;
diff --git a/command-list.txt b/command-list.txt
index 7e8cfec..0e7e147 100644
--- a/command-list.txt
+++ b/command-list.txt
@@ -61,6 +61,7 @@ git-index-pack                          plumbingmanipulators
 git-init                                mainporcelain common
 git-instaweb                            ancillaryinterrogators
 gitk                                    mainporcelain
+git-ln                                  mainporcelain
 git-log                                 mainporcelain common
 git-lost-found                          ancillarymanipulators	deprecated
 git-ls-files                            plumbinginterrogators
diff --git a/config.c b/config.c
index 5ea36a7..e0fd347 100644
--- a/config.c
+++ b/config.c
@@ -576,7 +576,13 @@ static int git_default_core_config(const char *var, const char *value)
 	}
 
 	if (!strcmp(var, "core.symlinks")) {
-		has_symlinks = git_config_bool(var, value);
+		if (0 == strcasecmp("copy", value)) {
+			copy_symlinks = 1;
+			has_symlinks = 1; // TODO: reconsider
+		} else {
+			has_symlinks = git_config_bool(var, value);
+			copy_symlinks = 0;
+		}
 		return 0;
 	}
 
diff --git a/entry.c b/entry.c
index 17a6bcc..b46417a 100644
--- a/entry.c
+++ b/entry.c
@@ -2,6 +2,17 @@
 #include "blob.h"
 #include "dir.h"
 #include "streaming.h"
+#include <string.h>
+#include <stdio.h>
+
+int nnn;
+int xlink(const char *from, const char *to)
+{
+	int ret = link(from,to);
+	printf("#%d,link %s,%s, ret=%d, errno=%d\n", nnn++, from, to, ret, errno);
+	return ret;
+}
+#define link(s,e) xlink(s,e)
 
 static void create_directories(const char *path, int path_len,
 			       const struct checkout *state)
@@ -134,6 +145,209 @@ static int streaming_write_entry(struct cache_entry *ce, char *path,
 	return result;
 }
 
+struct postlink {
+	struct cache_entry *ce;
+	char *from,*to;
+};
+static struct postlink *postlinks = NULL;
+static int npostlinks = 0;
+static int maxpostlinks = 0;
+static int resolved = 0;
+int lasttry = 0;
+
+void add_postlink(struct cache_entry *ce, char *from, char *to)
+{
+	if (npostlinks >= maxpostlinks) {
+		if (maxpostlinks == 0)
+			maxpostlinks = 64;
+		else
+			maxpostlinks *= 2;
+		postlinks = realloc(postlinks, maxpostlinks*sizeof(struct postlink));
+	}
+	postlinks[npostlinks].ce = ce;
+	postlinks[npostlinks].to = xstrdup(to);
+	postlinks[npostlinks].from = xstrdup(from);
+	npostlinks++;
+}
+
+void release_postlink(void)
+{
+	int i;
+	for (i = 0; i < npostlinks; ++i) {
+		free(postlinks[i].from);
+		free(postlinks[i].to);
+	}
+	free(postlinks);
+	postlinks = NULL;
+	npostlinks = 0;
+	maxpostlinks = 0;
+}
+
+static int recursive_link(const char *src, const char *dst) {
+	// printf("recursive_link %s %s\n", src, dst);
+	struct stat buf;
+	struct dirent *dp;
+	struct strbuf dstb = STRBUF_INIT;
+	struct strbuf srcb = STRBUF_INIT;
+	int err = stat(dst, &buf);
+	DIR *d = opendir(src);
+	if (!d)
+		return ENOENT;
+	if (err && errno == ENOENT) {
+		err = mkdir(dst, 0777);
+		if (err)
+			return err;
+		else
+			resolved++;
+	} else if (!(buf.st_mode & S_IFDIR))
+		return EEXIST;
+	err = 0;
+	while ((dp = readdir(d)) != NULL) {
+		if (is_dot_or_dotdot(dp->d_name))
+			continue;
+		strbuf_add(&dstb, dst, strlen(dst));
+		strbuf_add(&dstb, "/", 1);
+		strbuf_add(&dstb, dp->d_name, strlen(dp->d_name));
+		strbuf_add(&srcb, src, strlen(src));
+		strbuf_add(&srcb, "/", 1);
+		strbuf_add(&srcb, dp->d_name, strlen(dp->d_name));
+		if (!stat(srcb.buf, &buf)) {
+			if (buf.st_mode & S_IFDIR) {
+				if (recursive_link(srcb.buf, dstb.buf)) {
+					err = -1;
+				}
+			} else {
+				int r1,r2;
+				r1 = unlink(dstb.buf);
+				if (r1)
+					error("unlink %s = %d, errno=%d\n", dstb.buf, r1, errno);
+				r2 = link(srcb.buf, dstb.buf);
+				if (r2) {
+					if (maxpostlinks >= 0) {
+						error("cannot link %s to %s", srcb.buf, dstb.buf);
+					}
+					err = -1;
+				}
+				if (r1 && !r2)
+					resolved++;
+			}
+		} else {
+			if (maxpostlinks >= 0) {
+				error("cannot link %s to %s", srcb.buf, dstb.buf);
+			}
+			err = -1;
+		}
+		strbuf_reset(&srcb);
+		strbuf_reset(&dstb);
+	}
+	strbuf_release(&dstb);
+	strbuf_release(&srcb);
+	closedir(d);
+	return err;
+}
+
+static void resolvelink(struct strbuf *src, struct strbuf *dst)
+{
+	if (0 == strncmp(src->buf, "../", 3)) {
+		char *p = dst->buf + dst->len - 1;
+		while (p > dst->buf && *p != '/')
+			--p;
+		if (*p != '/')
+			return;
+		--p;
+		while (p > dst->buf && *p != '/')
+			--p;
+		strbuf_remove(src, 0, 3);
+		if (*p == '/')
+			strbuf_insert(src, 0, dst->buf, p - dst->buf + 1);
+		resolvelink(src, dst);
+	} else if (0 == strncmp(src->buf, "./", 2)) {
+		strbuf_remove(src, 0, 2);
+		resolvelink(src, dst);
+	}
+}
+
+static int fakesymlink(const char *old, const char *new)
+{
+	struct stat buf;
+	int st;
+	int err;
+	char dir[MAXPATHLEN];
+	char cdir[MAXPATHLEN];
+	char *dend;
+	struct strbuf src = STRBUF_INIT;
+	struct strbuf dst = STRBUF_INIT;
+	strcpy(dir, new);
+	dend = strrchr(dir, '/');
+	if (dend) {
+		*dend = 0;
+		if (!getcwd(cdir, MAXPATHLEN))
+			return -1;
+	}
+	strbuf_add(&src, old, strlen(old));
+	strbuf_add(&dst, new, strlen(new));
+	resolvelink(&src, &dst);
+	if (0 == (st = stat(src.buf, &buf))) {
+		if (buf.st_mode & (S_IFREG | S_IFLNK)) {
+			int r = unlink(dst.buf);
+			err = link(src.buf, dst.buf);
+			if (!err && !r)
+				resolved++;
+			if (err)
+				error("Failed to link %s %s\n", src.buf, dst.buf);
+		} else if (buf.st_mode & S_IFDIR) {
+			err = recursive_link(src.buf, dst.buf);
+			if (maxpostlinks < 0)
+				err = 1;
+//			printf("%d <- recursive_link\n", err);
+		} else {
+			error("mode:Failed to link %s %s\n", src.buf, dst.buf);
+			err = -1;
+		}
+	} else {
+		error("stat:Failed to link %s %s\n", src.buf, dst.buf);
+		err = -1;
+	}
+	strbuf_release(&src);
+	strbuf_release(&dst);
+	return err;
+}
+
+int checkout_remaining_link_copies(void)
+{
+	int j;
+	int err = 0;
+	maxpostlinks = -1;
+	resolved = 1;
+
+	/* stupid, each iteration should resolve at least one entry */
+	do {
+		if (resolved == 0)
+			lasttry = 1;
+		resolved = 0;
+		for (j = 0; j < npostlinks; ++j) {
+			struct postlink *e = &postlinks[j];
+			if (e->from) {
+				if (0 == fakesymlink(e->from, e->to)) {
+					if (e->ce) {
+						struct stat st;
+						if (0 == stat(e->to, &st))
+							fill_stat_cache_info(e->ce, &st);
+					}
+					free(e->from);
+					e->from = NULL;
+				} else {
+					if (lasttry) {
+						err = 1;
+					}
+				}
+			}
+		}
+	} while (resolved > 0 && !lasttry);
+	release_postlink();
+	return err;
+}
+
 static int write_entry(struct cache_entry *ce, char *path, const struct checkout *state, int to_tempfile)
 {
 	unsigned int ce_mode_s_ifmt = ce->ce_mode & S_IFMT;
@@ -161,8 +375,16 @@ static int write_entry(struct cache_entry *ce, char *path, const struct checkout
 			return error("unable to read sha1 file of %s (%s)",
 				path, sha1_to_hex(ce->sha1));
 
-		if (ce_mode_s_ifmt == S_IFLNK && has_symlinks && !to_tempfile) {
-			ret = symlink(new, path);
+		if (ce_mode_s_ifmt == S_IFLNK && (has_symlinks || copy_symlinks) && !to_tempfile) {
+			if (copy_symlinks) {
+				ret = fakesymlink(new, path);
+				if (ret < 0) {
+					// Could not copy now, assume we can do it later
+					add_postlink(ce, new, path);
+					ret = 0;
+				}
+			} else
+				ret = symlink(new, path);
 			free(new);
 			if (ret)
 				return error("unable to create symlink %s (%s)",
@@ -267,8 +489,16 @@ int checkout_entry(struct cache_entry *ce, const struct checkout *state, char *t
 			if (!state->force)
 				return error("%s is a directory", path);
 			remove_subtree(path);
-		} else if (unlink(path))
-			return error("unable to unlink old '%s' (%s)", path, strerror(errno));
+		} else {
+			if (unlink(path)) {
+//				printf("has_symlinks=%d, copy_symlinks=%d\n", has_symlinks, copy_symlinks);
+				if (copy_symlinks) {
+					remove_subtree(path);
+				} else {
+					return error("unable to unlink old '%s' (%s)", path, strerror(errno));
+				}
+			}
+		}
 	} else if (state->not_new)
 		return 0;
 	create_directories(path, len, state);
diff --git a/environment.c b/environment.c
index 71e438c..a1328ec 100644
--- a/environment.c
+++ b/environment.c
@@ -14,6 +14,7 @@
 int trust_executable_bit = 1;
 int trust_ctime = 1;
 int has_symlinks = 1;
+int copy_symlinks = 0;
 int minimum_abbrev = 4, default_abbrev = 7;
 int ignore_case;
 int assume_unchanged;
diff --git a/git-ln.sh b/git-ln.sh
new file mode 100755
index 0000000..d8df14f
--- /dev/null
+++ b/git-ln.sh
@@ -0,0 +1,132 @@
+#!/bin/sh
+[[ -n $LX ]] && set -x
+#
+# Copyright (C) 2012 Robin Rosenberg
+# Helper for adding symbolic links when file system does not support it
+#
+
+SUBDIRECTORY_OK=Yes
+OPTIONS_KEEPDASHDASH=
+OPTIONS_SPEC="\
+git ln -s [options] target linkname
+--
+f,force	force creating when linkname or target does not exist
+s,symbolic create symbolic link (mandatory)
+cached operate only on index
+"
+#usage() {
+	#	echo >&2 "usage: ln [-fs] target name"
+	#	exit 1
+#}
+
+symbolic=
+force=
+cached=
+copy=
+symlinks=$(git config --get core.symlinks|tr A-Z a-z)
+if [[ $symlinks == copy ]];then copy=1;fi
+
+. git-sh-setup
+. git-sh-i18n
+
+require_work_tree_exists
+if [ $? != 0 ]; then
+	usage
+fi
+while test $# != 0;do
+	case "$1" in
+		-f|--force)
+			force=1
+			shift
+			;;
+		-s|--symbolic)
+			symbolic=1
+			shift
+			;;
+		--cached)
+			cached=1
+			shift
+			;;
+		--)
+			if [[ $# == 3 ]];
+			then
+				target=$2
+				name=$3
+				shift 3
+				break
+			else
+				usage
+				exit 1
+			fi
+			;;
+		*)
+			usage
+			exit 1
+			;;
+		esac
+done
+if [[ -z $symbolic ]];then
+	usage
+fi
+GIT_DIR=$(git rev-parse --git-dir) || exit 1
+cwd=$(pwd)
+wd=$( (cd_to_toplevel && pwd) )
+namedir=$(dirname "$name")
+namebase=$(basename "$name")
+absnamedir=$( (cd "$namedir" && pwd) )
+cd "$namedir" || exit 1
+pwd
+if [[ ${absnamedir:0:${#wd}} != $wd ]]
+then
+	echo >&2 git-ln: $name is outside of working tree
+	exit 1
+fi
+reldir=${absnamedir:${#wd}}
+reldir=${reldir:1}
+if [[ -n $reldir ]];then
+	reldir="$reldir/"
+fi
+if [[ ! -e $target && ! $force ]];then
+	echo >&2 git-ln: $target does not exist
+	exit 1
+fi
+
+if [[ -n $force ]];then
+	if [[ -z $cached ]];then
+		if [[ -d $name && -n $copy ]];then
+			rm -rf "$name"
+		elif [[ ! -d $name ]];then
+			rm -f "$name"
+		fi
+	fi
+	git rm -f --cached "$name" >/dev/null || exit 1
+fi
+sha1=$(printf %s "$target" | git hash-object -w -t blob --stdin)
+if [ $? != 0 ];then exit 1;fi
+git update-index --add --cacheinfo 120000 $sha1 $reldir$name || exit $?
+if [[ -n $copy ]];then
+	cd "$cwd" || exit $?
+	if [[ -d $target ]];
+	then
+		(cd "$target" &&
+		find . -type d)|
+		while read d;do
+			mkdir -p "$name/$d" || exit $?
+		done
+		(cd "$target" &&
+		find . -type f)|
+		while read f;do
+			ln "$target/$f" "$name/$f" || exit $?
+		done
+	else
+		ln "$target" "$name" || exit $?
+	fi
+else
+	if [[ -e $name ]];then
+		echo >&2 git-ln: File $name exists
+		exit 1
+	fi
+	cd "$cwd" || exit $?
+	ln -s $target $name || exit $?
+fi
+exit 0
diff --git a/read-cache.c b/read-cache.c
index fda78bc..9ca8e42 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -181,8 +181,18 @@ static int ce_match_stat_basic(struct cache_entry *ce, struct stat *st)
 			changed |= MODE_CHANGED;
 		break;
 	case S_IFLNK:
+		//	ISLNK(st)	hassym	ISREG(st)	ISDIR(st)	copy	->
+		//	y			y		y								changed (old)
+		//	y			n		y								changed (old)
+		//	y			y		n								changed (old)
+		//  =			=		=			y			y		no
+		//  =			=		=			n			n		changed
+		//  =			=		=			y			n		changed
+		//  =			=		=			n			y		changed
+
 		if (!S_ISLNK(st->st_mode) &&
-		    (has_symlinks || !S_ISREG(st->st_mode)))
+		    (has_symlinks || !S_ISREG(st->st_mode))
+		    && !S_ISDIR(st->st_mode) && !copy_symlinks)
 			changed |= TYPE_CHANGED;
 		break;
 	case S_IFGITLINK:
@@ -195,36 +205,38 @@ static int ce_match_stat_basic(struct cache_entry *ce, struct stat *st)
 	default:
 		die("internal error: ce_mode is %o", ce->ce_mode);
 	}
-	if (ce->ce_mtime.sec != (unsigned int)st->st_mtime)
-		changed |= MTIME_CHANGED;
-	if (trust_ctime && ce->ce_ctime.sec != (unsigned int)st->st_ctime)
-		changed |= CTIME_CHANGED;
-
+	/* For copy as symlink we just ignore the lowlevel stat fields */
+	if (!copy_symlinks || !S_ISLNK(ce->ce_mode) || !S_ISDIR(st->st_mode)) {
+		if (ce->ce_mtime.sec != (unsigned int)st->st_mtime)
+			changed |= MTIME_CHANGED;
+		if (trust_ctime && ce->ce_ctime.sec != (unsigned int)st->st_ctime)
+			changed |= CTIME_CHANGED;
 #ifdef USE_NSEC
-	if (ce->ce_mtime.nsec != ST_MTIME_NSEC(*st))
-		changed |= MTIME_CHANGED;
-	if (trust_ctime && ce->ce_ctime.nsec != ST_CTIME_NSEC(*st))
-		changed |= CTIME_CHANGED;
+		if (ce->ce_mtime.nsec != ST_MTIME_NSEC(*st))
+			changed |= MTIME_CHANGED;
+		if (trust_ctime && ce->ce_ctime.nsec != ST_CTIME_NSEC(*st))
+			changed |= CTIME_CHANGED;
 #endif
 
-	if (ce->ce_uid != (unsigned int) st->st_uid ||
-	    ce->ce_gid != (unsigned int) st->st_gid)
-		changed |= OWNER_CHANGED;
-	if (ce->ce_ino != (unsigned int) st->st_ino)
-		changed |= INODE_CHANGED;
+		if (ce->ce_uid != (unsigned int) st->st_uid ||
+			ce->ce_gid != (unsigned int) st->st_gid)
+			changed |= OWNER_CHANGED;
+		if (ce->ce_ino != (unsigned int) st->st_ino)
+			changed |= INODE_CHANGED;
 
 #ifdef USE_STDEV
-	/*
-	 * st_dev breaks on network filesystems where different
-	 * clients will have different views of what "device"
-	 * the filesystem is on
-	 */
-	if (ce->ce_dev != (unsigned int) st->st_dev)
-		changed |= INODE_CHANGED;
+		/*
+		 * st_dev breaks on network filesystems where different
+		 * clients will have different views of what "device"
+		 * the filesystem is on
+		 */
+		if (ce->ce_dev != (unsigned int) st->st_dev)
+			changed |= INODE_CHANGED;
 #endif
 
-	if (ce->ce_size != (unsigned int) st->st_size)
-		changed |= DATA_CHANGED;
+		if (ce->ce_size != (unsigned int) st->st_size)
+			changed |= DATA_CHANGED;
+	}
 
 	/* Racily smudged entry? */
 	if (!ce->ce_size) {
diff --git a/t/t2401-checkout-symlink-as-copy.sh b/t/t2401-checkout-symlink-as-copy.sh
new file mode 100755
index 0000000..922af9d
--- /dev/null
+++ b/t/t2401-checkout-symlink-as-copy.sh
@@ -0,0 +1,142 @@
+#!/bin/sh
+#
+# Copyright (c) 2012 Robin Rosenberg
+
+test_description='git checkout and reset with symlinks as copy'
+
+. ./test-lib.sh
+
+fullfilelist="a
+linkd
+linkd/reald2
+linkd/reald2/zlink2
+linkd/reald2/zlink3
+linkd/realfile
+linkd/zlink
+linkfile
+reald
+reald/reald2
+reald/reald2/zlink2
+reald/reald2/zlink3
+reald/realfile
+reald/zlink
+text
+z"
+
+fullfilelist_except_linkd="a
+linkfile
+reald
+reald/reald2
+reald/reald2/zlink2
+reald/reald2/zlink3
+reald/realfile
+reald/zlink
+text
+z"
+
+fulllsfiles="a
+linkd
+linkfile
+reald/reald2/zlink2
+reald/reald2/zlink3
+reald/realfile
+reald/zlink
+text
+z"
+
+fulllsfiles_except_linkd="a
+linkfile
+reald/reald2/zlink2
+reald/reald2/zlink3
+reald/realfile
+reald/zlink
+text
+z"
+
+fulllstree="100644 blob 1269488f7fb1f4b56a8c0e5eb48cecbfadfa9219	a
+120000 blob afb748c1b973ba508f014b969b193f5370060583	linkd
+120000 blob 9a5d4d6a54a108d78fd356d95e15612ace0fc7ed	linkfile
+120000 blob bf219293900231f5f9fdb67fa004f2c83bec1635	reald/reald2/zlink2
+120000 blob a240f4229e24dde99ca0293d47d62906907e749c	reald/reald2/zlink3
+100644 blob d95f3ad14dee633a758d2e331151e950dd13e4ed	reald/realfile
+120000 blob 1856dae911a6b4e1a06f19218f97d94d2b1e5a96	reald/zlink
+120000 blob ae1910dff1c8f7f779f2154de8f3902702b90e8b	text
+120000 blob 2e65efe2a145dda7ee51d1741299f848e5bf752e	z"
+
+test_complete_master_state() {
+	test "$(find -L * -print)" = "$fullfilelist" &&
+	test "$(git ls-tree -r HEAD)" = "$fulllstree"
+}
+
+reset_master() {
+	rm -rf * &&
+	git reset --hard &&
+	test_complete_master_state
+}
+
+# setup work with or without real symlink support,
+# by default we use real symlink support
+test_expect_success setup '
+	git commit --allow-empty -m "empty" &&
+	git branch empty &&
+	echo >a data &&
+	git add a &&
+	git ln -s a z &&
+	mkdir reald &&
+	mkdir reald/reald2 &&
+	echo >../textf external_file &&
+	git ln -s ../textf text &&
+	echo >reald/realfile content &&
+	git add reald/realfile &&
+	git ln -s reald/realfile linkfile &&
+	(cd reald/reald2 && git ln -s ../realfile zlink2) &&
+	(cd reald/reald2 && git ln -s ../../z zlink3) &&
+	(cd reald && git ln -s ../z zlink) &&
+	git ln -s reald linkd &&
+	git commit -m "repo with symlinks" &&
+
+	test_complete_master_state &&
+
+	git config core.symlinks copy
+'
+
+test_expect_success 'reset --hard' '
+	rm -rf * &&
+	test "$(find * -print)" = "" &&
+
+	git reset --hard &&
+	test "$(find * -print )" = "$fullfilelist"
+'
+
+test_expect_success 'checkout -f empty' '
+	rm -rf * &&
+	git checkout -f empty &&
+	test "$(find * -print)" = ""
+'
+
+test_expect_success 'checkout -f master' '
+	rm -rf * &&
+	git checkout -f master &&
+	test_complete_master_state
+'
+
+test_expect_success 'rm --cached, i.e. link only ' '
+	git rm --cached linkd &&
+	test "$(find * -print)" = "$fullfilelist" &&
+	test "$(git ls-files)" = "$fulllsfiles_except_linkd"
+'
+
+test_expect_success 'rm copy of linked directory' '
+	reset_master &&
+	git rm linkd &&
+	test "$(find * -print)" = "$fullfilelist_except_linkd" &&
+	test "$(git ls-files)" = "$fulllsfiles_except_linkd"
+'
+
+test_expect_success 'rm without -f for a regular tree should NOT work' '
+	reset_master &&
+	! git rm reald &&
+	test_complete_master_state
+'
+
+test_done
diff --git a/unpack-trees.c b/unpack-trees.c
index 6d96366..bb2214d 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -229,6 +229,7 @@ static int check_updates(struct unpack_trees_options *o)
 			}
 		}
 	}
+	errs |= checkout_remaining_link_copies();
 	stop_progress(&progress);
 	if (o->update)
 		git_attr_set_direction(GIT_ATTR_CHECKIN, NULL);
@@ -1226,6 +1227,16 @@ static int verify_uptodate_1(struct cache_entry *ce,
 		 */
 		if (S_ISGITLINK(ce->ce_mode))
 			return 0;
+//
+//		if (copy_symlinks) {
+//			printf("%s, %0x A=%d, B=%d, c=%d\n", ce->name, st.st_mode, S_ISLNK(ce->ce_mode), (ce->ce_flags&CE_VALID), S_ISDIR(st.st_mode));
+//			if (S_ISLNK(ce->ce_mode) /*&& (ce->ce_flags&CE_VALID)*/ && S_ISDIR(st.st_mode))
+//				return 0;
+//			printf("%s, %0x A=%d, B=%d, c=%d\n", ce->name, st.st_mode, S_ISLNK(ce->ce_mode), (ce->ce_flags&CE_VALID), S_ISREG(st.st_mode));
+//			if (S_ISLNK(ce->ce_mode) /*&& (ce->ce_flags&CE_VALID)*/ && S_ISREG(st.st_mode))
+//				return 0;
+//
+//		}
 		errno = 0;
 	}
 	if (errno == ENOENT)
diff --git a/wrapper.c b/wrapper.c
index 68739aa..2a1a483 100644
--- a/wrapper.c
+++ b/wrapper.c
@@ -2,6 +2,7 @@
  * Various trivial helper wrappers around standard functions
  */
 #include "cache.h"
+#include "dir.h"
 
 static void do_nothing(size_t size)
 {
@@ -400,7 +401,23 @@ int rmdir_or_warn(const char *file)
 
 int remove_or_warn(unsigned int mode, const char *file)
 {
-	return S_ISGITLINK(mode) ? rmdir_or_warn(file) : unlink_or_warn(file);
+	if (S_ISGITLINK(mode))
+		return rmdir_or_warn(file);
+	if (S_ISLNK(mode) && copy_symlinks) {
+		struct stat sb;
+		if (!stat(file, &sb)) {
+			if (S_IFDIR & sb.st_mode) {
+				struct strbuf dir = STRBUF_INIT;
+				strbuf_add(&dir, file, strlen(file));
+				return remove_dir_recursively(&dir, 0);
+			} else {
+				return unlink_or_warn(file);
+			}
+		} else {
+			perror("oops");
+		}
+	}
+	return unlink_or_warn(file);
 }
 
 void warn_on_inaccessible(const char *path)
-- 
1.8.0.msysgit.0.3.gc00b80c.dirty

^ permalink raw reply related

* Re: [PATCH] Add directory pattern matching to attributes
From: Junio C Hamano @ 2012-12-05 23:35 UTC (permalink / raw)
  To: Jean-Noël AVILA; +Cc: git
In-Reply-To: <201212052310.30690.jn.avila@free.fr>

"Jean-Noël AVILA" <jn.avila@free.fr> writes:

> -static void prepare_attr_stack(const char *path)
> +static void prepare_attr_stack(const char *path, unsigned mode)
>  {
>  	struct attr_stack *elem, *info;
>  	int dirlen, len;
> @@ -645,28 +645,43 @@ static void prepare_attr_stack(const char *path)
>  }

Why?

The new "mode" parameter does not seem to be used in this function
at all.

>  static int path_matches(const char *pathname, int pathlen,
> -			const char *pattern,
> +			const unsigned mode, char *pattern,
>  			const char *base, int baselen)
>  {
> -	if (!strchr(pattern, '/')) {
> +	size_t len;
> +	char buf[PATH_MAX];
> +	char * lpattern = buf;
> +	len = strlen(pattern);
> +	if (PATH_MAX <= len)
> +		return 0;
> +	strncpy(buf,pattern,len);
> +	buf[len] ='\0';
> +	if (len && lpattern[len - 1] == '/') {
> +		if (S_ISDIR(mode))
> +			lpattern[len - 1] = '\0';
> +		else
> +			return 0;
> +	}
> +	if (!strchr(lpattern, '/')) {
>  		/* match basename */
>  		const char *basename = strrchr(pathname, '/');
>  		basename = basename ? basename + 1 : pathname;
> -		return (fnmatch_icase(pattern, basename, 0) == 0);
> +		return (fnmatch_icase(lpattern, basename, 0) == 0);
>  	}
>  	/*
>  	 * match with FNM_PATHNAME; the pattern has base implicitly
>  	 * in front of it.
>  	 */
> -	if (*pattern == '/')
> -		pattern++;
> +	if (*lpattern == '/')
> +		lpattern++;
>  	if (pathlen < baselen ||
>  	    (baselen && pathname[baselen] != '/') ||
>  	    strncmp(pathname, base, baselen))
>  		return 0;
>  	if (baselen != 0)
>  		baselen++;
> -	return fnmatch_icase(pattern, pathname + baselen, FNM_PATHNAME) == 0;
> +	return fnmatch_icase(lpattern, pathname + baselen, FNM_PATHNAME) == 0;
>  }

It appears to me that you are forcing the caller to tell this
function if the path is a directory, but in the attribute system,
the caller does not necessarily know if the path it is passing is
meant to be a directory or a regular file.  "check-attr" is meant to
be usable against a path that does not even exist on the working
tree, so using stat() or lstat() in it is not a solution.  In other
words, it is unfair (read: unworkable) to force it to append a
trailing slash after the path it calls this function with.

If you are interested in export-subst, all is not lost, though:

	$ git init
        $ mkdir a
        $ >a/b
        $ echo a export-ignore >.gitattributes
        $ git add a/b .gitattributes
	$ git commit -m initial
        $ git archive HEAD | tar tf -
        .gitattributes
        $ exit

You could change the "echo" to

	$ echo "a/*" export-ignore >.gitattributes

as well, but it seems to create an useless empty directory "a/" in
the output, which I think is an unrelated bug in "git archive".

This patch seems to be based on a stale codebase.  I do not think
I'd be opposed to change the sementics to allow the callers that
know that a path is a directory to optionally pass mode parameter by
ending the pathname with slash (in other words, have "git
check-attr" ask about a directory 'a' by saying "git check-attr
export-subst a/", and lose the "mode" argument in this patch), or
keep the "mode" parameter and instead allow "git check-attr" to ask
about a directory that does not exist in the working tree by a more
explicit "git check-attr --directory export-ignore a" or something.
Such an enhancement should be done on top of the current codebase.

Thanks.

^ permalink raw reply

* Re: [PATCH] Perform minimal stat comparison when some stat fields are not set
From: Junio C Hamano @ 2012-12-05 23:43 UTC (permalink / raw)
  To: Robin Rosenberg; +Cc: git, spearce
In-Reply-To: <1354742425-71417-1-git-send-email-robin.rosenberg@dewire.com>

Robin Rosenberg <robin.rosenberg@dewire.com> writes:

> At least JGit does sets uid, gid, ctime, ino and dev fields to zero
> on update. To Git this looks like the stat data does not match and
> a full file compare will be forced even it size and mtime match. This
> is in practice unnecessary. Sense JGit's presence by checking if ino
> and dev is zero.
>
> Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
> ---
>  read-cache.c | 21 +++++++++++++--------
>  1 file changed, 13 insertions(+), 8 deletions(-)
>
> diff --git a/read-cache.c b/read-cache.c
> index fda78bc..6f13a22 100644
> --- a/read-cache.c
> +++ b/read-cache.c
> @@ -197,21 +197,26 @@ static int ce_match_stat_basic(struct cache_entry *ce, struct stat *st)
>  	}
>  	if (ce->ce_mtime.sec != (unsigned int)st->st_mtime)
>  		changed |= MTIME_CHANGED;
> -	if (trust_ctime && ce->ce_ctime.sec != (unsigned int)st->st_ctime)
> +
> +	int minimal_stat = (ce->ce_ino == 0 && ce->ce_dev == 0);

decl-after-stmt.

Besides, is it sane to do this unconditionally to affect people who
do not use JGit?

^ permalink raw reply

* Re: Millisecond precision in timestamps?
From: Robin Rosenberg @ 2012-12-05 23:37 UTC (permalink / raw)
  To: David Aguilar
  Cc: Junio C Hamano, Jeff King, Shawn Pearce, Felipe Contreras, git,
	esr
In-Reply-To: <CAJDDKr6n2KSZz5zPHeWiYHAP7Zr02Ti-e24AX1yR_XAXAKhscg@mail.gmail.com>



----- Ursprungligt meddelande -----
> On Tue, Nov 27, 2012 at 11:58 PM, Eric S. Raymond <esr@thyrsus.com>
> wrote:
> > Junio C Hamano <gitster@pobox.com>:
> >> Roundtrip conversions may benefit from sub-second timestamps, but
> >> personally I think negative timestamps are more interesting and of
> >> practical use.
> >
> > You mean, as in times before the Unix epoch 1970-01-01T00:00:00Z?
> >
> > Interesting.  I hadn't thought of that.  I've never seen a software
> > project under version control with bits that old, which is
> > significant
> > because I've probably done more digging into ancient software than
> > anybody other than a specialist historian or two.
> 
> One example I've heard is someone wanting to throw the history
> of a country's laws into git so they can diff them.

Not sure any laws were passed on Feb 30th 1712 in sweden, but perhaps
you can define new time zones to handle that, but I doubt it is practically
doable when you get to countries and regions with less precise boundaries.

Seconds-since as a representation for dates is a dangerous and very
messy game. Java gets it wrong somewhere in 1910 and my guess is others
get it wrong too. There is change in time zones which triggers the bug.

-- robin

^ permalink raw reply

* Re: [PATCH] RFC Optionally handle symbolic links as copies
From: Junio C Hamano @ 2012-12-05 23:51 UTC (permalink / raw)
  To: Robin Rosenberg; +Cc: git
In-Reply-To: <1354747575-89277-1-git-send-email-robin.rosenberg@dewire.com>

Robin Rosenberg <robin.rosenberg@dewire.com> writes:

> If core.symlinks is set to copy then symbolic links in a git repository
> will be checked out as copies of the file it points to.

That all sounds nice on surface when the primary thing you care
about is to fetch and check out other people's code and extract it
to the working tree, but how well would that work on the checkin
side?  What happens if I check out a symlink that points at a file
(either in-tree or out-of-tree), make some changes that do not
involve the symlink, and before I make the commit, an unrelated
change is made to the file the symlink is pointing at?

> - git status - when do we report a diff. 
> 	- After checkout we should probably not
> 	- if the "linked" files change?

Yeah, exactly.

> 	- if a change in the copied directory chsnges

That, too.

> 	- if a file in the copied diretory is added/removed
> 	- update, should we update the copied structure automatically
> 	  when the link target changes

I personally do not think this is worth it.  It would be very useful
on the export/checkout side, so it may make sense to add it to "git
archive", though.

^ permalink raw reply

* Re: git-svn with non-standard repository layout
From: Stephen Bash @ 2012-12-05 23:54 UTC (permalink / raw)
  To: Piotr Krukowiecki; +Cc: Git Mailing List, Carsten Fuchs
In-Reply-To: <CAA01CsoS6xA-tGPw81tYmi1ETU8sQ08+oyHGg5ou1VGYrwd_SQ@mail.gmail.com>

----- Original Message -----
> From: "Piotr Krukowiecki" <piotr.krukowiecki@gmail.com>
> Sent: Wednesday, December 5, 2012 5:19:44 PM
> Subject: Re: git-svn with non-standard repository layout
> 
> Do you mean something like
> 
>    branches = branches/work/*/*:refs/remotes/work/*
>    branches = branches/{branch1,branch2}:refs/remotes/branches/*
> 
> instead of (currently used)
> 
>    branches = branches/work/*/*:refs/remotes/work/*
>    fetch = branches/branch1:refs/remotes/branches/branch1
>    fetch = branches/branch2:refs/remotes/branches/branch2

Essentially yes.  But I guess since you have branches at the same level as the work directory, you either have to add to the glob for each new branch or add another fetch line...  Doesn't seem like a big win to me.  Jumping on a tangent, I thought there could only be one wildcard on the left side of the ':' (and the '*' on the right).  If your work/*/* is actually working, that's quite interesting.
 
> BTW what's the difference between "fetch" and "branches" keys? I could
> only find one: "fetch" does not support glob arguments and "branches"
> do.

That's the only difference I've discovered, though someone more familiar with the code might be able to say more.

Here's my config for the Slimdevices repo I mentioned:

[svn-remote "svn"]
	url = http://svn.slimdevices.com/repos/slim

	fetch = trunk/server:refs/remotes/trunk
	fetch = 7.5/trunk/server:refs/remotes/7.5/trunk
	fetch = 7.6/trunk/server:refs/remotes/7.6/trunk
	fetch = 7.7/trunk/server:refs/remotes/7.7/trunk
	fetch = 7.8/trunk/server:refs/remotes/7.8/trunk

	branches = branches/*/server:refs/remotes/pre7/*
	branches = 7.5/branches/*/server:refs/remotes/7.5/*
	branches = 7.6/branches/*/server:refs/remotes/7.6/*
	branches = 7.7/branches/*/server:refs/remotes/7.7/*
	branches = 7.8/branches/*/server:refs/remotes/7.8/*

	tags = 7.5/tags/*/server:refs/remotes/7.5/tags/*
	tags = 7.6/tags/*/server:refs/remotes/7.6/tags/*
	tags = 7.7/tags/*/server:refs/remotes/7.7/tags/*
	tags = 7.8/tags/*/server:refs/remotes/7.8/tags/*

Lots of repetition, but now that I look at it this repo doesn't have the branches/work clash yours does, which simplifies the config.

HTH,
Stephen

^ permalink raw reply

* Re: Stitching histories of several repositories
From: Robin Rosenberg @ 2012-12-06  0:00 UTC (permalink / raw)
  To: Ramkumar Ramachandra; +Cc: Git List
In-Reply-To: <CALkWK0kCQQioCeuwWAAWfKodvOR+w+hB=11MYs5mGviN6Zy5qA@mail.gmail.com>



----- Ursprungligt meddelande -----
> Hi,
> 
> I've written a tool to stitch the first-parent histories of several
> git repositories.  To illustrate, consider that we have a toplevel
> git
> repository inside which the other repositories reside.
> 
[...]
> 
> I'd like to know whether the tool would be useful to a wider
> audience,
> before I polish it and consider submitting it for inclusion in
> contrib/.  I think the tool is especially useful for running bisect
> and tracking bugs that occur in large projects that consist of many
> git repositories.  Will a unified log showing commits in different
> submodules be useful?

I think it is useful. I did something like that creating a super-repo
for performing a bisect over EGit and JGit over a period over a year.
I don't think I restricted myself to the first parent, which was probably
a mistake.

The solution isn't perfect, but provided that is well-known you can
live with that. A problem I found was that many stiches versions weren't
even compilable so one might want to have the option of matching commits
using a window of some sort to identify compilable combinations and have
the ability to use only those for bisect. I'm not sure it's practical though.

-- robin

^ permalink raw reply

* Re: [PATCH] Perform minimal stat comparison when some stat fields are not set
From: Robin Rosenberg @ 2012-12-06  1:09 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, spearce
In-Reply-To: <7vhao0jc98.fsf@alter.siamese.dyndns.org>



----- Ursprungligt meddelande -----
> Robin Rosenberg <robin.rosenberg@dewire.com> writes:
> 
> > At least JGit does sets uid, gid, ctime, ino and dev fields to zero
> > on update. To Git this looks like the stat data does not match and
> > a full file compare will be forced even it size and mtime match.
> > This
> > is in practice unnecessary. Sense JGit's presence by checking if
> > ino
> > and dev is zero.
> >
> > Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
> > ---
> >  read-cache.c | 21 +++++++++++++--------
> >  1 file changed, 13 insertions(+), 8 deletions(-)
> >
> > diff --git a/read-cache.c b/read-cache.c
> > index fda78bc..6f13a22 100644
> > --- a/read-cache.c
> > +++ b/read-cache.c
> > @@ -197,21 +197,26 @@ static int ce_match_stat_basic(struct
> > cache_entry *ce, struct stat *st)
> >  	}
> >  	if (ce->ce_mtime.sec != (unsigned int)st->st_mtime)
> >  		changed |= MTIME_CHANGED;
> > -	if (trust_ctime && ce->ce_ctime.sec != (unsigned
> > int)st->st_ctime)
> > +
> > +	int minimal_stat = (ce->ce_ino == 0 && ce->ce_dev == 0);
> 
> decl-after-stmt.

Ok, btw. Which C version do we adhere to? C99 is quite old by now.

> Besides, is it sane to do this unconditionally to affect people who
> do not use JGit?
> 

Would a config option like core.minstat be better? The name would imply no dynamic detection.

- robin

^ permalink raw reply

* Re: [PATCH] RFC Optionally handle symbolic links as copies
From: Robin Rosenberg @ 2012-12-06  1:23 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vd2yojbw2.fsf@alter.siamese.dyndns.org>



----- Ursprungligt meddelande -----
> Robin Rosenberg <robin.rosenberg@dewire.com> writes:
> 
> > If core.symlinks is set to copy then symbolic links in a git
> > repository
> > will be checked out as copies of the file it points to.
> 
> That all sounds nice on surface when the primary thing you care
> about is to fetch and check out other people's code and extract it
> to the working tree, but how well would that work on the checkin
> side?  What happens if I check out a symlink that points at a file
> (either in-tree or out-of-tree), make some changes that do not
> involve the symlink, and before I make the commit, an unrelated
> change is made to the file the symlink is pointing at?
> 
> > - git status - when do we report a diff.
> > 	- After checkout we should probably not
> > 	- if the "linked" files change?
> 
> Yeah, exactly.
> 
> > 	- if a change in the copied directory chsnges
> 
> That, too.
> 
> > 	- if a file in the copied diretory is added/removed
> > 	- update, should we update the copied structure automatically
> > 	  when the link target changes

Some of the questions have proposals in the includes test script. A 
little more dangerous than having real symlinks ofcourse, but regardless
of what one does with or without copied symlinks one can make mistakes
and I feel letting Git do the copying is way better than having real
copies in the git repository. Another crappy scm which the users are
converting from does this and it works. A difference to git is that
it (ok clearcase) makes all files read-only so there are fewer mays
of making mistakes with the copies.

> I personally do not think this is worth it.  It would be very useful
> on the export/checkout side, so it may make sense to add it to "git
> archive", though.

It makes sense, but it does not solve the problem at hand.

-- robin

^ permalink raw reply

* Please pull l10n updates for 1.8.1 round 2
From: Jiang Xin @ 2012-12-06  2:54 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Trần Ngọc Quân, Git List

Hi, Junio

The following changes since commit f94c3251e1400c3cf349f7f84fea4db66b540113:

  Update draft release notes to 1.8.1 (2012-11-29 13:57:09 -0800)

are available in the git repository at:

  git://github.com/git-l10n/git-po master

for you to fetch changes up to 77cc392d6d60c5d22930174904adae182502b8a9:

  l10n: vi.po: update to git-v1.8.0.1-347-gf94c3 (2012-11-30 13:43:11 +0700)

----------------------------------------------------------------
Jiang Xin (1):
      l10n: Update git.pot (5 new, 1 removed messages)

Tran Ngoc Quan (1):
      l10n: vi.po: update to git-v1.8.0.1-347-gf94c3

 po/git.pot | 46 +++++++++++++++++++++++++++++++--------------
 po/vi.po   | 63 ++++++++++++++++++++++++++++++++++++++++++--------------------
 2 files changed, 75 insertions(+), 34 deletions(-)

--
Jiang Xin

^ permalink raw reply

* Re: [PATCH] RFC Optionally handle symbolic links as copies
From: Johannes Sixt @ 2012-12-06  6:59 UTC (permalink / raw)
  To: Robin Rosenberg; +Cc: git
In-Reply-To: <1354747575-89277-1-git-send-email-robin.rosenberg@dewire.com>

Am 12/5/2012 23:46, schrieb Robin Rosenberg:
> - git status - when do we report a diff. 
> 	- After checkout we should probably not

Are you saying that it should be ignored that the index records a symbolic
link, but the worktree contains a regular file and that the regular file
does not even contain the value of the symbolic link (like it would in the
core.symlinks=false case)?

-- Hannes

^ permalink raw reply

* Re: [PATCH] Perform minimal stat comparison when some stat fields are not set
From: Johannes Sixt @ 2012-12-06  7:21 UTC (permalink / raw)
  To: Robin Rosenberg; +Cc: Junio C Hamano, git, spearce
In-Reply-To: <805916413.19333293.1354756160521.JavaMail.root@dewire.com>

Am 12/6/2012 2:09, schrieb Robin Rosenberg:
>> Robin Rosenberg <robin.rosenberg@dewire.com> writes:
>>> At least JGit does sets uid, gid, ctime, ino and dev fields to zero
>>> on update. To Git this looks like the stat data does not match and
>>> a full file compare will be forced even it size and mtime match.
>>> This
>>> is in practice unnecessary. Sense JGit's presence by checking if
>>> ino
>>> and dev is zero.

Is this meant to better support C git and JGit working on the same repository?

MinGW git sets these two stat fields to zero as well. But we have less of
an interoparability problem between different git implementations in
practice on Windows, I think.

>> Besides, is it sane to do this unconditionally to affect people who
>> do not use JGit?
> 
> Would a config option like core.minstat be better? The name would imply no dynamic detection.

A configuration option is the way to go. We already have core.trustctime,
core.symlinks, core.filemode, core.ignoreCygwinFSTricks.

But your new mode is not "minimal". In some implementations or on some
filesystems, even more bits of stat information could be meaningless
(think of atime, rdev, nlink, uid, gid). Perhaps core.trustdevandino? Or
an enumeration core.ignoreCacheStat=ctime,dev,ino?

-- Hannes

^ permalink raw reply

* Re: Exploiting SHA1's "XOR weakness" allows for faster hash calculation
From: Sebastian Schuberth @ 2012-12-06  8:11 UTC (permalink / raw)
  To: Theodore Ts'o; +Cc: git
In-Reply-To: <20121205172011.GH18885@thunk.org>

On Wed, Dec 5, 2012 at 6:20 PM, Theodore Ts'o <tytso@mit.edu> wrote:

> It's only useful if you are trying to do brute-force password
> cracking, where the password is being hashed in a very specific way.
> (If for example the password was replicated N times in the input
> buffer for SHA-1, instead of keeping the padding constant in the rest
> of theinput buffer, this particular optimization would't apply.)
>
> In any case, it's not at all applicable for general purpose checksum
> calculations, and hence wouldn't apply to git.

Thanks for the explanation.

-- 
Sebastian Schuberth

^ permalink raw reply

* Re: [PATCH] Add directory pattern matching to attributes
From: Jean-Noël Avila @ 2012-12-06  9:02 UTC (permalink / raw)
  To: git
In-Reply-To: <7vlidcjcm9.fsf@alter.siamese.dyndns.org>

On 06/12/2012 00:35, Junio C Hamano wrote:
> "Jean-Noël AVILA" <jn.avila@free.fr> writes:
>
>> -static void prepare_attr_stack(const char *path)
>> +static void prepare_attr_stack(const char *path, unsigned mode)
>>   {
>>   	struct attr_stack *elem, *info;
>>   	int dirlen, len;
>> @@ -645,28 +645,43 @@ static void prepare_attr_stack(const char *path)
>>   }
> Why?
>
> The new "mode" parameter does not seem to be used in this function
> at all.
>
>>   static int path_matches(const char *pathname, int pathlen,
>> -			const char *pattern,
>> +			const unsigned mode, char *pattern,
>>   			const char *base, int baselen)
>>   {
>> -	if (!strchr(pattern, '/')) {
>> +	size_t len;
>> +	char buf[PATH_MAX];
>> +	char * lpattern = buf;
>> +	len = strlen(pattern);
>> +	if (PATH_MAX <= len)
>> +		return 0;
>> +	strncpy(buf,pattern,len);
>> +	buf[len] ='\0';
>> +	if (len && lpattern[len - 1] == '/') {
>> +		if (S_ISDIR(mode))
>> +			lpattern[len - 1] = '\0';
>> +		else
>> +			return 0;
>> +	}
>> +	if (!strchr(lpattern, '/')) {
>>   		/* match basename */
>>   		const char *basename = strrchr(pathname, '/');
>>   		basename = basename ? basename + 1 : pathname;
>> -		return (fnmatch_icase(pattern, basename, 0) == 0);
>> +		return (fnmatch_icase(lpattern, basename, 0) == 0);
>>   	}
>>   	/*
>>   	 * match with FNM_PATHNAME; the pattern has base implicitly
>>   	 * in front of it.
>>   	 */
>> -	if (*pattern == '/')
>> -		pattern++;
>> +	if (*lpattern == '/')
>> +		lpattern++;
>>   	if (pathlen < baselen ||
>>   	    (baselen && pathname[baselen] != '/') ||
>>   	    strncmp(pathname, base, baselen))
>>   		return 0;
>>   	if (baselen != 0)
>>   		baselen++;
>> -	return fnmatch_icase(pattern, pathname + baselen, FNM_PATHNAME) == 0;
>> +	return fnmatch_icase(lpattern, pathname + baselen, FNM_PATHNAME) == 0;
>>   }
> It appears to me that you are forcing the caller to tell this
> function if the path is a directory, but in the attribute system,
> the caller does not necessarily know if the path it is passing is
> meant to be a directory or a regular file.  "check-attr" is meant to
> be usable against a path that does not even exist on the working
> tree, so using stat() or lstat() in it is not a solution.  In other
> words, it is unfair (read: unworkable) to force it to append a
> trailing slash after the path it calls this function with.

Thank you for your comments. Changing the whole attr.h interface header
is definitely not a good option, but at some point, we may need more 
information
on the path to be able to match a path pattern against it.

>
> If you are interested in export-subst, all is not lost, though:
>
> 	$ git init
>          $ mkdir a
>          $ >a/b
>          $ echo a export-ignore >.gitattributes
>          $ git add a/b .gitattributes
> 	$ git commit -m initial
>          $ git archive HEAD | tar tf -
>          .gitattributes
>          $ exit
>
> You could change the "echo" to
>
> 	$ echo "a/*" export-ignore >.gitattributes
>
> as well, but it seems to create an useless empty directory "a/" in
> the output, which I think is an unrelated bug in "git archive".

This is quite different from the pattern matching documented for gitignore.

Moreover,

  $ mkdir -p not-ignored-dir/ignored-dir
  $ echo test >not-ignored-dir/ignored-dir/ignored
  $ echo 'ignored-dir/*' >.gitattributes
  $ git add not-ignored-dir .gitattributes
  $ git commit -m '.'
  $ git archive HEAD | tar tf -
.gitattributes
not-ignored-dir/
not-ignored-dir/ignored-dir/
not-ignored-dir/ignored-dir/ignored


>
> This patch seems to be based on a stale codebase.
Sorry. I thought the patch submissions had to be based on the 'maint' 
branch.

>    I do not think
> I'd be opposed to change the sementics to allow the callers that
> know that a path is a directory to optionally pass mode parameter by
> ending the pathname with slash (in other words, have "git
> check-attr" ask about a directory 'a' by saying "git check-attr
> export-subst a/", and lose the "mode" argument in this patch), or
> keep the "mode" parameter and instead allow "git check-attr" to ask
> about a directory that does not exist in the working tree by a more
> explicit "git check-attr --directory export-ignore a" or something.
> Such an enhancement should be done on top of the current codebase.

OK. I like the idea of proposing a path ending with '/' when it is meant 
to be
directory. This would not change the interface attr.h . I will rework 
with this idea.

Thank you.

^ permalink raw reply

* [PATCH] git-clean: Display more accurate delete messages
From: Zoltan Klinger @ 2012-12-06 10:15 UTC (permalink / raw)
  To: git; +Cc: Zoltan Klinger

Only print out the names of the files and directories that got actually
deleted.

Consider the following repo layout:
  |-- test.git/
        |-- foo/
             |-- bar/
                  |-- bar.txt
             |-- frotz.git/
                  |-- frotz.txt
        |-- tracked_file1
        |-- untracked_file1

Suppose the user issues 'git clean -fd' from the test.git directory.

When -d option is used and untracked directory 'foo' contains a
subdirectory 'frotz.git' that is managed by a different git repository
therefore it will not be removed.

  $ git clean -fd
  Removing foo/
  Removing untracked_file1

The message displayed to the user is slightly misleading. The foo/
directory has not been removed because of foo/frotz.git still exists.
On the other hand the subdirectory bar has been deleted but it's not
mentioned anywhere.

This behaviour is the result of the way the deletion of untracked
directories are reported. In the current implementation they are
deleted recursively but only the name of the top most directory is
printed out. The calling function does not know about any
subdirectories that could not be removed during the recursion.

Improve the way the deleted directories are reported back to
the user:
  (1) Modify the recursive delete function to run in both dry_run and
      delete modes.
  (2) During the recursion collect the name of the files and
      directories that will be or have been removed. Also collect the
      names of files and directories that could not be removed.
  (3) After finishing the deletes print out the names of all deleted
      files and any files or directories that failed to delete.

Consider the output of the improved version:

  $ git clean -fd
  Removed foo/bar/bar.txt
  Removed foo/bar
  Removed untracked_file1

Now it displays only the file and directory names that got actually
deleted.

Signed-off-by: Zoltan Klinger <zoltan.klinger@gmail.com>
---
Hi there,

My first patch. Hope you find it useful.

Looking forward to your feedback.

Cheers,
Zoltan

 builtin/clean.c |   64 +++++++++++++++++++++++++++++++------------------------
 dir.c           |   58 ++++++++++++++++++++++++++++++++++++++++---------
 dir.h           |    3 +++
 3 files changed, 87 insertions(+), 38 deletions(-)

diff --git a/builtin/clean.c b/builtin/clean.c
index 69c1cda..9b056b9 100644
--- a/builtin/clean.c
+++ b/builtin/clean.c
@@ -34,22 +34,31 @@ static int exclude_cb(const struct option *opt, const char *arg, int unset)
 	return 0;
 }
 
+static void print_result(const char *msg, struct string_list *lst)
+{
+  int i;
+  for (i = 0; i < lst->nr; i++)
+		printf("%s %s\n", msg, lst->items[i].string);
+}
+
 int cmd_clean(int argc, const char **argv, const char *prefix)
 {
 	int i;
-	int show_only = 0, remove_directories = 0, quiet = 0, ignored = 0;
+	int dry_run = 0, remove_directories = 0, quiet = 0, ignored = 0;
 	int ignored_only = 0, config_set = 0, errors = 0;
 	int rm_flags = REMOVE_DIR_KEEP_NESTED_GIT;
 	struct strbuf directory = STRBUF_INIT;
 	struct dir_struct dir;
 	static const char **pathspec;
 	struct strbuf buf = STRBUF_INIT;
+	struct string_list dels = STRING_LIST_INIT_DUP;
+	struct string_list errs = STRING_LIST_INIT_DUP;
 	struct string_list exclude_list = STRING_LIST_INIT_NODUP;
 	const char *qname;
 	char *seen = NULL;
 	struct option options[] = {
 		OPT__QUIET(&quiet, N_("do not print names of files removed")),
-		OPT__DRY_RUN(&show_only, N_("dry run")),
+		OPT__DRY_RUN(&dry_run, N_("dry run")),
 		OPT__FORCE(&force, N_("force")),
 		OPT_BOOLEAN('d', NULL, &remove_directories,
 				N_("remove whole directories")),
@@ -77,7 +86,7 @@ int cmd_clean(int argc, const char **argv, const char *prefix)
 	if (ignored && ignored_only)
 		die(_("-x and -X cannot be used together"));
 
-	if (!show_only && !force) {
+	if (!dry_run && !force) {
 		if (config_set)
 			die(_("clean.requireForce set to true and neither -n nor -f given; "
 				  "refusing to clean"));
@@ -150,43 +159,42 @@ int cmd_clean(int argc, const char **argv, const char *prefix)
 		if (S_ISDIR(st.st_mode)) {
 			strbuf_addstr(&directory, ent->name);
 			qname = quote_path_relative(directory.buf, directory.len, &buf, prefix);
-			if (show_only && (remove_directories ||
-			    (matches == MATCHED_EXACTLY))) {
-				printf(_("Would remove %s\n"), qname);
-			} else if (remove_directories ||
-				   (matches == MATCHED_EXACTLY)) {
-				if (!quiet)
-					printf(_("Removing %s\n"), qname);
-				if (remove_dir_recursively(&directory,
-							   rm_flags) != 0) {
-					warning(_("failed to remove %s"), qname);
-					errors++;
-				}
-			} else if (show_only) {
-				printf(_("Would not remove %s\n"), qname);
-			} else {
-				printf(_("Not removing %s\n"), qname);
+			if (remove_directories || (matches == MATCHED_EXACTLY)) {
+				remove_dir_recursively_with_dryrun(&directory, rm_flags, dry_run,
+						&dels, &errs, prefix);
 			}
 			strbuf_reset(&directory);
 		} else {
 			if (pathspec && !matches)
 				continue;
 			qname = quote_path_relative(ent->name, -1, &buf, prefix);
-			if (show_only) {
-				printf(_("Would remove %s\n"), qname);
-				continue;
-			} else if (!quiet) {
-				printf(_("Removing %s\n"), qname);
-			}
-			if (unlink(ent->name) != 0) {
-				warning(_("failed to remove %s"), qname);
-				errors++;
+			if (dry_run)
+				string_list_append(&dels, qname);
+			else {
+				if (unlink(ent->name) != 0)
+					string_list_append(&errs, qname);
+				else
+					string_list_append(&dels, qname);
 			}
 		}
 	}
+
+	if (!quiet) {
+		if (dry_run)
+			print_result("Would remove", &dels);
+		else
+			print_result("Removed", &dels);
+	}
+
+	errors = errs.nr;
+	if (errors)
+		print_result("Failed to remove", &errs);
+
 	free(seen);
 
 	strbuf_release(&directory);
+	string_list_clear(&dels, 0);
+	string_list_clear(&errs, 0);
 	string_list_clear(&exclude_list, 0);
 	return (errors != 0);
 }
diff --git a/dir.c b/dir.c
index 5a83aa7..f580c51 100644
--- a/dir.c
+++ b/dir.c
@@ -7,7 +7,9 @@
  */
 #include "cache.h"
 #include "dir.h"
+#include "quote.h"
 #include "refs.h"
+#include "string-list.h"
 
 struct path_simplify {
 	int len;
@@ -1294,11 +1296,25 @@ int is_empty_dir(const char *path)
 	return ret;
 }
 
-static int remove_dir_recurse(struct strbuf *path, int flag, int *kept_up)
+static void append_dir_name(struct string_list *dels, struct string_list *errs,
+    char *name, const char * prefix, int failed)
+{
+	struct strbuf buf = STRBUF_INIT;
+	const char *qname;
+	qname = quote_path_relative(name, strlen(name), &buf, prefix);
+	if (!failed && dels)
+		string_list_append(dels, qname);
+	else if (errs)
+		string_list_append(errs, qname);
+}
+
+static int remove_dir_recurse(struct strbuf *path, int flag, int *kept_up,
+	int dry_run, struct string_list *dels, struct string_list *errs,
+  const char *prefix)
 {
 	DIR *dir;
 	struct dirent *e;
-	int ret = 0, original_len = path->len, len, kept_down = 0;
+	int ret = 0, original_len = path->len, len, kept_down = 0, res = 0;
 	int only_empty = (flag & REMOVE_DIR_EMPTY_ONLY);
 	int keep_toplevel = (flag & REMOVE_DIR_KEEP_TOPLEVEL);
 	unsigned char submodule_head[20];
@@ -1315,8 +1331,13 @@ static int remove_dir_recurse(struct strbuf *path, int flag, int *kept_up)
 	dir = opendir(path->buf);
 	if (!dir) {
 		/* an empty dir could be removed even if it is unreadble */
-		if (!keep_toplevel)
-			return rmdir(path->buf);
+		if (!keep_toplevel) {
+			res = 0;
+			if (!dry_run)
+				res = rmdir(path->buf);
+			append_dir_name(dels, errs, path->buf, prefix, res);
+			return res;
+		}
 		else
 			return -1;
 	}
@@ -1334,10 +1355,16 @@ static int remove_dir_recurse(struct strbuf *path, int flag, int *kept_up)
 		if (lstat(path->buf, &st))
 			; /* fall thru */
 		else if (S_ISDIR(st.st_mode)) {
-			if (!remove_dir_recurse(path, flag, &kept_down))
+			if (!remove_dir_recurse(path, flag, &kept_down, dry_run, dels, errs, prefix))
 				continue; /* happy */
-		} else if (!only_empty && !unlink(path->buf))
-			continue; /* happy, too */
+		} else if (!only_empty) {
+			res = 0;
+			if (!dry_run)
+				res = unlink(path->buf);
+			append_dir_name(dels, errs, path->buf, prefix, res);
+			if (!res)
+				continue; /* happy, too */
+		}
 
 		/* path too long, stat fails, or non-directory still exists */
 		ret = -1;
@@ -1346,8 +1373,12 @@ static int remove_dir_recurse(struct strbuf *path, int flag, int *kept_up)
 	closedir(dir);
 
 	strbuf_setlen(path, original_len);
-	if (!ret && !keep_toplevel && !kept_down)
-		ret = rmdir(path->buf);
+	if (!ret && !keep_toplevel && !kept_down) {
+		ret = 0;
+		if (!dry_run)
+			ret = rmdir(path->buf);
+		append_dir_name(dels, errs, path->buf, prefix, res);
+	}
 	else if (kept_up)
 		/*
 		 * report the uplevel that it is not an error that we
@@ -1359,7 +1390,14 @@ static int remove_dir_recurse(struct strbuf *path, int flag, int *kept_up)
 
 int remove_dir_recursively(struct strbuf *path, int flag)
 {
-	return remove_dir_recurse(path, flag, NULL);
+	return remove_dir_recurse(path, flag, NULL, 0, NULL, NULL, NULL);
+}
+
+int remove_dir_recursively_with_dryrun(struct strbuf *path, int flag,
+		int dry_run, struct string_list *dels, struct string_list *errs,
+		const char *prefix)
+{
+	return remove_dir_recurse(path, flag, NULL, dry_run, dels, errs, prefix);
 }
 
 void setup_standard_excludes(struct dir_struct *dir)
diff --git a/dir.h b/dir.h
index f5c89e3..828bd49 100644
--- a/dir.h
+++ b/dir.h
@@ -131,6 +131,9 @@ extern void setup_standard_excludes(struct dir_struct *dir);
 #define REMOVE_DIR_KEEP_NESTED_GIT 02
 #define REMOVE_DIR_KEEP_TOPLEVEL 04
 extern int remove_dir_recursively(struct strbuf *path, int flag);
+extern int remove_dir_recursively_with_dryrun(struct strbuf *path,
+			int flag, int dryrun, struct string_list *dels,
+			struct string_list *errs, const char *prefix);
 
 /* tries to remove the path with empty directories along it, ignores ENOENT */
 extern int remove_path(const char *path);
-- 
1.7.9.5

^ permalink raw reply related

* Re: [PATCH] Add new git-cc-cmd helper to contrib
From: Ramkumar Ramachandra @ 2012-12-06 11:15 UTC (permalink / raw)
  To: Felipe Contreras; +Cc: git, Junio C Hamano
In-Reply-To: <CAMP44s1--1i+ZtG7eKe0ZpRizcBfq+wJv_VOVZfU+A+gpjLyhQ@mail.gmail.com>

Felipe Contreras wrote:
> [...]

What happened to this code?  I don't see it in `pu`.

Ram

^ permalink raw reply

* Re: [PATCH] Perform minimal stat comparison when some stat fields are not set
From: Robin Rosenberg @ 2012-12-06 11:16 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: Junio C Hamano, git, spearce
In-Reply-To: <50C0475F.1030206@viscovery.net>



----- Ursprungligt meddelande -----
> Am 12/6/2012 2:09, schrieb Robin Rosenberg:
> >> Robin Rosenberg <robin.rosenberg@dewire.com> writes:
> >>> At least JGit does sets uid, gid, ctime, ino and dev fields to
> >>> zero
> >>> on update. To Git this looks like the stat data does not match
> >>> and
> >>> a full file compare will be forced even it size and mtime match.
> >>> This
> >>> is in practice unnecessary. Sense JGit's presence by checking if
> >>> ino
> >>> and dev is zero.
> 
> Is this meant to better support C git and JGit working on the same
> repository?
> 
> MinGW git sets these two stat fields to zero as well. But we have
> less of
> an interoparability problem between different git implementations in
> practice on Windows, I think.

It is purely for performance in some situations.

> >> Besides, is it sane to do this unconditionally to affect people
> >> who
> >> do not use JGit?
> > 
> > Would a config option like core.minstat be better? The name would
> > imply no dynamic detection.
> 
> A configuration option is the way to go. We already have
> core.trustctime,
> core.symlinks, core.filemode, core.ignoreCygwinFSTricks.
> 
> But your new mode is not "minimal". In some implementations or on
> some
> filesystems, even more bits of stat information could be meaningless
> (think of atime, rdev, nlink, uid, gid). Perhaps core.trustdevandino?

I already excluded uid and gid so the only thing left is mtime and size.
I can't see any reason for anyone to look at atime (somebody read the file,
so what?), ok for rdev and nlink, but we don not look at them my patch
does not avoid looking at them.

> Or
> an enumeration core.ignoreCacheStat=ctime,dev,ino?

That would mean only one configuration option. Good.

-- robin

^ permalink raw reply

* Re: Please pull l10n updates for 1.8.1 round 2
From: Peter Krefting @ 2012-12-06 11:50 UTC (permalink / raw)
  To: Jiang Xin; +Cc: Junio C Hamano, Trần Ngọc Quân, Git List
In-Reply-To: <CANYiYbGG8DeneCRyxyfVoZ+O2yBm9ywOGZwQMZC9=VYVN5Pf=A@mail.gmail.com>

Jiang Xin:

> The following changes since commit f94c3251e1400c3cf349f7f84fea4db66b540113:
>
>  Update draft release notes to 1.8.1 (2012-11-29 13:57:09 -0800)
>
> are available in the git repository at:
>
>  git://github.com/git-l10n/git-po master
>
> for you to fetch changes up to 77cc392d6d60c5d22930174904adae182502b8a9:
>
>  l10n: vi.po: update to git-v1.8.0.1-347-gf94c3 (2012-11-30 13:43:11 +0700)

This doesn't seem to contain the update to the Swedish translation 
that I committed in https://github.com/nafmo/git-l10n-sv (commit 
bf3e8fe0c718b64b7e88bd3d74564c54f923aaed). Did I forgot to post a pull 
request to you, or what happened?

-- 
\\// Peter - http://www.softwolves.pp.se/

^ permalink raw reply

* Re: [PATCH] RFC Optionally handle symbolic links as copies
From: Robin Rosenberg @ 2012-12-06 11:51 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: git
In-Reply-To: <50C04255.8050209@viscovery.net>



----- Ursprungligt meddelande -----
> Am 12/5/2012 23:46, schrieb Robin Rosenberg:
> > - git status - when do we report a diff.
> > 	- After checkout we should probably not
> 
> Are you saying that it should be ignored that the index records a
> symbolic
> link, but the worktree contains a regular file and that the regular
> file
> does not even contain the value of the symbolic link (like it would
> in the
> core.symlinks=false case)?

Well we should have some notion of clean state. That could be handled
in more than one way. We could just ignore it, or compare the copy with
the content of the link target. The first is good enough for a proof-of
concept, but perhaps not as a solution since we don't have good other
means to check that the copy is stale.

-- robin

^ permalink raw reply

* Re: [PATCH v2] If `egrep` is aliased, temporary disable it in bash.completion
From: Adam Tkac @ 2012-12-06 14:05 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, SZEDER Gábor, Felipe Contreras
In-Reply-To: <7vk3t4qpoe.fsf@alter.siamese.dyndns.org>

On Thu, Nov 29, 2012 at 09:33:53AM -0800, Junio C Hamano wrote:
> Junio C Hamano <gitster@pobox.com> writes:
> 
> > Adam Tkac <atkac@redhat.com> writes:
> >
> >> Subject: Re: [PATCH v2] If `egrep` is aliased, temporary disable it in bash.completion
> >
> > The code does not seem to do anything special if it is not aliased,
> > though, so "If ..." part does not sound correct; perhaps you meant
> > "just in case egrep is aliased to something totally wacky" or
> > something?
> >
> > The script seems to use commands other than 'egrep' that too can be
> > aliased to do whatever unexpected things.  How does this patch get
> > away without backslashing them all, like
> >
> > 	\echo ...
> >         \sed ...
> >         \test ...
> >         \: comment ...
> > 	\git args ...
> >
> > and still fix problems for users?  Can't the same solution you would
> > give to users who alias one of the above to do something undesirable
> > be applied to those who alias egrep?
> >
> > Puzzled...
> 
> Sorry for having been more snarky than necessary (blame it to lack
> of caffeine).  What I was trying to get at were:
> 
>  * I have this suspicion that this patch exists only because you saw
>    somebody who aliases egrep to something unexpected by the use of
>    it in this script, and egrep *happened* to be the only such
>    "unreasonable" alias.  The reporter may not have aliased echo or
>    sed away, or the aliases to these command *happened* to produce
>    "acceptable" output (even though it might have been slightly
>    different from unaliased one, the difference *happened* not to
>    matter for the purpose of this script).
> 
>  * To the person who observes the same aliasing breakage due to his
>    aliasing sed to something else, you would solve his problem by
>    telling him "don't do that, then".  If that is the solution, why
>    wouldn't it work for egrep?
> 
>  * The next person who aliased other commands this script uses in
>    such a way that the behaviour of the alias differs sufficiently
>    from the unaliased version, you will have to patch the file
>    again, with the same backslashing.  This patch is not a solution,
>    but a band-aid that only works for a particular case you
>    *happened* to have seen.
> 
>  * A complete solution that follows the direction this patch
>    suggests would involve backslashing *all* commands that can
>    potentially aliased away.  Is that really the direction we would
>    want to go in (answer: I doubt it)?  Is that the only approach to
>    solve this aliasing issue (answer: I don't know, but we should
>    try to pursue it before applying a band-aid that is not a
>    solution)?
> 
> Is there a way to tell bash "do not alias-expand from here up to
> there"?  Perhaps "shopt -u expand_aliases" upon entry and restore
> its original value when we exit, or something?
> 
> IOW, something along this line?

This won't work, unfortunately, because shopt settings aren't inherited by
subshell (and for example egrep is called in subshell).

I discussed this issue with colleagues and we found basically two "fixes":

1. Tell people "do not use aliases which breaks completion script"
2. Prefix all commands with "command", i.e. `command egrep` etc.

In my opinion "2." is better long time solution, what do you think?

Regards, Adam

> 
>  contrib/completion/git-completion.bash | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git i/contrib/completion/git-completion.bash w/contrib/completion/git-completion.bash
> index 0b77eb1..193f53c 100644
> --- i/contrib/completion/git-completion.bash
> +++ w/contrib/completion/git-completion.bash
> @@ -23,6 +23,14 @@
>  #    3) Consider changing your PS1 to also show the current branch,
>  #       see git-prompt.sh for details.
>  
> +if shopt -q expand_aliases
> +then
> +	_git__aliases_were_enabled=yes
> +else
> +	_git__aliases_were_enabled=
> +fi
> +shopt -u expand_aliases
> +
>  case "$COMP_WORDBREAKS" in
>  *:*) : great ;;
>  *)   COMP_WORDBREAKS="$COMP_WORDBREAKS:"
> @@ -2504,3 +2512,8 @@ __git_complete gitk __gitk_main
>  if [ Cygwin = "$(uname -o 2>/dev/null)" ]; then
>  __git_complete git.exe __git_main
>  fi
> +
> +if test -n "$_git__aliases_were_enabled"
> +then
> +	shopt -s expand_aliases
> +fi
> 
> 

-- 
Adam Tkac, Red Hat, Inc.

^ permalink raw reply

* Re: [PATCH] git-clean: Display more accurate delete messages
From: Junio C Hamano @ 2012-12-06 17:37 UTC (permalink / raw)
  To: Zoltan Klinger; +Cc: git
In-Reply-To: <1354788938-26804-1-git-send-email-zoltan.klinger@gmail.com>

Zoltan Klinger <zoltan.klinger@gmail.com> writes:

> Only print out the names of the files and directories that got actually
> deleted.
>
> Consider the following repo layout:
>   |-- test.git/
>         |-- foo/
>              |-- bar/
>                   |-- bar.txt
>              |-- frotz.git/
>                   |-- frotz.txt
>         |-- tracked_file1
>         |-- untracked_file1
> ...
> Consider the output of the improved version:
>
>   $ git clean -fd
>   Removed foo/bar/bar.txt
>   Removed foo/bar
>   Removed untracked_file1

Hrm, following your discussion (ellided above), I would have
expected that you would show

    Removing directory foo/bar
    Removing untracked_file1

^ permalink raw reply

* Re: [PATCH v2] If `egrep` is aliased, temporary disable it in bash.completion
From: Junio C Hamano @ 2012-12-06 18:01 UTC (permalink / raw)
  To: Adam Tkac; +Cc: git, SZEDER Gábor, Felipe Contreras
In-Reply-To: <20121206140541.GA4892@redhat.com>

Adam Tkac <atkac@redhat.com> writes:

> On Thu, Nov 29, 2012 at 09:33:53AM -0800, Junio C Hamano wrote:
> ...
>> IOW, something along this line?
>
> This won't work, unfortunately, because shopt settings aren't inherited by
> subshell (and for example egrep is called in subshell).
>
> I discussed this issue with colleagues and we found basically two "fixes":
>
> 1. Tell people "do not use aliases which breaks completion script"
> 2. Prefix all commands with "command", i.e. `command egrep` etc.
>
> In my opinion "2." is better long time solution, what do you think?

Judging from what is in /etc/bash_completion.d/ (I am on Debian), I
think that others are divided.  Many but not all prefix "command" in
front of "grep", but nobody does the same for "egrep", "cut", "tr",
"sed", etc.

If it were up to me, I would say we pick #1, but I cc'ed the people
who have been more involved in our bash-completion code because they
are in a better position to argue between the two than I am.

Thoughts?

^ permalink raw reply

* Git repository and Maven project
From: Aleks @ 2012-12-06 21:00 UTC (permalink / raw)
  To: git

Can you help to clarify such question.
We have 2 different projects.
Name of first project say "server".
Second - "client".
Every project has own maven build structure.
Server produces the war archive for deployment.
The Client project produces the client jar for testing Server application.
Aside from these projects we should store different artefacts like 
prototypes for developing pages for server project.
There are two opinions about approach to arrangement git repository.

1) Create one repository https://git.org/my-project/src
Create 3 separate folders inside of src directory. So we will have such 
structure :
root of repository https://git.org/my-project/src/
contains 2 different maven projects and folder for prototypes files
https://git.org/my-project/src/server
https://git.org/my-project/src/client
https://git.org/my-project/src/prototypes

2)Create for every maven project and prototypes separate repository.
https://git.org/my-server/src
https://git.org/my-client/src
https://git.org/prototypes/src

The root of maven project begins from src folder in every repository ( 
https://git.org/my-server/src and https://git.org/my-client/src)
The xml files and images of prototypes will be inside of 
https://git.org/prototypes/src
I believe the second approach more proper git-approach.
Such approach allows team to use such advanced git features like 
branging, merging, stash etc.

What approach is proper?


-- 

Best regards
Aleks

^ permalink raw reply

* Re: Git repository and Maven project
From: Junio C Hamano @ 2012-12-06 21:35 UTC (permalink / raw)
  To: Aleks; +Cc: git
In-Reply-To: <50C1075E.1060407@gmail.com>

Aleks <oles.slosko@gmail.com> writes:

> Can you help to clarify such question.
> We have 2 different projects.
> Name of first project say "server".
> Second - "client".
> Every project has own maven build structure.
> Server produces the war archive for deployment.
> The Client project produces the client jar for testing Server application.
> Aside from these projects we should store different artefacts like
> prototypes for developing pages for server project.

It depends on how tightly coupled the versions of these three
"potentially separate but could be combined" pieces you would want
to make.  Is a particular version of the "server" software meant to
work with any random version of "prototypes"?  Is a particular
version of the "client" software meant to be used to test any random
version of "server"?

Having all three in the same repository means you are guaranteed,
and more importantly, your developers are *forced* to guarantee, a
checkout of a single commit will contain the state of these three
pieces that work well together.  A commit that changes only the
"server" subtree portion, without updating the corresponding assets
in "prototypes" hierarchy that are needed to support what was added
to the "server" part, would break the entire system, and hopefully
your QA procedure can detect and reject it.

Having all three in separate repositories means your developers can
be more loose but it may lead to QA nightmare if a proper procedure
is arranged around the entire process (which Git is only a small
part of).

> I believe the second approach more proper git-approach.
> Such approach allows team to use such advanced git features like
> branging, merging, stash etc.

There is no single "more proper git-approach"; it depends on your
requirements, which the above "how tightly coupled?" question is
an example of.  Branching, merging, etc. are orthogonal and can and
will be useful regardless of the repository arrangement you choose.

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox