Git development
 help / color / mirror / Atom feed
* [PATCH 1/2] t1400-update-ref: Add test verifying bug with symrefs in delete_ref()
From: Johan Herland @ 2012-10-21 10:40 UTC (permalink / raw)
  To: gitster; +Cc: git, rene.scharfe, vmiklos, Johan Herland
In-Reply-To: <1350816032-16312-1-git-send-email-johan@herland.net>

When deleting a ref through a symref (e.g. using 'git update-ref -d HEAD'
to delete refs/heads/master), we currently fail to remove the packed
version of that ref. This testcase demonstrates the bug.

Signed-off-by: Johan Herland <johan@herland.net>
---
 t/t1400-update-ref.sh | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh
index 4fd83a6..f7ec203 100755
--- a/t/t1400-update-ref.sh
+++ b/t/t1400-update-ref.sh
@@ -74,6 +74,24 @@ test_expect_success "delete $m (by HEAD)" '
 '
 rm -f .git/$m
 
+test_expect_success \
+	"create $m (by HEAD)" \
+	"git update-ref HEAD $A &&
+	 test $A"' = $(cat .git/'"$m"')'
+test_expect_success \
+	"pack refs" \
+	"git pack-refs --all"
+test_expect_success \
+	"move $m (by HEAD)" \
+	"git update-ref HEAD $B $A &&
+	 test $B"' = $(cat .git/'"$m"')'
+test_expect_failure "delete $m (by HEAD) should remove both packed and loose $m" '
+	git update-ref -d HEAD $B &&
+	! grep "$m" .git/packed-refs &&
+	! test -f .git/$m
+'
+rm -f .git/$m
+
 cp -f .git/HEAD .git/HEAD.orig
 test_expect_success "delete symref without dereference" '
 	git update-ref --no-deref -d HEAD &&
-- 
1.7.12.1.609.g5cd6968

^ permalink raw reply related

* [PATCH 2/2] Fix failure to delete a packed ref through a symref
From: Johan Herland @ 2012-10-21 10:40 UTC (permalink / raw)
  To: gitster; +Cc: git, rene.scharfe, vmiklos, Johan Herland
In-Reply-To: <1350816032-16312-1-git-send-email-johan@herland.net>

When deleting a ref through a symref (e.g. using 'git update-ref -d HEAD'
to delete refs/heads/master), we would remove the loose ref, but a packed
version of the same ref would remain, the end result being that instead of
deleting refs/heads/master we would appear to reset it to its state as of
the last repack.

This patch fixes the issue, by making sure we pass the correct ref name
when invoking repack_without_ref() from within delete_ref().

Signed-off-by: Johan Herland <johan@herland.net>
---
 refs.c                | 2 +-
 t/t1400-update-ref.sh | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/refs.c b/refs.c
index 726c53c..6cec1c8 100644
--- a/refs.c
+++ b/refs.c
@@ -1779,7 +1779,7 @@ int delete_ref(const char *refname, const unsigned char *sha1, int delopt)
 	 * packed one.  Also, if it was not loose we need to repack
 	 * without it.
 	 */
-	ret |= repack_without_ref(refname);
+	ret |= repack_without_ref(lock->ref_name);
 
 	unlink_or_warn(git_path("logs/%s", lock->ref_name));
 	invalidate_ref_cache(NULL);
diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh
index f7ec203..e415ee0 100755
--- a/t/t1400-update-ref.sh
+++ b/t/t1400-update-ref.sh
@@ -85,7 +85,7 @@ test_expect_success \
 	"move $m (by HEAD)" \
 	"git update-ref HEAD $B $A &&
 	 test $B"' = $(cat .git/'"$m"')'
-test_expect_failure "delete $m (by HEAD) should remove both packed and loose $m" '
+test_expect_success "delete $m (by HEAD) should remove both packed and loose $m" '
 	git update-ref -d HEAD $B &&
 	! grep "$m" .git/packed-refs &&
 	! test -f .git/$m
-- 
1.7.12.1.609.g5cd6968

^ permalink raw reply related

* [PATCH 0/2] Fix remaining issue when deleting symrefs
From: Johan Herland @ 2012-10-21 10:40 UTC (permalink / raw)
  To: gitster; +Cc: git, rene.scharfe, vmiklos, Johan Herland
In-Reply-To: <7vpq4flb1c.fsf@alter.siamese.dyndns.org>

The following two patches are based on rs/branch-del-symref, and fixes
the remaining failure to delete a packed ref through a symref.

The first patch demonstrates the bug with a testcase, and the second
patch fixes the bug.

Feel free to squash the two patches into one, if you prefer to keep
both the testcase and subsequent fix in a single commit.


Have fun! :)

...Johan

Johan Herland (2):
  t1400-update-ref: Add test verifying bug with symrefs in delete_ref()
  Fix failure to delete a packed ref through a symref

 refs.c                |  2 +-
 t/t1400-update-ref.sh | 18 ++++++++++++++++++
 2 files changed, 19 insertions(+), 1 deletion(-)

--
1.7.12.1.609.g5cd6968

^ permalink raw reply

* Links broken in ref docs.
From: Mike Norman @ 2012-10-21  7:31 UTC (permalink / raw)
  To: git

Many links on scm-git.org/docs simply reload the page.

For example, all of Sharing and Updating section simply reload the
docs page. And tons others. Must be a broken link or routing problem.
Repros on FF 14.0.1 and Chrome. Good luck!

IRC was useless.

Also, fuck your stupid email policies. :)

^ permalink raw reply

* Re: git config --unset does not remove section
From: Junio C Hamano @ 2012-10-21  7:09 UTC (permalink / raw)
  To: Angelo Borsotti; +Cc: git
In-Reply-To: <CAB9Jk9C_b_PScm3vEdvxsjqNHpTbz7OTTdFeFey7Sdb+M5gYFw@mail.gmail.com>

Angelo Borsotti <angelo.borsotti@gmail.com> writes:

> Another git config foo.bar true changes the config file into:
>
>     [foo]
>     [foo]
> 	bar = true
>
> Having two sections with the same name clutters the config file.

True.  It would be nice if the removal of the last element removes
the section header as well.

^ permalink raw reply

* Re: [PATCH v3 0/8] Fix GIT_CEILING_DIRECTORIES that contain symlinks
From: Junio C Hamano @ 2012-10-21  6:51 UTC (permalink / raw)
  To: Michael Haggerty; +Cc: Jiang Xin, Lea Wiemann, David Reiss, Johannes Sixt, git
In-Reply-To: <1350799057-13846-1-git-send-email-mhagger@alum.mit.edu>

Michael Haggerty <mhagger@alum.mit.edu> writes:

> This patch series has the side effect that all of the directories
> listed in GIT_CEILING_DIRECTORIES are accessed *unconditionally* to
> resolve any symlinks that are present in their paths.  It is
> admittedly odd that a feature intended to avoid accessing expensive
> directories would now *intentionally* access directories near the
> expensive ones.  In the above scenario this shouldn't be a problem,
> because /home would be the directory listed in
> GIT_CEILING_DIRECTORIES, and accessing /home itself shouldn't be
> expensive.

Interesting observation.  In the last sentence, "accessing /home"
does not exactly mean accessing /home, but accessing / to learn
about "home" in it, no?

> But there might be other scenarios for which this patch
> series causes a performance regression.

Yeah, after merging this to 'next', we should ask people who care
about CEILING to test it sufficiently.

Thanks for rerolling.

^ permalink raw reply

* git config --unset does not remove section
From: Angelo Borsotti @ 2012-10-21  6:46 UTC (permalink / raw)
  To: git

Hello,

git config --unset does not remove the section in the configuration
file when the removed key is the last one, leaving then an empty
section that is lo longer used. E.g.:

    git config foo.bar true

creates:

    [foo]
	bar = true

then:

    git config --unset foo.bar

changes the section into:

    [foo]

Another git config foo.bar true changes the config file into:

    [foo]
    [foo]
	bar = true

Having two sections with the same name clutters the config file.

-Angelo Borsotti

^ permalink raw reply

* Re: Subtree in Git
From: Junio C Hamano @ 2012-10-21  6:32 UTC (permalink / raw)
  To: Herman van Rink; +Cc: dag, greened, Hilco Wijbenga, Git Users
In-Reply-To: <50830374.9090308@initfour.nl>

Herman van Rink <rink@initfour.nl> writes:

> Junio, Could you please consider merging the single commit from my
> subtree-updates branch? https://github.com/helmo/git/tree/subtree-updates

In general, in areas like contrib/ where there is a volunteer area
maintainer, unless the change something ultra-urgent (e.g. serious
security fix) and the area maintainer is unavailable, I'm really
reluctant to bypass and take a single patch that adds many things
that are independent from each other.

Especially not immediately before tagging 1.8.0 final.

^ permalink raw reply

* [PATCH v3 7/8] normalize_ceiling_entry(): resolve symlinks
From: Michael Haggerty @ 2012-10-21  5:57 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Jiang Xin, Lea Wiemann, David Reiss, Johannes Sixt, git,
	Michael Haggerty
In-Reply-To: <1350799057-13846-1-git-send-email-mhagger@alum.mit.edu>

longest_ancestor_length() relies on a textual comparison of directory
parts to find the part of path that overlaps with one of the paths in
prefix_list.  But this doesn't work if any of the prefixes involves a
symbolic link, because the directories will look different even though
they might logically refer to the same directory.  So canonicalize the
paths listed in GIT_CEILING_DIRECTORIES using real_path_if_valid()
before passing them to longest_ancestor_length().

path is already in canonical form, so doesn't need to be canonicalized
again.

This fixes some problems with using GIT_CEILING_DIRECTORIES that
contains paths involving symlinks, including t4035 if run with --root
set to a path involving symlinks.

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
 setup.c | 20 +++++++++-----------
 1 file changed, 9 insertions(+), 11 deletions(-)

diff --git a/setup.c b/setup.c
index df97ad3..2eb5b75 100644
--- a/setup.c
+++ b/setup.c
@@ -622,24 +622,22 @@ static dev_t get_device_or_die(const char *path, const char *prefix, int prefix_
 }
 
 /*
- * A "string_list_each_func_t" function that normalizes an entry from
- * GIT_CEILING_DIRECTORIES or discards it if unusable.
+ * A "string_list_each_func_t" function that canonicalizes an entry
+ * from GIT_CEILING_DIRECTORIES using real_path_if_valid(), or
+ * discards it if unusable.
  */
 static int normalize_ceiling_entry(struct string_list_item *item, void *unused)
 {
-	const char *ceil = item->string;
-	int len = strlen(ceil);
-	char buf[PATH_MAX+1];
+	char *ceil = item->string;
+	const char *real_path;
 
-	if (len == 0 || len > PATH_MAX || !is_absolute_path(ceil))
+	if (!*ceil || !is_absolute_path(ceil))
 		return 0;
-	if (normalize_path_copy(buf, ceil) < 0)
+	real_path = real_path_if_valid(ceil);
+	if (!real_path)
 		return 0;
-	len = strlen(buf);
-	if (len > 1 && buf[len-1] == '/')
-		buf[--len] = '\0';
 	free(item->string);
-	item->string = xstrdup(buf);
+	item->string = xstrdup(real_path);
 	return 1;
 }
 
-- 
1.7.11.3

^ permalink raw reply related

* [PATCH v3 8/8] string_list_longest_prefix(): remove function
From: Michael Haggerty @ 2012-10-21  5:57 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Jiang Xin, Lea Wiemann, David Reiss, Johannes Sixt, git,
	Michael Haggerty
In-Reply-To: <1350799057-13846-1-git-send-email-mhagger@alum.mit.edu>

This function was added in f103f95b11d087f07c0c48bf784cd9197e18f203 in
the erroneous expectation that it would be used in the
reimplementation of longest_ancestor_length().  But it turned out to
be easier to use a function specialized for comparing path prefixes
(i.e., one that knows about slashes and root paths) than to prepare
the paths in such a way that a generic string prefix comparison
function can be used.  So delete string_list_longest_prefix() and its
documentation and test cases.

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
 Documentation/technical/api-string-list.txt |  8 --------
 string-list.c                               | 20 -------------------
 string-list.h                               |  8 --------
 t/t0063-string-list.sh                      | 30 -----------------------------
 test-string-list.c                          | 20 -------------------
 5 files changed, 86 deletions(-)

diff --git a/Documentation/technical/api-string-list.txt b/Documentation/technical/api-string-list.txt
index 94d7a2b..618400d 100644
--- a/Documentation/technical/api-string-list.txt
+++ b/Documentation/technical/api-string-list.txt
@@ -75,14 +75,6 @@ Functions
 	to be deleted.  Preserve the order of the items that are
 	retained.
 
-`string_list_longest_prefix`::
-
-	Return the longest string within a string_list that is a
-	prefix (in the sense of prefixcmp()) of the specified string,
-	or NULL if no such prefix exists.  This function does not
-	require the string_list to be sorted (it does a linear
-	search).
-
 `print_string_list`::
 
 	Dump a string_list to stdout, useful mainly for debugging purposes. It
diff --git a/string-list.c b/string-list.c
index c54b816..decfa74 100644
--- a/string-list.c
+++ b/string-list.c
@@ -136,26 +136,6 @@ void filter_string_list(struct string_list *list, int free_util,
 	list->nr = dst;
 }
 
