* [PATCH v4 6/8] longest_ancestor_length(): require prefix list entries to be normalized
From: Michael Haggerty @ 2012-10-28 16:16 UTC (permalink / raw)
To: Jeff King
Cc: Junio C Hamano, Jiang Xin, Lea Wiemann, Johannes Sixt, git,
Michael Haggerty
In-Reply-To: <1351440987-26636-1-git-send-email-mhagger@alum.mit.edu>
Move the responsibility for normalizing prefixes from
longest_ancestor_length() to its callers. Use slightly different
normalizations at the two callers:
In setup_git_directory_gently_1(), use the old normalization, which
ignores paths that are not usable. In the next commit we will change
this caller to also resolve symlinks in the paths from
GIT_CEILING_DIRECTORIES as part of the normalization.
In "test-path-utils longest_ancestor_length", use the old
normalization, but die() if any paths are unusable. Also change t0060
to only pass normalized paths to the test program (no empty entries or
non-absolute paths, strip trailing slashes from the paths, and remove
tests that thereby become redundant).
The point of this change is to reduce the scope of the ancestor_length
tests in t0060 from testing normalization+longest_prefix to testing
only mostly longest_prefix. This is necessary because when
setup_git_directory_gently_1() starts resolving symlinks as part of
its normalization, it will not be reasonable to do the same in the
test suite, because that would make the test results depend on the
contents of the root directory of the filesystem on which the test is
run. HOWEVER: under Windows, bash mangles arguments that look like
absolute POSIX paths into DOS paths. So we have to retain the level
of normalization done by normalize_path_copy() to convert the
bash-mangled DOS paths (which contain backslashes) into paths that use
forward slashes.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
path.c | 26 +++++++++++---------------
setup.c | 23 +++++++++++++++++++++++
t/t0060-path-utils.sh | 41 +++++++++++++----------------------------
test-path-utils.c | 45 ++++++++++++++++++++++++++++++++++++++++++++-
4 files changed, 91 insertions(+), 44 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 \
diff --git a/test-path-utils.c b/test-path-utils.c
index acb0560..0092cbf 100644
--- a/test-path-utils.c
+++ b/test-path-utils.c
@@ -1,6 +1,33 @@
#include "cache.h"
#include "string-list.h"
+/*
+ * A "string_list_each_func_t" function that normalizes an entry from
+ * GIT_CEILING_DIRECTORIES. If the path is unusable for some reason,
+ * die with an explanation.
+ */
+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)
+ die("Empty path is not supported");
+ if (len > PATH_MAX)
+ die("Path \"%s\" is too long", ceil);
+ if (!is_absolute_path(ceil))
+ die("Path \"%s\" is not absolute", ceil);
+ if (normalize_path_copy(buf, ceil) < 0)
+ die("Path \"%s\" could not be normalized", ceil);
+ len = strlen(buf);
+ if (len > 1 && buf[len-1] == '/')
+ die("Normalized path \"%s\" ended with slash", buf);
+ free(item->string);
+ item->string = xstrdup(buf);
+ return 1;
+}
+
int main(int argc, char **argv)
{
if (argc == 3 && !strcmp(argv[1], "normalize_path_copy")) {
@@ -33,10 +60,26 @@ int main(int argc, char **argv)
if (argc == 4 && !strcmp(argv[1], "longest_ancestor_length")) {
int len;
struct string_list ceiling_dirs = STRING_LIST_INIT_DUP;
+ char *path = xstrdup(argv[2]);
+ /*
+ * We have to normalize the arguments because under
+ * Windows, bash mangles arguments that look like
+ * absolute POSIX paths or colon-separate lists of
+ * absolute POSIX paths into DOS paths (e.g.,
+ * "/foo:/foo/bar" might be converted to
+ * "D:\Src\msysgit\foo;D:\Src\msysgit\foo\bar"),
+ * whereas longest_ancestor_length() requires paths
+ * that use forward slashes.
+ */
+ if (normalize_path_copy(path, path))
+ die("Path \"%s\" could not be normalized", argv[2]);
string_list_split(&ceiling_dirs, argv[3], PATH_SEP, -1);
- len = longest_ancestor_length(argv[2], &ceiling_dirs);
+ filter_string_list(&ceiling_dirs, 0,
+ normalize_ceiling_entry, NULL);
+ len = longest_ancestor_length(path, &ceiling_dirs);
string_list_clear(&ceiling_dirs, 0);
+ free(path);
printf("%d\n", len);
return 0;
}
--
1.8.0
^ permalink raw reply related
* [PATCH v4 7/8] setup_git_directory_gently_1(): resolve symlinks in ceiling paths
From: Michael Haggerty @ 2012-10-28 16:16 UTC (permalink / raw)
To: Jeff King
Cc: Junio C Hamano, Jiang Xin, Lea Wiemann, Johannes Sixt, git,
Michael Haggerty
In-Reply-To: <1351440987-26636-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(). (Also rename
normalize_ceiling_entry() to canonicalize_ceiling_entry() to reflect
the change.)
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.
Please note that test t0060 is *not* changed analogously, because that
would make the test suite results dependent on the contents of the
local root directory. However, real_path() is already tested
independently, and the "ancestor" tests cover the non-normalization
aspects of longest_ancestor_length(), so coverage remains sufficient.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
setup.c | 26 +++++++++++++-------------
1 file changed, 13 insertions(+), 13 deletions(-)
diff --git a/setup.c b/setup.c
index df97ad3..f108c4b 100644
--- a/setup.c
+++ b/setup.c
@@ -622,24 +622,23 @@ 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)
+static int canonicalize_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;
}
@@ -681,7 +680,8 @@ 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);
+ filter_string_list(&ceiling_dirs, 0,
+ canonicalize_ceiling_entry, NULL);
ceil_offset = longest_ancestor_length(cwd, &ceiling_dirs);
string_list_clear(&ceiling_dirs, 0);
}
--
1.8.0
^ permalink raw reply related
* [PATCH v4 8/8] string_list_longest_prefix(): remove function
From: Michael Haggerty @ 2012-10-28 16:16 UTC (permalink / raw)
To: Jeff King
Cc: Junio C Hamano, Jiang Xin, Lea Wiemann, Johannes Sixt, git,
Michael Haggerty
In-Reply-To: <1351440987-26636-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.8.0
^ permalink raw reply related
* [PATCH] parse_dirstat_params(): use string_list to split comma-separated string
From: Michael Haggerty @ 2012-10-28 16:50 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, git, Michael Haggerty
Use string_list_split_in_place() to split the comma-separated
parameters string. This simplifies the code and also fixes a bug: the
old code made calls like
memcmp(p, "lines", p_len)
which needn't work if p_len is different than the length of the
constant string (and could illegally access memory if p_len is larger
than the length of the constant string).
When p_len was less than the length of the constant string, the old
code would have allowed some abbreviations to be accepted (e.g., "cha"
for "changes") but this seems to have been a bug rather than a
feature, because (1) it is not documented; (2) no attempt was made to
handle ambiguous abbreviations, like "c" for "changes" vs
"cumulative".
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
diff.c | 40 +++++++++++++++++++++-------------------
1 file changed, 21 insertions(+), 19 deletions(-)
diff --git a/diff.c b/diff.c
index 35d3f07..a9cc8a9 100644
--- a/diff.c
+++ b/diff.c
@@ -15,6 +15,7 @@
#include "sigchain.h"
#include "submodule.h"
#include "ll-merge.h"
+#include "string-list.h"
#ifdef NO_FAST_WORKING_DIRECTORY
#define FAST_WORKING_DIRECTORY 0
@@ -68,26 +69,30 @@ static int parse_diff_color_slot(const char *var, int ofs)
return -1;
}
-static int parse_dirstat_params(struct diff_options *options, const char *params,
+static int parse_dirstat_params(struct diff_options *options, const char *params_string,
struct strbuf *errmsg)
{
- const char *p = params;
- int p_len, ret = 0;
+ char *params_copy = xstrdup(params_string);
+ struct string_list params = STRING_LIST_INIT_NODUP;
+ int ret = 0;
+ int i;
- while (*p) {
- p_len = strchrnul(p, ',') - p;
- if (!memcmp(p, "changes", p_len)) {
+ if (*params_copy)
+ string_list_split_in_place(¶ms, params_copy, ',', -1);
+ for (i = 0; i < params.nr; i++) {
+ const char *p = params.items[i].string;
+ if (!strcmp(p, "changes")) {
DIFF_OPT_CLR(options, DIRSTAT_BY_LINE);
DIFF_OPT_CLR(options, DIRSTAT_BY_FILE);
- } else if (!memcmp(p, "lines", p_len)) {
+ } else if (!strcmp(p, "lines")) {
DIFF_OPT_SET(options, DIRSTAT_BY_LINE);
DIFF_OPT_CLR(options, DIRSTAT_BY_FILE);
- } else if (!memcmp(p, "files", p_len)) {
+ } else if (!strcmp(p, "files")) {
DIFF_OPT_CLR(options, DIRSTAT_BY_LINE);
DIFF_OPT_SET(options, DIRSTAT_BY_FILE);
- } else if (!memcmp(p, "noncumulative", p_len)) {
+ } else if (!strcmp(p, "noncumulative")) {
DIFF_OPT_CLR(options, DIRSTAT_CUMULATIVE);
- } else if (!memcmp(p, "cumulative", p_len)) {
+ } else if (!strcmp(p, "cumulative")) {
DIFF_OPT_SET(options, DIRSTAT_CUMULATIVE);
} else if (isdigit(*p)) {
char *end;
@@ -99,24 +104,21 @@ static int parse_dirstat_params(struct diff_options *options, const char *params
while (isdigit(*++end))
; /* nothing */
}
- if (end - p == p_len)
+ if (!*end)
options->dirstat_permille = permille;
else {
- strbuf_addf(errmsg, _(" Failed to parse dirstat cut-off percentage '%.*s'\n"),
- p_len, p);
+ strbuf_addf(errmsg, _(" Failed to parse dirstat cut-off percentage '%s'\n"),
+ p);
ret++;
}
} else {
- strbuf_addf(errmsg, _(" Unknown dirstat parameter '%.*s'\n"),
- p_len, p);
+ strbuf_addf(errmsg, _(" Unknown dirstat parameter '%s'\n"), p);
ret++;
}
- p += p_len;
-
- if (*p)
- p++; /* more parameters, swallow separator */
}
+ string_list_clear(¶ms, 0);
+ free(params_copy);
return ret;
}
--
1.8.0
^ permalink raw reply related
* Re: git push tags
From: Johannes Sixt @ 2012-10-28 18:15 UTC (permalink / raw)
To: Angelo Borsotti; +Cc: git
In-Reply-To: <CAB9Jk9DMOwhDf3SvMzTmTZiyZg_4pgXx-evrfWkB3U4w-KqtVw@mail.gmail.com>
Am 25.10.2012 08:58, schrieb Angelo Borsotti:
> Hello,
>
> git push tag updates silently the specified tag. E.g.
>
> git init --bare release.git
> git clone release.git integrator
> cd integrator
> git branch -avv
> touch f1; git add f1; git commit -m A
> git tag v1
> git push origin tag v1
> touch f2; git add f2; git commit -m B
> git tag -f v1
> git push origin tag v1
>
> the second git push updates the tag in the remote repository. This is
> somehow counterintuitive because tags normally do not move (unless
> forced to that), and is not documented.
Tags are refs, just like branches. "Tags don't move" is just a
convention, and git doesn't even respect it (except possibly in one
place[1]). You can't reseat tags unless you use -f, which is exactly the
same with branches, which you can't reseat unless you use -f.
[1] By default, git fetch does not fetch tags that it already has.
> This is also harmful because it allows to change silently something
> (tags) that normally must not change.
You asked git push to push a tag, and the tag was pushed. What's wrong?
-- Hannes
^ permalink raw reply
* Re: crash on git diff-tree -Ganything <tree> for new files with textconv filter
From: Peter Oberndorfer @ 2012-10-28 19:56 UTC (permalink / raw)
To: Jeff King; +Cc: git, Junio C Hamano
In-Reply-To: <20121028120104.GE11434@sigill.intra.peff.net>
On 2012-10-28 13:01, Jeff King wrote:
> On Sat, Oct 27, 2012 at 08:37:24PM +0200, Peter Oberndorfer wrote:
>
>> It seems "git diff-tree -Ganything <tree>" crashes[1] with a null
>> pointer dereference
>> when run on a commit that adds a file (pdf) with a textconv filter.
>>
>> It can be reproduced with vanilla git by having a commit on top that
>> adds a file with a textconv filter and executing git diff-tree
>> -Ganything HEAD
>> But running git log -Ganything still works without a crash.
>> This problem seems to exist since the feature was first added in f506b8e8b5.
> Thanks for a thorough bug report. I didn't reproduce the crash, but I
> can see how it happens (it happens with diff-tree because we will reuse
> the working tree file in that instance; it could also happen if you
> turned on textconv caching).
>
>> While testing I also noticed the -S and -G act on the original file
>> instead of the textconv munged data.
>> Is this intentional or caused by accessing the wrong data?
> Both, perhaps. :)
Hi,
thanks for your patch for this!
>
> -G operates on the munged data; you can see it feed the munged data to
> xdiff in diff_grep. But the optimization for handling added and removed
> files accidentally fed the wrong pointer. Fixing that is a no-brainer,
> since the optimization is inconsistent with the regular code path.
>
> -S, however, predates the invention of textconv and has never used it.
> It is a little less clear that textconv is the right thing here, because
> it is not about grepping the diff, but about counting occurrences of the
> string inside the file content. I tend to think that doing so on the
> textconv'd data would be what people generally want, but it is a
> behavior change.
>
>> Wild guess: should we really access p->one->data and not mf1.ptr?
> Precisely. Thanks for your wild guess; it made finding the bug very
> easy. :)
>
>> Is there some more information i should provide?
> The patch below should fix it. I added tests, but please try your
> real-world test case on it to double-check.
I tested your patch, but now it crashes for another reason :-)
i have a file with exactly 12288(0x3000) bytes in the repository.
When the file is loaded, the data is placed luckily so the data end
falls at a page boundary.
Later diff_grep() calls regexec() which calls strlen() on the loaded buffer
and ends up reading beyond the actual data into the next page
which is not allocated and causes a pagefault.
Or it could possibly (randomly) match the regex on data that is not
actually part of a file...
Different memory allocation rules on Windows probably also have some
influence here.
My guess is that diff_filespec->data is not supposed to be zero terminated
and we should not invoke strlen() on a not zero terminated data.
But this should be decided by somebody who knows the rules.
Greetings Peter
^ permalink raw reply
* Re: git push tags
From: Chris Rorvick @ 2012-10-28 19:59 UTC (permalink / raw)
To: Johannes Sixt; +Cc: Angelo Borsotti, git
In-Reply-To: <508D7628.10509@kdbg.org>
On Sun, Oct 28, 2012 at 1:15 PM, Johannes Sixt <j6t@kdbg.org> wrote:
> Tags are refs, just like branches. "Tags don't move" is just a
> convention, and git doesn't even respect it (except possibly in one
> place[1]). You can't reseat tags unless you use -f, which is exactly the
> same with branches, which you can't reseat unless you use -f.
>
> [1] By default, git fetch does not fetch tags that it already has.
Also, git checkout <tag> puts you on a detached HEAD. This seems
pretty significant with regard to Git respecting a "tags don't move"
convention.
Chris Rorvick
^ permalink raw reply
* [PATCH] Documentation: improve the example of overriding LESS via core.pager
From: Patrick Palka @ 2012-10-28 20:12 UTC (permalink / raw)
To: git; +Cc: Patrick Palka
You can override an option set in the LESS variable by simply prefixing
the command line option with `-+`. This is more robust than the previous
example if the default LESS options are to ever change.
Signed-off-by: Patrick Palka <patrick@parcs.ath.cx>
---
Documentation/config.txt | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 11f320b..9a0544c 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -538,14 +538,14 @@ core.pager::
`LESS` variable to some other value. Alternately,
these settings can be overridden on a project or
global basis by setting the `core.pager` option.
- Setting `core.pager` has no affect on the `LESS`
+ Setting `core.pager` has no effect on the `LESS`
environment variable behaviour above, so if you want
to override git's default settings this way, you need
to be explicit. For example, to disable the S option
in a backward compatible manner, set `core.pager`
- to `less -+$LESS -FRX`. This will be passed to the
- shell by git, which will translate the final command to
- `LESS=FRSX less -+FRSX -FRX`.
+ to `less -+S`. This will be passed to the shell by
+ git, which will translate the final command to
+ `LESS=FRSX less -+S`.
core.whitespace::
A comma separated list of common whitespace problems to
--
1.7.10.4
^ permalink raw reply related
* Re: [PATCH v2] git-submodule add: Add -r/--record option.
From: Jens Lehmann @ 2012-10-28 20:48 UTC (permalink / raw)
To: W. Trevor King; +Cc: Git, Nahor, Phil Hord, Shawn O. Pearce
In-Reply-To: <20121025005307.GE801@odin.tremily.us>
Am 25.10.2012 02:53, schrieb W. Trevor King:
> On Wed, Oct 24, 2012 at 09:15:32PM +0200, Jens Lehmann wrote:
>> I still fail to see what adding that functionality to the submodule
>> command buys us (unless we also add code which really uses the branch
>> setting). What's wrong with doing a simple:
>>
>> git config -f .gitmodules submodule.<path>.branch <record_branch>
>>
>> on the command line when you want to use the branch setting for your
>> own purposes? You could easily wrap that into a helper script, no?
>
> Sure. But why maintain my own helper script if I can edit
> git-submodules.sh? It seems like a number of people are using this
> config option, and they generally store the same name in it that they
> use to create the submodule. This way I can save them time too.
But people are already using the "branch" setting in *different* ways:
Am 23.10.2012 22:55, schrieb W. Trevor King:
> As Phil pointed out, doing anything with this variable is ambiguous:
>
> On Mon, Oct 22, 2012 at 06:03:53PM -0400, Phil Hord wrote:
>> Some projects now use the 'branch' config value to record the tracking
>> branch for the submodule. Some ascribe different meaning to the
>> configuration if the value is given vs. undefined. For example, see
>> the Gerrit submodule-subscription mechanism. This change will cause
>> those workflows to behave differently than they do now.
I don't have a problem with the amount or complexity of the code being
added, But by adding that option we may be giving the impression that it
is officially sanctioned, or that it will be kept up to date by further
submodule commands. I added Shawn to the CC, maybe he can comment on how
the "branch" setting is used in Gerrit and what he thinks about adding
code to set that with "git submodule add -r <branch> ..." to core git.
^ permalink raw reply
* Re: [PATCHv3 0/2] Teach --recursive to submodule sync
From: Jens Lehmann @ 2012-10-28 21:02 UTC (permalink / raw)
To: Phil Hord; +Cc: git, phil.hord, Jeff King
In-Reply-To: <1351280683-8402-1-git-send-email-hordp@cisco.com>
Am 26.10.2012 21:44, schrieb Phil Hord:
> [PATCHv3 1/2] Teach --recursive to submodule sync
>
> Now with less noise and no redundant flags passed to the recursive call.
>
>
> [PATCHv3 2/2] Add tests for submodule sync --recursive
>
> The test remains unchanged.
Both are looking good.
Acked-By: Jens Lehmann <Jens.Lehmann@web.de>
^ permalink raw reply
* [PATCH] builtin/config.c: Fix a sparse warning
From: Ramsay Jones @ 2012-10-28 21:05 UTC (permalink / raw)
To: Jeff King; +Cc: GIT Mailing-list
Sparse issues an "Using plain integer as NULL pointer" warning while
checking a 'struct strbuf_list' initializer expression. The initial
field of the struct has pointer type, but the initializer expression
is given as '{0}'. In order to suppress the warning, we simply replace
the initializer with '{NULL}'.
Signed-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
---
Hi Jeff,
I try to catch these types of warnings while the patches are still
in the pu branch. I don't know how I missed this one, but I don't
remember your 'jk/config-ignore-duplicates' branch being in pu.
Sorry about that.
ATB,
Ramsay Jones
builtin/config.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/builtin/config.c b/builtin/config.c
index f881053..e796af4 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -165,7 +165,7 @@ static int collect_config(const char *key_, const char *value_, void *cb)
static int get_value(const char *key_, const char *regex_)
{
int ret = CONFIG_GENERIC_ERROR;
- struct strbuf_list values = {0};
+ struct strbuf_list values = {NULL};
int i;
if (use_key_regexp) {
--
1.8.0
^ permalink raw reply related
* [PATCH] pathspec.c: Fix some sparse warnings
From: Ramsay Jones @ 2012-10-28 21:09 UTC (permalink / raw)
To: Nguyen Thai Ngoc Duy; +Cc: git, Jeff King, GIT Mailing-list
Sparse issues a warning for all six external symbols defined in this
file. In order to suppress the warnings, we include the 'pathspec.h'
header file, which contains the relevant extern declarations for these
symbols.
Signed-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
---
Hi Nguyen,
I asked Adam to squash this patch into his 'as/check-ignore' branch
when he next re-rolled the branch. However, it appears that you
resubmitted that branch instead ... :-D
I don't know if this branch is ready to progress to next yet, but
could somebody (yourself, Adam or Jeff?) please squash this into
commit 1a88ae42 ("pathspec.c: move reusable code from builtin/add.c")
before it hits next.
Thanks!
ATB,
Ramsay Jones
pathspec.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/pathspec.c b/pathspec.c
index a9c5b5b..efdabe4 100644
--- a/pathspec.c
+++ b/pathspec.c
@@ -1,5 +1,6 @@
#include "cache.h"
#include "dir.h"
+#include "pathspec.h"
void validate_path(const char *prefix, const char *path)
{
--
1.8.0
^ permalink raw reply related
* Re: [PATCH v2] git-submodule add: Add -r/--record option.
From: W. Trevor King @ 2012-10-28 21:16 UTC (permalink / raw)
To: Jens Lehmann; +Cc: Git, Nahor, Phil Hord, Shawn O. Pearce
In-Reply-To: <508D9A12.6010104@web.de>
[-- Attachment #1: Type: text/plain, Size: 2764 bytes --]
On Sun, Oct 28, 2012 at 09:48:18PM +0100, Jens Lehmann wrote:
> Am 25.10.2012 02:53, schrieb W. Trevor King:
> > On Wed, Oct 24, 2012 at 09:15:32PM +0200, Jens Lehmann wrote:
> >> I still fail to see what adding that functionality to the submodule
> >> command buys us (unless we also add code which really uses the branch
> >> setting). What's wrong with doing a simple:
> >>
> >> git config -f .gitmodules submodule.<path>.branch <record_branch>
> >>
> >> on the command line when you want to use the branch setting for your
> >> own purposes? You could easily wrap that into a helper script, no?
> >
> > Sure. But why maintain my own helper script if I can edit
> > git-submodules.sh? It seems like a number of people are using this
> > config option, and they generally store the same name in it that they
> > use to create the submodule. This way I can save them time too.
>
> But people are already using the "branch" setting in *different* ways:
And they are usually storing the same string. Now, more easily. If
they want a different string, it is also easier. If they don't want
to use --record, they can do things however they were already doing
them. I don't see the problem.
> Am 23.10.2012 22:55, schrieb W. Trevor King:
> > As Phil pointed out, doing anything with this variable is ambiguous:
> >
> > On Mon, Oct 22, 2012 at 06:03:53PM -0400, Phil Hord wrote:
> >> Some projects now use the 'branch' config value to record the tracking
> >> branch for the submodule. Some ascribe different meaning to the
> >> configuration if the value is given vs. undefined. For example, see
> >> the Gerrit submodule-subscription mechanism. This change will cause
> >> those workflows to behave differently than they do now.
>
> I don't have a problem with the amount or complexity of the code being
> added, But by adding that option we may be giving the impression that it
> is officially sanctioned, or that it will be kept up to date by further
> submodule commands.
Storing something there will be officially sanctioned. Using it for
anything in particular will not be officially sanctioned. Phil's
submodule_<var-name> export in foreach will expose the variable so the
user can do whatever they think is appropriate with it, but it's still
up to the user to give the option some kind of semantic meaning.
> I added Shawn to the CC, maybe he can comment on how the "branch"
> setting is used in Gerrit and what he thinks about adding code to
> set that with "git submodule add -r <branch> ..." to core git.
Good idea.
Trevor
--
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Removing unreachable objects in the presence of broken links?
From: Geert Uytterhoeven @ 2012-10-28 21:21 UTC (permalink / raw)
To: git
Hi,
I managed to have a few missing objects in my development Linux kernel
repository, which uses another Linux kernel clone as an alternate.
Fortunately nothing is lost, as all missing objects are unreachable.
Probably they were in a branch that has been rebased, and the objects existed
for a small timespan in the alternate when I tried whether a patch created
in the development tree applied cleanly.
Is there a way to force removing unreachable objects in the presence of broken
links?
"git prune" doesn't do it, as it aborts when encountering the first
missing object.
Same with "git repack -[aA]d".
"git fsck" reported
broken link from tree 1330855dc33c1042b80d4c8ecbb6d56a19557ee8
to tree b6c8c53b804662d6a6435c62b6dec1612bfbeb46
broken link from tree f182e2fa155b9684b79ff6e17159d03d4de9a773
to blob d41f9ed0e2aba47ef62b4b4dd211b91cfe474ff8
missing blob d41f9ed0e2aba47ef62b4b4dd211b91cfe474ff8
missing tree b6c8c53b804662d6a6435c62b6dec1612bfbeb46
"git fsck --unreachable HEAD $(git for-each-ref
--format="%(objectname)" refs/heads)"
told me about lots of unreachable objects, including the tree objects that
contain the two broken links.
BTW, every time I now do a rebase that triggers a gc (after the actual rebase
operation has completed), I end up with "(no branch)", so I have to do:
git banch -D <branch>
git branch <branch>
git checkout <branch>
to get back on the branch.
This is with git version 1.7.0.4 (1:1.7.0.4-1ubuntu0.2).
Thanks!
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: Removing unreachable objects in the presence of broken links?
From: Andreas Schwab @ 2012-10-28 21:34 UTC (permalink / raw)
To: Geert Uytterhoeven; +Cc: git
In-Reply-To: <CAMuHMdUqUtDspOP2kE9wtGEr9aJHGGBG=HRomdY6NRa8gxar4A@mail.gmail.com>
Geert Uytterhoeven <geert@linux-m68k.org> writes:
> Is there a way to force removing unreachable objects in the presence of broken
> links?
Does it help to forcibly expire the reflogs?
Andreas.
--
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
"And now for something completely different."
^ permalink raw reply
* [PATCH] submodule status: remove unused orig_* variables
From: Jens Lehmann @ 2012-10-28 21:37 UTC (permalink / raw)
To: Phil Hord; +Cc: Git Mailing List, phil.hord, Jeff King
In-Reply-To: <508AE4AB.4070209@web.de>
When renaming orig_args to orig_flags in 98dbe63d (submodule: only
preserve flags across recursive status/update invocations) the call site
of the recursive cmd_status was forgotten. At that place orig_args is
still passed into the recursion, which is always empty since then. This
did not break anything because the orig_flags logic is not needed at all
when a function from the submodule script is called with eval, as that
inherits all the variables set by the option parsing done in the first
level of the recursion.
Now that we know that orig_flags and orig_args aren't needed at all,
let's just remove them from cmd_status().
Thanks-to: Phil Hord <hordp@cisco.com>
Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de>
---
Am 26.10.2012 21:29, schrieb Jens Lehmann:
> Am 26.10.2012 21:13, schrieb Phil Hord:
>> A test in t7404-submodule-foreach purports to test that
>> the --cached flag is properly noticed by --recursive calls
>> to the foreach command as it descends into nested
>> submodules. However, the test really does not perform this
>> test since the change it looks for is in a top-level
>> submodule handled by the first invocation of the command.
>> To properly test for the flag being passed to recursive
>> invocations, the change must be buried deeper in the
>> hierarchy.
>>
>> Move the change one level deeper so it properly verifies
>> the recursive machinery of the 'git submodule status'
>> command.
>
> Me thinks we should definitely do this.
And I also think this patch should go on top of Phil's patch after what
we learned so far.
I'm currently checking if we can also safely remove orig_flags from
cmd_update(). At least the test suite runs fine without it ...
git-submodule.sh | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/git-submodule.sh b/git-submodule.sh
index ab6b110..c287464 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -926,7 +926,6 @@ cmd_summary() {
cmd_status()
{
# parse $args after "submodule ... status".
- orig_flags=
while test $# -ne 0
do
case "$1" in
@@ -950,7 +949,6 @@ cmd_status()
break
;;
esac
- orig_flags="$orig_flags $(git rev-parse --sq-quote "$1")"
shift
done
@@ -990,7 +988,7 @@ cmd_status()
prefix="$displaypath/"
clear_local_git_env
cd "$sm_path" &&
- eval cmd_status "$orig_args"
+ eval cmd_status
) ||
die "$(eval_gettext "Failed to recurse into submodule path '\$sm_path'")"
fi
--
1.8.0.42.g2ea983b
^ permalink raw reply related
* Re: git push tags
From: Philip Oakley @ 2012-10-28 21:49 UTC (permalink / raw)
To: Chris Rorvick, Johannes Sixt; +Cc: Angelo Borsotti, git
In-Reply-To: <CAEUsAPYREy=CvPxy_Mzh5icVQo3=NV-AMC096Op0WWODLPH47Q@mail.gmail.com>
From: "Chris Rorvick" <chris@rorvick.com> Sent: Sunday, October 28, 2012
7:59 PM
> On Sun, Oct 28, 2012 at 1:15 PM, Johannes Sixt <j6t@kdbg.org> wrote:
>> Tags are refs, just like branches. "Tags don't move" is just a
>> convention, and git doesn't even respect it (except possibly in one
>> place[1]). You can't reseat tags unless you use -f, which is exactly
>> the
>> same with branches, which you can't reseat unless you use -f.
>>
>> [1] By default, git fetch does not fetch tags that it already has.
>
> Also, git checkout <tag> puts you on a detached HEAD. This seems
> pretty significant with regard to Git respecting a "tags don't move"
> convention.
Surely the convention is the other way around. That is, it is branches
that are _expected_ to move, hence unless you are checkout a branch
(movable) you will be on a detached head at a fixed place/sha1 [aka not
on a branch].
The checking out of a tag action doesn't make it more or less
significant.
I think Angelo's original post should be reviewed to see if the issue
can now be restated in a manner that shows up the implied conflict.
If I read it right it was where two users can tag two different commits
with the same tag name [e.g. 'Release_V3.3'] and the last person to push
wins, so anyone in the team can change what is to be the released
version!
Philip
>
^ permalink raw reply
* Re: [PATCH v2] git-submodule add: Add -r/--record option.
From: Shawn Pearce @ 2012-10-28 21:59 UTC (permalink / raw)
To: Jens Lehmann; +Cc: W. Trevor King, Git, Nahor, Phil Hord
In-Reply-To: <508D9A12.6010104@web.de>
On Sun, Oct 28, 2012 at 1:48 PM, Jens Lehmann <Jens.Lehmann@web.de> wrote:
> Am 23.10.2012 22:55, schrieb W. Trevor King:
>> As Phil pointed out, doing anything with this variable is ambiguous:
>>
>> On Mon, Oct 22, 2012 at 06:03:53PM -0400, Phil Hord wrote:
>>> Some projects now use the 'branch' config value to record the tracking
>>> branch for the submodule. Some ascribe different meaning to the
>>> configuration if the value is given vs. undefined. For example, see
>>> the Gerrit submodule-subscription mechanism. This change will cause
>>> those workflows to behave differently than they do now.
>
> I don't have a problem with the amount or complexity of the code being
> added, But by adding that option we may be giving the impression that it
> is officially sanctioned, or that it will be kept up to date by further
> submodule commands. I added Shawn to the CC, maybe he can comment on how
> the "branch" setting is used in Gerrit and what he thinks about adding
> code to set that with "git submodule add -r <branch> ..." to core git.
Looks like the Gerrit meaning is basically the same as Ævar's. Gerrit
updates the parent project as if you had done:
$ git submodule foreach 'git checkout $(git config --file
$toplevel/.gitmodules submodule.$name.branch) && git pull'
$ git commit -a -m "Updated submodules"
$ git push
and it does this automatically each time the submodule's branch is
modified by the Gerrit server.
On Tue, Oct 23, 2012 at 2:57 PM, W. Trevor King <wking@tremily.us> wrote:
> I'm not clear on what that means, but they accept special values like
> '.', so their usage is not compatible with Ævar's proposal.
"." is a special value to mean use the parent project's branch name.
So its more like this:
$ git submodule foreach 'git checkout $(git --git-dir $toplevel/.git
read-ref HEAD | sed s,^refs/heads/,,) && git pull'
$ git commit -a -m "Updated submodules"
$ git push
We use "." in Gerrit to make branching an entire forest of projects
easier. Setting up dev-fix-yy in the parent project will automatically
track dev-fix-yy in each submodule.
^ permalink raw reply
* Re: [PATCH v2] git-submodule add: Add -r/--record option.
From: W. Trevor King @ 2012-10-28 22:34 UTC (permalink / raw)
To: Shawn Pearce; +Cc: Jens Lehmann, W. Trevor King, Git, Nahor, Phil Hord
In-Reply-To: <CAJo=hJt_A5FCCcvR=sZ5Ni+-ZGq+MjxqkONbh9k+A46xBH9jzA@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1605 bytes --]
On Sun, Oct 28, 2012 at 02:59:33PM -0700, Shawn Pearce wrote:
> Looks like the Gerrit meaning is basically the same as Ævar's. Gerrit
> updates the parent project as if you had done:
>
> $ git submodule foreach 'git checkout $(git config --file
> $toplevel/.gitmodules submodule.$name.branch) && git pull'
> $ git commit -a -m "Updated submodules"
> $ git push
Ah, good, then we *are* all using the option for the same thing.
> On Tue, Oct 23, 2012 at 2:57 PM, W. Trevor King <wking@tremily.us> wrote:
> > I'm not clear on what that means, but they accept special values like
> > '.', so their usage is not compatible with Ævar's proposal.
>
> "." is a special value to mean use the parent project's branch name.
> So its more like this:
>
> $ git submodule foreach 'git checkout $(git --git-dir $toplevel/.git
> read-ref HEAD | sed s,^refs/heads/,,) && git pull'
> $ git commit -a -m "Updated submodules"
> $ git push
>
> We use "." in Gerrit to make branching an entire forest of projects
> easier. Setting up dev-fix-yy in the parent project will automatically
> track dev-fix-yy in each submodule.
Ok. If we wanted "." expansion to be a general submodule thing, it
would add a special case to Phil's submodule_<var-name> export. I
don't think such a special case would be worth the mental overhead,
but obviously the Gerrit folks think it is. I don't care either way
on this one.
Trevor
--
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH] pathspec.c: Fix some sparse warnings
From: Adam Spiers @ 2012-10-28 23:31 UTC (permalink / raw)
To: Ramsay Jones; +Cc: Nguyen Thai Ngoc Duy, Jeff King, GIT Mailing-list
In-Reply-To: <508D9EFF.3040900@ramsay1.demon.co.uk>
On Sun, Oct 28, 2012 at 9:09 PM, Ramsay Jones
<ramsay@ramsay1.demon.co.uk> wrote:
> Sparse issues a warning for all six external symbols defined in this
> file. In order to suppress the warnings, we include the 'pathspec.h'
> header file, which contains the relevant extern declarations for these
> symbols.
>
> Signed-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
> ---
>
> Hi Nguyen,
>
> I asked Adam to squash this patch into his 'as/check-ignore' branch
> when he next re-rolled the branch. However, it appears that you
> resubmitted that branch instead ... :-D
>
> I don't know if this branch is ready to progress to next yet, but
> could somebody (yourself, Adam or Jeff?) please squash this into
> commit 1a88ae42 ("pathspec.c: move reusable code from builtin/add.c")
> before it hits next.
Noted - thanks a lot Ramsay. My diary is a bit gentler this week so
I'm hoping to get some attention back on these patch series.
^ permalink raw reply
* [PATCH] Teach rm to remove submodules when given with a trailing '/'
From: Jens Lehmann @ 2012-10-28 23:28 UTC (permalink / raw)
To: Git Mailing List; +Cc: Jeff King
Doing a "git rm submod/" on a submodule results in an error:
fatal: pathspec 'submod/' did not match any files
This is really inconvenient as e.g. using TAB completion in a shell on a
submodule automatically adds the trailing '/' when it completes the path
of the submodule directory. The user has then to remove the '/' herself to
make a "git rm" succeed. Doing a "git rm -r somedir/" is working fine, so
there is no reason why that shouldn't work for submodules too.
Teach git rm to not error out when a '/' is appended to the path of a
submodule. Achieve this by chopping off trailing slashes from the path
names given if they represent directories. Add tests to make sure that
logic only applies to directories and not to files.
Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de>
---
This patch applies on top of the jl/submodule-rm branch merged into
current next.
builtin/rm.c | 7 +++++++
t/t3600-rm.sh | 17 +++++++++++++++++
2 files changed, 24 insertions(+)
diff --git a/builtin/rm.c b/builtin/rm.c
index 2aea3b5..5381d3f 100644
--- a/builtin/rm.c
+++ b/builtin/rm.c
@@ -234,6 +234,13 @@ int cmd_rm(int argc, const char **argv, const char *prefix)
if (read_cache() < 0)
die(_("index file corrupt"));
+ /* Remove trailing '/' from directories to find submodules in the index */
+ for (i = 0; i < argc; i++) {
+ size_t pathlen = strlen(argv[i]);
+ if (pathlen && is_directory(argv[i]) && (argv[i][pathlen - 1] == '/'))
+ argv[i] = xmemdupz(argv[i], pathlen - 1);
+ }
+
pathspec = get_pathspec(prefix, argv);
refresh_index(&the_index, REFRESH_QUIET, pathspec, NULL, NULL);
diff --git a/t/t3600-rm.sh b/t/t3600-rm.sh
index 97254e8..06f6384 100755
--- a/t/t3600-rm.sh
+++ b/t/t3600-rm.sh
@@ -302,6 +302,23 @@ test_expect_success 'rm removes work tree of unmodified submodules' '
test_cmp expect actual
'
+test_expect_success 'rm removes a submodule with a trailing /' '
+ git reset --hard &&
+ git submodule update &&
+ git rm submod/ &&
+ test ! -d submod &&
+ git status -s -uno --ignore-submodules=none > actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'rm fails when given a file with a trailing /' '
+ test_must_fail git rm empty/
+'
+
+test_expect_success 'rm succeeds when given a directory with a trailing /' '
+ git rm -r frotz/
+'
+
test_expect_success 'rm of a populated submodule with different HEAD fails unless forced' '
git reset --hard &&
git submodule update &&
--
1.8.0.42.g3346551
^ permalink raw reply related
* Re: git push tags
From: Drew Northup @ 2012-10-28 23:58 UTC (permalink / raw)
To: Philip Oakley
Cc: Chris Rorvick, Johannes Sixt, Angelo Borsotti, git, Kacper Kornet
In-Reply-To: <4B8097A9D6854CDFA27E7CF6574B37BA@PhilipOakley>
On Sun, Oct 28, 2012 at 5:49 PM, Philip Oakley <philipoakley@iee.org> wrote:
> If I read it right it was where two users can tag two different commits with
> the same tag name [e.g. 'Release_V3.3'] and the last person to push wins, so
> anyone in the team can change what is to be the released version!
Philip,
Please look at Kacper's patch and Angelo's response to it. He seems to
be asking that tags not be permitted to be pushed as if doing so were
a "fast-forward" update.
This weekend I was, in part, trying to figure out what the correct CC
list for that patch would be, what the documentation change would be,
what changes would need to be made to the advice, what test would need
to be included, and so on to build a proper patch bundle. All of that
while tring to keep the house from "falling in" (I've been doing some
cleaning) and prepare for the Northeastern USA Coast to be doused and
blasted by Sandy in about a day and a half. If we decide to continue
in the path that Kacper and I have stumbled upon (with Angelo's
prodding) I'd appreciate a little help putting all of this together to
mesh with the aforementioned patch. (Heck, if there's somebody better
than me to take this over I'd be game for that too...)
--
-Drew Northup
--------------------------------------------------------------
"As opposed to vegetable or mineral error?"
-John Pescatore, SANS NewsBites Vol. 12 Num. 59
^ permalink raw reply
* gitweb
From: rh @ 2012-10-28 23:56 UTC (permalink / raw)
To: git
I'm not using gitweb I was thinking about using it and was looking at the
cgi and saw this in this file:
https://github.com/git/git/blob/master/gitweb/gitweb.perl
I think I understand the intention but the outcome is wrong.
our %highlight_ext = (
# main extensions, defining name of syntax;
# see files in /usr/share/highlight/langDefs/ directory
map { $_ => $_ }
qw(py c cpp rb java css php sh pl js tex bib xml awk bat ini spec tcl sql make),
# alternate extensions, see /etc/highlight/filetypes.conf
'h' => 'c',
map { $_ => 'sh' } qw(bash zsh ksh),
map { $_ => 'cpp' } qw(cxx c++ cc),
map { $_ => 'php' } qw(php3 php4 php5 phps),
map { $_ => 'pl' } qw(perl pm), # perhaps also 'cgi'
map { $_ => 'make'} qw(mak mk),
map { $_ => 'xml' } qw(xhtml html htm),
);
I think the intent is better met with this, (the print is for show)
our %he = ();
$he{'h'} = 'c';
$he{$_} = $_ for (qw(py c cpp rb java css php sh pl js tex bib xml awk bat ini spec tcl sql make));
$he{$_} = 'cpp' for (qw(cxx c++ cc));
$he{$_} = 'php' for (qw(php3 php4 php5 phps));
$he{$_} = 'pl' for (qw(cgi perl pm));
$he{$_} = 'make' for (qw(mak mk));
$he{$_} = 'xml' for (qw(xhtml html htm));
$he{$_} = 'sh' for (qw(bash zsh ksh));
print "$he{$_} $_\n" for(sort {$he{$a} cmp $he{$b}} keys %he);
But then again maybe I misunderstood the intent. And maybe everyone's happy
with it as-is.
^ permalink raw reply
* Re: [PATCH v3 0/8] Fix GIT_CEILING_DIRECTORIES that contain symlinks
From: David Aguilar @ 2012-10-29 0:15 UTC (permalink / raw)
To: Junio C Hamano
Cc: Michael Haggerty, Jiang Xin, Lea Wiemann, David Reiss,
Johannes Sixt, git, Lars R. Damerow, Jeff King, Marc Jordan
In-Reply-To: <7v7gqkgvxe.fsf@alter.siamese.dyndns.org>
On Sat, Oct 20, 2012 at 11:51 PM, Junio C Hamano <gitster@pobox.com> wrote:
> 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.
GIT_CEILING_DIRECTORIES was always about trying to avoid
hitting them at all; they can be (busy) NFS volumes there.
Here's the description from the 1.6.0 release notes:
* A new environment variable GIT_CEILING_DIRECTORIES can be used to stop
the discovery process of the toplevel of working tree; this may be useful
when you are working in a slow network disk and are outside any working tree,
as bash-completion and "git help" may still need to run in these places.
In 8030e44215fe8f34edd57d711a35f2f0f97a0423 Lars added
GIT_ONE_FILESYSTEM to fix a related issue.
Do you guys have GIT_CEILING_DIRECTORIES set too?
We use GIT_CEILING_DIRECTORIES and I'm pretty sure
we don't want every git command hitting them, so this would
be a regression when seen from the POV of our current usage
of this variable, which would be a bummer.
Is there another way to accomplish this without the performance hit?
Maybe something that can be solved with configuration?
I'd be happy to lend a hand if you guys have some ideas
on how we can help keep it fast. Thoughts?
Original patches for those just joining us:
http://thread.gmane.org/gmane.comp.version-control.git/208102
--
David
^ permalink raw reply
* Re: [PATCH v3 0/8] Fix GIT_CEILING_DIRECTORIES that contain symlinks
From: Junio C Hamano @ 2012-10-29 1:42 UTC (permalink / raw)
To: David Aguilar
Cc: Michael Haggerty, Jiang Xin, Lea Wiemann, David Reiss,
Johannes Sixt, git, Lars R. Damerow, Jeff King, Marc Jordan
In-Reply-To: <CAJDDKr4ki+NjSeuZpAU6bM=YAQ_3mdHCtawstdCqe9Ewvp=arQ@mail.gmail.com>
David Aguilar <davvid@gmail.com> wrote:
>Is there another way to accomplish this without the performance hit?
Perhaps not canonicalize elements on the CEILING list ourselves? If we make it a user error to put symlinked alias in the variable, and document it clearly, wouldn't it suffice?
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox