[PATCH v6 00/10] Add --graft option to git replace
Here is a small series to implement: git replace [-f] --graft commit [parent...] This patch series goes on top of the patch series that implements --edit. The changes since v5, thanks to Junio, are: - new patch 1/10 to clean up redirection style in t6050 - new patches 8/10, 9/10 and 10/10 to check mergetags - add functions to test parents in patch 3/10 and 7/10 - improve testing signed commits in patch 7/10 - improve warning when removing commit signature in patch 6/10 Christian Couder (10): replace: cleanup redirection style in tests replace: add --graft option replace: add test for --graft Documentation: replace: add --graft option contrib: add convert-grafts-to-replace-refs.sh replace: remove signature when using --graft replace: add test for --graft with signed commit commit: add for_each_mergetag() replace: check mergetags when using --graft replace: add test for --graft with a mergetag Documentation/git-replace.txt | 10 +++ builtin/replace.c | 126 +++- commit.c | 47 +++ commit.h | 7 ++ contrib/convert-grafts-to-replace-refs.sh | 28 +++ log-tree.c| 15 +--- t/t6050-replace.sh| 135 -- 7 files changed, 332 insertions(+), 36 deletions(-) create mode 100755 contrib/convert-grafts-to-replace-refs.sh -- 2.0.0.421.g786a89d.dirty -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v6 01/10] replace: cleanup redirection style in tests
Signed-off-by: Christian Couder chrisc...@tuxfamily.org --- t/t6050-replace.sh | 48 1 file changed, 24 insertions(+), 24 deletions(-) diff --git a/t/t6050-replace.sh b/t/t6050-replace.sh index 68b3cb2..fb07ad2 100755 --- a/t/t6050-replace.sh +++ b/t/t6050-replace.sh @@ -27,36 +27,36 @@ HASH6= HASH7= test_expect_success 'set up buggy branch' ' - echo line 1 hello - echo line 2 hello - echo line 3 hello - echo line 4 hello + echo line 1 hello + echo line 2 hello + echo line 3 hello + echo line 4 hello add_and_commit_file hello 4 lines HASH1=$(git rev-parse --verify HEAD) - echo line BUG hello - echo line 6 hello - echo line 7 hello - echo line 8 hello + echo line BUG hello + echo line 6 hello + echo line 7 hello + echo line 8 hello add_and_commit_file hello 4 more lines with a BUG HASH2=$(git rev-parse --verify HEAD) - echo line 9 hello - echo line 10 hello + echo line 9 hello + echo line 10 hello add_and_commit_file hello 2 more lines HASH3=$(git rev-parse --verify HEAD) - echo line 11 hello + echo line 11 hello add_and_commit_file hello 1 more line HASH4=$(git rev-parse --verify HEAD) - sed -e s/BUG/5/ hello hello.new + sed -e s/BUG/5/ hello hello.new mv hello.new hello add_and_commit_file hello BUG fixed HASH5=$(git rev-parse --verify HEAD) - echo line 12 hello - echo line 13 hello + echo line 12 hello + echo line 13 hello add_and_commit_file hello 2 more lines HASH6=$(git rev-parse --verify HEAD) - echo line 14 hello - echo line 15 hello - echo line 16 hello + echo line 14 hello + echo line 15 hello + echo line 16 hello add_and_commit_file hello again 3 more lines HASH7=$(git rev-parse --verify HEAD) ' @@ -95,7 +95,7 @@ test_expect_success 'tag replaced commit' ' ' test_expect_success 'git fsck works' ' - git fsck master fsck_master.out + git fsck master fsck_master.out grep dangling commit $R fsck_master.out grep dangling tag $(cat .git/refs/tags/mytag) fsck_master.out test -z $(git fsck) @@ -217,14 +217,14 @@ test_expect_success 'fetch branch with replacement' ' ( cd clone_dir git fetch origin refs/heads/tofetch:refs/heads/parallel3 - git log --pretty=oneline parallel3 output.txt + git log --pretty=oneline parallel3 output.txt ! grep $PARA3 output.txt - git show $PARA3 para3.txt + git show $PARA3 para3.txt grep A U Thor para3.txt git fetch origin refs/replace/*:refs/replace/* - git log --pretty=oneline parallel3 output.txt + git log --pretty=oneline parallel3 output.txt grep $PARA3 output.txt - git show $PARA3 para3.txt + git show $PARA3 para3.txt grep O Thor para3.txt ) ' @@ -302,7 +302,7 @@ test_expect_success 'test --format medium' ' echo $PARA3 - $S echo $MYTAG - $HASH1 } | sort expected - git replace -l --format medium | sort actual + git replace -l --format medium | sort actual test_cmp expected actual ' @@ -314,7 +314,7 @@ test_expect_success 'test --format long' ' echo $PARA3 (commit) - $S (commit) echo $MYTAG (tag) - $HASH1 (commit) } | sort expected - git replace --format=long | sort actual + git replace --format=long | sort actual test_cmp expected actual ' -- 2.0.0.421.g786a89d.dirty -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v6 02/10] replace: add --graft option
The usage string for this option is:
git replace [-f] --graft commit [parent...]
First we create a new commit that is the same as commit
except that its parents are [parents...]
Then we create a replace ref that replace commit with
the commit we just created.
With this new option, it should be straightforward to
convert grafts to replace refs.
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
Signed-off-by: Junio C Hamano gits...@pobox.com
---
builtin/replace.c | 74 ++-
1 file changed, 73 insertions(+), 1 deletion(-)
diff --git a/builtin/replace.c b/builtin/replace.c
index 1bb491d..ad47237 100644
--- a/builtin/replace.c
+++ b/builtin/replace.c
@@ -17,6 +17,7 @@
static const char * const git_replace_usage[] = {
N_(git replace [-f] object replacement),
N_(git replace [-f] --edit object),
+ N_(git replace [-f] --graft commit [parent...]),
N_(git replace -d object...),
N_(git replace [--format=format] [-l [pattern]]),
NULL
@@ -294,6 +295,66 @@ static int edit_and_replace(const char *object_ref, int
force)
return replace_object_sha1(object_ref, old, replacement, new, force);
}
+static void replace_parents(struct strbuf *buf, int argc, const char **argv)
+{
+ struct strbuf new_parents = STRBUF_INIT;
+ const char *parent_start, *parent_end;
+ int i;
+
+ /* find existing parents */
+ parent_start = buf-buf;
+ parent_start += 46; /* tree + hex sha1 + \n */
+ parent_end = parent_start;
+
+ while (starts_with(parent_end, parent ))
+ parent_end += 48; /* parent + hex sha1 + \n */
+
+ /* prepare new parents */
+ for (i = 1; i argc; i++) {
+ unsigned char sha1[20];
+ if (get_sha1(argv[i], sha1) 0)
+ die(_(Not a valid object name: '%s'), argv[i]);
+ lookup_commit_or_die(sha1, argv[i]);
+ strbuf_addf(new_parents, parent %s\n, sha1_to_hex(sha1));
+ }
+
+ /* replace existing parents with new ones */
+ strbuf_splice(buf, parent_start - buf-buf, parent_end - parent_start,
+ new_parents.buf, new_parents.len);
+
+ strbuf_release(new_parents);
+}
+
+static int create_graft(int argc, const char **argv, int force)
+{
+ unsigned char old[20], new[20];
+ const char *old_ref = argv[0];
+ struct commit *commit;
+ struct strbuf buf = STRBUF_INIT;
+ const char *buffer;
+ unsigned long size;
+
+ if (get_sha1(old_ref, old) 0)
+ die(_(Not a valid object name: '%s'), old_ref);
+ commit = lookup_commit_or_die(old, old_ref);
+
+ buffer = get_commit_buffer(commit, size);
+ strbuf_add(buf, buffer, size);
+ unuse_commit_buffer(commit, buffer);
+
+ replace_parents(buf, argc, argv);
+
+ if (write_sha1_file(buf.buf, buf.len, commit_type, new))
+ die(_(could not write replacement commit for: '%s'), old_ref);
+
+ strbuf_release(buf);
+
+ if (!hashcmp(old, new))
+ return error(new commit is the same as the old one: '%s',
sha1_to_hex(old));
+
+ return replace_object_sha1(old_ref, old, replacement, new, force);
+}
+
int cmd_replace(int argc, const char **argv, const char *prefix)
{
int force = 0;
@@ -303,12 +364,14 @@ int cmd_replace(int argc, const char **argv, const char
*prefix)
MODE_LIST,
MODE_DELETE,
MODE_EDIT,
+ MODE_GRAFT,
MODE_REPLACE
} cmdmode = MODE_UNSPECIFIED;
struct option options[] = {
OPT_CMDMODE('l', list, cmdmode, N_(list replace refs),
MODE_LIST),
OPT_CMDMODE('d', delete, cmdmode, N_(delete replace refs),
MODE_DELETE),
OPT_CMDMODE('e', edit, cmdmode, N_(edit existing object),
MODE_EDIT),
+ OPT_CMDMODE('g', graft, cmdmode, N_(change a commit's
parents), MODE_GRAFT),
OPT_BOOL('f', force, force, N_(replace the ref if it
exists)),
OPT_STRING(0, format, format, N_(format), N_(use this
format)),
OPT_END()
@@ -325,7 +388,10 @@ int cmd_replace(int argc, const char **argv, const char
*prefix)
usage_msg_opt(--format cannot be used when not listing,
git_replace_usage, options);
- if (force cmdmode != MODE_REPLACE cmdmode != MODE_EDIT)
+ if (force
+ cmdmode != MODE_REPLACE
+ cmdmode != MODE_EDIT
+ cmdmode != MODE_GRAFT)
usage_msg_opt(-f only makes sense when writing a replacement,
git_replace_usage, options);
@@ -348,6 +414,12 @@ int cmd_replace(int argc, const char **argv, const char
*prefix)
git_replace_usage, options);
return edit_and_replace(argv[0], force);
+ case
[PATCH v6 06/10] replace: remove signature when using --graft
It could be misleading to keep a signature in a
replacement commit, so let's remove it.
Note that there should probably be a way to sign
the replacement commit created when using --graft,
but this can be dealt with in another commit or
patch series.
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
builtin/replace.c | 5 +
commit.c | 34 ++
commit.h | 2 ++
3 files changed, 41 insertions(+)
diff --git a/builtin/replace.c b/builtin/replace.c
index ad47237..cc29ef2 100644
--- a/builtin/replace.c
+++ b/builtin/replace.c
@@ -344,6 +344,11 @@ static int create_graft(int argc, const char **argv, int
force)
replace_parents(buf, argc, argv);
+ if (remove_signature(buf)) {
+ warning(_(the original commit '%s' has a gpg signature.),
old_ref);
+ warning(_(the signature will be removed in the replacement
commit!));
+ }
+
if (write_sha1_file(buf.buf, buf.len, commit_type, new))
die(_(could not write replacement commit for: '%s'), old_ref);
diff --git a/commit.c b/commit.c
index fb7897c..54e157d 100644
--- a/commit.c
+++ b/commit.c
@@ -1177,6 +1177,40 @@ int parse_signed_commit(const struct commit *commit,
return saw_signature;
}
+int remove_signature(struct strbuf *buf)
+{
+ const char *line = buf-buf;
+ const char *tail = buf-buf + buf-len;
+ int in_signature = 0;
+ const char *sig_start = NULL;
+ const char *sig_end = NULL;
+
+ while (line tail) {
+ const char *next = memchr(line, '\n', tail - line);
+ next = next ? next + 1 : tail;
+
+ if (in_signature line[0] == ' ')
+ sig_end = next;
+ else if (starts_with(line, gpg_sig_header)
+line[gpg_sig_header_len] == ' ') {
+ sig_start = line;
+ sig_end = next;
+ in_signature = 1;
+ } else {
+ if (*line == '\n')
+ /* dump the whole remainder of the buffer */
+ next = tail;
+ in_signature = 0;
+ }
+ line = next;
+ }
+
+ if (sig_start)
+ strbuf_remove(buf, sig_start - buf-buf, sig_end - sig_start);
+
+ return sig_start != NULL;
+}
+
static void handle_signed_tag(struct commit *parent, struct
commit_extra_header ***tail)
{
struct merge_remote_desc *desc;
diff --git a/commit.h b/commit.h
index 2e1492a..4234dae 100644
--- a/commit.h
+++ b/commit.h
@@ -327,6 +327,8 @@ struct commit *get_merge_parent(const char *name);
extern int parse_signed_commit(const struct commit *commit,
struct strbuf *message, struct strbuf
*signature);
+extern int remove_signature(struct strbuf *buf);
+
extern void print_commit_list(struct commit_list *list,
const char *format_cur,
const char *format_last);
--
2.0.0.421.g786a89d.dirty
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v6 05/10] contrib: add convert-grafts-to-replace-refs.sh
This patch adds into contrib/ an example script to convert
grafts from an existing grafts file into replace refs using
the new --graft option of git replace.
While at it let's mention this new script in the
git replace documentation for the --graft option.
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
Signed-off-by: Junio C Hamano gits...@pobox.com
---
Documentation/git-replace.txt | 4 +++-
contrib/convert-grafts-to-replace-refs.sh | 28
2 files changed, 31 insertions(+), 1 deletion(-)
create mode 100755 contrib/convert-grafts-to-replace-refs.sh
diff --git a/Documentation/git-replace.txt b/Documentation/git-replace.txt
index 491875e..e1be2e6 100644
--- a/Documentation/git-replace.txt
+++ b/Documentation/git-replace.txt
@@ -79,7 +79,9 @@ OPTIONS
content as commit except that its parents will be
[parent...] instead of commit's parents. A replacement ref
is then created to replace commit with the newly created
- commit.
+ commit. See contrib/convert-grafts-to-replace-refs.sh for an
+ example script based on this option that can convert grafts to
+ replace refs.
-l pattern::
--list pattern::
diff --git a/contrib/convert-grafts-to-replace-refs.sh
b/contrib/convert-grafts-to-replace-refs.sh
new file mode 100755
index 000..0cbc917
--- /dev/null
+++ b/contrib/convert-grafts-to-replace-refs.sh
@@ -0,0 +1,28 @@
+#!/bin/sh
+
+# You should execute this script in the repository where you
+# want to convert grafts to replace refs.
+
+GRAFTS_FILE=${GIT_DIR:-.git}/info/grafts
+
+. $(git --exec-path)/git-sh-setup
+
+test -f $GRAFTS_FILE || die Could not find graft file: '$GRAFTS_FILE'
+
+grep '^[^# ]' $GRAFTS_FILE |
+while read definition
+do
+ if test -n $definition
+ then
+ echo Converting: $definition
+ git replace --graft $definition ||
+ die Conversion failed for: $definition
+ fi
+done
+
+mv $GRAFTS_FILE $GRAFTS_FILE.bak ||
+ die Could not rename '$GRAFTS_FILE' to '$GRAFTS_FILE.bak'
+
+echo Success!
+echo All the grafts in '$GRAFTS_FILE' have been converted to replace refs!
+echo The grafts file '$GRAFTS_FILE' has been renamed: '$GRAFTS_FILE.bak'
--
2.0.0.421.g786a89d.dirty
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v6 10/10] replace: add test for --graft with a mergetag
Signed-off-by: Christian Couder chrisc...@tuxfamily.org --- t/t6050-replace.sh | 22 ++ 1 file changed, 22 insertions(+) diff --git a/t/t6050-replace.sh b/t/t6050-replace.sh index 15fd541..3bb8d06 100755 --- a/t/t6050-replace.sh +++ b/t/t6050-replace.sh @@ -416,4 +416,26 @@ test_expect_success GPG '--graft with a signed commit' ' git replace -d $HASH8 ' +test_expect_success GPG 'set up a merge commit with a mergetag' ' + git reset --hard HEAD + git checkout -b test_branch HEAD~2 + echo line 1 from test branch hello + echo line 2 from test branch hello + git add hello + test_tick + git commit -m hello: 2 more lines from a test branch + HASH9=$(git rev-parse --verify HEAD) + git tag -s -m tag for testing with a mergetag test_tag HEAD + git checkout master + git merge -s ours test_tag + HASH10=$(git rev-parse --verify HEAD) + git cat-file commit $HASH10 | grep ^mergetag object +' + +test_expect_success GPG '--graft on a commit with a mergetag' ' + test_must_fail git replace --graft $HASH10 $HASH8^1 + git replace --graft $HASH10 $HASH8^1 $HASH9 + git replace -d $HASH10 +' + test_done -- 2.0.0.421.g786a89d.dirty -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v6 04/10] Documentation: replace: add --graft option
Signed-off-by: Christian Couder chrisc...@tuxfamily.org Signed-off-by: Junio C Hamano gits...@pobox.com --- Documentation/git-replace.txt | 8 1 file changed, 8 insertions(+) diff --git a/Documentation/git-replace.txt b/Documentation/git-replace.txt index 61461b9..491875e 100644 --- a/Documentation/git-replace.txt +++ b/Documentation/git-replace.txt @@ -10,6 +10,7 @@ SYNOPSIS [verse] 'git replace' [-f] object replacement 'git replace' [-f] --edit object +'git replace' [-f] --graft commit [parent...] 'git replace' -d object... 'git replace' [--format=format] [-l [pattern]] @@ -73,6 +74,13 @@ OPTIONS newly created object. See linkgit:git-var[1] for details about how the editor will be chosen. +--graft commit [parent...]:: + Create a graft commit. A new commit is created with the same + content as commit except that its parents will be + [parent...] instead of commit's parents. A replacement ref + is then created to replace commit with the newly created + commit. + -l pattern:: --list pattern:: List replace refs for objects that match the given pattern (or -- 2.0.0.421.g786a89d.dirty -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v6 07/10] replace: add test for --graft with signed commit
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
t/t6050-replace.sh | 25 +
1 file changed, 25 insertions(+)
diff --git a/t/t6050-replace.sh b/t/t6050-replace.sh
index d80a89e..15fd541 100755
--- a/t/t6050-replace.sh
+++ b/t/t6050-replace.sh
@@ -7,6 +7,7 @@ test_description='Tests replace refs functionality'
exec /dev/null
. ./test-lib.sh
+. $TEST_DIRECTORY/lib-gpg.sh
add_and_commit_file()
{
@@ -391,4 +392,28 @@ test_expect_success '--graft with and without already
replaced object' '
git replace -d $HASH5
'
+test_expect_success GPG 'set up a signed commit' '
+ echo line 17 hello
+ echo line 18 hello
+ git add hello
+ test_tick
+ git commit --quiet -S -m hello: 2 more lines in a signed commit
+ HASH8=$(git rev-parse --verify HEAD)
+ git verify-commit $HASH8
+'
+
+test_expect_success GPG '--graft with a signed commit' '
+ git cat-file commit $HASH8 orig
+ git replace --graft $HASH8
+ git cat-file commit $HASH8 repl
+ commit_buffer_contains_parents $HASH8
+ commit_has_parents $HASH8
+ test_must_fail git verify-commit $HASH8
+ sed -n -e /^tree /p -e /^author /p -e /^committer /p orig
expected
+ echo expected
+ sed -e /^$/q repl actual
+ test_cmp expected actual
+ git replace -d $HASH8
+'
+
test_done
--
2.0.0.421.g786a89d.dirty
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v6 03/10] replace: add test for --graft
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
Signed-off-by: Junio C Hamano gits...@pobox.com
---
t/t6050-replace.sh | 40
1 file changed, 40 insertions(+)
diff --git a/t/t6050-replace.sh b/t/t6050-replace.sh
index fb07ad2..d80a89e 100755
--- a/t/t6050-replace.sh
+++ b/t/t6050-replace.sh
@@ -18,6 +18,33 @@ add_and_commit_file()
git commit --quiet -m $_file: $_msg
}
+commit_buffer_contains_parents()
+{
+git cat-file commit $1 payload
+sed -n -e '/^$/q' -e '/^parent /p' payload actual
+shift
+: expected
+for _parent
+do
+ echo parent $_parent expected || return 1
+done
+test_cmp actual expected
+}
+
+commit_has_parents()
+{
+_parent_number=1
+_commit=$1
+shift
+for _parent
+do
+ _found=$(git rev-parse --verify $_commit^$_parent_number) || return 1
+ test $_found = $_parent || return 1
+ _parent_number=$(( $_parent_number + 1 ))
+done
+test_must_fail git rev-parse --verify $_commit^$_parent_number
+}
+
HASH1=
HASH2=
HASH3=
@@ -351,4 +378,17 @@ test_expect_success 'replace ref cleanup' '
test -z $(git replace)
'
+test_expect_success '--graft with and without already replaced object' '
+ test $(git log --oneline | wc -l) = 7
+ git replace --graft $HASH5
+ test $(git log --oneline | wc -l) = 3
+ commit_buffer_contains_parents $HASH5
+ commit_has_parents $HASH5
+ test_must_fail git replace --graft $HASH5 $HASH4 $HASH3
+ git replace --force -g $HASH5 $HASH4 $HASH3
+ commit_buffer_contains_parents $HASH5 $HASH4 $HASH3
+ commit_has_parents $HASH5 $HASH4 $HASH3
+ git replace -d $HASH5
+'
+
test_done
--
2.0.0.421.g786a89d.dirty
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v6 09/10] replace: check mergetags when using --graft
When using --graft, with a mergetag in the original
commit, we should check that the commit pointed to by
the mergetag is still a parent of then new commit we
create, otherwise the mergetag could be misleading.
If the commit pointed to by the mergetag is no more
a parent of the new commit, we could remove the
mergetag, but in this case there is a good chance
that the title or other elements of the commit might
also be misleading. So let's just error out and
suggest to use --edit instead on the commit.
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
builtin/replace.c | 47 +++
1 file changed, 47 insertions(+)
diff --git a/builtin/replace.c b/builtin/replace.c
index cc29ef2..2290529 100644
--- a/builtin/replace.c
+++ b/builtin/replace.c
@@ -13,6 +13,7 @@
#include refs.h
#include parse-options.h
#include run-command.h
+#include tag.h
static const char * const git_replace_usage[] = {
N_(git replace [-f] object replacement),
@@ -325,6 +326,50 @@ static void replace_parents(struct strbuf *buf, int argc,
const char **argv)
strbuf_release(new_parents);
}
+struct check_mergetag_data {
+ int argc;
+ const char **argv;
+};
+
+static void check_one_mergetag(struct commit *commit,
+ struct commit_extra_header *extra,
+ void *data)
+{
+ struct check_mergetag_data *mergetag_data = (struct check_mergetag_data
*)data;
+ const char *ref = mergetag_data-argv[0];
+ unsigned char tag_sha1[20];
+ struct tag *tag;
+ int i;
+
+ hash_sha1_file(extra-value, extra-len, typename(OBJ_TAG), tag_sha1);
+ tag = lookup_tag(tag_sha1);
+ if (!tag)
+ die(_(bad mergetag in commit '%s'), ref);
+ if (parse_tag_buffer(tag, extra-value, extra-len))
+ die(_(malformed mergetag in commit '%s'), ref);
+
+ /* iterate over new parents */
+ for (i = 1; i mergetag_data-argc; i++) {
+ unsigned char sha1[20];
+ if (get_sha1(mergetag_data-argv[i], sha1) 0)
+ die(_(Not a valid object name: '%s'),
mergetag_data-argv[i]);
+ if (!hashcmp(tag-tagged-sha1, sha1))
+ return; /* found */
+ }
+
+ die(_(original commit '%s' contains mergetag '%s' that is discarded;
+ use --edit instead of --graft), ref, sha1_to_hex(tag_sha1));
+}
+
+static void check_mergetags(struct commit *commit, int argc, const char **argv)
+{
+ struct check_mergetag_data mergetag_data;
+
+ mergetag_data.argc = argc;
+ mergetag_data.argv = argv;
+ for_each_mergetag(check_one_mergetag, commit, mergetag_data);
+}
+
static int create_graft(int argc, const char **argv, int force)
{
unsigned char old[20], new[20];
@@ -349,6 +394,8 @@ static int create_graft(int argc, const char **argv, int
force)
warning(_(the signature will be removed in the replacement
commit!));
}
+ check_mergetags(commit, argc, argv);
+
if (write_sha1_file(buf.buf, buf.len, commit_type, new))
die(_(could not write replacement commit for: '%s'), old_ref);
--
2.0.0.421.g786a89d.dirty
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v6 08/10] commit: add for_each_mergetag()
In the same way as there is for_each_ref() to
iterate on refs, it might be useful to have
for_each_mergetag() to iterate on the mergetags
of a given commit.
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
commit.c | 13 +
commit.h | 5 +
log-tree.c | 15 ---
3 files changed, 22 insertions(+), 11 deletions(-)
diff --git a/commit.c b/commit.c
index 54e157d..0f83ff7 100644
--- a/commit.c
+++ b/commit.c
@@ -1348,6 +1348,19 @@ struct commit_extra_header
*read_commit_extra_headers(struct commit *commit,
return extra;
}
+void for_each_mergetag(each_mergetag_fn fn, struct commit *commit, void *data)
+{
+ struct commit_extra_header *extra, *to_free;
+
+ to_free = read_commit_extra_headers(commit, NULL);
+ for (extra = to_free; extra; extra = extra-next) {
+ if (strcmp(extra-key, mergetag))
+ continue; /* not a merge tag */
+ fn(commit, extra, data);
+ }
+ free_commit_extra_headers(to_free);
+}
+
static inline int standard_header_field(const char *field, size_t len)
{
return ((len == 4 !memcmp(field, tree , 5)) ||
diff --git a/commit.h b/commit.h
index 4234dae..c81ba85 100644
--- a/commit.h
+++ b/commit.h
@@ -312,6 +312,11 @@ extern struct commit_extra_header
*read_commit_extra_headers(struct commit *, co
extern void free_commit_extra_headers(struct commit_extra_header *extra);
+typedef void (*each_mergetag_fn)(struct commit *commit, struct
commit_extra_header *extra,
+void *cb_data);
+
+extern void for_each_mergetag(each_mergetag_fn fn, struct commit *commit, void
*data);
+
struct merge_remote_desc {
struct object *obj; /* the named object, could be a tag */
const char *name;
diff --git a/log-tree.c b/log-tree.c
index 10e6844..706ed4c 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -413,10 +413,11 @@ static int is_common_merge(const struct commit *commit)
!commit-parents-next-next);
}
-static void show_one_mergetag(struct rev_info *opt,
+static void show_one_mergetag(struct commit *commit,
struct commit_extra_header *extra,
- struct commit *commit)
+ void *data)
{
+ struct rev_info *opt = (struct rev_info *)data;
unsigned char sha1[20];
struct tag *tag;
struct strbuf verify_message;
@@ -463,15 +464,7 @@ static void show_one_mergetag(struct rev_info *opt,
static void show_mergetag(struct rev_info *opt, struct commit *commit)
{
- struct commit_extra_header *extra, *to_free;
-
- to_free = read_commit_extra_headers(commit, NULL);
- for (extra = to_free; extra; extra = extra-next) {
- if (strcmp(extra-key, mergetag))
- continue; /* not a merge tag */
- show_one_mergetag(opt, extra, commit);
- }
- free_commit_extra_headers(to_free);
+ for_each_mergetag(show_one_mergetag, commit, opt);
}
void show_log(struct rev_info *opt)
--
2.0.0.421.g786a89d.dirty
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 2/2] test-config: Add tests for the config_set API
Ramkumar Ramachandra artag...@gmail.com writes: A couple of quick nits. Tanay Abhra wrote: +test_expect_success 'clear default config' ' + rm -f .git/config +' Unnecessary; a fresh temporary directory is created for each test run. Hmm, fresh, but not empty. Anyway, the next test does a cat on this file, so it will erase its content, so the rm -f is actually not needed. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v6 1/2] add `config_set` API for caching config-like files
Currently `git_config()` uses a callback mechanism and file rereads for config values. Due to this approach, it is not uncommon for the config files to be parsed several times during the run of a git program, with different callbacks picking out different variables useful to themselves. Add a `config_set`, that can be used to construct an in-memory cache for config-like files that the caller specifies (i.e., files like `.gitmodules`, `~/.gitconfig` etc.). Add two external functions `git_configset_get_value` and `git_configset_get_value_multi` for querying from the config sets. `git_configset_get_value` follows `last one wins` semantic (i.e. if there are multiple matches for the queried key in the files of the configset the value returned will be the last entry in `value_list`). `git_configset_get_value_multi` returns a list of values sorted in order of increasing priority (i.e. last match will be at the end of the list). Add type specific query functions like `git_configset_get_bool` and similar. Add a default `config_set`, `the_config_set` to cache all key-value pairs read from usual config files (repo specific .git/config, user wide ~/.gitconfig, XDG config and the global /etc/gitconfig). `the_config_set` is populated using `git_config()`. Add two external functions `git_config_get_value` and `git_config_get_value_multi` for querying in a non-callback manner from `the_config_set`. Also, add type specific query functions that are implemented as a thin wrapper around the `config_set` API. Signed-off-by: Tanay Abhra tanay...@gmail.com --- Documentation/technical/api-config.txt | 134 +++ Makefile | 1 + cache.h| 34 config-hash.c | 295 + config.c | 3 + 5 files changed, 467 insertions(+) create mode 100644 config-hash.c diff --git a/Documentation/technical/api-config.txt b/Documentation/technical/api-config.txt index 230b3a0..65a6717 100644 --- a/Documentation/technical/api-config.txt +++ b/Documentation/technical/api-config.txt @@ -77,6 +77,81 @@ To read a specific file in git-config format, use `git_config_from_file`. This takes the same callback and data parameters as `git_config`. +Querying For Specific Variables +--- + +For programs wanting to query for specific variables in a non-callback +manner, the config API provides two functions `git_config_get_value` +and `git_config_get_value_multi`. They both read values from an internal +cache generated previously from reading the config files. + +`int git_config_get_value(const char *key, const char **value)`:: + + Finds the highest-priority value for the configuration variable `key`, + stores the pointer to it in `value` and returns 0. When the + configuration variable `key` is not found, returns 1 without touching + `value`. The caller should not free or modify `value`, as it is owned + by the cache. + +`const struct string_list *git_config_get_value_multi(const char *key)`:: + + Finds and returns the value list, sorted in order of increasing priority + for the configuration variable `key`. When the configuration variable + `key` is not found, returns NULL. The caller should not free or modify + the returned pointer, as it is owned by the cache. + +`void git_config_clear(void)`:: + + Resets and invalidates the config cache. + +The config API also provides type specific API functions which do conversion +as well as retrieval for the queried variable, including: + +`int git_config_get_int(const char *key, int *dest)`:: + + Finds and parses the value to an integer for the configuration variable + `key`. Dies on error; otherwise, stores the value of the parsed integer in + `dest` and returns 0. When the configuration variable `key` is not found, + returns 1 without touching `dest`. + +`int git_config_get_ulong(const char *key, unsigned long *dest)`:: + + Similar to `git_config_get_int` but for unsigned longs. + +`int git_config_get_bool(const char *key, int *dest)`:: + + Finds and parses the value into a boolean value, for the configuration + variable `key`respecting keywords like true and false. Integer + values are converted into true/false values (when they are non-zero or + zero, respectively). Other values cause a die(). If parsing is successful, + stores the value of the parsed result in `dest` and returns 0. When the + configuration variable `key` is not found, returns 1 without touching + `dest`. + +`int git_config_get_bool_or_int(const char *key, int *is_bool, int *dest)`:: + + Similar to `git_config_get_bool`, except that integers are copied as-is, + and `is_bool` flag is unset. + +`int git_config_get_maybe_bool(const char *key, int *dest)`:: + + Similar to `git_config_get_bool`, except
[PATCH v6 0/3] git config cache special querying api utilizing the cache
Hi, [PATCH V6]: Style nits and mistakes corrected. Diff between v6 and v5[8] is at the bottom. Thanks to Matthieu, Ramsay and Ram for their suggestions. [PATCH V5]: `config_set` now uses a single hashmap. Corrected style nits raised in the thread[7]. Thanks to Junio and Matthieu for their suggestions. [PATCH v4]: Introduced `config_set` construct which points to a ordered set of config-files cached as hashmaps. Added relevant API functions. For more details see the documentation. Rewrote the git_config_get* family to use `config_set` internally. Added tests for both config_set API and git_config_get family. Added type specific API functions which parses the found value and converts it into a specific type. Most of the changes implemented are the result of discussion in [6]. Thanks to Eric, Ramsay, Junio, Matthieu Karsten for their suggestions and review. [PATCH v3]: Added flag for NULL values that were causing segfaults in some cases. Added test-config for usage examples. Minor changes and corrections. Refer to discussion thread[5] for more details. Thanks to Matthieu, Jeff and Junio for their valuable suggestions. [PATCH v2]:Changed the string_list to a struct instead of pointer to a struct. Added string-list initilization functions. Minor mistakes corrected acoording to review comments[4]. Thanks to Eric and Matthieu for their review. [PATCH V1]:Most of the invaluable suggestions by Eric Sunshine, Torsten Bogershausen and Jeff King has been implemented[1]. Complete rewrite of config_cache*() family using git_config() as hook as suggested by Jeff. Thanks for the review. [RFC V2]: Improved according to the suggestions by Eric Sunshine and Torsten Bogershausen. Added cache invalidation when config file is changed.[2] I am using git_config_set_multivar_in_file() as an update hook. This is patch is for this year's GSoC. My project is Git Config API improvements. The link of my proposal is appended below [3]. The aim of this patch series is to generate a cache for querying values from the config files in a non-callback manner as the current method reads and parses the config files every time a value is queried for. The cache is generated from hooking the update_cache function to the current parsing and callback mechanism in config.c. It is implemented as an hashmap using the hashmap-api with variables and its corresponding values list as its members. The values in the list are sorted in order of increasing priority. The cache is initialised the first time when any of the new query functions is called. It is invalidated by using git_config_set_multivar_in_file() as an update hook. We add two new functions to the config-api, git_config_get_string() and git_config_get_string_multi() for querying in a non callback manner from the cache. [1] http://marc.info/?t=14017206626r=1w=2 [2] http://git.661346.n2.nabble.com/RFC-PATCH-0-2-Git-config-cache-amp-special-querying-api-utilizing-the-cache-td7611691.html [3] https://drive.google.com/file/d/0B4suZ-aHqDcnSUZJRXVTTnZUN1E/edit?usp=sharing [4] http://thread.gmane.org/gmane.comp.version-control.git/251073/focus=251369 [5] http://thread.gmane.org/gmane.comp.version-control.git/251704/ [6] http://thread.gmane.org/gmane.comp.version-control.git/252329/ [7] http://marc.info/?t=14042811521r=1w=2 [8] http://article.gmane.org/gmane.comp.version-control.git/252942/ Tanay Abhra (2): config-hash.c test-config .gitignore | 1 + Documentation/technical/api-config.txt | 134 +++ Makefile | 2 + cache.h| 34 config-hash.c | 295 + config.c | 3 + t/t1308-config-hash.sh | 168 +++ test-config.c | 125 ++ 8 files changed, 762 insertions(+) create mode 100644 config-hash.c create mode 100755 t/t1308-config-hash.sh create mode 100644 test-config.c -- 1.9.0.GIT Diff between v6 and v5: diff --git a/Documentation/technical/api-config.txt b/Documentation/technical/api-config.txt index bdf86d0..65a6717 100644 --- a/Documentation/technical/api-config.txt +++ b/Documentation/technical/api-config.txt @@ -110,7 +110,7 @@ as well as retrieval for the queried variable, including: `int git_config_get_int(const char *key, int *dest)`:: Finds and parses the value to an integer for the configuration variable - `key`. Dies on error; otherwise, stores pointer to the parsed integer in + `key`. Dies on error; otherwise, stores the value of the parsed integer in `dest` and returns 0. When the configuration variable `key` is not found, returns 1 without touching `dest`. @@
[PATCH v6 2/2] test-config: Add tests for the config_set API
Expose the `config_set` C API as a set of simple commands in order to
facilitate testing. Add tests for the `config_set` API as well as for
`git_config_get_*()` family for the usual config files.
Signed-off-by: Tanay Abhra tanay...@gmail.com
---
.gitignore | 1 +
Makefile | 1 +
t/t1308-config-hash.sh | 168 +
test-config.c | 125
4 files changed, 295 insertions(+)
create mode 100755 t/t1308-config-hash.sh
create mode 100644 test-config.c
diff --git a/.gitignore b/.gitignore
index 42294e5..eeb66e2 100644
--- a/.gitignore
+++ b/.gitignore
@@ -176,6 +176,7 @@
/gitweb/static/gitweb.js
/gitweb/static/gitweb.min.*
/test-chmtime
+/test-config
/test-ctype
/test-date
/test-delta
diff --git a/Makefile b/Makefile
index d503f78..e875070 100644
--- a/Makefile
+++ b/Makefile
@@ -548,6 +548,7 @@ X =
PROGRAMS += $(patsubst %.o,git-%$X,$(PROGRAM_OBJS))
TEST_PROGRAMS_NEED_X += test-chmtime
+TEST_PROGRAMS_NEED_X += test-config
TEST_PROGRAMS_NEED_X += test-ctype
TEST_PROGRAMS_NEED_X += test-date
TEST_PROGRAMS_NEED_X += test-delta
diff --git a/t/t1308-config-hash.sh b/t/t1308-config-hash.sh
new file mode 100755
index 000..ad99f8b
--- /dev/null
+++ b/t/t1308-config-hash.sh
@@ -0,0 +1,168 @@
+#!/bin/sh
+
+test_description='Test git config-hash API in different settings'
+
+. ./test-lib.sh
+
+#'check section.key value' verifies that the entry for section.key is
+#'value'
+check() {
+ echo $2 expected
+ test-config get_value $1 actual 21
+ test_cmp actual expected
+}
+
+test_expect_success 'setup default config' '
+ cat .git/config EOF
+ [core]
+ penguin = very blue
+ Movie = BadPhysics
+ UPPERCASE = true
+ MixedCase = true
+ my =
+ foo
+ baz = sam
+ [Cores]
+ WhatEver = Second
+ baz = bar
+ [cores]
+ baz = bat
+ [CORES]
+ baz = ball
+ [my Foo bAr]
+ hi = mixed-case
+ [my FOO BAR]
+ hi = upper-case
+ [my foo bar]
+ hi = lower-case
+ [core]
+ baz = bat
+ baz = hask
+ [lamb]
+ chop = 65
+ head = none
+ [goat]
+ legs = 4
+ head = true
+ skin = false
+ nose = 1
+ horns
+ EOF
+'
+
+test_expect_success 'get value for a simple key' '
+ check core.penguin very blue
+'
+
+test_expect_success 'get value for a key with value as an empty string' '
+ check core.my
+'
+
+test_expect_success 'get value for a key with value as NULL' '
+ check core.foo (NULL)
+'
+
+test_expect_success 'upper case key' '
+ check core.UPPERCASE true
+'
+
+test_expect_success 'mixed case key' '
+ check core.MixedCase true
+'
+
+test_expect_success 'key and value with mixed case' '
+ check core.Movie BadPhysics
+'
+
+test_expect_success 'key with case sensitive subsection' '
+ check my.Foo bAr.hi mixed-case
+ check my.FOO BAR.hi upper-case
+ check my.foo bar.hi lower-case
+'
+
+test_expect_success 'key with case insensitive section header' '
+ check cores.baz ball
+ check Cores.baz ball
+ check CORES.baz ball
+ check coreS.baz ball
+'
+
+test_expect_success 'find value with misspelled key' '
+ check my.fOo Bar.hi Value not found for \my.fOo Bar.hi\
+'
+
+test_expect_success 'find value with the highest priority' '
+ check core.baz hask
+'
+
+test_expect_success 'find integer value for a key' '
+ echo 65 expect
+ test-config get_int lamb.chop actual
+ test_cmp expect actual
+'
+
+test_expect_success 'find integer if value is non parse-able' '
+ echo 65 expect
+ test_must_fail test-config get_int lamb.head actual
+ test_must_fail test_cmp expect actual
+'
+
+test_expect_success 'find bool value for the entered key' '
+ cat expect -\EOF
+ 1
+ 0
+ 1
+ 1
+ 1
+ EOF
+ test-config get_bool goat.head actual
+ test-config get_bool goat.skin actual
+ test-config get_bool goat.nose actual
+ test-config get_bool goat.horns actual
+ test-config get_bool goat.legs actual
+ test_cmp expect actual
+'
+
+test_expect_success 'find multiple values' '
+ cat expect -\EOF
+ sam
+ bat
+ hask
+ EOF
+ test-config get_value_multi core.bazactual
+ test_cmp expect actual
+'
+
+test_expect_success 'find value from a configset' '
+ cat config2 -\EOF
+ [core]
+ baz = lama
+ [my]
+ new = silk
+ [core]
+ baz = ball
+ EOF
+ echo silk expect
+ test-config configset_get_value my.new config2 .git/config actual
Adding 'Signed-off-by' to 'subtree add --squash' commits
Is it possible to sign off squashed commits created by the 'git subtree add ... --squash' command? I ask, because the Signed-off-by tag is a requirement for a project I work on, but I've been unable to achieve this. A thorough search of Google/StackOverflow reveals nothing. Regards, Stephen Finucane PS: A visualisation. The issue is the second squash commit |\ Merge: 000 000 | | Author: Stephen Finucane stephen.finuc...@intel.com | | Date: Mon Jul 7 10:21:51 2014 +0100 | | | | Add xxx as a git subtree | | | | - There is no problem with this commit. | | | | Signed-off-by: Stephen Finucane stephen.finuc...@intel.com | | | * commit | Author: Stephen Finucane stephen.finuc...@intel.com | Date: Mon Jul 7 10:21:51 2014 +0100 | | Squashed 'path/' content from commit 000 | | git-subtree-dir: path | git-subtree-split: | * commit | Author: Stephen Finucane stephen.finuc...@intel.com | Date: Mon Jul 7 10:04:38 2014 +0100 ... -- Intel Shannon Limited Registered in Ireland Registered Office: Collinstown Industrial Park, Leixlip, County Kildare Registered Number: 308263 Business address: Dromore House, East Park, Shannon, Co. Clare This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 00/28] Support multiple checkouts
On Mon, Jul 7, 2014 at 3:46 AM, Max Kirillov m...@max630.net wrote: Hi. What future does this have? Currently it is marked as Stalled, but still mergeable with some trivial conflicts and seem to be working (except some bugs in interaction with submodules, see below). It would be very nice if this feature is officially supported. It's to be re-rolled soon. I have a patch about sparse-checkout and Dennis may contribute another one to enable checkout --to from a bare repository. By the way Dennis has been testing this feature and he reported (off-list) it worked fine, which is good news. I have done nothing so far because my git time (or energy to be precise) is limited these days, and I wanted to see if Dennis reported any new bugs. I also have a comment about how it interacts with submodules. Would it be more appropriate to mark modules as a per-checkout directory? Because each of the working tree's submodule is obviously a separated directory in filesystem, and in most cases (at least in my practice) they are checked-out to different revisions. Submodule interaction is something I have avoided so far because I'm not a big user and admittedly does not follow its development closely. I think we could get this in first, then let submodule people aware about this feature and let them decide how to use it. So, currently (before proper linking of submodules checkouts implemented), if make submodules per-checkout (actually it appears to somehow work even with current code, maybe because some submodule code ignores the common_dir), one could run git submodule update if necessary, and get fully separated clones, which would work normally. It still may break if submodules are removed, added or renamed, but this seems to be inevitable until config is separated to per-checkout and common parts, which I suppose is a much bigger task. -- Duy -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 00/28] Support multiple checkouts
On ma, 2014-07-07 at 17:25 +0700, Duy Nguyen wrote: I also have a comment about how it interacts with submodules. Would it be more appropriate to mark modules as a per-checkout directory? Because each of the working tree's submodule is obviously a separated directory in filesystem, and in most cases (at least in my practice) they are checked-out to different revisions. Submodule interaction is something I have avoided so far because I'm not a big user and admittedly does not follow its development closely. I think we could get this in first, then let submodule people aware about this feature and let them decide how to use it. I do intend to use checkout --to and submodule update on the same repository, but have not yet done so. I will poke at that later this month. If you can easily reproduce errors, I would appreciate to know how, because my use of submodules is very limited. -- Dennis Kaarsemaker http://www.kaarsemaker.net -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Adding 'Signed-off-by' to 'subtree add --squash' commits
Finucane, Stephen stephen.finuc...@intel.com writes: Is it possible to sign off squashed commits created by the 'git subtree add ... --squash' command? If it isn't directly possible, you can always use git commit --amend -C HEAD -s to modify the commit afterwards. Andreas. -- Andreas Schwab, sch...@linux-m68k.org GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 And now for something completely different. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] cache-tree: Create/update cache-tree on checkout
David Turner dtur...@twopensource.com writes: I am not convinced that doing an equivalent of write-tree when you switch branches is the right approach in the first place. You will eventually write it out as a tree, and having a relatively undamaged cache-tree will help you when you do so, but spending the cycles necessary to compute a fully populated cache-tree, only to let it degrade over time until you are ready to write it out as a tree, somehow sounds like asking for a duplicated work upfront. As I understand it, the cache-tree extension was originally designed to speed up writing the tree later. However, as Karsten Blees's work (and my own tests) have shown, it also speeds up git status. I use git status a lot while working, and I've talked to a few others who do the same. So I think it's worth spending extra time when switching branches to have a good working experience within that branch. You are reading the history of the subsystem correctly. Since b65982b6 (Optimize diff-index --cached using cache-tree, 2009-05-20), having an undamaged cache-tree does help with git status as well. In the new version of the patchset (which I'll post shortly), I've added an option WRITE_TREE_REPAIR, which does all of the work to compute a new tree object, but only adds it to the cache-tree if it already exists on-disk. This is a little wasteful for the reason that you note. So if you would like, I could add a config option to skip it. But I think it is a good default. OK, sounds good. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 1/2] add `config_set` API for caching config files
Tanay Abhra tanay...@gmail.com writes: On 7/4/2014 2:47 PM, Matthieu Moy wrote: Tanay Abhra tanay...@gmail.com writes: Hi, I have cooked up a single hashmap implementation. What are your thoughts about it? I had a quick look, and it looks good to me. I'll make a more detailed review when you send the next series. One more doubt, does filename,linenr for every value has any use other than raising semantic error in typespecific API functions. For example, if we call git_config_get_int(foo.bar), we can show to the user value not a int at filename, linenr. Other than that I cannot think of any other use of it. Currently `git_config_int` dies if value put for parsing is not an int. Junio and Karsten, both raised the point for saving filename,linenr, but I can't find any use cases for it other than what I mentioned above. Yes, error reporting is what the pair needs to be kept for. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 2/2] test-config: Add tests for the config_set API
Tanay Abhra tanay...@gmail.com writes:
diff --git a/t/t1308-config-hash.sh b/t/t1308-config-hash.sh
new file mode 100755
index 000..ad99f8b
--- /dev/null
+++ b/t/t1308-config-hash.sh
@@ -0,0 +1,168 @@
+#!/bin/sh
+
+test_description='Test git config-hash API in different settings'
You may want to call it config_set API now.
+#'check section.key value' verifies that the entry for section.key is
+#'value'
Style: space after #.
+check() {
+ echo $2 expected
+ test-config get_value $1 actual 21
+ test_cmp actual expected
+}
You need to -chain these lines, to catch potential test-config
failures (if it returns 1 after sending the right output, you won't
notice).
The doc says
- test_cmp expected actual
You swapped the order of parameters.
+test_expect_success 'setup default config' '
+ cat .git/config EOF
Missing here (sorry, I should have noticed the first time).
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 00/14] Add submodule test harness
Jens Lehmann jens.lehm...@web.de writes:
Junio, do you want me to resend 02/14 without the non-portable echo -n
or could you just squash the following diff in?
Amended locally here already; thanks, both.
-8
diff --git a/t/lib-submodule-update.sh b/t/lib-submodule-update.sh
index 24c9fd7..3584755 100755
--- a/t/lib-submodule-update.sh
+++ b/t/lib-submodule-update.sh
@@ -304,7 +304,7 @@ test_submodule_switch () {
(
cd submodule_update
git branch -t add_sub1 origin/add_sub1
- echo -n sub1
+ sub1
test_must_fail $command add_sub1
test_superproject_content origin/no_submodule
test_must_be_empty sub1
@@ -547,7 +547,7 @@ test_submodule_forced_switch () {
(
cd submodule_update
git branch -t add_sub1 origin/add_sub1
- echo -n sub1
+ sub1
$command add_sub1
test_superproject_content origin/add_sub1
test_dir_is_empty sub1
-- 2.0.1.458.gf680257.dirty
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 0/3] git config cache special querying api utilizing the cache
Tanay Abhra tanay...@gmail.com writes: test_expect_success 'find value with misspelled key' ' - echo Value not found for \my.fOo Bar.hi\ expect - test_must_fail test-config get_value my.fOo Bar.hi actual - test_cmp expect actual + check my.fOo Bar.hi Value not found for \my.fOo Bar.hi\ ' This one is wrong: you did need the test_must_fail here. (That's related to my other message about -chaining in check) Other than my minor remarks, the patches now sounds good. Tanay: you should be able to send a v7 very soon, and it should hopefully be ready for pu. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Adding 'Signed-off-by' to 'subtree add --squash' commits
On Mon, Jul 07, 2014 at 06:55:07PM +0200, Andreas Schwab wrote: Finucane, Stephen stephen.finuc...@intel.com writes: Is it possible to sign off squashed commits created by the 'git subtree add ... --squash' command? If it isn't directly possible, you can always use git commit --amend -C HEAD -s to modify the commit afterwards. I think that is sensible, though these days you can spell -C HEAD as --no-edit, which I think is a bit more obvious. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v1 1/4] hashmap: factor out getting an int hash code from a, SHA1
Karsten Blees karsten.bl...@gmail.com writes:
Copying the first bytes of a SHA1 is duplicated in six places, however,
the implications (wrong byte order on little-endian systems) is documented
only once.
s/wrong /different /; but other than that I think this is a good
change.
+`unsigned int sha1hash(const unsigned char *sha1)`::
+
+ Converts a cryptographic hash (e.g. SHA-1) into an int-sized hash code
+ for use in hash tables. Cryptographic hashes are supposed to have
+ uniform distribution, so in contrast to `memhash()`, this just copies
+ the first `sizeof(int)` bytes without shuffling any bits. Note that
+ the results will be different on big-endian and little-endian
+ platforms, so they should not be stored or transferred over the net!
Tone down with s/!/./, perhaps?
Another thing we may want to caution against is to use it as a
tie-breaker that affects the final outcome the user can observe, but
that may be something that goes without saying. I dunno..
diff --git a/hashmap.h b/hashmap.h
index a816ad4..ed5425a 100644
--- a/hashmap.h
+++ b/hashmap.h
@@ -13,6 +13,17 @@ extern unsigned int strihash(const char *buf);
extern unsigned int memhash(const void *buf, size_t len);
extern unsigned int memihash(const void *buf, size_t len);
+static inline unsigned int sha1hash(const unsigned char *sha1)
+{
+ /*
+ * Equivalent to 'return *(int *)sha1;', but safe on platforms that
+ * don't support unaligned reads.
+ */
s/int/unsigned /; other than that, the explanation is good.
+ unsigned int hash;
+ memcpy(hash, sha1, sizeof(hash));
+ return hash;
+}
+
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 00/28] Support multiple checkouts
On Mon, Jul 07, 2014 at 12:49:01PM +0200, Dennis Kaarsemaker wrote: I do intend to use checkout --to and submodule update on the same repository, but have not yet done so. I will poke at that later this month. If you can easily reproduce errors, I would appreciate to know how, because my use of submodules is very limited. I have collected all my tests to this script: https://raw.githubusercontent.com/max630/git/tmp/multiple_work_trees_dev/t/t7410-submodule-checkout-to.sh -- Max -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v1 3/4] hashmap: add simplified hashmap_get_from_hash() API
Karsten Blees karsten.bl...@gmail.com writes:
Hashmap entries are typically looked up by just a key. The hashmap_get()
API expects an initialized entry structure instead, to support compound
keys. This flexibility is currently only needed by find_dir_entry() in
name-hash.c (and compat/win32/fscache.c in the msysgit fork). All other
(currently five) call sites of hashmap_get() have to set up a near emtpy
s/emtpy/empty/;
entry structure, resulting in duplicate code like this:
struct hashmap_entry keyentry;
hashmap_entry_init(keyentry, hash(key));
return hashmap_get(map, keyentry, key);
Add a hashmap_get_from_hash() API that allows hashmap lookups by just
specifying the key and its hash code, i.e.:
return hashmap_get_from_hash(map, hash(key), key);
While I think the *_get_from_hash() is an improvement over the
three-line sequence, I find it somewhat strange that callers of it
still must compute the hash code themselves, instead of letting the
hashmap itself to apply the user supplied hash function to the key
to derive it. After all, the map must know how to compare two
entries via a user-supplied cmpfn given at the map initialization
time, and the algorithm to derive the hash code falls into the same
category, in the sense that both must stay the same during the
lifetime of a hashmap, no? Unless there is a situation where you
need to be able to initialize a hashmap_entry without knowing which
hashmap the entry will eventually be stored, it feels dubious API
that exposes to the outside callers hashmap_entry_init() that takes
the hash code without taking the hashmap itself.
In other words, why isn't hashmap_get() more like this:
void *hashmap_get(const struct hashmap *map, const void *key)
{
struct hashmap_entry keyentry;
hashmap_entry_init(keyentry, map-hash(key));
return *find_entry_ptr(map, keyentry, key);
}
with hashmap_entry_init() purely a static helper in hashmap.c?
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v1 4/4] hashmap: add string interning API
Karsten Blees karsten.bl...@gmail.com writes:
Interning short strings with high probability of duplicates can reduce the
memory footprint and speed up comparisons.
Add strintern() and memintern() APIs that use a hashmap to manage the pool
of unique, interned strings.
Note: strintern(getenv()) could be used to sanitize git's use of getenv(),
in case we ever encounter a platform where a call to getenv() invalidates
previous getenv() results (which is allowed by POSIX).
I think the attribute name/value strings are already interned, so
they may want to be converted to use this (or vice-versa).
Signed-off-by: Karsten Blees bl...@dcon.de
---
Documentation/technical/api-hashmap.txt | 15 +
hashmap.c | 38
+
hashmap.h | 8 +++
t/t0011-hashmap.sh | 13 +++
test-hashmap.c | 14
5 files changed, 88 insertions(+)
diff --git a/Documentation/technical/api-hashmap.txt
b/Documentation/technical/api-hashmap.txt
index f9215d6..00c4c29 100644
--- a/Documentation/technical/api-hashmap.txt
+++ b/Documentation/technical/api-hashmap.txt
@@ -193,6 +193,21 @@ more entries.
`hashmap_iter_first` is a combination of both (i.e. initializes the iterator
and returns the first entry, if any).
+`const char *strintern(const char *string)`::
+`const void *memintern(const void *data, size_t len)`::
+
+ Returns the unique, interned version of the specified string or data,
+ similar to the `String.intern` API in Java and .NET, respectively.
+ Interned strings remain valid for the entire lifetime of the process.
++
+Can be used as `[x]strdup()` or `xmemdupz` replacement, except that interned
+strings / data must not be modified or freed.
++
+Interned strings are best used for short strings with high probability of
+duplicates.
++
+Uses a hashmap to store the pool of interned strings.
+
Usage example
-
diff --git a/hashmap.c b/hashmap.c
index d1b8056..f693839 100644
--- a/hashmap.c
+++ b/hashmap.c
@@ -226,3 +226,41 @@ void *hashmap_iter_next(struct hashmap_iter *iter)
current = iter-map-table[iter-tablepos++];
}
}
+
+struct pool_entry {
+ struct hashmap_entry ent;
+ size_t len;
+ unsigned char data[FLEX_ARRAY];
+};
+
+static int pool_entry_cmp(const struct pool_entry *e1,
+ const struct pool_entry *e2,
+ const unsigned char *keydata)
+{
+ return e1-data != keydata
+(e1-len != e2-len || memcmp(e1-data, keydata, e1-len));
+}
+
+const void *memintern(const void *data, size_t len)
+{
+ static struct hashmap map;
+ struct pool_entry key, *e;
+
+ /* initialize string pool hashmap */
+ if (!map.tablesize)
+ hashmap_init(map, (hashmap_cmp_fn) pool_entry_cmp, 0);
+
+ /* lookup interned string in pool */
+ hashmap_entry_init(key, memhash(data, len));
+ key.len = len;
+ e = hashmap_get(map, key, data);
+ if (!e) {
+ /* not found: create it */
+ e = xmallocz(sizeof(struct pool_entry) + len);
+ hashmap_entry_init(e, key.ent.hash);
+ e-len = len;
+ memcpy(e-data, data, len);
+ hashmap_add(map, e);
+ }
+ return e-data;
+}
diff --git a/hashmap.h b/hashmap.h
index 12f0668..507884b 100644
--- a/hashmap.h
+++ b/hashmap.h
@@ -87,4 +87,12 @@ static inline void *hashmap_iter_first(struct hashmap *map,
return hashmap_iter_next(iter);
}
+/* string interning */
+
+extern const void *memintern(const void *data, size_t len);
+static inline const char *strintern(const char *string)
+{
+ return memintern(string, strlen(string));
+}
+
#endif
diff --git a/t/t0011-hashmap.sh b/t/t0011-hashmap.sh
index 391e2b6..f97c805 100755
--- a/t/t0011-hashmap.sh
+++ b/t/t0011-hashmap.sh
@@ -237,4 +237,17 @@ test_expect_success 'grow / shrink' '
'
+test_expect_success 'string interning' '
+
+test_hashmap intern value1
+intern Value1
+intern value2
+intern value2
+ value1
+Value1
+value2
+value2
+
+'
+
test_done
diff --git a/test-hashmap.c b/test-hashmap.c
index 3c9f67b..07aa7ec 100644
--- a/test-hashmap.c
+++ b/test-hashmap.c
@@ -234,6 +234,20 @@ int main(int argc, char *argv[])
/* print table sizes */
printf(%u %u\n, map.tablesize, map.size);
+ } else if (!strcmp(intern, cmd) l1) {
+
+ /* test that strintern works */
+ const char *i1 = strintern(p1);
+ const char *i2 = strintern(p1);
+ if (strcmp(i1, p1))
+ printf(strintern(%s) returns %s\n, p1, i1);
+ else if (i1 == p1)
+
Re: [BUG] rebase no longer omits local commits
John Keeping j...@keeping.me.uk writes:
Perhaps we shuld do something like this (which passes the test suite):
-- 8 --
diff --git a/git-rebase.sh b/git-rebase.sh
index 06c810b..0c6c5d3 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -544,7 +544,8 @@ if test $fork_point = t
then
new_upstream=$(git merge-base --fork-point $upstream_name \
${switch_to:-HEAD})
- if test -n $new_upstream
+ if test -n $new_upstream
+! git merge-base --is-ancestor $new_upstream $upstream_name
then
upstream=$new_upstream
fi
-- 8 --
Since the intent of `--fork-point` is to find the best starting point
for the $upstream...$orig_head range, if the fork point is behind the
new location of the upstream then should we leave the upstream as it
was?
Probably; but the check to avoid giving worse fork-point should be
in the implementation of merge-base --fork-point itself, so that
we do not have to do the above to both rebase and pull --rebase,
no?
I haven't thought through this completely, but it seems like we should
be doing a check like the above, at least when we're in
$fork_point=auto mode.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] t/Makefile: always test all lint targets when running tests
Jens Lehmann jens.lehm...@web.de writes: Only the two targets test-lint-duplicates and test-lint-executable are currently executed when running the test target. This was done on purpose when the TEST_LINT variable was added in 81127d74. But as this does not include the test-lint-shell-syntax target added the same day in commit c7ce70ac, it is easy to accidentally add non portable shell constructs without noticing that when running the test suite. I not running the lint-shell-syntax that is fundamentally flaky to avoid false positives is very much on purpose. The flakiness is not the fault of the implementor of the lint-shell-syntax, but comes from the approach taken to pretend that simple pattern matching can parse shell scripts. It may not complain on the current set of scripts, but that is not really by design but by accident. So I am not very enthusiastic to see this change myself. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] refs: Fix valgrind suppression file
David Turner dtur...@twopensource.com writes:
Add all of the ways in which check_refname_format violates valgrind's
expectations to the valgrind suppression file; remove an assumption about
the call chain of check_refname_format from same.
Signed-off-by: David Turner dtur...@twitter.com
---
t/valgrind/default.supp | 15 +++
1 file changed, 11 insertions(+), 4 deletions(-)
I'll queue, but it makes me feel more and more disgusted with that
SSE patch, to be honest.
diff --git a/t/valgrind/default.supp b/t/valgrind/default.supp
index 332ab1a..9d51c92 100644
--- a/t/valgrind/default.supp
+++ b/t/valgrind/default.supp
@@ -50,10 +50,17 @@
fun:copy_ref
}
{
- ignore-sse-check_refname_format
+ ignore-sse-check_refname_format-addr
Memcheck:Addr8
fun:check_refname_format
- fun:cmd_check_ref_format
- fun:handle_builtin
- fun:main
+}
+{
+ ignore-sse-check_refname_format-cond
+ Memcheck:Cond
+ fun:check_refname_format
+}
+{
+ ignore-sse-check_refname_format-value
+ Memcheck:Value8
+ fun:check_refname_format
}
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] symlinks: remove PATH_MAX limitation
Karsten Blees karsten.bl...@gmail.com writes:
'git checkout' fails if a directory is longer than PATH_MAX, because the
lstat_cache in symlinks.c checks if the leading directory exists using
PATH_MAX-bounded string operations.
Remove the limitation by using strbuf instead.
Good.
diff --git a/cache.h b/cache.h
index df65231..44aa439 100644
--- a/cache.h
+++ b/cache.h
@@ -1090,12 +1090,16 @@ struct checkout {
extern int checkout_entry(struct cache_entry *ce, const struct checkout
*state, char *topath);
struct cache_def {
- char path[PATH_MAX + 1];
- int len;
+ struct strbuf path;
int flags;
int track_flags;
int prefix_len_stat_func;
};
+#define CACHE_DEF_INIT { STRBUF_INIT, 0, 0, 0 }
+static inline void cache_def_free(struct cache_def *cache)
+{
+ strbuf_release(cache-path);
+}
The above cache_def_free(cache) does not free the cache itself, but
only its associated data, so the name cache_def_free() is somewhat
misleading.
It seems that we use the name that consists solely of something
and free if the pointer to something itself is also freed. For
example diff_free_filespec_data(struct diff_filespec *) frees data
associated with the filespec but not the filespec itself, which is
done with diff_free_filespec().
Another name that may not be misleading would be cache_def_clear();
I think I'd prefer it over cache_def_free_data().
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 1/4] cache-tree: Create/update cache-tree on checkout
David Turner dtur...@twopensource.com writes: When git checkout checks out a branch, create or update the cache-tree so that subsequent operations are faster. update_main_cache_tree learned a new flag, WRITE_TREE_REPAIR. When WRITE_TREE_REPAIR is set, portions of the cache-tree which do not correspond to existing tree objects are invalidated (and portions which do are marked as valid). No new tree objects are created. Signed-off-by: David Turner dtur...@twitter.com --- Looks safe, if a bit wasteful, to me. Thanks; will queue. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 3/4] cache-tree: subdirectory tests
David Turner dtur...@twopensource.com writes:
Add tests to confirm that invalidation of subdirectories nether over-
nor under-invalidates.
Signed-off-by: David Turner dtur...@twitter.com
---
t/t0090-cache-tree.sh | 28 +---
1 file changed, 25 insertions(+), 3 deletions(-)
diff --git a/t/t0090-cache-tree.sh b/t/t0090-cache-tree.sh
index 98fb1ab..8437c5f 100755
--- a/t/t0090-cache-tree.sh
+++ b/t/t0090-cache-tree.sh
@@ -21,10 +21,13 @@ test_shallow_cache_tree () {
cmp_cache_tree expect
}
+# Test that the cache-tree for a given directory is invalid.
+# If no directory is given, check that the root is invalid
test_invalid_cache_tree () {
- echo invalid (0 subtrees) expect
- printf SHA #(ref) (%d entries, 0 subtrees)\n $(git ls-files|wc -l)
expect
- cmp_cache_tree expect
+ test-dump-cache-tree actual
+ sed -e s/$_x40/SHA/ -e s/[0-9]* subtrees//g actual filtered
+ expect=$(printf invalid $1 ()\n)
It would be saner to do 'printf string %s more string $1' than
embedding caller-supplied $1 inside the format specifier.
+ fgrep $expect filtered
We'd actually want to see fewer uses of 'fgrep' in the tests for two
reasons.
Is having an entry that is invalidated the only thing we care about
in this test? Shouldn't the caller expect These subtrees and
nothing else must be invalidated, in which case the helper should
check not just the expected invalid dir1/ appears in the output
but no other unexpected invalid somethingelse/ appears (and this
no other unexpected output makes use of grep family in tests like
this less desirable).
In other words, wouldn't it be better to do the helper along the
lines of:
test_invalidated_cache_tree () {
if test $# != 0
then
printf invalid %s ()\n $@
fi expect
test-dump-cache-tree |
sed -n -e '/^invalid /p' actual
test_cmp expect actual
}
and use
test_invalidated_cache_tree dir1
when we expect only dir1 and dir2 (but not dir2 or anything else) is
invalidated?
Thanks.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 2/4] test-dump-cache-tree: invalid trees are not errors
David Turner dtur...@twopensource.com writes: Do not treat known-invalid trees as errors even when their count is incorrect. Because git already knows that these trees are invalid, nothing depends on the count field. s/count/subtree_nr/; they are different. nothing depends on is not quite correct. The field keeps track of the number of subdirectories in the directory recorded in the cache-tree, and it *must* be maintained correctly (we iterate over the it-down[] array up to that number). The number of subdirectories the directory actually has, which we can discover by counting entries in the main part of the index, may be different from subtree_nr, if we haven't run update_one() on it, e.g. we may have added a path in a new subdirectory, at which time we would have invalidated the directory and its it-down[] array does not know the new subdirectory. While reading 3/4, I wondered if it makes sense to show the (N subtrees) for invalidated node, but as a debugging measure it helped me often while developping the framework, so we may not want to drop the subtree_nr from the output for invalidated entries. The change itself looks good. Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 00/14] Add submodule test harness
On 2014-07-07 19.05, Junio C Hamano wrote: Jens Lehmann jens.lehm...@web.de writes: Junio, do you want me to resend 02/14 without the non-portable echo -n or could you just squash the following diff in? Amended locally here already; thanks, both. There seems to be some other trouble under Mac OS, not yet fully tracked down, (may be related to the diff -r) And Msysgit complains error: fchmod on c:/xxxt/trash directory.t7613-merge-submodule/submodule_update_repo/.git/modules/sub1/config.lock failed: Function not implemented -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 4/4] cache-tree: Write updated cache-tree after commit
David Turner dtur...@twopensource.com writes:
During the commit process, update the cache-tree. Write this updated
cache-tree so that it's ready for subsequent commands.
Add test code which demonstrates that git commit now writes the cache
tree. Make all tests test the entire cache-tree, not just the root
level.
Signed-off-by: David Turner dtur...@twitter.com
---
builtin/commit.c | 31 +++---
t/t0090-cache-tree.sh | 87
++-
2 files changed, 91 insertions(+), 27 deletions(-)
diff --git a/builtin/commit.c b/builtin/commit.c
index 9cfef6c..5981755 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -342,6 +342,8 @@ static char *prepare_index(int argc, const char **argv,
const char *prefix,
discard_cache();
read_cache_from(index_lock.filename);
+ if (update_main_cache_tree(WRITE_TREE_SILENT) = 0)
+ write_cache(fd, active_cache, active_nr);
commit_style = COMMIT_NORMAL;
return index_lock.filename;
@@ -363,10 +365,18 @@ static char *prepare_index(int argc, const char **argv,
const char *prefix,
fd = hold_locked_index(index_lock, 1);
add_files_to_cache(also ? prefix : NULL, pathspec, 0);
refresh_cache_or_die(refresh_flags);
- update_main_cache_tree(WRITE_TREE_SILENT);
- if (write_cache(fd, active_cache, active_nr) ||
- close_lock_file(index_lock))
- die(_(unable to write new_index file));
+ if (active_cache_changed
+ || !cache_tree_fully_valid(active_cache_tree)) {
+ update_main_cache_tree(WRITE_TREE_SILENT);
+ active_cache_changed = 1;
+ }
+ if (active_cache_changed) {
+ if (write_cache(fd, active_cache, active_nr) ||
+ close_lock_file(index_lock))
+ die(_(unable to write new_index file));
+ } else {
+ rollback_lock_file(index_lock);
+ }
commit_style = COMMIT_NORMAL;
return index_lock.filename;
}
@@ -383,14 +393,10 @@ static char *prepare_index(int argc, const char **argv,
const char *prefix,
if (!only !pathspec.nr) {
fd = hold_locked_index(index_lock, 1);
refresh_cache_or_die(refresh_flags);
- if (active_cache_changed) {
- update_main_cache_tree(WRITE_TREE_SILENT);
- if (write_cache(fd, active_cache, active_nr) ||
- commit_locked_index(index_lock))
- die(_(unable to write new_index file));
- } else {
- rollback_lock_file(index_lock);
- }
+ update_main_cache_tree(WRITE_TREE_SILENT);
+ if (write_cache(fd, active_cache, active_nr) ||
+ commit_locked_index(index_lock))
+ die(_(unable to write new_index file));
commit_style = COMMIT_AS_IS;
return get_index_file();
}
Hmph. The above two hunks somehow feel the other way around. When
doing a non-partial, non as-is commit, i.e. git commit $paths, the
original used to call 'write-cache' unconditionally, because it is
very unlikely for us to see !active_cache_changed after calling the
add-files-to-cache with user-supplied pathspec (if there is nothing
to change, the user is being silly to say git commit $paths in the
first place). I would have expected that the patch would have left
that code path alone (it seems to be doing the right thing already).
On the other hand, git commit to commit the contents of the index
as-is is being cautious not to write things out unnecessarily, but
as you found out, it would be a good idea to fully populate the
cache-tree in this code path and write the otherwise already-good
index file out, even if we see !active_cache_changed after we called
refresh_cache_or_die(). So I would have expected that the patch
would have kept the write only necessary carefulness instead of
calling write-cache unconditionally.
That is, something like:
fd = hold_locked_index();
refresh_cache_or_die();
+ if (!cache_tree_fully_valid())
+ active_cache_changed = 1;
if (active_cache_changed) {
update_main_cache_tree();
diff --git a/t/t0090-cache-tree.sh b/t/t0090-cache-tree.sh
index 8437c5f..15f1484 100755
--- a/t/t0090-cache-tree.sh
+++ b/t/t0090-cache-tree.sh
@@ -7,8 +7,14 @@ cache-tree extension.
. ./test-lib.sh
+grep_nonmatch_ok () {
+grep $@
dq around it, i.e. $@.
+test $? = 2 return 1
POSIX.1 only specifies 1 not necessarily 2 as an error status.
+return 0
+}
Having said all that I do not think the helper is
Re: [PATCH 3/5] Run the perf test suite for profile feedback too
Andi Kleen a...@firstfloor.org writes: From: Andi Kleen a...@linux.intel.com Open: If the perf test suite is representative enough it may be reasonable to only run that and skip the much longer full test suite. Thoughts? I do not think it right now is representative, nor it was meant to become so. The operations are those that people cared about and tuned, and hopefully it would cover stuff actual end users care about in the real life, though. Signed-off-by: Andi Kleen a...@linux.intel.com --- Makefile | 1 + 1 file changed, 1 insertion(+) diff --git a/Makefile b/Makefile index a9770ac..ba64be9 100644 --- a/Makefile +++ b/Makefile @@ -1647,6 +1647,7 @@ ifeq ($(filter all,$(MAKECMDGOALS)),all) all:: profile-clean $(MAKE) PROFILE=GEN all $(MAKE) PROFILE=GEN -j1 test + $(MAKE) PROFILE=GEN -j1 perf endif endif -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [BUG] rebase no longer omits local commits
On Mon, Jul 07, 2014 at 10:56:23AM -0700, Junio C Hamano wrote:
John Keeping j...@keeping.me.uk writes:
Perhaps we shuld do something like this (which passes the test suite):
-- 8 --
diff --git a/git-rebase.sh b/git-rebase.sh
index 06c810b..0c6c5d3 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -544,7 +544,8 @@ if test $fork_point = t
then
new_upstream=$(git merge-base --fork-point $upstream_name \
${switch_to:-HEAD})
- if test -n $new_upstream
+ if test -n $new_upstream
+ ! git merge-base --is-ancestor $new_upstream $upstream_name
then
upstream=$new_upstream
fi
-- 8 --
Since the intent of `--fork-point` is to find the best starting point
for the $upstream...$orig_head range, if the fork point is behind the
new location of the upstream then should we leave the upstream as it
was?
Probably; but the check to avoid giving worse fork-point should be
in the implementation of merge-base --fork-point itself, so that
we do not have to do the above to both rebase and pull --rebase,
no?
I don't think so, since in that case we're not actually finding the fork
point as defined in the documentation, we're finding the upstream rebase
wants.
Having played with this a bit, I think we shouldn't be replacing the
upstream with the fork point but should instead add the fork point as an
additional negative ref:
$upstream...$orig_head ^$fork_point
Here's a script that creates a repository showing this:
-- 8 --
#!/bin/sh
git init rebase-test
cd rebase-test
echo one file
git add file
git commit -m one
echo frist file2
git add file2
git commit -m first
git branch --track dev
echo first file2
git commit -a --amend --no-edit
echo two file
git commit -a -m two
echo three file
git commit -a -m three
echo second file2
git commit -a -m second
git checkout dev
git cherry-pick -2 master
echo four file
git commit -a -m four
printf '\nWithout fork point (old behaviour)\n'
git rev-list --oneline --cherry @{u}...
printf '\nFork point as upstream (current behaviour)\n'
git rev-list --oneline --cherry $(git merge-base --fork-point master HEAD)...
printf '\nWith fork point\n'
git rev-list --oneline --cherry @{u}... ^$(git merge-base --fork-point master
HEAD)
-- 8 --
In this case the rebase should be clean since the only applicable patch
changes three to four in file, but the current rebase code fails
both with `--fork-point` and with `--no-fork-point`.
With `--fork-point` we try to apply two and three which have already
been cherry-picked across (as Ted originally reported) and with
`--no-fork-point`, we try to apply first which conflicts because we
have the version prior to it being fixed up on master.
I hacked up git-rebase to test this and the change to use the fork point
as in the last line of the script above does indeed make the rebase go
through cleanly, but I have not yet looked at how to cleanly patch in
that behaviour.
I haven't tested git-pull, but it looks like it has always (since 2009)
behaved in the way `git rebase --fork-point` does now, failing to detect
cherry-picked commits that are now in the upstream.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Test failure in t9814-git-p4-rename.sh - my environment or bad test?
Pete Wyckoff p...@padd.com writes: I'm not sure how to robustify this. At least doing the multiple comparisons should make the tests work again. The goal of this series of tests is to make sure that copy detection is working, not to verify that the correct copy choice was made. That should be in other (non-p4) tests. Choosing any of these as the copy source is fine is a sensible way to fix the problem with this test. Would it be a better solution to avoid having multiple/ambiguous copy source candidates in the first place, by the way? Do send patches based on Junio's master. I can ack, and they'll show up in a future git release. Thanks! -- Pete -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Potential bug in ls-files (version 1.7.9.5)
I'd appreciate if someone could confirm that this is a bug in git, as this works as expected with git 1.8.5.2. I'm not sure if this 1.7.X is still supported, but I think it's still the latest git-core available in Ubuntu 12.04 repositories. I'm having problems with git ls-files --others --ignored --exclude-standard not listing some ignored files. My project has this directory structure . ├── aspnet │ ├── .gitignore │ ├── __init__.py │ ├── lib │ │ ├── lots of big stuff My aspnet/.gitignore lists lib/*, and git add aspnet/lib/foo reports that this path is ignored. But git ls-files --others --ignored --exclude-standard does not list the files under lib. These are untracked files, they do show up in output if I do git ls-files --others, but not if I provide the ignored flag. With 1.8.5.2, all of the files under lib are listed if I ls-files --ignored --others --exclude-standard Tracks http://stackoverflow.com/questions/24620108/show-all-ignored-files-in-git Thanks, -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/5] Run the perf test suite for profile feedback too
On Mon, Jul 07, 2014 at 02:06:57PM -0700, Junio C Hamano wrote: Andi Kleen a...@firstfloor.org writes: From: Andi Kleen a...@linux.intel.com Open: If the perf test suite is representative enough it may be reasonable to only run that and skip the much longer full test suite. Thoughts? I do not think it right now is representative, nor it was meant to become so. The operations are those that people cared about and tuned, and hopefully it would cover stuff actual end users care about in the real life, though. I ended up answering the question by creating two separate makefile targets in the next patch. profile to run the full test suite and profile-fast to run only the performance test. profile-fast as the name implies is a lot faster to build, and fast enough that it's not annoying. I'll remove the Open -Andi -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 03/10] replace: add test for --graft
Christian Couder chrisc...@tuxfamily.org writes:
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
Signed-off-by: Junio C Hamano gits...@pobox.com
---
t/t6050-replace.sh | 40
1 file changed, 40 insertions(+)
diff --git a/t/t6050-replace.sh b/t/t6050-replace.sh
index fb07ad2..d80a89e 100755
--- a/t/t6050-replace.sh
+++ b/t/t6050-replace.sh
@@ -18,6 +18,33 @@ add_and_commit_file()
git commit --quiet -m $_file: $_msg
}
+commit_buffer_contains_parents()
SP before (), perhaps?
+{
+git cat-file commit $1 payload
+sed -n -e '/^$/q' -e '/^parent /p' payload actual
+shift
+: expected
+for _parent
+do
+ echo parent $_parent expected || return 1
+done
You can redirect the entire loop to simplify the above 5 lines,
which would read better, no?
for _parent
do
echo parent $_parent
done expect
+test_cmp actual expected
As test_cmp normally runs diff, it is better to compare expect
with actual, not the other way around; running the test verbosely,
i.e. t6050-replace.sh -v, shows how the actual output differs from
what is expected when diagnosing breakage and would be more useful
that way.
If your goal is to test both the object contents with this code
*and* the overall system behaviour with the $child^$parent below,
perhaps they should be in the same commit_has_parents function,
without forcing the caller to choose between the two or call both,
no?
+test_expect_success '--graft with and without already replaced object' '
+ test $(git log --oneline | wc -l) = 7
+ git replace --graft $HASH5
+ test $(git log --oneline | wc -l) = 3
+ commit_buffer_contains_parents $HASH5
+ commit_has_parents $HASH5
+ test_must_fail git replace --graft $HASH5 $HASH4 $HASH3
+ git replace --force -g $HASH5 $HASH4 $HASH3
+ commit_buffer_contains_parents $HASH5 $HASH4 $HASH3
+ commit_has_parents $HASH5 $HASH4 $HASH3
+ git replace -d $HASH5
+'
+
test_done
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Potential bug in ls-files (version 1.7.9.5)
Hi Hamilton, Hamilton Turner wrote: My project has this directory structure . ├── aspnet │ ├── .gitignore │ ├── __init__.py │ ├── lib │ │ ├── lots of big stuff My aspnet/.gitignore lists lib/*, and git add aspnet/lib/foo reports that this path is ignored. But git ls-files --others --ignored --exclude-standard does not list the files under lib. These are untracked files, they do show up in output if I do git ls-files --others, but not if I provide the ignored flag. With 1.8.5.2, all of the files under lib are listed if I ls-files --ignored --others --exclude-standard This was probably fixed by v1.7.11.2~12^2~5 (ls-files -i: pay attention to exclusion of leading paths, 2012-06-01). [...] I'm not sure if this 1.7.X is still supported, but I think it's still the latest git-core available in Ubuntu 12.04 repositories. I haven't had much luck in the past working with the Ubuntu maintainers to update old releases. :( https://launchpad.net/~git-core/+archive/ppa may help if PPAs are usable in your environment. Thanks and hope that helps, Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 09/10] replace: check mergetags when using --graft
Christian Couder chrisc...@tuxfamily.org writes: When using --graft, with a mergetag in the original commit, we should check that the commit pointed to by the mergetag is still a parent of then new commit we create, otherwise the mergetag could be misleading. If the commit pointed to by the mergetag is no more a parent of the new commit, we could remove the mergetag, but in this case there is a good chance that the title or other elements of the commit might also be misleading. So let's just error out and suggest to use --edit instead on the commit. I do not quite understand the reasoning. If you have a merge you earlier made with a signed tag, and then want to redo the merge with an updated tip of that branch (perhaps the side branch was earlier based on maint but now was rebased on master), it will perfectly be normal to expect that the title or other elements of the resulting merge to stay the same. Why is it a good idea to error it out? If the argument were 'replace --graft that changes the parents is likely to affect merge summary message, so error out and suggest to use --edit instead', regardless of the 'mergetag', I'd understand it, but that would essentially mean that 'replace --graft' should never be used, so... -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 08/10] commit: add for_each_mergetag()
Christian Couder chrisc...@tuxfamily.org writes:
In the same way as there is for_each_ref() to
iterate on refs, it might be useful to have
for_each_mergetag() to iterate on the mergetags
of a given commit.
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
Heh, might be useful is an understatement ;-) We won't apply a
change that might be useful very lightly, but this refactoring
already uses the helper in existing code, showing that it *is*
useful, no?
Let's have this early in the series, or perhaps even independent of
the replace series.
Thanks.
commit.c | 13 +
commit.h | 5 +
log-tree.c | 15 ---
3 files changed, 22 insertions(+), 11 deletions(-)
diff --git a/commit.c b/commit.c
index 54e157d..0f83ff7 100644
--- a/commit.c
+++ b/commit.c
@@ -1348,6 +1348,19 @@ struct commit_extra_header
*read_commit_extra_headers(struct commit *commit,
return extra;
}
+void for_each_mergetag(each_mergetag_fn fn, struct commit *commit, void
*data)
+{
+ struct commit_extra_header *extra, *to_free;
+
+ to_free = read_commit_extra_headers(commit, NULL);
+ for (extra = to_free; extra; extra = extra-next) {
+ if (strcmp(extra-key, mergetag))
+ continue; /* not a merge tag */
+ fn(commit, extra, data);
+ }
+ free_commit_extra_headers(to_free);
+}
+
static inline int standard_header_field(const char *field, size_t len)
{
return ((len == 4 !memcmp(field, tree , 5)) ||
diff --git a/commit.h b/commit.h
index 4234dae..c81ba85 100644
--- a/commit.h
+++ b/commit.h
@@ -312,6 +312,11 @@ extern struct commit_extra_header
*read_commit_extra_headers(struct commit *, co
extern void free_commit_extra_headers(struct commit_extra_header *extra);
+typedef void (*each_mergetag_fn)(struct commit *commit, struct
commit_extra_header *extra,
+ void *cb_data);
+
+extern void for_each_mergetag(each_mergetag_fn fn, struct commit *commit,
void *data);
+
struct merge_remote_desc {
struct object *obj; /* the named object, could be a tag */
const char *name;
diff --git a/log-tree.c b/log-tree.c
index 10e6844..706ed4c 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -413,10 +413,11 @@ static int is_common_merge(const struct commit *commit)
!commit-parents-next-next);
}
-static void show_one_mergetag(struct rev_info *opt,
+static void show_one_mergetag(struct commit *commit,
struct commit_extra_header *extra,
- struct commit *commit)
+ void *data)
{
+ struct rev_info *opt = (struct rev_info *)data;
unsigned char sha1[20];
struct tag *tag;
struct strbuf verify_message;
@@ -463,15 +464,7 @@ static void show_one_mergetag(struct rev_info *opt,
static void show_mergetag(struct rev_info *opt, struct commit *commit)
{
- struct commit_extra_header *extra, *to_free;
-
- to_free = read_commit_extra_headers(commit, NULL);
- for (extra = to_free; extra; extra = extra-next) {
- if (strcmp(extra-key, mergetag))
- continue; /* not a merge tag */
- show_one_mergetag(opt, extra, commit);
- }
- free_commit_extra_headers(to_free);
+ for_each_mergetag(show_one_mergetag, commit, opt);
}
void show_log(struct rev_info *opt)
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 4/4] cache-tree: Write updated cache-tree after commit
Junio C Hamano gits...@pobox.com writes: diff --git a/builtin/commit.c b/builtin/commit.c index 9cfef6c..5981755 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -342,6 +342,8 @@ static char *prepare_index(int argc, const char **argv, const char *prefix, discard_cache(); read_cache_from(index_lock.filename); +if (update_main_cache_tree(WRITE_TREE_SILENT) = 0) +write_cache(fd, active_cache, active_nr); commit_style = COMMIT_NORMAL; return index_lock.filename; I'll push out a tentative result of merging this series (with some proposed fix-ups) sometime later today, but this part interacts with Duy's split-index topic in a funny way, so I'd exclude it from the merge result for now. Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v5 1/4] cache-tree: Create/update cache-tree on checkout
When git checkout checks out a branch, create or update the
cache-tree so that subsequent operations are faster.
update_main_cache_tree learned a new flag, WRITE_TREE_REPAIR. When
WRITE_TREE_REPAIR is set, portions of the cache-tree which do not
correspond to existing tree objects are invalidated (and portions which
do are marked as valid). No new tree objects are created.
Signed-off-by: David Turner dtur...@twitter.com
---
builtin/checkout.c| 8
cache-tree.c | 12 +++-
cache-tree.h | 1 +
t/t0090-cache-tree.sh | 19 ---
4 files changed, 36 insertions(+), 4 deletions(-)
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 07cf555..054214f 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -553,6 +553,14 @@ static int merge_working_tree(const struct checkout_opts
*opts,
}
}
+ if (!active_cache_tree)
+ active_cache_tree = cache_tree();
+
+ if (!cache_tree_fully_valid(active_cache_tree))
+ cache_tree_update(active_cache_tree,
+ (const struct cache_entry * const
*)active_cache,
+ active_nr, WRITE_TREE_SILENT |
WRITE_TREE_REPAIR);
+
if (write_cache(newfd, active_cache, active_nr) ||
commit_locked_index(lock_file))
die(_(unable to write new index file));
diff --git a/cache-tree.c b/cache-tree.c
index 7fa524a..f951d7d 100644
--- a/cache-tree.c
+++ b/cache-tree.c
@@ -239,9 +239,12 @@ static int update_one(struct cache_tree *it,
struct strbuf buffer;
int missing_ok = flags WRITE_TREE_MISSING_OK;
int dryrun = flags WRITE_TREE_DRY_RUN;
+ int repair = flags WRITE_TREE_REPAIR;
int to_invalidate = 0;
int i;
+ assert(!(dryrun repair));
+
*skip_count = 0;
if (0 = it-entry_count has_sha1_file(it-sha1))
@@ -374,7 +377,14 @@ static int update_one(struct cache_tree *it,
#endif
}
- if (dryrun)
+ if (repair) {
+ unsigned char sha1[20];
+ hash_sha1_file(buffer.buf, buffer.len, tree_type, sha1);
+ if (has_sha1_file(sha1))
+ hashcpy(it-sha1, sha1);
+ else
+ to_invalidate = 1;
+ } else if (dryrun)
hash_sha1_file(buffer.buf, buffer.len, tree_type, it-sha1);
else if (write_sha1_file(buffer.buf, buffer.len, tree_type, it-sha1)) {
strbuf_release(buffer);
diff --git a/cache-tree.h b/cache-tree.h
index f1923ad..666d18f 100644
--- a/cache-tree.h
+++ b/cache-tree.h
@@ -39,6 +39,7 @@ int update_main_cache_tree(int);
#define WRITE_TREE_IGNORE_CACHE_TREE 2
#define WRITE_TREE_DRY_RUN 4
#define WRITE_TREE_SILENT 8
+#define WRITE_TREE_REPAIR 16
/* error return codes */
#define WRITE_TREE_UNREADABLE_INDEX (-1)
diff --git a/t/t0090-cache-tree.sh b/t/t0090-cache-tree.sh
index 6c33e28..98fb1ab 100755
--- a/t/t0090-cache-tree.sh
+++ b/t/t0090-cache-tree.sh
@@ -44,14 +44,14 @@ test_expect_success 'read-tree HEAD establishes cache-tree'
'
test_expect_success 'git-add invalidates cache-tree' '
test_when_finished git reset --hard; git read-tree HEAD
- echo I changed this file foo
+ echo I changed this file foo
git add foo
test_invalid_cache_tree
'
test_expect_success 'update-index invalidates cache-tree' '
test_when_finished git reset --hard; git read-tree HEAD
- echo I changed this file foo
+ echo I changed this file foo
git update-index --add foo
test_invalid_cache_tree
'
@@ -85,9 +85,22 @@ test_expect_success 'reset --hard without index gives
cache-tree' '
test_shallow_cache_tree
'
-test_expect_failure 'checkout gives cache-tree' '
+test_expect_success 'checkout gives cache-tree' '
+ git tag current
git checkout HEAD^
test_shallow_cache_tree
'
+test_expect_success 'checkout -b gives cache-tree' '
+ git checkout current
+ git checkout -b prev HEAD^
+ test_shallow_cache_tree
+'
+
+test_expect_success 'checkout -B gives cache-tree' '
+ git checkout current
+ git checkout -B prev HEAD^
+ test_shallow_cache_tree
+'
+
test_done
--
2.0.0.390.gcb682f8
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v5 4/4] cache-tree: Write updated cache-tree after commit
During the commit process, update the cache-tree. Write this updated
cache-tree so that it's ready for subsequent commands.
Add test code which demonstrates that git commit now writes the cache
tree. Make all tests test the entire cache-tree, not just the root
level.
Signed-off-by: David Turner dtur...@twitter.com
---
builtin/commit.c | 9 +-
t/t0090-cache-tree.sh | 79 ++-
2 files changed, 73 insertions(+), 15 deletions(-)
diff --git a/builtin/commit.c b/builtin/commit.c
index 9cfef6c..99c9054 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -342,6 +342,8 @@ static char *prepare_index(int argc, const char **argv,
const char *prefix,
discard_cache();
read_cache_from(index_lock.filename);
+ if (update_main_cache_tree(WRITE_TREE_SILENT) = 0)
+ write_cache(fd, active_cache, active_nr);
commit_style = COMMIT_NORMAL;
return index_lock.filename;
@@ -383,8 +385,12 @@ static char *prepare_index(int argc, const char **argv,
const char *prefix,
if (!only !pathspec.nr) {
fd = hold_locked_index(index_lock, 1);
refresh_cache_or_die(refresh_flags);
- if (active_cache_changed) {
+ if (active_cache_changed
+ || !cache_tree_fully_valid(active_cache_tree)) {
update_main_cache_tree(WRITE_TREE_SILENT);
+ active_cache_changed = 1;
+ }
+ if (active_cache_changed) {
if (write_cache(fd, active_cache, active_nr) ||
commit_locked_index(index_lock))
die(_(unable to write new_index file));
@@ -435,6 +441,7 @@ static char *prepare_index(int argc, const char **argv,
const char *prefix,
fd = hold_locked_index(index_lock, 1);
add_remove_files(partial);
refresh_cache(REFRESH_QUIET);
+ update_main_cache_tree(WRITE_TREE_SILENT);
if (write_cache(fd, active_cache, active_nr) ||
close_lock_file(index_lock))
die(_(unable to write new_index file));
diff --git a/t/t0090-cache-tree.sh b/t/t0090-cache-tree.sh
index 496c034..8c89689 100755
--- a/t/t0090-cache-tree.sh
+++ b/t/t0090-cache-tree.sh
@@ -8,7 +8,7 @@ cache-tree extension.
. ./test-lib.sh
cmp_cache_tree () {
- test-dump-cache-tree actual
+ test-dump-cache-tree | sed -e '/#(ref)/d' actual
sed s/$_x40/SHA/ actual filtered
test_cmp $1 filtered
}
@@ -16,8 +16,26 @@ cmp_cache_tree () {
# We don't bother with actually checking the SHA1:
# test-dump-cache-tree already verifies that all existing data is
# correct.
-test_shallow_cache_tree () {
- printf SHA (%d entries, 0 subtrees)\n $(git ls-files|wc -l) expect
+generate_expected_cache_tree () {
+ dir=$1${1:+/}
+ parent=$2
+ # ls-files might have foo/bar, foo/bar/baz, and foo/bar/quux
+ # We want to count only foo because it's the only direct child
+ subtrees=$(git ls-files|grep /|cut -d / -f 1|uniq)
+ subtree_count=$(echo $subtrees|awk '$1 {++c} END {print c}')
+ entries=$(git ls-files|wc -l)
+ printf SHA $dir (%d entries, %d subtrees)\n $entries $subtree_count
+ for subtree in $subtrees
+ do
+ cd $subtree
+ generate_expected_cache_tree $dir$subtree $dir || return 1
+ cd ..
+ done
+ dir=$parent
+}
+
+test_cache_tree () {
+ generate_expected_cache_tree expect
cmp_cache_tree expect
}
@@ -33,14 +51,14 @@ test_no_cache_tree () {
cmp_cache_tree expect
}
-test_expect_failure 'initial commit has cache-tree' '
+test_expect_success 'initial commit has cache-tree' '
test_commit foo
- test_shallow_cache_tree
+ test_cache_tree
'
test_expect_success 'read-tree HEAD establishes cache-tree' '
git read-tree HEAD
- test_shallow_cache_tree
+ test_cache_tree
'
test_expect_success 'git-add invalidates cache-tree' '
@@ -58,6 +76,18 @@ test_expect_success 'git-add in subdir invalidates
cache-tree' '
test_invalid_cache_tree
'
+cat before \EOF
+SHA (3 entries, 2 subtrees)
+SHA dir1/ (1 entries, 0 subtrees)
+SHA dir2/ (1 entries, 0 subtrees)
+EOF
+
+cat expect \EOF
+invalid (2 subtrees)
+invalid dir1/ (0 subtrees)
+SHA dir2/ (1 entries, 0 subtrees)
+EOF
+
test_expect_success 'git-add in subdir does not invalidate sibling cache-tree'
'
git tag no-children
test_when_finished git reset --hard no-children; git read-tree HEAD
@@ -65,8 +95,10 @@ test_expect_success 'git-add in subdir does not invalidate
sibling cache-tree' '
test_commit dir1/a
test_commit dir2/b
echo I changed this file dir1/a
+ cmp_cache_tree before
+ echo I
[PATCH v5 2/4] test-dump-cache-tree: invalid trees are not errors
Do not treat known-invalid trees as errors even when their subtree_nr is
incorrect. Because git already knows that these trees are invalid,
an incorrect subtree_nr will not cause problems.
Add a couple of comments.
Signed-off-by: David Turner dtur...@twitter.com
---
test-dump-cache-tree.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/test-dump-cache-tree.c b/test-dump-cache-tree.c
index 47eab97..cbbbd8e 100644
--- a/test-dump-cache-tree.c
+++ b/test-dump-cache-tree.c
@@ -26,16 +26,16 @@ static int dump_cache_tree(struct cache_tree *it,
return 0;
if (it-entry_count 0) {
+ /* invalid */
dump_one(it, pfx, );
dump_one(ref, pfx, #(ref) );
- if (it-subtree_nr != ref-subtree_nr)
- errs = 1;
}
else {
dump_one(it, pfx, );
if (hashcmp(it-sha1, ref-sha1) ||
ref-entry_count != it-entry_count ||
ref-subtree_nr != it-subtree_nr) {
+ /* claims to be valid but is lying */
dump_one(ref, pfx, #(ref) );
errs = 1;
}
--
2.0.0.390.gcb682f8
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v5 3/4] cache-tree: subdirectory tests
Add tests to confirm that invalidation of subdirectories neither over-
nor under-invalidates.
Signed-off-by: David Turner dtur...@twitter.com
---
t/t0090-cache-tree.sh | 26 +++---
1 file changed, 23 insertions(+), 3 deletions(-)
diff --git a/t/t0090-cache-tree.sh b/t/t0090-cache-tree.sh
index 98fb1ab..496c034 100755
--- a/t/t0090-cache-tree.sh
+++ b/t/t0090-cache-tree.sh
@@ -22,9 +22,10 @@ test_shallow_cache_tree () {
}
test_invalid_cache_tree () {
- echo invalid (0 subtrees) expect
- printf SHA #(ref) (%d entries, 0 subtrees)\n $(git ls-files|wc -l)
expect
- cmp_cache_tree expect
+ printf invalid %s ()\n $@
expect
+ test-dump-cache-tree | \
+ sed -n -e s/$_x40/SHA/ -e s/[0-9]* subtrees//g -e '/#(ref)/d' -e
'/^invalid /p' actual
+ test_cmp expect actual
}
test_no_cache_tree () {
@@ -49,6 +50,25 @@ test_expect_success 'git-add invalidates cache-tree' '
test_invalid_cache_tree
'
+test_expect_success 'git-add in subdir invalidates cache-tree' '
+ test_when_finished git reset --hard; git read-tree HEAD
+ mkdir dirx
+ echo I changed this file dirx/foo
+ git add dirx/foo
+ test_invalid_cache_tree
+'
+
+test_expect_success 'git-add in subdir does not invalidate sibling cache-tree'
'
+ git tag no-children
+ test_when_finished git reset --hard no-children; git read-tree HEAD
+ mkdir dir1 dir2
+ test_commit dir1/a
+ test_commit dir2/b
+ echo I changed this file dir1/a
+ git add dir1/a
+ test_invalid_cache_tree dir1/
+'
+
test_expect_success 'update-index invalidates cache-tree' '
test_when_finished git reset --hard; git read-tree HEAD
echo I changed this file foo
--
2.0.0.390.gcb682f8
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Branch list by date
I built this terribly-written alias because I wanted to see a list of branches by date of commit. The output looks like this: $ git bbd 11 months ago pipette_editor 7 weeks ago ensure-ie-rendering-edge 6 weeks ago strings-yml 5 weeks ago message-when-validation-fails 4 weeks ago new-parsers 11 days ago tax 8 hours ago search 7 hours ago browse 16 minutes ago master 8 seconds ago org-read And the alias, in all its glory: [alias] bbd = !export head=$(git symbolic-ref HEAD); git for-each-ref --sort=committerdate --format 'color=$(if [[ %(refname) = $head ]]; then printf \\\e[32m\; fi); printf \\\e[01;30m%%15s\\e(B\\e[m %%s%%s\\n\ %(committerdate:relative) $color %(refname:short)' refs/heads/ --shell | sh I write this missive with dual purpose: firstly to share a potentially useful tool, and secondly to suggest that this feature (with a less mind-wrenchingly disgusting implementation) might be included in mainline git, as for example `git branch [-t] | [--by-time]`. Until the ocean swallows us all, j -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 09/10] replace: check mergetags when using --graft
From: Junio C Hamano gits...@pobox.com Christian Couder chrisc...@tuxfamily.org writes: When using --graft, with a mergetag in the original commit, we should check that the commit pointed to by the mergetag is still a parent of then new commit we create, otherwise the mergetag could be misleading. If the commit pointed to by the mergetag is no more a parent of the new commit, we could remove the mergetag, but in this case there is a good chance that the title or other elements of the commit might also be misleading. So let's just error out and suggest to use --edit instead on the commit. I do not quite understand the reasoning. If you have a merge you earlier made with a signed tag, and then want to redo the merge with an updated tip of that branch (perhaps the side branch was earlier based on maint but now was rebased on master), it will perfectly be normal to expect that the title or other elements of the resulting merge to stay the same. Yeah, but then you might also want to have a mergetag for the updated tip of the branch and --graft will not put it in the new commit, so it would be better to use --edit in this case. Why is it a good idea to error it out? Because sometimes, in complex cases, it is misleading to do as if you can do the right thing, when there is a good chance you cannot. If the argument were 'replace --graft that changes the parents is likely to affect merge summary message, so error out and suggest to use --edit instead', regardless of the 'mergetag', I'd understand it, but that would essentially mean that 'replace --graft' should never be used, so... I think that when replace --graft is used on a regular commit there is much better chance that the resulting replacement commit will be as the user expect it to be. Thanks, Christian. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 03/10] replace: add test for --graft
From: Junio C Hamano gits...@pobox.com
Christian Couder chrisc...@tuxfamily.org writes:
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
Signed-off-by: Junio C Hamano gits...@pobox.com
---
t/t6050-replace.sh | 40
1 file changed, 40 insertions(+)
diff --git a/t/t6050-replace.sh b/t/t6050-replace.sh
index fb07ad2..d80a89e 100755
--- a/t/t6050-replace.sh
+++ b/t/t6050-replace.sh
@@ -18,6 +18,33 @@ add_and_commit_file()
git commit --quiet -m $_file: $_msg
}
+commit_buffer_contains_parents()
SP before (), perhaps?
Ok.
+{
+git cat-file commit $1 payload
+sed -n -e '/^$/q' -e '/^parent /p' payload actual
+shift
+: expected
+for _parent
+do
+echo parent $_parent expected || return 1
+done
You can redirect the entire loop to simplify the above 5 lines,
which would read better, no?
for _parent
do
echo parent $_parent
done expect
Ok, thanks.
+test_cmp actual expected
As test_cmp normally runs diff, it is better to compare expect
with actual, not the other way around; running the test verbosely,
i.e. t6050-replace.sh -v, shows how the actual output differs from
what is expected when diagnosing breakage and would be more useful
that way.
Ok.
If your goal is to test both the object contents with this code
*and* the overall system behaviour with the $child^$parent below,
perhaps they should be in the same commit_has_parents function,
without forcing the caller to choose between the two or call both,
no?
Yeah, will do.
Thanks,
Christian.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 08/10] commit: add for_each_mergetag()
From: Junio C Hamano gits...@pobox.com Christian Couder chrisc...@tuxfamily.org writes: In the same way as there is for_each_ref() to iterate on refs, it might be useful to have for_each_mergetag() to iterate on the mergetags of a given commit. Signed-off-by: Christian Couder chrisc...@tuxfamily.org --- Heh, might be useful is an understatement ;-) We won't apply a change that might be useful very lightly, but this refactoring already uses the helper in existing code, showing that it *is* useful, no? Let's have this early in the series, or perhaps even independent of the replace series. Ok, thanks, Christian. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