-char *string_list_longest_prefix(const struct string_list *prefixes,
-				 const char *string)
-{
-	int i, max_len = -1;
-	char *retval = NULL;
-
-	for (i = 0; i < prefixes->nr; i++) {
-		char *prefix = prefixes->items[i].string;
-		if (!prefixcmp(string, prefix)) {
-			int len = strlen(prefix);
-			if (len > max_len) {
-				retval = prefix;
-				max_len = len;
-			}
-		}
-	}
-
-	return retval;
-}
-
 void string_list_clear(struct string_list *list, int free_util)
 {
 	if (list->items) {
diff --git a/string-list.h b/string-list.h
index 5efd07b..3a6a6dc 100644
--- a/string-list.h
+++ b/string-list.h
@@ -38,14 +38,6 @@ int for_each_string_list(struct string_list *list,
 void filter_string_list(struct string_list *list, int free_util,
 			string_list_each_func_t want, void *cb_data);
 
-/*
- * Return the longest string in prefixes that is a prefix (in the
- * sense of prefixcmp()) of string, or NULL if no such prefix exists.
- * This function does not require the string_list to be sorted (it
- * does a linear search).
- */
-char *string_list_longest_prefix(const struct string_list *prefixes, const char *string);
-
 
 /* Use these functions only on sorted lists: */
 int string_list_has_string(const struct string_list *list, const char *string);
diff --git a/t/t0063-string-list.sh b/t/t0063-string-list.sh
index 41c8826..dbfc05e 100755
--- a/t/t0063-string-list.sh
+++ b/t/t0063-string-list.sh
@@ -17,14 +17,6 @@ test_split () {
 	"
 }
 
-test_longest_prefix () {
-	test "$(test-string-list longest_prefix "$1" "$2")" = "$3"
-}
-
-test_no_longest_prefix () {
-	test_must_fail test-string-list longest_prefix "$1" "$2"
-}
-
 test_split "foo:bar:baz" ":" "-1" <<EOF
 3
 [0]: "foo"
@@ -96,26 +88,4 @@ test_expect_success "test remove_duplicates" '
 	test a:b:c = "$(test-string-list remove_duplicates a:a:a:b:b:b:c:c:c)"
 '
 
-test_expect_success "test longest_prefix" '
-	test_no_longest_prefix - '' &&
-	test_no_longest_prefix - x &&
-	test_longest_prefix "" x "" &&
-	test_longest_prefix x x x &&
-	test_longest_prefix "" foo "" &&
-	test_longest_prefix : foo "" &&
-	test_longest_prefix f foo f &&
-	test_longest_prefix foo foobar foo &&
-	test_longest_prefix foo foo foo &&
-	test_no_longest_prefix bar foo &&
-	test_no_longest_prefix bar:bar foo &&
-	test_no_longest_prefix foobar foo &&
-	test_longest_prefix foo:bar foo foo &&
-	test_longest_prefix foo:bar bar bar &&
-	test_longest_prefix foo::bar foo foo &&
-	test_longest_prefix foo:foobar foo foo &&
-	test_longest_prefix foobar:foo foo foo &&
-	test_longest_prefix foo: bar "" &&
-	test_longest_prefix :foo bar ""
-'
-
 test_done
diff --git a/test-string-list.c b/test-string-list.c
index 4693295..00ce6c9 100644
--- a/test-string-list.c
+++ b/test-string-list.c
@@ -97,26 +97,6 @@ int main(int argc, char **argv)
 		return 0;
 	}
 
-	if (argc == 4 && !strcmp(argv[1], "longest_prefix")) {
-		/* arguments: <colon-separated-prefixes>|- <string> */
-		struct string_list prefixes = STRING_LIST_INIT_DUP;
-		int retval;
-		const char *prefix_string = argv[2];
-		const char *string = argv[3];
-		const char *match;
-
-		parse_string_list(&prefixes, prefix_string);
-		match = string_list_longest_prefix(&prefixes, string);
-		if (match) {
-			printf("%s\n", match);
-			retval = 0;
-		}
-		else
-			retval = 1;
-		string_list_clear(&prefixes, 0);
-		return retval;
-	}
-
 	fprintf(stderr, "%s: unknown function name: %s\n", argv[0],
 		argv[1] ? argv[1] : "(there was none)");
 	return 1;
-- 
1.7.11.3

^ permalink raw reply related

* [PATCH v3 5/8] longest_ancestor_length(): take a string_list argument for prefixes
From: Michael Haggerty @ 2012-10-21  5:57 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Jiang Xin, Lea Wiemann, David Reiss, Johannes Sixt, git,
	Michael Haggerty
In-Reply-To: <1350799057-13846-1-git-send-email-mhagger@alum.mit.edu>

Change longest_ancestor_length() to take the prefixes argument as a
string_list rather than as a PATH_SEP-separated string.  This will
make it easier to change the caller to alter the entries before
calling longest_ancestor_length().

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
 cache.h           |  2 +-
 path.c            | 22 +++++++++-------------
 setup.c           | 11 +++++++++--
 test-path-utils.c |  8 +++++++-
 4 files changed, 26 insertions(+), 17 deletions(-)

diff --git a/cache.h b/cache.h
index b0d75bc..8103385 100644
--- a/cache.h
+++ b/cache.h
@@ -718,7 +718,7 @@ const char *real_path_if_valid(const char *path);
 const char *absolute_path(const char *path);
 const char *relative_path(const char *abs, const char *base);
 int normalize_path_copy(char *dst, const char *src);
-int longest_ancestor_length(const char *path, const char *prefix_list);
+int longest_ancestor_length(const char *path, struct string_list *prefixes);
 char *strip_path_suffix(const char *path, const char *suffix);
 int daemon_avoid_alias(const char *path);
 int offset_1st_component(const char *path);
diff --git a/path.c b/path.c
index f455e8e..b80d2e6 100644
--- a/path.c
+++ b/path.c
@@ -570,30 +570,27 @@ int normalize_path_copy(char *dst, const char *src)
 
 /*
  * path = Canonical absolute path
- * prefix_list = Colon-separated list of absolute paths
+ * prefixes = string_list containing absolute paths
  *
- * Determines, for each path in prefix_list, whether the "prefix" really
+ * Determines, for each path in prefixes, whether the "prefix" really
  * is an ancestor directory of path.  Returns the length of the longest
  * ancestor directory, excluding any trailing slashes, or -1 if no prefix
- * is an ancestor.  (Note that this means 0 is returned if prefix_list is
- * "/".) "/foo" is not considered an ancestor of "/foobar".  Directories
+ * is an ancestor.  (Note that this means 0 is returned if prefixes is
+ * ["/"].) "/foo" is not considered an ancestor of "/foobar".  Directories
  * are not considered to be their own ancestors.  path must be in a
  * canonical form: empty components, or "." or ".." components are not
- * allowed.  prefix_list may be null, which is like "".
+ * allowed.  Empty strings in prefixes are ignored.
  */
-int longest_ancestor_length(const char *path, const char *prefix_list)
+int longest_ancestor_length(const char *path, struct string_list *prefixes)
 {
-	struct string_list prefixes = STRING_LIST_INIT_DUP;
 	char buf[PATH_MAX+1];
 	int i, max_len = -1;
 
-	if (prefix_list == NULL || !strcmp(path, "/"))
+	if (!strcmp(path, "/"))
 		return -1;
 
-	string_list_split(&prefixes, prefix_list, PATH_SEP, -1);
-
-	for (i = 0; i < prefixes.nr; i++) {
-		const char *ceil = prefixes.items[i].string;
+	for (i = 0; i < prefixes->nr; i++) {
+		const char *ceil = prefixes->items[i].string;
 		int len = strlen(ceil);
 
 		if (len == 0 || len > PATH_MAX || !is_absolute_path(ceil))
@@ -611,7 +608,6 @@ int longest_ancestor_length(const char *path, const char *prefix_list)
 		}
 	}
 
-	string_list_clear(&prefixes, 0);
 	return max_len;
 }
 
diff --git a/setup.c b/setup.c
index 3a1b2fd..b4cd356 100644
--- a/setup.c
+++ b/setup.c
@@ -1,5 +1,6 @@
 #include "cache.h"
 #include "dir.h"
+#include "string-list.h"
 
 static int inside_git_dir = -1;
 static int inside_work_tree = -1;
@@ -627,10 +628,11 @@ static dev_t get_device_or_die(const char *path, const char *prefix, int prefix_
 static const char *setup_git_directory_gently_1(int *nongit_ok)
 {
 	const char *env_ceiling_dirs = getenv(CEILING_DIRECTORIES_ENVIRONMENT);
+	struct string_list ceiling_dirs = STRING_LIST_INIT_DUP;
 	static char cwd[PATH_MAX+1];
 	const char *gitdirenv, *ret;
 	char *gitfile;
-	int len, offset, offset_parent, ceil_offset;
+	int len, offset, offset_parent, ceil_offset = -1;
 	dev_t current_device = 0;
 	int one_filesystem = 1;
 
@@ -655,7 +657,12 @@ static const char *setup_git_directory_gently_1(int *nongit_ok)
 	if (gitdirenv)
 		return setup_explicit_git_dir(gitdirenv, cwd, len, nongit_ok);
 
-	ceil_offset = longest_ancestor_length(cwd, env_ceiling_dirs);
+	if (env_ceiling_dirs) {
+		string_list_split(&ceiling_dirs, env_ceiling_dirs, PATH_SEP, -1);
+		ceil_offset = longest_ancestor_length(cwd, &ceiling_dirs);
+		string_list_clear(&ceiling_dirs, 0);
+	}
+
 	if (ceil_offset < 0 && has_dos_drive_prefix(cwd))
 		ceil_offset = 1;
 
diff --git a/test-path-utils.c b/test-path-utils.c
index 3bc20e9..acb0560 100644
--- a/test-path-utils.c
+++ b/test-path-utils.c
@@ -1,4 +1,5 @@
 #include "cache.h"
+#include "string-list.h"
 
 int main(int argc, char **argv)
 {
@@ -30,7 +31,12 @@ int main(int argc, char **argv)
 	}
 
 	if (argc == 4 && !strcmp(argv[1], "longest_ancestor_length")) {
-		int len = longest_ancestor_length(argv[2], argv[3]);
+		int len;
+		struct string_list ceiling_dirs = STRING_LIST_INIT_DUP;
+
+		string_list_split(&ceiling_dirs, argv[3], PATH_SEP, -1);
+		len = longest_ancestor_length(argv[2], &ceiling_dirs);
+		string_list_clear(&ceiling_dirs, 0);
 		printf("%d\n", len);
 		return 0;
 	}
-- 
1.7.11.3

^ permalink raw reply related

* [PATCH v3 6/8] longest_ancestor_length(): require prefix list entries to be normalized
From: Michael Haggerty @ 2012-10-21  5:57 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Jiang Xin, Lea Wiemann, David Reiss, Johannes Sixt, git,
	Michael Haggerty
In-Reply-To: <1350799057-13846-1-git-send-email-mhagger@alum.mit.edu>

Move the responsibility for normalizing the prefixes passed to
longest_ancestor_length() to its caller.  In t0060, only test
longest_ancestor_lengths using normalized paths: remove empty entries
and non-absolute paths, strip trailing slashes from the paths, and
remove tests that thereby become redundant.

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
 path.c                | 26 +++++++++++---------------
 setup.c               | 23 +++++++++++++++++++++++
 t/t0060-path-utils.sh | 41 +++++++++++++----------------------------
 3 files changed, 47 insertions(+), 43 deletions(-)

diff --git a/path.c b/path.c
index b80d2e6..d3d3f8b 100644
--- a/path.c
+++ b/path.c
@@ -570,20 +570,20 @@ int normalize_path_copy(char *dst, const char *src)
 
 /*
  * path = Canonical absolute path
- * prefixes = string_list containing absolute paths
+ * prefixes = string_list containing normalized, absolute paths without
+ * trailing slashes (except for the root directory, which is denoted by "/").
  *
- * Determines, for each path in prefixes, whether the "prefix" really
+ * Determines, for each path in prefixes, whether the "prefix"
  * is an ancestor directory of path.  Returns the length of the longest
  * ancestor directory, excluding any trailing slashes, or -1 if no prefix
  * is an ancestor.  (Note that this means 0 is returned if prefixes is
  * ["/"].) "/foo" is not considered an ancestor of "/foobar".  Directories
  * are not considered to be their own ancestors.  path must be in a
  * canonical form: empty components, or "." or ".." components are not
- * allowed.  Empty strings in prefixes are ignored.
+ * allowed.
  */
 int longest_ancestor_length(const char *path, struct string_list *prefixes)
 {
-	char buf[PATH_MAX+1];
 	int i, max_len = -1;
 
 	if (!strcmp(path, "/"))
@@ -593,19 +593,15 @@ int longest_ancestor_length(const char *path, struct string_list *prefixes)
 		const char *ceil = prefixes->items[i].string;
 		int len = strlen(ceil);
 
-		if (len == 0 || len > PATH_MAX || !is_absolute_path(ceil))
-			continue;
-		if (normalize_path_copy(buf, ceil) < 0)
-			continue;
-		len = strlen(buf);
-		if (len > 0 && buf[len-1] == '/')
-			buf[--len] = '\0';
+		if (len == 1 && ceil[0] == '/')
+			len = 0; /* root matches anything, with length 0 */
+		else if (!strncmp(path, ceil, len) && path[len] == '/')
+			; /* match of length len */
+		else
+			continue; /* no match */
 
-		if (!strncmp(path, buf, len) &&
-		    path[len] == '/' &&
-		    len > max_len) {
+		if (len > max_len)
 			max_len = len;
-		}
 	}
 
 	return max_len;
diff --git a/setup.c b/setup.c
index b4cd356..df97ad3 100644
--- a/setup.c
+++ b/setup.c
@@ -622,6 +622,28 @@ static dev_t get_device_or_die(const char *path, const char *prefix, int prefix_
 }
 
 /*
+ * A "string_list_each_func_t" function that normalizes an entry from
+ * GIT_CEILING_DIRECTORIES or discards it if unusable.
+ */
+static int normalize_ceiling_entry(struct string_list_item *item, void *unused)
+{
+	const char *ceil = item->string;
+	int len = strlen(ceil);
+	char buf[PATH_MAX+1];
+
+	if (len == 0 || len > PATH_MAX || !is_absolute_path(ceil))
+		return 0;
+	if (normalize_path_copy(buf, ceil) < 0)
+		return 0;
+	len = strlen(buf);
+	if (len > 1 && buf[len-1] == '/')
+		buf[--len] = '\0';
+	free(item->string);
+	item->string = xstrdup(buf);
+	return 1;
+}
+
+/*
  * We cannot decide in this function whether we are in the work tree or
  * not, since the config can only be read _after_ this function was called.
  */
@@ -659,6 +681,7 @@ static const char *setup_git_directory_gently_1(int *nongit_ok)
 
 	if (env_ceiling_dirs) {
 		string_list_split(&ceiling_dirs, env_ceiling_dirs, PATH_SEP, -1);
+		filter_string_list(&ceiling_dirs, 0, normalize_ceiling_entry, NULL);
 		ceil_offset = longest_ancestor_length(cwd, &ceiling_dirs);
 		string_list_clear(&ceiling_dirs, 0);
 	}
diff --git a/t/t0060-path-utils.sh b/t/t0060-path-utils.sh
index 4ef2345..09a42a4 100755
--- a/t/t0060-path-utils.sh
+++ b/t/t0060-path-utils.sh
@@ -93,47 +93,32 @@ norm_path /d1/s1//../s2/../../d2 /d2 POSIX
 norm_path /d1/.../d2 /d1/.../d2 POSIX
 norm_path /d1/..././../d2 /d1/d2 POSIX
 
-ancestor / "" -1
 ancestor / / -1
-ancestor /foo "" -1
-ancestor /foo : -1
-ancestor /foo ::. -1
-ancestor /foo ::..:: -1
 ancestor /foo / 0
 ancestor /foo /fo -1
 ancestor /foo /foo -1
-ancestor /foo /foo/ -1
 ancestor /foo /bar -1
-ancestor /foo /bar/ -1
 ancestor /foo /foo/bar -1
-ancestor /foo /foo:/bar/ -1
-ancestor /foo /foo/:/bar/ -1
-ancestor /foo /foo::/bar/ -1
-ancestor /foo /:/foo:/bar/ 0
-ancestor /foo /foo:/:/bar/ 0
-ancestor /foo /:/bar/:/foo 0
-ancestor /foo/bar "" -1
+ancestor /foo /foo:/bar -1
+ancestor /foo /:/foo:/bar 0
+ancestor /foo /foo:/:/bar 0
+ancestor /foo /:/bar:/foo 0
 ancestor /foo/bar / 0
 ancestor /foo/bar /fo -1
-ancestor /foo/bar foo -1
 ancestor /foo/bar /foo 4
-ancestor /foo/bar /foo/ 4
 ancestor /foo/bar /foo/ba -1
 ancestor /foo/bar /:/fo 0
 ancestor /foo/bar /foo:/foo/ba 4
 ancestor /foo/bar /bar -1
-ancestor /foo/bar /bar/ -1
-ancestor /foo/bar /fo: -1
-ancestor /foo/bar :/fo -1
-ancestor /foo/bar /foo:/bar/ 4
-ancestor /foo/bar /:/foo:/bar/ 4
-ancestor /foo/bar /foo:/:/bar/ 4
-ancestor /foo/bar /:/bar/:/fo 0
-ancestor /foo/bar /:/bar/ 0
-ancestor /foo/bar .:/foo/. 4
-ancestor /foo/bar .:/foo/.:.: 4
-ancestor /foo/bar /foo/./:.:/bar 4
-ancestor /foo/bar .:/bar -1
+ancestor /foo/bar /fo -1
+ancestor /foo/bar /foo:/bar 4
+ancestor /foo/bar /:/foo:/bar 4
+ancestor /foo/bar /foo:/:/bar 4
+ancestor /foo/bar /:/bar:/fo 0
+ancestor /foo/bar /:/bar 0
+ancestor /foo/bar /foo 4
+ancestor /foo/bar /foo:/bar 4
+ancestor /foo/bar /bar -1
 
 test_expect_success 'strip_path_suffix' '
 	test c:/msysgit = $(test-path-utils strip_path_suffix \
-- 
1.7.11.3

^ permalink raw reply related

* [PATCH v3 4/8] longest_ancestor_length(): use string_list_split()
From: Michael Haggerty @ 2012-10-21  5:57 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Jiang Xin, Lea Wiemann, David Reiss, Johannes Sixt, git,
	Michael Haggerty
In-Reply-To: <1350799057-13846-1-git-send-email-mhagger@alum.mit.edu>


Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
 path.c | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/path.c b/path.c
index cbbdf7d..f455e8e 100644
--- a/path.c
+++ b/path.c
@@ -12,6 +12,7 @@
  */
 #include "cache.h"
 #include "strbuf.h"
+#include "string-list.h"
 
 static char bad_path[] = "/bad-path/";
 
@@ -582,20 +583,22 @@ int normalize_path_copy(char *dst, const char *src)
  */
 int longest_ancestor_length(const char *path, const char *prefix_list)
 {
+	struct string_list prefixes = STRING_LIST_INIT_DUP;
 	char buf[PATH_MAX+1];
-	const char *ceil, *colon;
-	int len, max_len = -1;
+	int i, max_len = -1;
 
 	if (prefix_list == NULL || !strcmp(path, "/"))
 		return -1;
 
-	for (colon = ceil = prefix_list; *colon; ceil = colon+1) {
-		for (colon = ceil; *colon && *colon != PATH_SEP; colon++);
-		len = colon - ceil;
+	string_list_split(&prefixes, prefix_list, PATH_SEP, -1);
+
+	for (i = 0; i < prefixes.nr; i++) {
+		const char *ceil = prefixes.items[i].string;
+		int len = strlen(ceil);
+
 		if (len == 0 || len > PATH_MAX || !is_absolute_path(ceil))
 			continue;
-		strlcpy(buf, ceil, len+1);
-		if (normalize_path_copy(buf, buf) < 0)
+		if (normalize_path_copy(buf, ceil) < 0)
 			continue;
 		len = strlen(buf);
 		if (len > 0 && buf[len-1] == '/')
@@ -608,6 +611,7 @@ int longest_ancestor_length(const char *path, const char *prefix_list)
 		}
 	}
 
+	string_list_clear(&prefixes, 0);
 	return max_len;
 }
 
-- 
1.7.11.3

^ permalink raw reply related

* [PATCH v3 3/8] Introduce new function real_path_if_valid()
From: Michael Haggerty @ 2012-10-21  5:57 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Jiang Xin, Lea Wiemann, David Reiss, Johannes Sixt, git,
	Michael Haggerty
In-Reply-To: <1350799057-13846-1-git-send-email-mhagger@alum.mit.edu>

The function is like real_path(), except that it returns NULL on error
instead of dying.

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
 abspath.c | 5 +++++
 cache.h   | 1 +
 2 files changed, 6 insertions(+)

diff --git a/abspath.c b/abspath.c
index f8a526f..40cdc46 100644
--- a/abspath.c
+++ b/abspath.c
@@ -153,6 +153,11 @@ const char *real_path(const char *path)
 	return real_path_internal(path, 1);
 }
 
+const char *real_path_if_valid(const char *path)
+{
+	return real_path_internal(path, 0);
+}
+
 static const char *get_pwd_cwd(void)
 {
 	static char cwd[PATH_MAX + 1];
diff --git a/cache.h b/cache.h
index a58df84..b0d75bc 100644
--- a/cache.h
+++ b/cache.h
@@ -714,6 +714,7 @@ static inline int is_absolute_path(const char *path)
 }
 int is_directory(const char *);
 const char *real_path(const char *path);
+const char *real_path_if_valid(const char *path);
 const char *absolute_path(const char *path);
 const char *relative_path(const char *abs, const char *base);
 int normalize_path_copy(char *dst, const char *src);
-- 
1.7.11.3

^ permalink raw reply related

* [PATCH v3 2/8] real_path_internal(): add comment explaining use of cwd
From: Michael Haggerty @ 2012-10-21  5:57 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Jiang Xin, Lea Wiemann, David Reiss, Johannes Sixt, git,
	Michael Haggerty
In-Reply-To: <1350799057-13846-1-git-send-email-mhagger@alum.mit.edu>


Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
 abspath.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/abspath.c b/abspath.c
index a7ab8e9..f8a526f 100644
--- a/abspath.c
+++ b/abspath.c
@@ -35,7 +35,14 @@ static const char *real_path_internal(const char *path, int die_on_error)
 {
 	static char bufs[2][PATH_MAX + 1], *buf = bufs[0], *next_buf = bufs[1];
 	char *retval = NULL;
+
+	/*
+	 * If we have to temporarily chdir(), store the original CWD
+	 * here so that we can chdir() back to it at the end of the
+	 * function:
+	 */
 	char cwd[1024] = "";
+
 	int buf_index = 1;
 
 	int depth = MAXDEPTH;
-- 
1.7.11.3

^ permalink raw reply related

* [PATCH v3 1/8] Introduce new static function real_path_internal()
From: Michael Haggerty @ 2012-10-21  5:57 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Jiang Xin, Lea Wiemann, David Reiss, Johannes Sixt, git,
	Michael Haggerty
In-Reply-To: <1350799057-13846-1-git-send-email-mhagger@alum.mit.edu>

It accepts a new parameter, die_on_error.  If die_on_error is false,
it simply cleans up after itself and returns NULL rather than dying.

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
 abspath.c | 93 ++++++++++++++++++++++++++++++++++++++++++++++++---------------
 1 file changed, 72 insertions(+), 21 deletions(-)

diff --git a/abspath.c b/abspath.c
index 05f2d79..a7ab8e9 100644
--- a/abspath.c
+++ b/abspath.c
@@ -15,15 +15,26 @@ int is_directory(const char *path)
 #define MAXDEPTH 5
 
 /*
- * Use this to get the real path, i.e. resolve links. If you want an
- * absolute path but don't mind links, use absolute_path.
+ * Return the real path (i.e., absolute path, with symlinks resolved
+ * and extra slashes removed) equivalent to the specified path.  (If
+ * you want an absolute path but don't mind links, use
+ * absolute_path().)  The return value is a pointer to a static
+ * buffer.
+ *
+ * The input and all intermediate paths must be shorter than MAX_PATH.
+ * The directory part of path (i.e., everything up to the last
+ * dir_sep) must denote a valid, existing directory, but the last
+ * component need not exist.  If die_on_error is set, then die with an
+ * informative error message if there is a problem.  Otherwise, return
+ * NULL on errors (without generating any output).
  *
  * If path is our buffer, then return path, as it's already what the
  * user wants.
  */
-const char *real_path(const char *path)
+static const char *real_path_internal(const char *path, int die_on_error)
 {
 	static char bufs[2][PATH_MAX + 1], *buf = bufs[0], *next_buf = bufs[1];
+	char *retval = NULL;
 	char cwd[1024] = "";
 	int buf_index = 1;
 
@@ -35,11 +46,19 @@ const char *real_path(const char *path)
 	if (path == buf || path == next_buf)
 		return path;
 
-	if (!*path)
-		die("The empty string is not a valid path");
+	if (!*path) {
+		if (die_on_error)
+			die("The empty string is not a valid path");
+		else
+			goto error_out;
+	}
 
-	if (strlcpy(buf, path, PATH_MAX) >= PATH_MAX)
-		die ("Too long path: %.*s", 60, path);
+	if (strlcpy(buf, path, PATH_MAX) >= PATH_MAX) {
+		if (die_on_error)
+			die("Too long path: %.*s", 60, path);
+		else
+			goto error_out;
+	}
 
 	while (depth--) {
 		if (!is_directory(buf)) {
@@ -54,20 +73,36 @@ const char *real_path(const char *path)
 		}
 
 		if (*buf) {
-			if (!*cwd && !getcwd(cwd, sizeof(cwd)))
-				die_errno ("Could not get current working directory");
+			if (!*cwd && !getcwd(cwd, sizeof(cwd))) {
+				if (die_on_error)
+					die_errno("Could not get current working directory");
+				else
+					goto error_out;
+			}
 
-			if (chdir(buf))
-				die_errno ("Could not switch to '%s'", buf);
+			if (chdir(buf)) {
+				if (die_on_error)
+					die_errno("Could not switch to '%s'", buf);
+				else
+					goto error_out;
+			}
+		}
+		if (!getcwd(buf, PATH_MAX)) {
+			if (die_on_error)
+				die_errno("Could not get current working directory");
+			else
+				goto error_out;
 		}
-		if (!getcwd(buf, PATH_MAX))
-			die_errno ("Could not get current working directory");
 
 		if (last_elem) {
 			size_t len = strlen(buf);
-			if (len + strlen(last_elem) + 2 > PATH_MAX)
-				die ("Too long path name: '%s/%s'",
-						buf, last_elem);
+			if (len + strlen(last_elem) + 2 > PATH_MAX) {
+				if (die_on_error)
+					die("Too long path name: '%s/%s'",
+					    buf, last_elem);
+				else
+					goto error_out;
+			}
 			if (len && !is_dir_sep(buf[len-1]))
 				buf[len++] = '/';
 			strcpy(buf + len, last_elem);
@@ -77,10 +112,18 @@ const char *real_path(const char *path)
 
 		if (!lstat(buf, &st) && S_ISLNK(st.st_mode)) {
 			ssize_t len = readlink(buf, next_buf, PATH_MAX);
-			if (len < 0)
-				die_errno ("Invalid symlink '%s'", buf);
-			if (PATH_MAX <= len)
-				die("symbolic link too long: %s", buf);
+			if (len < 0) {
+				if (die_on_error)
+					die_errno("Invalid symlink '%s'", buf);
+				else
+					goto error_out;
+			}
+			if (PATH_MAX <= len) {
+				if (die_on_error)
+					die("symbolic link too long: %s", buf);
+				else
+					goto error_out;
+			}
 			next_buf[len] = '\0';
 			buf = next_buf;
 			buf_index = 1 - buf_index;
@@ -89,10 +132,18 @@ const char *real_path(const char *path)
 			break;
 	}
 
+	retval = buf;
+error_out:
+	free(last_elem);
 	if (*cwd && chdir(cwd))
 		die_errno ("Could not change back to '%s'", cwd);
 
-	return buf;
+	return retval;
+}
+
+const char *real_path(const char *path)
+{
+	return real_path_internal(path, 1);
 }
 
 static const char *get_pwd_cwd(void)
-- 
1.7.11.3

^ permalink raw reply related

* [PATCH v3 0/8] Fix GIT_CEILING_DIRECTORIES that contain symlinks
From: Michael Haggerty @ 2012-10-21  5:57 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Jiang Xin, Lea Wiemann, David Reiss, Johannes Sixt, git,
	Michael Haggerty

v3 of the series, reworked WRT v2:

* Change longest_ancestor_length() to take a string_list for its
  prefixes argument (instead of a PATH_SEP-separated string).

* Move the responsibility for canonicalizing prefixes from
  longest_ancestor_length() to its caller in
  setup_git_directory_gently_1().  This keeps
  longest_ancestor_length() testable, though the test inputs now have
  to be canonicalized.

* Remove function string_list_longest_prefix(), which was mainly
  intended to be used in the implementation of
  longest_ancestor_length() but is now unneeded.

Thanks to Junio for his comments.

I would like to explicitly point out a possible objection to this
whole enterprise.  GIT_CEILING_DIRECTORIES is used to prevent git from
mucking around in certain directories, to avoid expensive accesses
(for example of remote directories).  The original motivation for the
feature was a user whose home directory /home/$USER was automounted.
When git was invoked outside of a git tree, it would crawl up the
filesystem tree, eventually reaching /home, and then try to access the
paths

    /home
    /home/.git
    /home/.git/objects
    /home/objects

The last three accesses would be very expensive because the system
would attempt to automount the entries listed.

This patch series has the side effect that all of the directories
listed in GIT_CEILING_DIRECTORIES are accessed *unconditionally* to
resolve any symlinks that are present in their paths.  It is
admittedly odd that a feature intended to avoid accessing expensive
directories would now *intentionally* access directories near the
expensive ones.  In the above scenario this shouldn't be a problem,
because /home would be the directory listed in
GIT_CEILING_DIRECTORIES, and accessing /home itself shouldn't be
expensive.  But there might be other scenarios for which this patch
series causes a performance regression.

Another point: After this change, it would be trivially possible to
support non-absolute paths in GIT_CEILING_DIRECTORIES; just remove the
call to is_absolute_path() from normalize_ceiling_entry().  This might
be convenient for the test suite.

Michael Haggerty (8):
  Introduce new static function real_path_internal()
  real_path_internal(): add comment explaining use of cwd
  Introduce new function real_path_if_valid()
  longest_ancestor_length(): use string_list_split()
  longest_ancestor_length(): take a string_list argument for prefixes
  longest_ancestor_length(): require prefix list entries to be
    normalized
  normalize_ceiling_entry(): resolve symlinks
  string_list_longest_prefix(): remove function

 Documentation/technical/api-string-list.txt |   8 ---
 abspath.c                                   | 105 ++++++++++++++++++++++------
 cache.h                                     |   3 +-
 path.c                                      |  46 ++++++------
 setup.c                                     |  32 ++++++++-
 string-list.c                               |  20 ------
 string-list.h                               |   8 ---
 t/t0060-path-utils.sh                       |  41 ++++-------
 t/t0063-string-list.sh                      |  30 --------
 test-path-utils.c                           |   8 ++-
 test-string-list.c                          |  20 ------
 11 files changed, 157 insertions(+), 164 deletions(-)

-- 
1.7.11.3

^ permalink raw reply

* Re: What's cooking in git.git (Oct 2012, #06; Fri, 19)
From: Junio C Hamano @ 2012-10-21  5:57 UTC (permalink / raw)
  To: Nguyen Thai Ngoc Duy; +Cc: git
In-Reply-To: <CACsJy8A2PdG69hB1=YgHMAdibO=7_Uu5qvmyAqcrhdBVWy761g@mail.gmail.com>

Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:

>>  I suspect that this needs to be plugged to pathspec matching code;
>>  otherwise "git log -- 'Docum*/**/*.txt'" would not show the log for
>>  commits that touch Documentation/git.txt, which would be confusing
>>  to the users.
>
> I do want non-recursive "*" in pathspec and "**" can help retain the
> recursive "*" semantics. But can we just flip the coin at some point
> and change "*" semantics in pathspec from recursive to non-recursive?

Fair enough; that indeed is a valid concern.  Something like
":(glob)" magic may be necessary as an early step.  Longer term
(like Git 3.0 where we might say "screw the existing users" and
redesign "if we were doing Git from scratch"), it would be nice to
have excludes, attributes and pathspecs all share the same syntax,
though.

^ permalink raw reply

* Re: What's cooking in git.git (Oct 2012, #06; Fri, 19)
From: Nguyen Thai Ngoc Duy @ 2012-10-21  4:24 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vmwzii37w.fsf@alter.siamese.dyndns.org>

On Sat, Oct 20, 2012 at 4:03 AM, Junio C Hamano <gitster@pobox.com> wrote:
> * nd/wildmatch (2012-10-15) 13 commits
>
>  Allows pathname patterns in .gitignore and .gitattributes files
>  with double-asterisks "foo/**/bar" to match any number of directory
>  hierarchies.
>
>  I suspect that this needs to be plugged to pathspec matching code;
>  otherwise "git log -- 'Docum*/**/*.txt'" would not show the log for
>  commits that touch Documentation/git.txt, which would be confusing
>  to the users.

I do want non-recursive "*" in pathspec and "**" can help retain the
recursive "*" semantics. But can we just flip the coin at some point
and change "*" semantics in pathspec from recursive to non-recursive?

I have no problem with that but then we might want to change other
places that use fnmatch without FNM_PATHNAME too, e.g. apply, branch,
for-each-ref... Or we could go with new syntax
":(glob)Docu*/**/*.txt". A bit ugly.
-- 
Duy

^ permalink raw reply

* Re: Report a bug, about track remote repository.
From: Cheeray Huang @ 2012-10-21  2:44 UTC (permalink / raw)
  To: Philip Oakley; +Cc: git
In-Reply-To: <F15116C3B49A439AB06C8068F79E4C0F@PhilipOakley>

On 2012年10月21日 07:46, Philip Oakley wrote:
>
> At this point you have created the 'conflict' - You can't have two 
> different branches that both track the same identical remote branch 
> and expect that they can be both different and identical at the same 
> time.

> Only one push (from two branches trying) can suceed. You either force 
> the remote to match the current branch, and loose any information that 
> it had about the other branch, or the remote stays with one branch. 
> Simply don't do it [that way]. If the local branches are different, 
> then you need distinct remote branches.

Yes, if it will lead a conflict, I think git should give a warning for 
it and prevent these operations rather than give a comment to inform you 
to push something.



-- 
Best Regards!

Qiyu Huang( Cheeray )

^ permalink raw reply

* Re: looking for suggestions for managing a tree of server configs
From: david @ 2012-10-21  2:34 UTC (permalink / raw)
  To: Drew Northup; +Cc: Junio C Hamano, git
In-Reply-To: <CAM9Z-nmHxyqnyq1fChhv7hP_awgsaO2FT1t29PAwrvZkaA-hgg@mail.gmail.com>

On Sat, 20 Oct 2012, Drew Northup wrote:

> On Sun, Oct 14, 2012 at 12:57 AM,  <david@lang.hm> wrote:
>> On Sat, 13 Oct 2012, Junio C Hamano wrote:
>>> david@lang.hm writes:
>>>> I've got a directory tree that holds config data for all my
>>>> servers. This consists of one directory per server (which is updated
>>>> periodically from what is currently configured on that server), plus
>>>> higher level summary reports and similar information.
>>>>
>>>> today I have just a single git tree covering everything, and I make a
>>>> commit each time one of the per-server directories is updated, and
>>>> again when the top-level stuff is created.
>>>
>>> It is quite clear to me what you are keeping at the top-level files,
>>> but if a large portion of the configuration for these servers are
>>> shared, it might not be a bad idea to have a canonical "gold-master"
>>> configuration branch, to which the shared updates are applied, with
>>> a branch per server that forks from that canonical branch to keep
>>> the machine specific tweaks as differences from the canonical stuff,
>>> instead of having N subdirectories (one per machine).
>>
>> In an ideal world yes, but right now these machines are updated by many
>> different tools (unforuntantly including 'vi'), so these directories aren't
>> the config to be pushed out to the boxes (i.e. what they should be), it's
>> instead an archived 'what is', the result of changes from all the tools.
>>
>> The systems are all built with a standard image, but the automation tools I
>> do have tend to push identical files out to many of the systems (or files
>> identical except for a couple of lines)
>
> David,
> Is there any particular reason you aren't using etckeeper?

not really, I've thought of that as a tool for managing a single system. 
Some of the data in configs is sensitive (and much of it is not in /etc), 
but I guess I should be able to work around those issues.

I can e-mail 'patches' to the central server, but I'm then back to the 
same question that I started out with.

How can I sanely organize all these different, but similar sets of files 
on the central server?

David Lang

^ permalink raw reply

* [PATCH] Fix git p4 sync errors
From: Matt Arsenault @ 2012-10-21  1:59 UTC (permalink / raw)
  To: git

>From 425e4dc6992d07aa00039c5bb8e8c76def591fd3 Mon Sep 17 00:00:00 2001
From: Matt Arsenault <arsenm2@gmail.com>
Date: Sat, 20 Oct 2012 18:48:45 -0700
Subject: [PATCH] git-p4: Fix not using -s option to describe

This solves errors in some cases when syncing renamed files.
Other places where describe is used use the -s, except this one.

Signed-off-by: Matt Arsenault <arsenm2@gmail.com>
---
 git-p4.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/git-p4.py b/git-p4.py
index 882b1bb..e203508 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -2543,7 +2543,7 @@ class P4Sync(Command, P4UserMap):
     def importChanges(self, changes):
         cnt = 1
         for change in changes:
-            description = p4Cmd(["describe", str(change)])
+            description = p4Cmd(["describe", "-s", str(change)])
             self.updateOptionDict(description)
 
             if not self.silent:
-- 
1.7.12.2

^ permalink raw reply related

* Re: looking for suggestions for managing a tree of server configs
From: Drew Northup @ 2012-10-21  1:50 UTC (permalink / raw)
  To: david; +Cc: Junio C Hamano, git
In-Reply-To: <alpine.DEB.2.02.1210132153040.6253@asgard.lang.hm>

On Sun, Oct 14, 2012 at 12:57 AM,  <david@lang.hm> wrote:
> On Sat, 13 Oct 2012, Junio C Hamano wrote:
>> david@lang.hm writes:
>>> I've got a directory tree that holds config data for all my
>>> servers. This consists of one directory per server (which is updated
>>> periodically from what is currently configured on that server), plus
>>> higher level summary reports and similar information.
>>>
>>> today I have just a single git tree covering everything, and I make a
>>> commit each time one of the per-server directories is updated, and
>>> again when the top-level stuff is created.
>>
>> It is quite clear to me what you are keeping at the top-level files,
>> but if a large portion of the configuration for these servers are
>> shared, it might not be a bad idea to have a canonical "gold-master"
>> configuration branch, to which the shared updates are applied, with
>> a branch per server that forks from that canonical branch to keep
>> the machine specific tweaks as differences from the canonical stuff,
>> instead of having N subdirectories (one per machine).
>
> In an ideal world yes, but right now these machines are updated by many
> different tools (unforuntantly including 'vi'), so these directories aren't
> the config to be pushed out to the boxes (i.e. what they should be), it's
> instead an archived 'what is', the result of changes from all the tools.
>
> The systems are all built with a standard image, but the automation tools I
> do have tend to push identical files out to many of the systems (or files
> identical except for a couple of lines)

David,
Is there any particular reason you aren't using etckeeper?

-- 
-Drew Northup
--------------------------------------------------------------
"As opposed to vegetable or mineral error?"
-John Pescatore, SANS NewsBites Vol. 12 Num. 59

^ permalink raw reply

* Re: Report a bug, about track remote repository.
From: Philip Oakley @ 2012-10-20 23:46 UTC (permalink / raw)
  To: Cheeray Huang, git
In-Reply-To: <5082F255.9060600@gmail.com>

From: "Cheeray Huang" <cheeray.huang@gmail.com>
> Hi,
>
> I think I found a bug, when I used local branches to track remote 
> branch. But I'm not very sure, can anyone double check this?  I'd like 
> to finger this out. I think you can reproduce this bug as below steps:
>
> precondition:
>
> Suppose that you have a remote branch in repository, named 
> origin/work. And then you want to track it with a local branch.
>
> Steps:
>
> 1. So you can do this:
>
> git checkout -t origin/work
>
> now, you have a local branch also named "work" to track "origin/work".
> It works nicely, you can use "push/pull" command without any detail 
> parameters to sync anything with the remote branch.
>
> 2. Create another branch, ex. named "work2", to track "origin/work" 
> again, though maybe there are not so many people that will do like 
> this.

At this point you have created the 'conflict' - You can't have two 
different branches that both track the same identical remote branch and 
expect that they can be both different and identical at the same time.

>
> You will find that local branch "work2" can't "push" to "origin/work".
> ex. After you committed something in work2, you typed "git status", 
> git would tell you:
>
> Your branch is ahead of 'origin/work' by x commit.
>
> And then you used "git push", git can't display the information about 
> changing hash value in remote branch, just printed "everything is up 
> to date".

Only one push (from two branches trying) can suceed. You either force 
the remote to match the current branch, and loose any information that 
it had about the other branch, or the remote stays with one branch. 
Simply don't do it [that way]. If the local branches are different, then 
you need distinct remote branches.

>
> Actually, you can use some exact parameters to solve this,  such as:
>
> git push origin work2:work
>
> But, I still think it is a bug.
>
> BTW, I found this bug when I used github. I don't know whether it is 
> related to github.
>
>
> -- 
> B&R,
> Cheeray

^ permalink raw reply

* Re: Subtree in Git
From: Herman van Rink @ 2012-10-20 20:03 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: dag, greened, Hilco Wijbenga, Git Users
In-Reply-To: <nng4npe6zsj.fsf@transit.us.cray.com>

On 07/11/2012 06:14 PM, dag@cray.com wrote:
> Herman van Rink <rink@initfour.nl> writes:
>
>>> It's hard to tell what's what with one big diff.  Each command should
>>> get its own commit plus more if infrastructure work has to be done.  I
>>> realize it's a bit of a pain to reformulate this but git rebase -i makes
>>> it easy and the history will be much better long-term.
>>>
>>> Each command should be described briefly in the commit log.
>> That would indeed be nice, but as some parts interdependent it would be
>> rather complicated.
> Do the interdependent parts first, then.  These should be pure
> infrastructure.
>
>> And what is the use if their not fully independently testable.
> The command should be testable as soon as they are fully implemented,
> no?
>
> I'm thinking about a sequence like this:
>
> - Infrastructure for command A (and possibly B, C, etc. if they are
>   interdependent).
> - Command A + tests
> - Infrastructure for command B
> - Command B + tests
> - etc.
>
>> If you want to fake a nice history tree then go ahead, I just don't have
>> the energy to go through these commits again just for that.
> Well, I can't do this either, both because it would take time to get up
> to speed on the patches and because I have a million other things going
> on at the moment.  So unfortunately, this is going to sit until someone
> can take it up.
>
> Unless Junio accepts your patches, of course.  :)

Junio, Could you please consider merging the single commit from my
subtree-updates branch? https://github.com/helmo/git/tree/subtree-updates

I've seen a few reactions on the git userlist refer to issues which have
long been solved in these collected updates.


>
>>> Some questions/comments:
>>>
>>> - Is .gittrees the right solution?  I like the feature it provides but
>>>   an external file feels a bit hacky.  I wonder if there is a better way
>>>   to track this metadata.  Notes maybe?  Other git experts will have to
>>>   chime in with suggestions.
>> It's similar to what git submodule does. And when you add this file to
>> the index you can use it on other checkouts as well.
> Well, I guess I'm not strongly opposed, I was just asking the question.
>
>>> - This code seems to be repeated a lot.  Maybe it should be a utility
>>>   function.
>> Yes that's there three times...
> So you agree it should be factored?
>
>>> - I removed all this stuff in favor of the test library.  Please don't
>>>   reintroduce it.  These new tests will have to be rewritten in terms of
>>>   the existing test infrastructure.  It's not too hard.
>> I've left it in to be able to verify your new tests. Once all the new
>> tests are passing we can get rid of the old one, not before.
>> And as all the old tests are contained in test.sh it should not interfere...
> No, I'm very strongly against putting this back in.  The new tests will
> have to be updated to the upstream test infrastructure.
>
>                                       -Dave
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>


-- 

Met vriendelijke groet / Regards,

Herman van Rink
Initfour websolutions

^ 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