[PATCH v3 00/11] add options to for-each-ref
This is continuation of the patch series 'Create ref-filter from for-each-ref' found at : http://thread.gmane.org/gmane.comp.version-control.git/271563 The previous version of this series can be found at : http://thread.gmane.org/gmane.comp.version-control.git/271575 Changes made: * Add PARSE_OPT_HIDDEN back in 09/11 * Whitespace changes and added missing information in 01/11 * Reduce redundancy in 05/11 Thanks to Junio C Hamano, Matthieu Moy, and Christian Couder for all the suggestions. -- Regards, Karthik Nayak -- 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
rebase -i might leave CHERRY_PICK_HEAD behind
Hi, When skipping an empty commit with 'git rebase --continue' a CHERRY_PICK_HEAD file might be left behind. What I did boils down to this: echo one file git add file git commit -m first echo two file git commit -a -m second echo three file git commit -a -m third git rebase -i HEAD^^ # change todo list to edit the second commit echo three file git commit -a --amend # this effectively makes the third commit an empty commit # and rebase will ask what to do: git rebase --continue The previous cherry-pick is now empty, possibly due to conflict resolution. If you wish to commit it anyway, use: git commit --allow-empty Otherwise, please use 'git reset' rebase in progress; onto 7335bbe7a5 You are currently rebasing branch 'master' on '7335bbe7a5'. nothing to commit, working directory clean Could not apply d19f82ac6d467247117fd734ed039b03ef923c86... third # I didn't want an empty commit, but didn't read that carefully, so I did: git rebase --continue Successfully rebased and updated refs/heads/master. # and was rewarded for my lack of attention with the following bash prompt: test/rebase-empty-continue (master|CHERRY-PICKING)$ # indeed: ls -l .git/CHERRY_PICK_HEAD -rw-r--r-- 1 szeder szeder 41 Jun 16 13:22 .git/CHERRY_PICK_HEAD On one hand, it's user error: it told me to run 'git reset' to achive what I want but I didn't. Note, however, how it told me about 'git reset': while 'git commit --allow-empty' is greatly emphasized by indentation and empty lines before and after, 'git reset' blends in quite well into the rebase progress. It was late, I was tired, and there was a questionable penalty on Copa América as well ;), so I simply didn't notice. On the other hand, 1. 'git rebase' claimed that Successfully rebased..., yet it left cruft behind. I think it shouldn't. 2. 'git rebase --continue' didn't complain by the lack of prior 'git reset' and finished doing exacly what I expected from it to do (except leaving CHERRY_PICK_HEAD behind, of course). Perhaps it should complain, like it does when the worktree is dirty. Alternatively, it could just continue to DWIM, as it does already, but then it should remove CHERRY_PICK_HEAD as well. Gábor -- 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] [RFC] Make git-update-ref invoke the update and post-update hooks
On Mon, Jun 15, 2015 at 3:45 PM, Junio C Hamano gits...@pobox.com wrote: Those who want to bypass your policy can use vi .git/packed-refs and/or rm .git/refs/heads/foo; you would not propose to patch rm and vi to pay attention to git configuration, because they are not about git at all, and there are cases where these bare-metal level editing of repositories is necessary. You would instead tell them don't do that. When I say update-ref is a low-level plumbing command that should not enforce policy, I am suggesting you to treat the command just like you would treat rm and vi. I suppose I see your point, though the line they've crossed at that point is from benign to malicious. When you assume they've crossed that line, hooks and configuration parameters are mostly useless in other git commands as well. My assumption here is they want to do something productive and aren't intentionally seeking to subvert policies, but rather aren't aware of the policies and/or correct procedure. Thanks for your responses, I appreciate your time. -Brian -- 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
Using clean/smudge filters with difftool
Hi everyone, I created a clean filter to apply on some files before commiting them. The filter works correctly when I commit the file and is also applied when I usethe iff command line tool. However, when using difftool with meld, the filter is not applied and the different versions of the files are compared without any filtering. Is there a way to apply the clean/smudge filters when comparing the working copy of a file to the HEAD version in a gui diff tool? I'm using git version 2.4.3 under Ubuntu. Best, Florian -- 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 v3 02/11] tag: libify parse_opt_points_at()
Rename 'parse_opt_points_at()' to 'parse_opt_object_name()' and move it from 'tag.c' to 'parse-options'. This now acts as a common parse_opt function which accepts an objectname and stores it into a sha1_array. Based-on-patch-by: Jeff King p...@peff.net Mentored-by: Christian Couder christian.cou...@gmail.com Mentored-by: Matthieu Moy matthieu@grenoble-inp.fr Signed-off-by: Karthik Nayak karthik@gmail.com --- builtin/tag.c | 21 ++--- parse-options-cb.c | 17 + parse-options.h| 1 + 3 files changed, 20 insertions(+), 19 deletions(-) diff --git a/builtin/tag.c b/builtin/tag.c index 5f6cdc5..e36c43e 100644 --- a/builtin/tag.c +++ b/builtin/tag.c @@ -546,23 +546,6 @@ static int strbuf_check_tag_ref(struct strbuf *sb, const char *name) return check_refname_format(sb-buf, 0); } -static int parse_opt_points_at(const struct option *opt __attribute__((unused)), - const char *arg, int unset) -{ - unsigned char sha1[20]; - - if (unset) { - sha1_array_clear(points_at); - return 0; - } - if (!arg) - return error(_(switch 'points-at' requires an object)); - if (get_sha1(arg, sha1)) - return error(_(malformed object name '%s'), arg); - sha1_array_append(points_at, sha1); - return 0; -} - static int parse_opt_sort(const struct option *opt, const char *arg, int unset) { int *sort = opt-value; @@ -625,8 +608,8 @@ int cmd_tag(int argc, const char **argv, const char *prefix) parse_opt_with_commit, (intptr_t)HEAD, }, { - OPTION_CALLBACK, 0, points-at, NULL, N_(object), - N_(print only tags of the object), 0, parse_opt_points_at + OPTION_CALLBACK, 0, points-at, points_at, N_(object), + N_(print only tags of the object), 0, parse_opt_object_name }, OPT_END() }; diff --git a/parse-options-cb.c b/parse-options-cb.c index be8c413..de75411 100644 --- a/parse-options-cb.c +++ b/parse-options-cb.c @@ -4,6 +4,7 @@ #include commit.h #include color.h #include string-list.h +#include sha1-array.h /*- some often used options -*/ @@ -92,6 +93,22 @@ int parse_opt_with_commit(const struct option *opt, const char *arg, int unset) return 0; } +int parse_opt_object_name(const struct option *opt, const char *arg, int unset) +{ + unsigned char sha1[20]; + + if (unset) { + sha1_array_clear(opt-value); + return 0; + } + if (!arg) + return -1; + if (get_sha1(arg, sha1)) + return error(_(malformed object name '%s'), arg); + sha1_array_append(opt-value, sha1); + return 0; +} + int parse_opt_tertiary(const struct option *opt, const char *arg, int unset) { int *target = opt-value; diff --git a/parse-options.h b/parse-options.h index c71e9da..36c71fe 100644 --- a/parse-options.h +++ b/parse-options.h @@ -220,6 +220,7 @@ extern int parse_opt_approxidate_cb(const struct option *, const char *, int); extern int parse_opt_expiry_date_cb(const struct option *, const char *, int); extern int parse_opt_color_flag_cb(const struct option *, const char *, int); extern int parse_opt_verbosity_cb(const struct option *, const char *, int); +extern int parse_opt_object_name(const struct option *, const char *, int); extern int parse_opt_with_commit(const struct option *, const char *, int); extern int parse_opt_tertiary(const struct option *, const char *, int); extern int parse_opt_string_list(const struct option *, const char *, int); -- 2.4.3.436.g722e2ce.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 v3 03/11] ref-filter: implement '--points-at' option
In 'tag -l' we have '--points-at' option which lets users list only tags which point to a particular commit. Implement this option in 'ref-filter.{c,h}' so that other commands can benefit from this. This is duplicated from tag.c, we will eventually remove that when we port tag.c to use ref-filter APIs. Based-on-patch-by: Jeff King p...@peff.net Mentored-by: Christian Couder christian.cou...@gmail.com Mentored-by: Matthieu Moy matthieu@grenoble-inp.fr Signed-off-by: Karthik Nayak karthik@gmail.com --- builtin/tag.c | 4 ref-filter.c | 26 ++ ref-filter.h | 1 + 3 files changed, 31 insertions(+) diff --git a/builtin/tag.c b/builtin/tag.c index e36c43e..280981f 100644 --- a/builtin/tag.c +++ b/builtin/tag.c @@ -56,6 +56,10 @@ static int match_pattern(const char **patterns, const char *ref) return 0; } +/* + * This is currently duplicated in ref-filter.c, and will eventually be + * removed as we port tag.c to use the ref-filter APIs. + */ static const unsigned char *match_points_at(const char *refname, const unsigned char *sha1) { diff --git a/ref-filter.c b/ref-filter.c index 43502a4..591e281 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -842,6 +842,29 @@ static int match_name_as_path(const char **pattern, const char *refname) return 0; } +/* + * Given a ref (sha1, refname) see if it points to a one of the sha1s + * in a sha1_array. + */ +static int match_points_at(struct sha1_array *points_at, const unsigned char *sha1, + const char *refname) +{ + struct object *obj; + + if (!points_at || !points_at-nr) + return 1; + + if (sha1_array_lookup(points_at, sha1) = 0) + return 1; + + obj = parse_object_or_die(sha1, refname); + if (obj-type == OBJ_TAG + sha1_array_lookup(points_at, ((struct tag *)obj)-tagged-sha1) = 0) + return 1; + + return 0; +} + /* Allocate space for a new ref_array_item and copy the objectname and flag to it */ static struct ref_array_item *new_ref_array_item(const char *refname, const unsigned char *objectname, @@ -875,6 +898,9 @@ static int ref_filter_handler(const char *refname, const struct object_id *oid, if (*filter-name_patterns !match_name_as_path(filter-name_patterns, refname)) return 0; + if (!match_points_at(filter-points_at, oid-hash, refname)) + return 0; + /* * We do not open the object yet; sort may only need refname * to do its job and the resulting list may yet to be pruned diff --git a/ref-filter.h b/ref-filter.h index 6997984..c2856b8 100644 --- a/ref-filter.h +++ b/ref-filter.h @@ -42,6 +42,7 @@ struct ref_array { struct ref_filter { const char **name_patterns; + struct sha1_array points_at; }; struct ref_filter_cbdata { -- 2.4.3.436.g722e2ce.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] git-prompt.sh: Document GIT_PS1_STATESEPARATOR
Quoting Joe Cridge joe.cri...@me.com: The environment variable GIT_PS1_STATESEPARATOR can be used to set the separator between the branch name and the state symbols in the prompt. At present the variable is not mentioned in the inline documentation which makes it difficult for the casual user to identify. Thanks, makes sense. Signed-off-by: Joe Cridge joe.cri...@me.com --- contrib/completion/git-prompt.sh | 4 1 file changed, 4 insertions(+) diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh index f18aedc..366f0bc 100644 --- a/contrib/completion/git-prompt.sh +++ b/contrib/completion/git-prompt.sh @@ -66,6 +66,10 @@ # git always compare HEAD to @{upstream} # svn always compare HEAD to your SVN upstream # +# You can change the separator between the branch name and the above +# state symbols by setting GIT_PS1_STATESEPARATOR. The default separator +# is SP. This is not a specification of a protocol or file or input/output format, where we formally use SP and LF. Perhaps we could spell out SP as a space here, for the sake of the casual user? +# # By default, __git_ps1 will compare HEAD to your SVN upstream if it can # find one, or @{upstream} otherwise. Once you have set # GIT_PS1_SHOWUPSTREAM, you can override it on a per-repository basis by -- 2.4.2 -- 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 v3 05/11] ref-filter: add parse_opt_merge_filter()
Add 'parse_opt_merge_filter()' to parse '--merged' and '--no-merged' options and write MACROS for the same. This is copied from 'builtin/branch.c' which will eventually be removed when we port 'branch.c' to use ref-filter APIs. Based-on-patch-by: Jeff King p...@peff.net Mentored-by: Christian Couder christian.cou...@gmail.com Mentored-by: Matthieu Moy matthieu@grenoble-inp.fr Signed-off-by: Karthik Nayak karthik@gmail.com --- builtin/branch.c | 4 ref-filter.c | 21 + ref-filter.h | 11 +++ 3 files changed, 36 insertions(+) diff --git a/builtin/branch.c b/builtin/branch.c index b42e5b6..ddd90e6 100644 --- a/builtin/branch.c +++ b/builtin/branch.c @@ -745,6 +745,10 @@ static void rename_branch(const char *oldname, const char *newname, int force) strbuf_release(newsection); } +/* + * This function is duplicated in ref-filter. It will eventually be removed + * when we port branch.c to use ref-filter APIs. + */ static int opt_parse_merge_filter(const struct option *opt, const char *arg, int unset) { merge_filter = ((opt-long_name[0] == 'n') diff --git a/ref-filter.c b/ref-filter.c index 591e281..6502179 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -1125,3 +1125,24 @@ int parse_opt_ref_sorting(const struct option *opt, const char *arg, int unset) s-atom = parse_ref_filter_atom(arg, arg+len); return 0; } + +int parse_opt_merge_filter(const struct option *opt, const char *arg, int unset) +{ + struct ref_filter *rf = opt-value; + unsigned char sha1[20]; + + rf-merge = starts_with(opt-long_name, no) + ? REF_FILTER_MERGED_OMIT + : REF_FILTER_MERGED_INCLUDE; + + if (!arg) + arg = HEAD; + if (get_sha1(arg, sha1)) + die(_(malformed object name %s), arg); + + rf-merge_commit = lookup_commit_reference_gently(sha1, 0); + if (!rf-merge_commit) + return opterror(opt, must point to a commit, 0); + + return 0; +} diff --git a/ref-filter.h b/ref-filter.h index c2856b8..ad2902b 100644 --- a/ref-filter.h +++ b/ref-filter.h @@ -50,6 +50,15 @@ struct ref_filter_cbdata { struct ref_filter *filter; }; +/* Macros for checking --merged and --no-merged options */ +#define _OPT_MERGED_NO_MERGED(option, filter, h) \ + { OPTION_CALLBACK, 0, option, (filter), N_(commit), (h), \ + PARSE_OPT_LASTARG_DEFAULT | PARSE_OPT_NONEG, \ + parse_opt_merge_filter, (intptr_t) HEAD \ + } +#define OPT_MERGED(f, h) _OPT_MERGED_NO_MERGED(merged, f, h) +#define OPT_NO_MERGED(f, h) _OPT_MERGED_NO_MERGED(no-merged, f, h) + /* * API for filtering a set of refs. Based on the type of refs the user * has requested, we iterate through those refs and apply filters @@ -71,5 +80,7 @@ void show_ref_array_item(struct ref_array_item *info, const char *format, int qu int parse_opt_ref_sorting(const struct option *opt, const char *arg, int unset); /* Default sort option based on refname */ struct ref_sorting *ref_default_sorting(void); +/* Function to parse --merged and --no-merged options */ +int parse_opt_merge_filter(const struct option *opt, const char *arg, int unset); #endif /* REF_FILTER_H */ -- 2.4.3.436.g722e2ce.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 v3 06/11] ref-filter: implement '--merged' and '--no-merged' options
In 'branch -l' we have '--merged' option which only lists refs (branches) merged into the named commit and '--no-merged' option which only lists refs (branches) not merged into the named commit. Implement these two options in ref-filter.{c,h} so that other commands can benefit from this. Based-on-patch-by: Jeff King p...@peff.net Mentored-by: Christian Couder christian.cou...@gmail.com Mentored-by: Matthieu Moy matthieu@grenoble-inp.fr Signed-off-by: Karthik Nayak karthik@gmail.com --- ref-filter.c | 68 ref-filter.h | 8 +++ 2 files changed, 72 insertions(+), 4 deletions(-) diff --git a/ref-filter.c b/ref-filter.c index 6502179..06328eb 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -9,6 +9,7 @@ #include tag.h #include quote.h #include ref-filter.h +#include revision.h typedef enum { FIELD_STR, FIELD_ULONG, FIELD_TIME } cmp_type; @@ -889,6 +890,7 @@ static int ref_filter_handler(const char *refname, const struct object_id *oid, struct ref_filter_cbdata *ref_cbdata = cb_data; struct ref_filter *filter = ref_cbdata-filter; struct ref_array_item *ref; + struct commit *commit = NULL; if (flag REF_BAD_NAME) { warning(ignoring ref with broken name %s, refname); @@ -901,12 +903,19 @@ static int ref_filter_handler(const char *refname, const struct object_id *oid, if (!match_points_at(filter-points_at, oid-hash, refname)) return 0; + if (filter-merge_commit) { + commit = lookup_commit_reference_gently(oid-hash, 1); + if (!commit) + return 0; + } + /* * We do not open the object yet; sort may only need refname * to do its job and the resulting list may yet to be pruned * by maxcount logic. */ ref = new_ref_array_item(refname, oid-hash, flag); + ref-commit = commit; REALLOC_ARRAY(ref_cbdata-array-items, ref_cbdata-array-nr + 1); ref_cbdata-array-items[ref_cbdata-array-nr++] = ref; @@ -932,6 +941,50 @@ void ref_array_clear(struct ref_array *array) array-nr = array-alloc = 0; } +static void do_merge_filter(struct ref_filter_cbdata *ref_cbdata) +{ + struct rev_info revs; + int i, old_nr; + struct ref_filter *filter = ref_cbdata-filter; + struct ref_array *array = ref_cbdata-array; + struct commit_list *p, *to_clear = NULL; + + init_revisions(revs, NULL); + + for (i = 0; i array-nr; i++) { + struct ref_array_item *item = array-items[i]; + add_pending_object(revs, item-commit-object, item-refname); + commit_list_insert(item-commit, to_clear); + } + + filter-merge_commit-object.flags |= UNINTERESTING; + add_pending_object(revs, filter-merge_commit-object, ); + + revs.limited = 1; + if (prepare_revision_walk(revs)) + die(_(revision walk setup failed)); + + old_nr = array-nr; + array-nr = 0; + + for (i = 0; i old_nr; i++) { + struct ref_array_item *item = array-items[i]; + struct commit *commit = item-commit; + + int is_merged = !!(commit-object.flags UNINTERESTING); + + if (is_merged == (filter-merge == REF_FILTER_MERGED_INCLUDE)) + array-items[array-nr++] = array-items[i]; + else + free_array_item(item); + } + + for (p = to_clear; p; p = p-next) + clear_commit_marks(p-item, ALL_REV_FLAGS); + clear_commit_marks(filter-merge_commit, ALL_REV_FLAGS); + free_commit_list(to_clear); +} + /* * API for filtering a set of refs. Based on the type of refs the user * has requested, we iterate through those refs and apply filters @@ -941,17 +994,24 @@ void ref_array_clear(struct ref_array *array) int filter_refs(struct ref_array *array, struct ref_filter *filter, unsigned int type) { struct ref_filter_cbdata ref_cbdata; + int ret = 0; ref_cbdata.array = array; ref_cbdata.filter = filter; + /* Simple per-ref filtering */ if (type (FILTER_REFS_ALL | FILTER_REFS_INCLUDE_BROKEN)) - return for_each_rawref(ref_filter_handler, ref_cbdata); + ret = for_each_rawref(ref_filter_handler, ref_cbdata); else if (type FILTER_REFS_ALL) - return for_each_ref(ref_filter_handler, ref_cbdata); - else + ret = for_each_ref(ref_filter_handler, ref_cbdata); + else if (type) die(filter_refs: invalid type); - return 0; + + /* Filters that need revision walking */ + if (filter-merge_commit) + do_merge_filter(ref_cbdata); + + return ret; } static int cmp_ref_sorting(struct ref_sorting *s, struct ref_array_item *a, struct ref_array_item *b) diff --git
[PATCH v3 11/11] for-each-ref: add '--contains' option
Add the '--contains' option provided by 'ref-filter'. The '--contains' option lists only refs which are contain the mentioned commit (HEAD if no commit is explicitly given). Add documentation and tests for the same. Mentored-by: Christian Couder christian.cou...@gmail.com Mentored-by: Matthieu Moy matthieu@grenoble-inp.fr Signed-off-by: Karthik Nayak karthik@gmail.com --- Documentation/git-for-each-ref.txt | 5 + builtin/for-each-ref.c | 2 ++ t/t6301-for-each-ref-filter.sh | 13 + 3 files changed, 20 insertions(+) diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt index d61a756..7a949f3 100644 --- a/Documentation/git-for-each-ref.txt +++ b/Documentation/git-for-each-ref.txt @@ -11,6 +11,7 @@ SYNOPSIS 'git for-each-ref' [--count=count] [--shell|--perl|--python|--tcl] [(--sort=key)...] [--format=format] [pattern...] [--points-at object] [(--merged | --no-merged) object] + [--contains object] DESCRIPTION --- @@ -74,6 +75,10 @@ OPTIONS Only list refs whose tips are not reachable from the specified commit (HEAD if not specified). +--contains [commit]:: + Only list tags which contain the specified commit (HEAD if not + specified). + FIELD NAMES --- diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c index 00913f4..8ccbb1c 100644 --- a/builtin/for-each-ref.c +++ b/builtin/for-each-ref.c @@ -9,6 +9,7 @@ static char const * const for_each_ref_usage[] = { N_(git for-each-ref [options] [pattern]), N_(git for-each-ref [--points-at object]), N_(git for-each-ref [(--merged | --no-merged) object]), + N_(git for-each-ref [--contains object]), NULL }; @@ -41,6 +42,7 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix) parse_opt_object_name), OPT_MERGED(filter, N_(print only refs that are merged)), OPT_NO_MERGED(filter, N_(print only refs that are not merged)), + OPT_CONTAINS(filter.with_commit, N_(print only refs which contain the commit)), OPT_END(), }; diff --git a/t/t6301-for-each-ref-filter.sh b/t/t6301-for-each-ref-filter.sh index 593c3cb..ab7928c 100755 --- a/t/t6301-for-each-ref-filter.sh +++ b/t/t6301-for-each-ref-filter.sh @@ -64,4 +64,17 @@ test_expect_success 'filtering with --no-merged' ' test_cmp expect actual ' +test_expect_success 'filtering with --contains' ' + cat expect -\EOF + refs/heads/master + refs/heads/side + refs/odd/spot + refs/tags/four + refs/tags/three + refs/tags/two + EOF + git for-each-ref --format=%(refname) --contains=two actual + test_cmp expect actual +' + test_done -- 2.4.3.436.g722e2ce.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 v3 10/11] ref-filter: implement '--contains' option
'tag -l' and 'branch -l' have two different ways of finding out if a certain ref contains a commit. Implement both these methods in ref-filter and give the caller of ref-filter API the option to pick which implementation to be used. 'branch -l' uses 'is_descendant_of()' from commit.c which is left as the default implementation to be used. 'tag -l' uses a more specific algorithm since ffc4b80. This implementation is used whenever the 'with_commit_tag_algo' bit is set in 'struct ref_filter'. Mentored-by: Christian Couder christian.cou...@gmail.com Mentored-by: Matthieu Moy matthieu@grenoble-inp.fr Signed-off-by: Karthik Nayak karthik@gmail.com --- ref-filter.c | 113 ++- ref-filter.h | 3 ++ 2 files changed, 115 insertions(+), 1 deletion(-) diff --git a/ref-filter.c b/ref-filter.c index 06328eb..3d9a7c6 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -818,6 +818,114 @@ static void get_ref_atom_value(struct ref_array_item *ref, int atom, struct atom *v = ref-value[atom]; } +enum contains_result { + CONTAINS_UNKNOWN = -1, + CONTAINS_NO = 0, + CONTAINS_YES = 1 +}; + +/* + * Mimicking the real stack, this stack lives on the heap, avoiding stack + * overflows. + * + * At each recursion step, the stack items points to the commits whose + * ancestors are to be inspected. + */ +struct contains_stack { + int nr, alloc; + struct contains_stack_entry { + struct commit *commit; + struct commit_list *parents; + } *contains_stack; +}; + +static int in_commit_list(const struct commit_list *want, struct commit *c) +{ + for (; want; want = want-next) + if (!hashcmp(want-item-object.sha1, c-object.sha1)) + return 1; + return 0; +} + +/* + * Test whether the candidate or one of its parents is contained in the list. + * Do not recurse to find out, though, but return -1 if inconclusive. + */ +static enum contains_result contains_test(struct commit *candidate, + const struct commit_list *want) +{ + /* was it previously marked as containing a want commit? */ + if (candidate-object.flags TMP_MARK) + return 1; + /* or marked as not possibly containing a want commit? */ + if (candidate-object.flags UNINTERESTING) + return 0; + /* or are we it? */ + if (in_commit_list(want, candidate)) { + candidate-object.flags |= TMP_MARK; + return 1; + } + + if (parse_commit(candidate) 0) + return 0; + + return -1; +} + +static void push_to_contains_stack(struct commit *candidate, struct contains_stack *contains_stack) +{ + ALLOC_GROW(contains_stack-contains_stack, contains_stack-nr + 1, contains_stack-alloc); + contains_stack-contains_stack[contains_stack-nr].commit = candidate; + contains_stack-contains_stack[contains_stack-nr++].parents = candidate-parents; +} + +static enum contains_result contains_tag_algo(struct commit *candidate, + const struct commit_list *want) +{ + struct contains_stack contains_stack = { 0, 0, NULL }; + int result = contains_test(candidate, want); + + if (result != CONTAINS_UNKNOWN) + return result; + + push_to_contains_stack(candidate, contains_stack); + while (contains_stack.nr) { + struct contains_stack_entry *entry = contains_stack.contains_stack[contains_stack.nr - 1]; + struct commit *commit = entry-commit; + struct commit_list *parents = entry-parents; + + if (!parents) { + commit-object.flags |= UNINTERESTING; + contains_stack.nr--; + } + /* +* If we just popped the stack, parents-item has been marked, +* therefore contains_test will return a meaningful 0 or 1. +*/ + else switch (contains_test(parents-item, want)) { + case CONTAINS_YES: + commit-object.flags |= TMP_MARK; + contains_stack.nr--; + break; + case CONTAINS_NO: + entry-parents = parents-next; + break; + case CONTAINS_UNKNOWN: + push_to_contains_stack(parents-item, contains_stack); + break; + } + } + free(contains_stack.contains_stack); + return contains_test(candidate, want); +} + +static int commit_contains(struct ref_filter *filter, struct commit *commit) +{ + if (filter-with_commit_tag_algo) + return contains_tag_algo(commit, filter-with_commit); + return is_descendant_of(commit, filter-with_commit); +} + /* * Return 1 if the refname matches one of the patterns, otherwise 0. * A pattern
[PATCH v3 08/11] parse-option: rename parse_opt_with_commit()
Rename parse_opt_with_commit() to parse_opt_commit_object_name() to show that it can be used to obtain a list of commits and is not constricted to usage of '--contains' option. Mentored-by: Christian Couder christian.cou...@gmail.com Mentored-by: Matthieu Moy matthieu@grenoble-inp.fr Signed-off-by: Karthik Nayak karthik@gmail.com --- builtin/branch.c | 4 ++-- builtin/tag.c | 4 ++-- parse-options-cb.c | 2 +- parse-options.h| 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/builtin/branch.c b/builtin/branch.c index ddd90e6..ddd728e 100644 --- a/builtin/branch.c +++ b/builtin/branch.c @@ -828,13 +828,13 @@ int cmd_branch(int argc, const char **argv, const char *prefix) OPTION_CALLBACK, 0, contains, with_commit, N_(commit), N_(print only branches that contain the commit), PARSE_OPT_LASTARG_DEFAULT, - parse_opt_with_commit, (intptr_t)HEAD, + parse_opt_commit_object_name, (intptr_t)HEAD, }, { OPTION_CALLBACK, 0, with, with_commit, N_(commit), N_(print only branches that contain the commit), PARSE_OPT_HIDDEN | PARSE_OPT_LASTARG_DEFAULT, - parse_opt_with_commit, (intptr_t) HEAD, + parse_opt_commit_object_name, (intptr_t) HEAD, }, OPT__ABBREV(abbrev), diff --git a/builtin/tag.c b/builtin/tag.c index 280981f..2d6610a 100644 --- a/builtin/tag.c +++ b/builtin/tag.c @@ -603,13 +603,13 @@ int cmd_tag(int argc, const char **argv, const char *prefix) OPTION_CALLBACK, 0, contains, with_commit, N_(commit), N_(print only tags that contain the commit), PARSE_OPT_LASTARG_DEFAULT, - parse_opt_with_commit, (intptr_t)HEAD, + parse_opt_commit_object_name, (intptr_t)HEAD, }, { OPTION_CALLBACK, 0, with, with_commit, N_(commit), N_(print only tags that contain the commit), PARSE_OPT_HIDDEN | PARSE_OPT_LASTARG_DEFAULT, - parse_opt_with_commit, (intptr_t)HEAD, + parse_opt_commit_object_name, (intptr_t)HEAD, }, { OPTION_CALLBACK, 0, points-at, points_at, N_(object), diff --git a/parse-options-cb.c b/parse-options-cb.c index de75411..8bec5e4 100644 --- a/parse-options-cb.c +++ b/parse-options-cb.c @@ -77,7 +77,7 @@ int parse_opt_verbosity_cb(const struct option *opt, const char *arg, return 0; } -int parse_opt_with_commit(const struct option *opt, const char *arg, int unset) +int parse_opt_commit_object_name(const struct option *opt, const char *arg, int unset) { unsigned char sha1[20]; struct commit *commit; diff --git a/parse-options.h b/parse-options.h index 36c71fe..8542d9c 100644 --- a/parse-options.h +++ b/parse-options.h @@ -221,7 +221,7 @@ extern int parse_opt_expiry_date_cb(const struct option *, const char *, int); extern int parse_opt_color_flag_cb(const struct option *, const char *, int); extern int parse_opt_verbosity_cb(const struct option *, const char *, int); extern int parse_opt_object_name(const struct option *, const char *, int); -extern int parse_opt_with_commit(const struct option *, const char *, int); +extern int parse_opt_commit_object_name(const struct option *, const char *, int); extern int parse_opt_tertiary(const struct option *, const char *, int); extern int parse_opt_string_list(const struct option *, const char *, int); extern int parse_opt_noop_cb(const struct option *, const char *, int); -- 2.4.3.436.g722e2ce.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 v3 07/11] for-each-ref: add '--merged' and '--no-merged' options
Add the '--merged' and '--no-merged' options provided by 'ref-filter'. The '--merged' option lets the user to only list refs merged into the named commit. The '--no-merged' option lets the user to only list refs not merged into the named commit. Add documentation and tests for the same. Based-on-patch-by: Jeff King p...@peff.net Mentored-by: Christian Couder christian.cou...@gmail.com Mentored-by: Matthieu Moy matthieu@grenoble-inp.fr Signed-off-by: Karthik Nayak karthik@gmail.com --- Documentation/git-for-each-ref.txt | 10 +- builtin/for-each-ref.c | 3 +++ t/t6301-for-each-ref-filter.sh | 21 + 3 files changed, 33 insertions(+), 1 deletion(-) diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt index 0524ac4..d61a756 100644 --- a/Documentation/git-for-each-ref.txt +++ b/Documentation/git-for-each-ref.txt @@ -10,7 +10,7 @@ SYNOPSIS [verse] 'git for-each-ref' [--count=count] [--shell|--perl|--python|--tcl] [(--sort=key)...] [--format=format] [pattern...] - [--points-at object] + [--points-at object] [(--merged | --no-merged) object] DESCRIPTION --- @@ -66,6 +66,14 @@ OPTIONS --points-at object:: Only list refs of the given object. +--merged [commit]:: + Only list refs whose tips are reachable from the + specified commit (HEAD if not specified). + +--no-merged [commit]:: + Only list refs whose tips are not reachable from the + specified commit (HEAD if not specified). + FIELD NAMES --- diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c index 2dee149..00913f4 100644 --- a/builtin/for-each-ref.c +++ b/builtin/for-each-ref.c @@ -8,6 +8,7 @@ static char const * const for_each_ref_usage[] = { N_(git for-each-ref [options] [pattern]), N_(git for-each-ref [--points-at object]), + N_(git for-each-ref [(--merged | --no-merged) object]), NULL }; @@ -38,6 +39,8 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix) OPT_CALLBACK(0, points-at, filter.points_at, N_(object), N_(print only refs of the object), parse_opt_object_name), + OPT_MERGED(filter, N_(print only refs that are merged)), + OPT_NO_MERGED(filter, N_(print only refs that are not merged)), OPT_END(), }; diff --git a/t/t6301-for-each-ref-filter.sh b/t/t6301-for-each-ref-filter.sh index 1ad65f0..593c3cb 100755 --- a/t/t6301-for-each-ref-filter.sh +++ b/t/t6301-for-each-ref-filter.sh @@ -43,4 +43,25 @@ test_expect_success 'filtering with --points-at' ' test_cmp expect actual ' +test_expect_success 'filtering with --merged' ' + cat expect -\EOF + refs/heads/master + refs/odd/spot + refs/tags/one + refs/tags/three + refs/tags/two + EOF + git for-each-ref --format=%(refname) --merged=master actual + test_cmp expect actual +' + +test_expect_success 'filtering with --no-merged' ' + cat expect -\EOF + refs/heads/side + refs/tags/four + EOF + git for-each-ref --format=%(refname) --no-merged=master actual + test_cmp expect actual +' + test_done -- 2.4.3.436.g722e2ce.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 v3 09/11] parse-options.h: add macros for '--contains' option
Add a macro for using the '--contains' option in parse-options.h also include an optional '--with' option macro which performs the same action as '--contains'. Make tag.c use this new macro. Mentored-by: Christian Couder christian.cou...@gmail.com Mentored-by: Matthieu Moy matthieu@grenoble-inp.fr Signed-off-by: Karthik Nayak karthik@gmail.com --- builtin/tag.c | 14 ++ parse-options.h | 7 +++ 2 files changed, 9 insertions(+), 12 deletions(-) diff --git a/builtin/tag.c b/builtin/tag.c index 2d6610a..767162e 100644 --- a/builtin/tag.c +++ b/builtin/tag.c @@ -595,23 +595,13 @@ int cmd_tag(int argc, const char **argv, const char *prefix) OPT_GROUP(N_(Tag listing options)), OPT_COLUMN(0, column, colopts, N_(show tag list in columns)), + OPT_CONTAINS(with_commit, N_(print only tags that contain the commit)), + OPT_WITH(with_commit, N_(print only tags that contain the commit)), { OPTION_CALLBACK, 0, sort, tag_sort, N_(type), N_(sort tags), PARSE_OPT_NONEG, parse_opt_sort }, { - OPTION_CALLBACK, 0, contains, with_commit, N_(commit), - N_(print only tags that contain the commit), - PARSE_OPT_LASTARG_DEFAULT, - parse_opt_commit_object_name, (intptr_t)HEAD, - }, - { - OPTION_CALLBACK, 0, with, with_commit, N_(commit), - N_(print only tags that contain the commit), - PARSE_OPT_HIDDEN | PARSE_OPT_LASTARG_DEFAULT, - parse_opt_commit_object_name, (intptr_t)HEAD, - }, - { OPTION_CALLBACK, 0, points-at, points_at, N_(object), N_(print only tags of the object), 0, parse_opt_object_name }, diff --git a/parse-options.h b/parse-options.h index 8542d9c..2b555ff 100644 --- a/parse-options.h +++ b/parse-options.h @@ -243,5 +243,12 @@ extern int parse_opt_noop_cb(const struct option *, const char *, int); OPT_COLOR_FLAG(0, color, (var), (h)) #define OPT_COLUMN(s, l, v, h) \ { OPTION_CALLBACK, (s), (l), (v), N_(style), (h), PARSE_OPT_OPTARG, parseopt_column_callback } +#define _OPT_CONTAINS_OR_WITH(name, variable, help, flag)\ + { OPTION_CALLBACK, 0, name, (variable), N_(commit), (help), \ + PARSE_OPT_LASTARG_DEFAULT | flag, \ + parse_opt_commit_object_name, (intptr_t) HEAD \ + } +#define OPT_CONTAINS(v, h) _OPT_CONTAINS_OR_WITH(contains, v, h, 0) +#define OPT_WITH(v, h) _OPT_CONTAINS_OR_WITH(with, v, h, PARSE_OPT_HIDDEN) #endif -- 2.4.3.436.g722e2ce.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 v3 04/11] for-each-ref: add '--points-at' option
Add the '--points-at' option provided by 'ref-filter'. The option lets the user to pick only refs which point to a particular commit. Add documentation and tests for the same. Based-on-patch-by: Jeff King p...@peff.net Mentored-by: Christian Couder christian.cou...@gmail.com Mentored-by: Matthieu Moy matthieu@grenoble-inp.fr Signed-off-by: Karthik Nayak karthik@gmail.com --- Documentation/git-for-each-ref.txt | 3 +++ builtin/for-each-ref.c | 9 +++-- t/t6301-for-each-ref-filter.sh | 10 ++ 3 files changed, 20 insertions(+), 2 deletions(-) diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt index 7f8d9a5..0524ac4 100644 --- a/Documentation/git-for-each-ref.txt +++ b/Documentation/git-for-each-ref.txt @@ -10,6 +10,7 @@ SYNOPSIS [verse] 'git for-each-ref' [--count=count] [--shell|--perl|--python|--tcl] [(--sort=key)...] [--format=format] [pattern...] + [--points-at object] DESCRIPTION --- @@ -62,6 +63,8 @@ OPTIONS the specified host language. This is meant to produce a scriptlet that can directly be `eval`ed. +--points-at object:: + Only list refs of the given object. FIELD NAMES --- diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c index 7919206..2dee149 100644 --- a/builtin/for-each-ref.c +++ b/builtin/for-each-ref.c @@ -7,6 +7,7 @@ static char const * const for_each_ref_usage[] = { N_(git for-each-ref [options] [pattern]), + N_(git for-each-ref [--points-at object]), NULL }; @@ -34,9 +35,15 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix) OPT_STRING( 0 , format, format, N_(format), N_(format to use for the output)), OPT_CALLBACK(0 , sort, sorting_tail, N_(key), N_(field name to sort on), parse_opt_ref_sorting), + OPT_CALLBACK(0, points-at, filter.points_at, +N_(object), N_(print only refs of the object), +parse_opt_object_name), OPT_END(), }; + memset(array, 0, sizeof(array)); + memset(filter, 0, sizeof(filter)); + parse_options(argc, argv, prefix, opts, for_each_ref_usage, 0); if (maxcount 0) { error(invalid --count argument: `%d', maxcount); @@ -55,8 +62,6 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix) /* for warn_ambiguous_refs */ git_config(git_default_config, NULL); - memset(array, 0, sizeof(array)); - memset(filter, 0, sizeof(filter)); filter.name_patterns = argv; filter_refs(array, filter, FILTER_REFS_ALL | FILTER_REFS_INCLUDE_BROKEN); ref_array_sort(sorting, array); diff --git a/t/t6301-for-each-ref-filter.sh b/t/t6301-for-each-ref-filter.sh index bfcb866..1ad65f0 100755 --- a/t/t6301-for-each-ref-filter.sh +++ b/t/t6301-for-each-ref-filter.sh @@ -33,4 +33,14 @@ test_expect_success 'filtering with fnmatch' ' test_cmp expect actual ' +test_expect_success 'filtering with --points-at' ' + cat expect -\EOF + refs/heads/master + refs/odd/spot + refs/tags/three + EOF + git for-each-ref --format=%(refname) --points-at=master actual + test_cmp expect actual +' + test_done -- 2.4.3.436.g722e2ce.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 v3 04/11] for-each-ref: add '--points-at' option
On Tue, Jun 16, 2015 at 9:30 PM, Matthieu Moy matthieu@grenoble-inp.fr wrote: Karthik Nayak karthik@gmail.com writes: --- a/Documentation/git-for-each-ref.txt +++ b/Documentation/git-for-each-ref.txt @@ -62,6 +63,8 @@ OPTIONS the specified host language. This is meant to produce a scriptlet that can directly be `eval`ed. +--points-at object:: + Only list refs of the given object. I would find Only list refs pointing to the given object clearer. Noted. Thanks -- Regards, Karthik Nayak -- 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: rebase -i might leave CHERRY_PICK_HEAD behind
Hi, On 2015-06-16 14:06, SZEDER Gábor wrote: When skipping an empty commit with 'git rebase --continue' a CHERRY_PICK_HEAD file might be left behind. Yeah, I noticed that, too... it even survives the cleanup of the finished rebase under certain circumstances. Maybe something like this? -- snipsnap -- diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh index dc3133f..16e0a82 100644 --- a/git-rebase--interactive.sh +++ b/git-rebase--interactive.sh @@ -849,7 +849,11 @@ continue) # do we have anything to commit? if git diff-index --cached --quiet HEAD -- then - : Nothing to commit -- skip this + : Nothing to commit -- skip this commit + + test ! -f $GIT_DIR/CHERRY_PICK_HEAD || + rm $GIT_DIR/CHERRY_PICK_HEAD || + die Could not remove CHERRY_PICK_HEAD else if ! test -f $author_script then -- 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 v3 01/11] t6301: for-each-ref tests for ref-filter APIs
Karthik Nayak karthik@gmail.com writes: Add tests for 'for-each-ref' which utilizes the ref-filter APIs. Currently it's redundant with the tests in 't6300' but more tests will be eventually added as we implement more options into 'for-each-ref'. Actually, I'm not convinced that the actual tests have a real value (since as you say, it's redundant with t6300). Perhaps we can limit this commit to the setup. +++ b/t/t6301-for-each-ref-filter.sh @@ -0,0 +1,36 @@ +#!/bin/sh + +test_description='test for-each-refs usage of ref-filter APIs' + +. ./test-lib.sh +. $TEST_DIRECTORY/lib-gpg.sh You are not using lib-gpg.sh, right? If first thought it was an incorrect cut-and-paste, but I actually think that you need to setup a signed tag to properly test the --points-at option (it does not only list refs pointing directly at a commit, but also refs pointing at a tag pointing at the given commit). Such signed tag could be added here and used later in PATCH 04. -- 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] git-prompt.sh: Document GIT_PS1_STATESEPARATOR
Quoting SZEDER Gábor sze...@ira.uka.de: diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh index f18aedc..366f0bc 100644 --- a/contrib/completion/git-prompt.sh +++ b/contrib/completion/git-prompt.sh @@ -66,6 +66,10 @@ # git always compare HEAD to @{upstream} # svn always compare HEAD to your SVN upstream # +# You can change the separator between the branch name and the above +# state symbols by setting GIT_PS1_STATESEPARATOR. The default separator +# is SP. This is not a specification of a protocol or file or input/output format, where we formally use SP and LF. Perhaps we could spell out SP as a space here, for the sake of the casual user? Yes, I agree. Would it also be worth changing lines 25 and 29? @@ 18,12 @@ #3b) Alternatively, for a slightly faster prompt, __git_ps1 can #be used for PROMPT_COMMAND in Bash or for precmd() in Zsh #with two parameters, pre and post, which are strings #you would put in $PS1 before and after the status string #generated by the git-prompt machinery. e.g. #Bash: PROMPT_COMMAND='__git_ps1 \u@\h:\w \\\$ ' # will show username, at-sign, host, colon, cwd, then # various status string, followed by dollar and SP, as # your prompt. #ZSH: precmd () { __git_ps1 %n :%~$ |%s } # will show username, pipe, then various status string, # followed by colon, cwd, dollar and SP, as your prompt.-- 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] Documentation on git-checkout --ours/--theirs improved
Simon Eugster simon...@gmail.com writes: 2015-06-15 22:10 GMT+02:00 Junio C Hamano gits...@pobox.com: Simon A. Eugster simon...@gmail.com writes: --- - Lack of explanation as to why this is a good thing. - Lack of sign-off. Why is there still 1/2, if its effect is wholly annulled by a subsequent step 2/2? Sorry for that, still trying to find out how git send-email works. I do not think git send-email is involved in that process in any way. The problem is you made the updates on top of the previous one, without squashing. You fed two commits, instead of a squashed one commit, to git send-email, and the command obliged and sent them out. +During merging, we assume the role of the canonical history’s keeper, +which, in case of a rebase, is the remote history, and our private commits +look to the keeper as “their” commits which need to be integrated on top +of “our” work. ++ +Normal merging: + +local -abC -- canonical history + | git checkout --ours + v +MERGE -abC + ^ + | git checkout --theirs +origin/master ---Xyz + +Rebasing: + +local ---Abc + | git checkout --theirs + v +REBASE xyZ + ^ + | git checkout --ours +origin/master -xyZ-- canonical history + I can see that an arrow with canonical history points at different things between the two pictures, but other than that, I am not sure what these are trying to illustrate. Especially between abc and xyz, why does the former choose abc while the latter choooses xyz? Are these pictures meant to show what happens when the user says checkout --ours during a conflicted integration (whether it is a merge or a rebase)? I tried to create a picture which shows the difference of ours and theirs when merging vs. rebasing, but apparently it did not turn out well, and I will just leave it away. I'll wait for several days to see what other people would say, if they care to comment on this. Maybe they can come up with a more intuitive picture, or maybe they say textual description is sufficiently clear that we do not need an illustration. I dunno. 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/3] pkt-line: support tracing verbatim pack contents
On Fri, Jun 12, 2015 at 5:28 PM, Jeff King p...@peff.net wrote: When debugging the pack protocol, it is sometimes useful to store the verbatim pack that we sent or received on the wire. Looking at the on-disk result is often not helpful for a few reasons: 1. If the operation is a clone, we destroy the repo on failure, leaving nothing on disk. 2. If the pack is small, we unpack it immediately, and the full pack never hits the disk. 3. If we feed the pack to index-pack --fix-thin, the resulting pack has the extra delta bases added to it. We already have a GIT_TRACE_PACKET mechanism for tracing packets. Let's extend it with GIT_TRACE_PACK to dump the verbatim packfile. FWIW, this also works for me - I have no preference between my patches and Jeff's. I suspect yours are much better given that you have a clue about git internals ;). One bit of feedback is that it might be worth mentioning (though I don't feel strongly) that GIT_TRACE_PACK works with or without GIT_TRACE_PACKET - that wasn't immediately obvious to me, but it makes sense once I read the code. Thanks! There are a few other positive fallouts that come from rearranging this code: - We currently disable the packet trace after seeing the PACK header, even though we may get human-readable lines on other sidebands; now we include them in the trace. - We currently try to print PACK ... in the trace to indicate that the packfile has started. But because we disable packet tracing, we never printed this line. We will now do so. Signed-off-by: Jeff King p...@peff.net --- Documentation/git.txt | 13 +++- pkt-line.c| 59 ++- t/t5601-clone.sh | 7 ++ trace.c | 7 ++ trace.h | 1 + 5 files changed, 71 insertions(+), 16 deletions(-) diff --git a/Documentation/git.txt b/Documentation/git.txt index 45b64a7..8c44d14 100644 --- a/Documentation/git.txt +++ b/Documentation/git.txt @@ -1000,9 +1000,20 @@ Unsetting the variable, or setting it to empty, 0 or Enables trace messages for all packets coming in or out of a given program. This can help with debugging object negotiation or other protocol issues. Tracing is turned off at a packet - starting with PACK. + starting with PACK (but see 'GIT_TRACE_PACK' below). See 'GIT_TRACE' for available trace output options. +'GIT_TRACE_PACK':: + Enables tracing of packfiles sent or received by a + given program. Unlike other trace output, this trace is + verbatim: no headers, and no quoting of binary data. You almost + certainly want to direct into a file (e.g., + `GIT_TRACE_PACK=/tmp/my.pack`) rather than displaying it on the + terminal or mixing it with other trace output. ++ +Note that this is currently only implemented for the client side +of clones and fetches. + 'GIT_TRACE_PERFORMANCE':: Enables performance related trace messages, e.g. total execution time of each Git command. diff --git a/pkt-line.c b/pkt-line.c index e75af02..2e122c0 100644 --- a/pkt-line.c +++ b/pkt-line.c @@ -4,16 +4,51 @@ char packet_buffer[LARGE_PACKET_MAX]; static const char *packet_trace_prefix = git; static struct trace_key trace_packet = TRACE_KEY_INIT(PACKET); +static struct trace_key trace_pack = TRACE_KEY_INIT(PACK); void packet_trace_identity(const char *prog) { packet_trace_prefix = xstrdup(prog); } +static int packet_trace_pack(const char *buf, unsigned int len, int sideband) +{ + if (!sideband) { + trace_verbatim(trace_pack, buf, len); + return 1; + } else if (len *buf == '\1') { + trace_verbatim(trace_pack, buf + 1, len - 1); + return 1; + } else { + /* it's another non-pack sideband */ + return 0; + } +} + static void packet_trace(const char *buf, unsigned int len, int write) { int i; struct strbuf out; + static int in_pack, sideband; + + if (!trace_want(trace_packet) !trace_want(trace_pack)) + return; + + if (in_pack) { + if (packet_trace_pack(buf, len, sideband)) + return; + } else if (starts_with(buf, PACK) || starts_with(buf, \1PACK)) { + in_pack = 1; + sideband = *buf == '\1'; + packet_trace_pack(buf, len, sideband); + + /* +* Make a note in the human-readable trace that the pack data +* started. +*/ + buf = PACK ...; + len = strlen(buf); + } if (!trace_want(trace_packet)) return; @@ -24,21 +59,15 @@ static void packet_trace(const char *buf, unsigned int len, int write)
Re: [PATCH v3 01/11] t6301: for-each-ref tests for ref-filter APIs
On Tue, Jun 16, 2015 at 8:51 PM, Matthieu Moy matthieu@grenoble-inp.fr wrote: Karthik Nayak karthik@gmail.com writes: Add tests for 'for-each-ref' which utilizes the ref-filter APIs. Currently it's redundant with the tests in 't6300' but more tests will be eventually added as we implement more options into 'for-each-ref'. Actually, I'm not convinced that the actual tests have a real value (since as you say, it's redundant with t6300). Perhaps we can limit this commit to the setup. Well I just wanted some minimal tests, but yeah, we could limit this only to the setup. +++ b/t/t6301-for-each-ref-filter.sh @@ -0,0 +1,36 @@ +#!/bin/sh + +test_description='test for-each-refs usage of ref-filter APIs' + +. ./test-lib.sh +. $TEST_DIRECTORY/lib-gpg.sh You are not using lib-gpg.sh, right? If first thought it was an incorrect cut-and-paste, but I actually think that you need to setup a signed tag to properly test the --points-at option (it does not only list refs pointing directly at a commit, but also refs pointing at a tag pointing at the given commit). Such signed tag could be added here and used later in PATCH 04. It is a cut-paste error. Thanks will look into it. -- Regards, Karthik Nayak -- 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 v3 05/11] ref-filter: add parse_opt_merge_filter()
On Tue, Jun 16, 2015 at 9:48 PM, Matthieu Moy matthieu@grenoble-inp.fr wrote: Karthik Nayak karthik@gmail.com writes: This is copied from 'builtin/branch.c' which will eventually be removed when we port 'branch.c' to use ref-filter APIs. Earlier in the series you took code from tag.c. I think you should focus on either merge or tag, get a ref-filter-based replacement that passes the tests for it, and then consider the other. The fact that the test pass for a rewritten command is important to check the correctness of the these patches. I'm not asking you to remove commits from this series though. Just impatient to see one command fully replaced (actually, I see that you have more commits than you sent in your branch, so I guess it will come soon on the list) :-). The idea is to currently get ref-filter to support all options and port it over to for-each-ref which would be the first command to completely use ref-filter. And like you said, the challenge is to then ensure tag.c and branch.c to use ref-filter and make them pass all tests. If you see my branch on Github, I have been pushing patches to port tag.c to use ref-filter. I just finish porting tag.c to use ref-filter. Now to include missing options in it, then I'll push the series to the list. (Must be quite a co-incidence cause I just pushed a patch for tag.c to completely use ref-filter ) Like you mentioned tag.c and branch.c both have their own setup for which small changes need to be made in ref-filter. So taking one step at a time, I'll finish up with tag.c and end it with branch.c -- Regards, Karthik Nayak -- 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 v3 04/11] for-each-ref: add '--points-at' option
Karthik Nayak karthik@gmail.com writes: --- a/Documentation/git-for-each-ref.txt +++ b/Documentation/git-for-each-ref.txt @@ -62,6 +63,8 @@ OPTIONS the specified host language. This is meant to produce a scriptlet that can directly be `eval`ed. +--points-at object:: + Only list refs of the given object. I would find Only list refs pointing to the given object clearer. -- 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 v3 05/11] ref-filter: add parse_opt_merge_filter()
Karthik Nayak karthik@gmail.com writes: This is copied from 'builtin/branch.c' which will eventually be removed when we port 'branch.c' to use ref-filter APIs. Earlier in the series you took code from tag.c. I think you should focus on either merge or tag, get a ref-filter-based replacement that passes the tests for it, and then consider the other. The fact that the test pass for a rewritten command is important to check the correctness of the these patches. I'm not asking you to remove commits from this series though. Just impatient to see one command fully replaced (actually, I see that you have more commits than you sent in your branch, so I guess it will come soon on the list) :-). -- 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 3/3] pkt-line: support tracing verbatim pack contents
On Tue, Jun 16, 2015 at 09:39:19AM -0700, Junio C Hamano wrote: As to the documentation, I have a feeling that, unless the reader and/or the user intimately knows that TRACE_PACK is implemented by hooking into the same mechanism that TRACE_PACKET needs to, s/he would not even wonder if TRACE_PACKET needs to be enabled when asking for TRACE_PACK. Yes, one is a proper substring of the other, but the similarity between the two stops there. While I do not think it would hurt very much to mention that they are independent, I have a slight suspicion that it might make it more likely to get user confused. Yes, I was just re-reading the documentation based on Augie's comment, and it seems pretty clear to me. Of course I wrote it, so that is not saying much. Augie, I'd be happy to hear a proposed wording change if you have one. I do kind of hate the name TRACE_PACK for two reasons: - it _is_ so close to TRACE_PACKET; maybe TRACE_PACKFILE would be better - it does not indicate that it is about on-the-wire packs. I.e., it has nothing to do with git repack. But I could not think of a good succinct name to indicate that. -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] Fix power checking on OS X
Panagiotis Astithas past...@gmail.com writes: The output of pmset -g batt changed at some point from Currently drawing from 'AC Power' to the slightly different Now drawing from 'AC Power'. Starting the match from drawing makes the check work in both old and new versions of OS X. Signed-off-by: Panagiotis Astithas past...@gmail.com --- Hmph, is there any difference between this one and the original one that I already queued several days ago? contrib/hooks/pre-auto-gc-battery | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contrib/hooks/pre-auto-gc-battery b/contrib/hooks/pre-auto-gc-battery index 9d0c2d1..6a2cdeb 100755 --- a/contrib/hooks/pre-auto-gc-battery +++ b/contrib/hooks/pre-auto-gc-battery @@ -33,7 +33,7 @@ elif grep -q AC Power \+: 1 /proc/pmu/info 2/dev/null then exit 0 elif test -x /usr/bin/pmset /usr/bin/pmset -g batt | - grep -q Currently drawing from 'AC Power' + grep -q drawing from 'AC Power' then exit 0 fi -- 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/3] pkt-line: support tracing verbatim pack contents
On Tue, Jun 16, 2015 at 11:38:41AM -0400, Augie Fackler wrote: We already have a GIT_TRACE_PACKET mechanism for tracing packets. Let's extend it with GIT_TRACE_PACK to dump the verbatim packfile. FWIW, this also works for me - I have no preference between my patches and Jeff's. I suspect yours are much better given that you have a clue about git internals ;). I like mine better than yours, if only because it hooks into the existing tracing mechanism. But I am sad that neither of our proposals works for tracing pushed packs (something that is useful for the opposite situation: you have a sane git server, and some unknown misbehaving _client_ is sending you bogus packs). Here's a rough cut at the trace stdin idea I mentioned earlier (which is essentially an internal tee). You can collect the incoming pack like: GIT_TRACE=1 \ GIT_TRACE_STDIN=/tmp/foo.pack \ GIT_TRACE_STDIN_FOR=index-pack \ git fetch ... The STDIN_FOR hack is there because otherwise you get the stdin for multiple processes intermingled. I think a cleaner way to do it would be to allow something like: GIT_TRACE_STDIN=/tmp/stdin.%p and replace %p with the PID of the tracing process. I like this approach because it works everywhere (for pushed packs, but also for any other git commands you may want to debug). The downside versus the other patches is that to replay the index-pack command, you need to collect both its stdin and its arguments (which you can get from the GIT_TRACE output). On the other hand, it gives you a more realistic replay of what happened (e.g., if there is a bug triggered by the --pack-header code). Here is the patch: --- diff --git a/git.c b/git.c index 44374b1..d58866e 100644 --- a/git.c +++ b/git.c @@ -616,6 +616,80 @@ static void restore_sigpipe_to_default(void) signal(SIGPIPE, SIG_DFL); } +static int copy_stdin(int in, int out, void *data) +{ + struct trace_key *key = data; + while (1) { + char buf[8192]; + ssize_t len = xread(in, buf, sizeof(buf)); + if (!len) + break; + if (len 0) { + warning(error reading stdin trace: %s, + strerror(errno)); + break; + } + + trace_verbatim(key, buf, len); + if (write_in_full(out, buf, len) 0) { + warning(error writing stdin trace: %s, + strerror(errno)); + break; + } + } + close(in); + close(out); + return 0; +} + +static int trace_stdin_for(const char *cmd) +{ + int ret = 1; + const char *x = getenv(GIT_TRACE_STDIN_FOR); + + if (x) { + struct string_list items = STRING_LIST_INIT_DUP; + string_list_split(items, x, ':', -1); + ret = unsorted_string_list_has_string(items, cmd); + string_list_clear(items, 0); + } + + return ret; +} + +static void trace_stdin(const char *cmd) +{ + static struct trace_key key = TRACE_KEY_INIT(STDIN); + static struct async async; + + if (!trace_stdin_for(cmd) || !trace_want(key)) + return; + + memset(async, 0, sizeof(async)); + async.proc = copy_stdin; + async.data = key; + async.in = dup(0); + async.out = -1; + + if (async.in 0 || start_async(async) 0) { + warning(unable to trace stdin: %s, strerror(errno)); + return ; + } + + /* +* At this point we've handed stdin off to the async process, +* so there we are past the point of no return. +*/ + if (dup2(async.out, 0)) + die_errno(unable to redirect stdin from async process); + close(async.out); + + /* +* leak async; we would know to finish_async() only when we are +* exiting, and there is no point then +*/ +} + int main(int argc, char **av) { const char **argv = (const char **) av; @@ -674,6 +748,8 @@ int main(int argc, char **av) } cmd = argv[0]; + trace_stdin(cmd); + /* * We use PATH to find git commands, but we prepend some higher * precedence paths: the --exec-path option, the GIT_EXEC_PATH -- 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/3] pkt-line: support tracing verbatim pack contents
On Tue, Jun 16, 2015 at 01:14:03PM -0400, Augie Fackler wrote: Yeah, not having it for the push side is a slight bummer, but in general I haven't had problems debugging git clients pushing bogus data in the same way that I've had problems with weirdness in new server features. Being in charge of a large git server farm, I think I have the opposite problem. :) Here's a rough cut at the trace stdin idea I mentioned earlier (which is essentially an internal tee). You can collect the incoming pack like: Neat, but not sure I like the extra overhead of having to grab the full trace and then reconstruct some arguments to be able to diagnose the pack. Having the verbatim pack just land on disk is really handy, because then any existing tools one has cooked up (my team has a few weird one-off ones by now) just work without extra fussing or looking up steps to reconstruct the whole file. I guess there is really room for both. Just because you _can_ accomplish the same thing with both does not mean we cannot have two ways to do it (an easy way, and a harder, more flexible way). -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 3/3] pkt-line: support tracing verbatim pack contents
On Tue, Jun 16, 2015 at 12:52:46PM -0400, Augie Fackler wrote: Yeah, I don't have one - I may have been making assumptions because I knew something of the background on your implementation. i'd say let it live and if people provide feedback later figure out a reword (writing for humans is not a strong suit of mine, alas.) OK, let's leave it as-is, then. +1 on TRACE_PACKFILE - also makes it harder to accidentally leave off the ET and get binary spew when one didn't expect that. Here's a re-roll of 3/3, using TRACE_PACKFILE. I think the documentation is enough to allay my other concern (that the name does not imply it is about the network protocol). -- 8 -- Subject: pkt-line: support tracing verbatim pack contents When debugging the pack protocol, it is sometimes useful to store the verbatim pack that we sent or received on the wire. Looking at the on-disk result is often not helpful for a few reasons: 1. If the operation is a clone, we destroy the repo on failure, leaving nothing on disk. 2. If the pack is small, we unpack it immediately, and the full pack never hits the disk. 3. If we feed the pack to index-pack --fix-thin, the resulting pack has the extra delta bases added to it. We already have a GIT_TRACE_PACKET mechanism for tracing packets. Let's extend it with GIT_TRACE_PACKFILE to dump the verbatim packfile. There are a few other positive fallouts that come from rearranging this code: - We currently disable the packet trace after seeing the PACK header, even though we may get human-readable lines on other sidebands; now we include them in the trace. - We currently try to print PACK ... in the trace to indicate that the packfile has started. But because we disable packet tracing, we never printed this line. We will now do so. Signed-off-by: Jeff King p...@peff.net --- Documentation/git.txt | 13 +++- pkt-line.c| 59 ++- t/t5601-clone.sh | 7 ++ trace.c | 7 ++ trace.h | 1 + 5 files changed, 71 insertions(+), 16 deletions(-) diff --git a/Documentation/git.txt b/Documentation/git.txt index 45b64a7..3453669 100644 --- a/Documentation/git.txt +++ b/Documentation/git.txt @@ -1000,9 +1000,20 @@ Unsetting the variable, or setting it to empty, 0 or Enables trace messages for all packets coming in or out of a given program. This can help with debugging object negotiation or other protocol issues. Tracing is turned off at a packet - starting with PACK. + starting with PACK (but see 'GIT_TRACE_PACKFILE' below). See 'GIT_TRACE' for available trace output options. +'GIT_TRACE_PACKFILE':: + Enables tracing of packfiles sent or received by a + given program. Unlike other trace output, this trace is + verbatim: no headers, and no quoting of binary data. You almost + certainly want to direct into a file (e.g., + `GIT_TRACE_PACKFILE=/tmp/my.pack`) rather than displaying it on + the terminal or mixing it with other trace output. ++ +Note that this is currently only implemented for the client side +of clones and fetches. + 'GIT_TRACE_PERFORMANCE':: Enables performance related trace messages, e.g. total execution time of each Git command. diff --git a/pkt-line.c b/pkt-line.c index e75af02..08a1427 100644 --- a/pkt-line.c +++ b/pkt-line.c @@ -4,16 +4,51 @@ char packet_buffer[LARGE_PACKET_MAX]; static const char *packet_trace_prefix = git; static struct trace_key trace_packet = TRACE_KEY_INIT(PACKET); +static struct trace_key trace_pack = TRACE_KEY_INIT(PACKFILE); void packet_trace_identity(const char *prog) { packet_trace_prefix = xstrdup(prog); } +static int packet_trace_pack(const char *buf, unsigned int len, int sideband) +{ + if (!sideband) { + trace_verbatim(trace_pack, buf, len); + return 1; + } else if (len *buf == '\1') { + trace_verbatim(trace_pack, buf + 1, len - 1); + return 1; + } else { + /* it's another non-pack sideband */ + return 0; + } +} + static void packet_trace(const char *buf, unsigned int len, int write) { int i; struct strbuf out; + static int in_pack, sideband; + + if (!trace_want(trace_packet) !trace_want(trace_pack)) + return; + + if (in_pack) { + if (packet_trace_pack(buf, len, sideband)) + return; + } else if (starts_with(buf, PACK) || starts_with(buf, \1PACK)) { + in_pack = 1; + sideband = *buf == '\1'; + packet_trace_pack(buf, len, sideband); + + /* +* Make a note in the human-readable trace that the pack data +* started. +*/ + buf = PACK ...; + len = strlen(buf); + }
Re: [PATCH 3/3] pkt-line: support tracing verbatim pack contents
On Tue, Jun 16, 2015 at 1:18 PM, Jeff King p...@peff.net wrote: On Tue, Jun 16, 2015 at 01:14:03PM -0400, Augie Fackler wrote: Yeah, not having it for the push side is a slight bummer, but in general I haven't had problems debugging git clients pushing bogus data in the same way that I've had problems with weirdness in new server features. Being in charge of a large git server farm, I think I have the opposite problem. :) I've got plenty of servers too, but we haven't typically seen push-time problems. Maybe we're lucky, and now that I've said something and tempted Murphy I'll suffer torment. :) Here's a rough cut at the trace stdin idea I mentioned earlier (which is essentially an internal tee). You can collect the incoming pack like: Neat, but not sure I like the extra overhead of having to grab the full trace and then reconstruct some arguments to be able to diagnose the pack. Having the verbatim pack just land on disk is really handy, because then any existing tools one has cooked up (my team has a few weird one-off ones by now) just work without extra fussing or looking up steps to reconstruct the whole file. I guess there is really room for both. Just because you _can_ accomplish the same thing with both does not mean we cannot have two ways to do it (an easy way, and a harder, more flexible way). *nod* that might make the most sense - given that we both seem to have use cases in mind for verbatim packs on pulls, that seems like a good thing to have easy to deploy. (It still seems to me more likely to write custom servers than clients, but then again custom VCS servers has been my life for a while, so I likely have a weird perspective on things.) -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] Fix power checking on OS X
On Tue, Jun 16, 2015 at 8:32 PM, Junio C Hamano gits...@pobox.com wrote: Panagiotis Astithas past...@gmail.com writes: The output of pmset -g batt changed at some point from Currently drawing from 'AC Power' to the slightly different Now drawing from 'AC Power'. Starting the match from drawing makes the check work in both old and new versions of OS X. Signed-off-by: Panagiotis Astithas past...@gmail.com --- Hmph, is there any difference between this one and the original one that I already queued several days ago? None, I was just following the process outlined in SubmittingPatches that mentions a separate email after the discussion is over. -- 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] Fix power checking on OS X
Ah, thanks. Not many people do that, so I was wondering what was going on ;-). On Tue, Jun 16, 2015 at 10:35 AM, Panagiotis Astithas past...@gmail.com wrote: On Tue, Jun 16, 2015 at 8:32 PM, Junio C Hamano gits...@pobox.com wrote: Panagiotis Astithas past...@gmail.com writes: The output of pmset -g batt changed at some point from Currently drawing from 'AC Power' to the slightly different Now drawing from 'AC Power'. Starting the match from drawing makes the check work in both old and new versions of OS X. Signed-off-by: Panagiotis Astithas past...@gmail.com --- Hmph, is there any difference between this one and the original one that I already queued several days ago? None, I was just following the process outlined in SubmittingPatches that mentions a separate email after the discussion is over. -- 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/3] pkt-line: support tracing verbatim pack contents
Augie Fackler au...@google.com writes: On Fri, Jun 12, 2015 at 5:28 PM, Jeff King p...@peff.net wrote: When debugging the pack protocol, it is sometimes useful to store the verbatim pack that we sent or received on the wire. Looking at the on-disk result is often not helpful for a few reasons: 1. If the operation is a clone, we destroy the repo on failure, leaving nothing on disk. 2. If the pack is small, we unpack it immediately, and the full pack never hits the disk. 3. If we feed the pack to index-pack --fix-thin, the resulting pack has the extra delta bases added to it. We already have a GIT_TRACE_PACKET mechanism for tracing packets. Let's extend it with GIT_TRACE_PACK to dump the verbatim packfile. FWIW, this also works for me - I have no preference between my patches and Jeff's. I suspect yours are much better given that you have a clue about git internals ;). One bit of feedback is that it might be worth mentioning (though I don't feel strongly) that GIT_TRACE_PACK works with or without GIT_TRACE_PACKET - that wasn't immediately obvious to me, but it makes sense once I read the code. Thanks! Thanks, both. I think this series makes sense. As to the documentation, I have a feeling that, unless the reader and/or the user intimately knows that TRACE_PACK is implemented by hooking into the same mechanism that TRACE_PACKET needs to, s/he would not even wonder if TRACE_PACKET needs to be enabled when asking for TRACE_PACK. Yes, one is a proper substring of the other, but the similarity between the two stops there. While I do not think it would hurt very much to mention that they are independent, I have a slight suspicion that it might make it more likely to get user confused. -- 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/3] pkt-line: support tracing verbatim pack contents
On Tue, Jun 16, 2015 at 12:43 PM, Jeff King p...@peff.net wrote: On Tue, Jun 16, 2015 at 09:39:19AM -0700, Junio C Hamano wrote: As to the documentation, I have a feeling that, unless the reader and/or the user intimately knows that TRACE_PACK is implemented by hooking into the same mechanism that TRACE_PACKET needs to, s/he would not even wonder if TRACE_PACKET needs to be enabled when asking for TRACE_PACK. Yes, one is a proper substring of the other, but the similarity between the two stops there. While I do not think it would hurt very much to mention that they are independent, I have a slight suspicion that it might make it more likely to get user confused. Yes, I was just re-reading the documentation based on Augie's comment, and it seems pretty clear to me. Of course I wrote it, so that is not saying much. Augie, I'd be happy to hear a proposed wording change if you have one. Yeah, I don't have one - I may have been making assumptions because I knew something of the background on your implementation. i'd say let it live and if people provide feedback later figure out a reword (writing for humans is not a strong suit of mine, alas.) I do kind of hate the name TRACE_PACK for two reasons: - it _is_ so close to TRACE_PACKET; maybe TRACE_PACKFILE would be better +1 on TRACE_PACKFILE - also makes it harder to accidentally leave off the ET and get binary spew when one didn't expect that. AF -- 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 v2 7/7] bisect: allows any terms set by user
Matthieu Moy matthieu@grenoble-inp.fr writes: I would say for the current bisection, i.e. $ git bisect start $ git bisect terms foo bar # Scope starts here ... The first 'bar' commit is deadbeef. # Scope ends here $ git bisect terms foo bar You need to start by git bisect start Do you want me to do it for you [Y/n]? I personnaly don't like this autostart. In the first version of this patch 7/7 'git bisect terms' has to be called before the start. I think it is better because we can then use 'git bisect start rev1 rev2 ...' The next 'bisect start rev1 rev2' would be in bad/good mode. But this have to be discuted, do the user have to type 'git bisect terms' each bisection if he wants to use special terms ? To me, yes. If we allow arbitrary terms, then they will most likely be specific to one bisect session (e.g. if I bisect present/absent once, it tells me nothing about what I'll want for my next bisection). Ok. 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/3] trace: add GIT_TRACE_STDIN
On Tue, Jun 16, 2015 at 03:49:07PM -0400, Jeff King wrote: Another option would be to stop trying to intercept stdin in git.c, and instead make this a feature of run-command.c. That is, right before we exec a process, tee its stdin there. That means that you cannot do: GIT_TRACE_STDIN=/tmp/foo.out git foo to collect the stdin of foo. But that is not really an interesting case anyway. You can run tee yourself if you want. The interesting cases are the ones where git is spawning a sub-process, and you want to intercept the data moving between the git processes. Hmm. I guess we do not actually have to move the stdin interception there. We can just move the config-checking there, like the patch below. It basically just converts trace.foo.bar into GIT_TRACE_BAR when we are running foo as a git command. This does work, but is perhaps potentially confusing to the user, because it only kicks in when _git_ runs foo. IOW, this works: git config trace.upload-pack.foo /path/to/foo.out git daemon and will trace as you expect. But then running: git upload-pack yourself will do nothing. I dunno. --- run-command.c | 17 + trace.c | 44 trace.h | 9 + 3 files changed, 70 insertions(+) diff --git a/run-command.c b/run-command.c index 4d73e90..2febbb5 100644 --- a/run-command.c +++ b/run-command.c @@ -284,6 +284,23 @@ int start_command(struct child_process *cmd) if (!cmd-argv) cmd-argv = cmd-args.argv; + if (cmd-git_cmd) { + /* +* Load any extra variables into env_array. But +* if we weren't going to use it (in favor of env), +* then consolidate the two. Make sure the original env +* goes after what we add, so that it can override. +* +* We cannot just keep two lists, because we may hand off the +* single list to a spawn() implementation. +*/ + trace_config_for(cmd-argv[0], cmd-env_array); + if (cmd-env_array.argc cmd-env) { + for (; *cmd-env; cmd-env++) + argv_array_push(cmd-env_array, *cmd-env); + cmd-env = NULL; + } + } if (!cmd-env) cmd-env = cmd-env_array.argv; diff --git a/trace.c b/trace.c index a7ec484..86c988e 100644 --- a/trace.c +++ b/trace.c @@ -24,6 +24,7 @@ #include cache.h #include quote.h +#include argv-array.h static size_t expand_trace_name(struct strbuf *out, const char *fmt, void *data) @@ -449,3 +450,46 @@ void trace_command_performance(const char **argv) sq_quote_argv(command_line, argv, 0); command_start_time = getnanotime(); } + +struct trace_config_data { + const char *want_cmd; + struct argv_array *out; +}; + +static int trace_config_cb(const char *var, const char *value, void *vdata) +{ + struct trace_config_data *data = vdata; + const char *have_cmd, *key; + int have_len; + + if (!parse_config_key(var, trace, have_cmd, have_len, key) + have_cmd + !strncmp(data-want_cmd, have_cmd, have_len) + data-want_cmd[have_len] == '\0') { + struct strbuf buf = STRBUF_INIT; + + strbuf_addstr(buf, GIT_TRACE_); + while (*key) + strbuf_addch(buf, toupper(*key++)); + + /* +* Environment always takes precedence over config, so do not +* override existing variables. We cannot rely on setenv()'s +* overwrite flag here, because we may pass the list off to +* a spawn() implementation, which always overwrites. +*/ + if (!getenv(buf.buf)) + argv_array_pushf(data-out, %s=%s, buf.buf, value); + + strbuf_release(buf); + } + return 0; +} + +void trace_config_for(const char *cmd, struct argv_array *out) +{ + struct trace_config_data data; + data.want_cmd = cmd; + data.out = out; + git_config(trace_config_cb, data); +} diff --git a/trace.h b/trace.h index 179b249..83618e9 100644 --- a/trace.h +++ b/trace.h @@ -4,6 +4,8 @@ #include git-compat-util.h #include strbuf.h +struct argv_array; + struct trace_key { const char * const key; int fd; @@ -20,6 +22,13 @@ extern uint64_t getnanotime(void); extern void trace_command_performance(const char **argv); extern void trace_verbatim(struct trace_key *key, const void *buf, unsigned len); +/** + * Load any trace-related config for git command cmd, and insert the matching + * environment variables into out, which is suitable for use by run-command + * and friends. + */ +void trace_config_for(const char *cmd, struct argv_array *out); + #ifndef HAVE_VARIADIC_MACROS
[PATCH] mergetools: add config option to disable auto-merge
For some mergetools, the current invocation of git mergetool will include an auto-merge flag. By default the flag is included, however if the git config option 'merge.automerge' is set to 'false', then that flag will now be omitted. Signed-off-by: Michael Rappazzo rappa...@gmail.com --- Documentation/config.txt| 6 ++ Documentation/git-mergetool.txt | 19 +-- mergetools/araxis | 4 +++- mergetools/diffmerge| 4 +++- mergetools/kdiff3 | 4 +++- 5 files changed, 28 insertions(+), 9 deletions(-) diff --git a/Documentation/config.txt b/Documentation/config.txt index 43bb53c..658d8f7 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -1864,6 +1864,12 @@ mergetool.meld.hasOutput:: to `true` tells Git to unconditionally use the `--output` option, and `false` avoids using `--output`. +mergetool.automerge:: + When invoking a custom merge tool which includes an auto-merge + option, Git will include that option by default. If this variable + is set to `false` then the auto-merge option is not used when + invoking the custom merge tool. + mergetool.keepBackup:: After performing a merge, the original file with conflict markers can be saved as a file with a `.orig` extension. If this variable diff --git a/Documentation/git-mergetool.txt b/Documentation/git-mergetool.txt index e846c2e..3461d3a 100644 --- a/Documentation/git-mergetool.txt +++ b/Documentation/git-mergetool.txt @@ -79,16 +79,23 @@ success of the resolution after the custom tool has exited. Prompt before each invocation of the merge resolution program to give the user a chance to skip the path. -TEMPORARY FILES -`git mergetool` creates `*.orig` backup files while resolving merges. -These are safe to remove once a file has been merged and its -`git mergetool` session has completed. - +CONFIGURATION OPTIONS +- +mergetool.keepBackup:: + `git mergetool` creates `*.orig` backup files while resolving merges. + These are safe to remove once a file has been merged and its + `git mergetool` session has completed. ++ Setting the `mergetool.keepBackup` configuration variable to `false` causes `git mergetool` to automatically remove the backup as files are successfully merged. +mergetool.automerge:: + For some mergetools, the current invocation of git mergetool will + include an auto-merge flag. By default the flag is included, however if + the git config option `merge.automerge` is set to `false`, then that + flag will be omitted. + GIT --- Part of the linkgit:git[1] suite diff --git a/mergetools/araxis b/mergetools/araxis index 64f97c5..00e63da 100644 --- a/mergetools/araxis +++ b/mergetools/araxis @@ -6,7 +6,9 @@ merge_cmd () { touch $BACKUP if $base_present then - $merge_tool_path -wait -merge -3 -a1 \ + automerge=-merge + test $(git config --get --bool mergetool.automerge) = false automerge='' + $merge_tool_path -wait ${automerge} -3 -a1 \ $BASE $LOCAL $REMOTE $MERGED /dev/null 21 else $merge_tool_path -wait -2 \ diff --git a/mergetools/diffmerge b/mergetools/diffmerge index f138cb4..287489b 100644 --- a/mergetools/diffmerge +++ b/mergetools/diffmerge @@ -5,7 +5,9 @@ diff_cmd () { merge_cmd () { if $base_present then - $merge_tool_path --merge --result=$MERGED \ + automerge=--merge + test $(git config --get --bool mergetool.automerge) = false automerge='' + $merge_tool_path ${automerge} --result=$MERGED \ $LOCAL $BASE $REMOTE else $merge_tool_path --merge \ diff --git a/mergetools/kdiff3 b/mergetools/kdiff3 index 793d129..8e1d063 100644 --- a/mergetools/kdiff3 +++ b/mergetools/kdiff3 @@ -7,7 +7,9 @@ diff_cmd () { merge_cmd () { if $base_present then - $merge_tool_path --auto \ + automerge=--auto + test $(git config --get --bool mergetool.automerge) = false automerge='' + $merge_tool_path ${automerge} \ --L1 $MERGED (Base) \ --L2 $MERGED (Local) \ --L3 $MERGED (Remote) \ -- 2.4.2 -- 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] mergetools: add config option to disable auto-merge
During conflict resolution, invoking a mergetool which supports auto-merge will pass that option by default to the underlying tool. This patch is intended to allow one to set 'merge.automerge' to 'false' in order to override this behavior. Michael Rappazzo (1): mergetools: add config option to disable auto-merge Documentation/config.txt| 6 ++ Documentation/git-mergetool.txt | 19 +-- mergetools/araxis | 4 +++- mergetools/diffmerge| 4 +++- mergetools/kdiff3 | 4 +++- 5 files changed, 28 insertions(+), 9 deletions(-) -- 2.4.2 -- 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/3] pkt-line: support tracing verbatim pack contents
On Tue, Jun 16, 2015 at 1:10 PM, Jeff King p...@peff.net wrote: On Tue, Jun 16, 2015 at 11:38:41AM -0400, Augie Fackler wrote: We already have a GIT_TRACE_PACKET mechanism for tracing packets. Let's extend it with GIT_TRACE_PACK to dump the verbatim packfile. FWIW, this also works for me - I have no preference between my patches and Jeff's. I suspect yours are much better given that you have a clue about git internals ;). I like mine better than yours, if only because it hooks into the existing tracing mechanism. But I am sad that neither of our proposals works for tracing pushed packs (something that is useful for the opposite situation: you have a sane git server, and some unknown misbehaving _client_ is sending you bogus packs). Yeah, not having it for the push side is a slight bummer, but in general I haven't had problems debugging git clients pushing bogus data in the same way that I've had problems with weirdness in new server features. Here's a rough cut at the trace stdin idea I mentioned earlier (which is essentially an internal tee). You can collect the incoming pack like: Neat, but not sure I like the extra overhead of having to grab the full trace and then reconstruct some arguments to be able to diagnose the pack. Having the verbatim pack just land on disk is really handy, because then any existing tools one has cooked up (my team has a few weird one-off ones by now) just work without extra fussing or looking up steps to reconstruct the whole file. -- 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
[ANNOUNCE] Git v2.4.4
The latest maintenance release Git v2.4.4 is now available at the usual places. The tarballs are found at: https://www.kernel.org/pub/software/scm/git/ The following public repositories all have a copy of the 'v2.4.4' tag and the 'maint' branch that the tag points at: url = https://kernel.googlesource.com/pub/scm/git/git url = git://repo.or.cz/alt-git.git url = https://code.google.com/p/git-core/ url = git://git.sourceforge.jp/gitroot/git-core/git.git url = git://git-core.git.sourceforge.net/gitroot/git-core/git-core url = https://github.com/gitster/git Git v2.4.4 Release Notes Fixes since v2.4.3 -- * l10n updates for German. * An earlier leakfix to bitmap testing code was incomplete. * git clean pathspec... tried to lstat(2) and complain even for paths outside the given pathspec. * Communication between the HTTP server and http_backend process can lead to a dead-lock when relaying a large ref negotiation request. Diagnose the situation better, and mitigate it by reading such a request first into core (to a reasonable limit). * The clean/smudge interface did not work well when filtering an empty contents (failed and then passed the empty input through). It can be argued that a filter that produces anything but empty for an empty input is nonsense, but if the user wants to do strange things, then why not? * Make git stash something --help error out, so that users can safely say git stash drop --help. * Clarify that log --raw and log --format=raw are unrelated concepts. * Catch a programmer mistake to feed a pointer not an array to ARRAY_SIZE() macro, by using a couple of GCC extensions. Also contains typofixes, documentation updates and trivial code clean-ups. Changes since v2.4.3 are as follows: Alex Henrie (1): blame, log: format usage strings similarly to those in documentation David Turner (1): clean: only lstat files in pathspec Elia Pinto (1): git-compat-util.h: implement a different ARRAY_SIZE macro for for safely deriving the size of array Jeff King (8): http-backend: fix die recursion with custom handler t5551: factor out tag creation stash: complain about unknown flags stash: recognize --help for subcommands test_bitmap_walk: free bitmap with bitmap_free http-backend: spool ref negotiation requests to buffer clone: use OPT_STRING_LIST for --reference clone: reorder --dissociate and --reference options Jim Hill (1): sha1_file: pass empty buffer to index empty file Junio C Hamano (1): Git 2.4.4 Matthieu Moy (2): Documentation/log: clarify what --raw means Documentation/log: clarify sha1 non-abbreviation in log --raw Michael Coleman (1): Documentation/git-commit: grammofix Michael J Gruber (3): l10n: de.po: grammar fix l10n: de.po: punctuation fixes l10n: de.po: translation fix for fall-back to 3way merge Phillip Sz (1): l10n: de.po: change error message from sagen to Meinten Sie René Scharfe (3): use file_exists() to check if a file exists in the worktree clean: remove unused variable buf dir: remove unused variable sb Stefan Beller (2): submodule doc: reorder introductory paragraphs glossary: add remote, submodule, superproject -- 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
What's cooking in git.git (Jun 2015, #04; Tue, 16)
Here are the topics that have been cooking. Commits prefixed with '-' are only in 'pu' (proposed updates) while commits prefixed with '+' are in 'next'. Hopefully the pre-release freeze can start late next week, and the next cycle would reopen mid next month. In the meantime, let's shift the focus on making sure that what has already been merged to 'master' are good (i.e. regression hunting and fixes). You can find the changes described here in the integration branches of the repositories listed at http://git-blame.blogspot.com/p/git-public-repositories.html -- [New Topics] * jk/pkt-log-pack (2015-06-16) 3 commits - pkt-line: support tracing verbatim pack contents - pkt-line: tighten sideband PACK check when tracing - pkt-line: simplify starts_with checks in packet tracing Enhance packet tracing machinery to allow capturing an incoming pack data to a file for debugging. * jk/stash-require-clean-index (2015-06-15) 1 commit (merged to 'next' on 2015-06-16 at beb4883) + Revert stash: require a clean index to apply A hotfix for the topic already in 'master'. Will merge to 'master'. * kb/i18n-doc (2015-06-15) 1 commit - Documentation/i18n.txt: clarify character encoding support Comments? * kb/use-nsec-doc (2015-06-15) 1 commit - Makefile / racy-git.txt: clarify USE_NSEC prerequisites Comments? * kn/for-each-ref (2015-06-15) 11 commits - ref-filter: make 'ref_array_item' use a FLEX_ARRAY for refname - for-each-ref: introduce filter_refs() - ref-filter: move code from 'for-each-ref' - ref-filter: add 'ref-filter.h' - for-each-ref: rename variables called sort to sorting - for-each-ref: rename some functions and make them public - for-each-ref: introduce 'ref_array_clear()' - for-each-ref: introduce new structures for better organisation - for-each-ref: rename 'refinfo' to 'ref_array_item' - for-each-ref: clean up code - for-each-ref: extract helper functions out of grab_single_ref() GSoC project to rebuild ref listing by branch and tag based on the for-each-ref machinery. Will merge to 'next'. * mh/init-delete-refs-api (2015-06-15) 13 commits - SQUASH avoid die(variable) - refs: move the remaining ref module declarations to refs.h - initial_ref_transaction_commit(): check for ref D/F conflicts - initial_ref_transaction_commit(): check for duplicate refs - refs: remove some functions from the module's public interface - initial_ref_transaction_commit(): function for initial ref creation - repack_without_refs(): make function private - prune_remote(): use delete_refs() - delete_refs(): improve error message - delete_refs(): new function for the refs API - delete_ref(): handle special case more explicitly - remove_branches(): remove temporary - delete_ref(): move declaration to refs.h Clean up refs API and make git clone less intimate with the implementation detail. Needs reroll. * mh/replace-refs (2015-06-12) 1 commit - Allow to control where the replace refs are looked for Add an environment variable to tell Git to look into refs hierarchy other than refs/replace/ for the object replacement data. * nd/multiple-work-trees (2015-06-12) 1 commit - checkout: don't check worktrees when not necessary git checkout [tree-ish] paths spent unnecessary cycles checking if the current branch was checked out elsewhere, when we know we are not switching the branches ourselves. Will merge to 'next'. * pa/auto-gc-mac-osx (2015-06-12) 1 commit (merged to 'next' on 2015-06-16 at 151ec95) + hooks/pre-auto-gc: adjust power checking for newer OS X Recent Mac OS X updates breaks the logic to detect that the machine is on the AC power in the sample pre-auto-gc script. Will merge to 'master'. * pt/t0302-needs-sanity (2015-06-12) 1 commit (merged to 'next' on 2015-06-16 at f0d8e17) + t0302: unreadable test needs SANITY prereq Will merge to 'master'. -- [Graduated to master] * ah/send-email-sendmail-alias (2015-05-27) 2 commits (merged to 'next' on 2015-06-04 at 9d9bd68) + t9001: write $HOME/, not ~/, to help shells without tilde expansion + send-email: add sendmail email aliases format (this branch is used by es/send-email-sendmail-alias.) git send-email learned the alias file format used by the sendmail program (in an abbreviated form). * jc/diff-ws-error-highlight (2015-05-26) 4 commits (merged to 'next' on 2015-06-01 at 6046560) + diff.c: --ws-error-highlight=kind option + diff.c: add emit_del_line() and emit_context_line() + t4015: separate common setup and per-test expectation + t4015: modernise style Allow whitespace breakages in deleted and context lines to be also painted in the output. * jk/clone-dissociate (2015-05-27) 2 commits (merged to 'next' on 2015-06-01 at 19e3ec3) + clone: reorder --dissociate and --reference options + clone: use OPT_STRING_LIST for --reference Code clean-up. *
[PATCH v2 7/7] bisect: allows any terms set by user
Matthieu Moy matthieu@grenoble-inp.fr writes: # terms_defined is 0 when the user did not define the terms explicitely # yet. This is the case when running 'git bisect start bad_rev good_rev' # before we see an explicit reference to a term. terms_defined=0 The thing is: 'git bisect reset git bisect new HEAD (autostart ?) y' is strictly equivalent to 'git bisect reset git bisect start git bisect new HEAD' In both case terms_defined value would be 0 while we kinda know that a term has been used. It just that in the code it is not taken in account yet. I don't really mind doing the change but I'm just pointing out that it can be confusing. 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/RFC 0/3] add GIT_TRACE_STDIN
On Tue, Jun 16, 2015 at 01:23:31PM -0400, Augie Fackler wrote: I guess there is really room for both. Just because you _can_ accomplish the same thing with both does not mean we cannot have two ways to do it (an easy way, and a harder, more flexible way). *nod* that might make the most sense - given that we both seem to have use cases in mind for verbatim packs on pulls, that seems like a good thing to have easy to deploy. My ulterior motive is that I actually already have a similar thing in place _just_ for pack-objects, and I'd like to get rid of my custom hack. :) In that case it is not about saving the packfile, but rather saving the parameters to create it (I was interested in finding out why git was spending so much CPU to serve some particular requests, and being able to run the same pack-generation repeatedly is helpful). Here are the patches I came up with: [1/3]: trace: implement %p placeholder for filenames [2/3]: trace: add pid to each output line [3/3]: trace: add GIT_TRACE_STDIN They apply on top of the TRACE_PACKFILE patches, only because they also need the new trace_verbatim(). But I am not altogether happy with the result; see the comments I'll add to 3/3. -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: Should the --encoding argument to log/show commands make any guarantees about their output?
On 2015-06-16 11.38, Jan-Philip Gehrcke wrote: On 15.06.2015 18:21, Torsten Bögershausen wrote: On 2015-06-15 10.50, Jan-Philip Gehrcke wrote: Let me describe what I think it currently does: The program attempts to re-code a log message, so it follows the chain raw input - unicode - raw output Not sure what raw input/output means. But there is only one reencode step involved, e.g. input(8859) - output(UTF-8) I probably need to correct myself: pretty.c void format_commit_message(const struct commit *commit, The the message is converted from the commit encoding into UTF-8. When the log encoding is different from UTF-8, There is a second conversio (as you said). (But not in your case, here the second conversion is skipped) [snip] I'd be willing to contribute, but of course there must be a discussion and an agreement before that, if there is need to change something at all, and what exactly. [] What do you think? See commit.c: /* * This verifies that the buffer is in proper utf8 format. * * If it isn't, it assumes any non-utf8 characters are Latin1, * and does the conversion. */ static int verify_utf8(struct strbuf *buf) I think the --encoding option would have ideal semantics for described behavior. However, I guess maintaining backwards compatibility is an issue here. On the other hand, I realize that the --encoding option undergoes changes: the docs for git log in release 2.4.3 do not even list the --encoding option anymore. Why is that? I haven't found a corresponding changelog/release notes entry. It still seems to work: git log --encoding ISO-8859-1 | xxd | grep gersha | less includes this line: 7465 6e20 42f6 6765 7273 6861 7573 656e ten B.gershausen So my suggestion (in short): move verify_utf8() from commit.c into utf8.c, make it non-static, and add a prototype in utf8.h Use that function in pretty.c (and commit.c), test it. Make a patch out of it and send it to the list. In the ideal world the patch will include a test case, but I don't know how easy it is to create such a commit. -- 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 1/3] trace: implement %p placeholder for filenames
This lets you trace output on a per-process basis by naming the trace file after the PID. This is probably not all that useful for GIT_TRACE itself (which is about a global view of the process hierarchy anyway), but will be useful for traces which produce large amounts of data (e.g., whole packfiles). Signed-off-by: Jeff King p...@peff.net --- This is the bare minimum to let you distinguish between the stdin of different processes. If we are going to use the trace subsystem to collect more long-term debugging logs, it would probably make sense to add placeholders for the program name and a timestamp. Then you could, for example, leave more verbose debugging enabled on your servers all the time. Documentation/git.txt | 3 ++- trace.c | 19 +-- 2 files changed, 19 insertions(+), 3 deletions(-) diff --git a/Documentation/git.txt b/Documentation/git.txt index 3453669..80f6392 100644 --- a/Documentation/git.txt +++ b/Documentation/git.txt @@ -984,7 +984,8 @@ trace messages into this file descriptor. Alternatively, if the variable is set to an absolute path (starting with a '/' character), Git will interpret this as a file path and will try to write the trace messages -into it. +into it. If the filename contains the string `%p`, that string +will be replaced with the PID of the traced process. + Unsetting the variable, or setting it to empty, 0 or false (case insensitive) disables trace messages. diff --git a/trace.c b/trace.c index 7393926..e1d1360 100644 --- a/trace.c +++ b/trace.c @@ -25,6 +25,16 @@ #include cache.h #include quote.h +static size_t expand_trace_name(struct strbuf *out, const char *fmt, + void *data) +{ + if (*fmt == 'p') { + strbuf_addf(out, %lu, (unsigned long)getpid()); + return 1; + } + return 0; +} + /* Get a trace file descriptor from key env variable. */ static int get_trace_fd(struct trace_key *key) { @@ -49,17 +59,22 @@ static int get_trace_fd(struct trace_key *key) else if (strlen(trace) == 1 isdigit(*trace)) key-fd = atoi(trace); else if (is_absolute_path(trace)) { - int fd = open(trace, O_WRONLY | O_APPEND | O_CREAT, 0666); + struct strbuf name = STRBUF_INIT; + int fd; + + strbuf_expand(name, trace, expand_trace_name, NULL); + fd = open(name.buf, O_WRONLY | O_APPEND | O_CREAT, 0666); if (fd == -1) { fprintf(stderr, Could not open '%s' for tracing: %s\n Defaulting to tracing on stderr...\n, - trace, strerror(errno)); + name.buf, strerror(errno)); key-fd = STDERR_FILENO; } else { key-fd = fd; key-need_close = 1; } + strbuf_release(name); } else { fprintf(stderr, What does '%s' for %s mean?\n If you want to trace into a file, then please set -- 2.4.3.699.g84b4da7 -- 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/3] trace: add GIT_TRACE_STDIN
On Tue, Jun 16, 2015 at 03:37:16PM -0400, Jeff King wrote: For instance: GIT_TRACE=/tmp/processes.out \ GIT_TRACE_STDIN=/tmp/stdin.%p \ git daemon ... After a fetch, processes.out will contain a line like: 15:19:08.275493 [pid=13196] git.c:348 trace: built-in: git 'pack-objects' '--revs' '--thin' '--stdout' '--progress' '--delta-base-offset' And stdin.13196 (the pid picked from the above line) will contain its stdin. So this is the part that I don't like. Collecting stdin is expensive, and the commands above would be totally inappropriate for a production server, for two reasons: 1. It requires restarting git-daemon to turn on debugging. 2. It logs for _everything_. Every repo, and every process. I mentioned before that I have a similar patch for logging pack-objects. That is triggered via config, which solves both of these problems. What I'd really like is to be able to do: git config trace.pack-objects.stdin fetches/stdin.%p in a particular repository, collect data for a few minutes, and then turn it back off. The patch below implements that, but it doesn't quite work as you might hope. The problem is that we cannot read the repository config so early in the git.c startup; we do not even know if we have a repository yet. Pushing the config-reading later is too late; we may already have looked at GIT_TRACE_* and decided whether to trace (and possibly even written trace records!). It's possible we could hack around it by rearranging the startup sequence, but that sequence is notoriously fragile. Another option would be to stop trying to intercept stdin in git.c, and instead make this a feature of run-command.c. That is, right before we exec a process, tee its stdin there. That means that you cannot do: GIT_TRACE_STDIN=/tmp/foo.out git foo to collect the stdin of foo. But that is not really an interesting case anyway. You can run tee yourself if you want. The interesting cases are the ones where git is spawning a sub-process, and you want to intercept the data moving between the git processes. -Peff --- diff --git a/git.c b/git.c index e7e58e3..cbb7e9b 100644 --- a/git.c +++ b/git.c @@ -675,6 +675,38 @@ static void trace_stdin(void) */ } +static int trace_config_cb(const char *var, const char *value, void *data) +{ + const char *want_cmd = data; + const char *have_cmd, *key; + int have_len; + + if (!parse_config_key(var, trace, have_cmd, have_len, key) + have_cmd + !strncmp(want_cmd, have_cmd, have_len) + want_cmd[have_len] == '\0') { + struct strbuf buf = STRBUF_INIT; + strbuf_addstr(buf, GIT_TRACE_); + while (*key) + strbuf_addch(buf, toupper(*key++)); + setenv(buf.buf, value, 0); + strbuf_release(buf); + } + return 0; +} + +static void load_trace_config(const char *cmd, const char **argv) +{ + /* XXX we don't know the cmd for sure until we parse the options */ + if (!strcmp(cmd, git)) + cmd = argv[1]; + else + skip_prefix(cmd, git-, cmd); + + if (cmd) + git_config(trace_config_cb, (void *)cmd); +} + int main(int argc, char **av) { const char **argv = (const char **) av; @@ -698,6 +730,7 @@ int main(int argc, char **av) git_setup_gettext(); + load_trace_config(cmd, argv); trace_command_performance(argv); trace_stdin(); -- 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] Documentation/describe: improve one-line summary
Matthieu Moy matthieu@imag.fr writes: diff --git a/Documentation/git-describe.txt b/Documentation/git-describe.txt index d20ca40..e045fc7 100644 --- a/Documentation/git-describe.txt +++ b/Documentation/git-describe.txt @@ -3,7 +3,7 @@ git-describe(1) NAME -git-describe - Show the most recent tag that is reachable from a commit +git-describe - Describe a commit using the most recent tag reachable from it While it feels a bit funny to describe a describe command as that which describes, this definitely is an improvement. 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: Troubleshoot clone issue to NFS.
On Fri, Jun 05, 2015 at 08:18:17AM -0400, Jeff King wrote: On Fri, Jun 05, 2015 at 12:01:16PM +, steve.nor...@thomsonreuters.com wrote: On Sunday, May 24, 2015 @ 10:01 AM Duy Nguyen [mailto:pclo...@gmail.com] did scribble: In case you want to back away from option 2 because it starts to leak raciness, which your old commit tried to fix in the first place. I think the only other place that tests for lots of non-existent loose objects is write_sha1_file (e.g. tar -xf bigtarball.tar.gz; cd bigtarball; git init; git add .). But the number of calls should be much smaller compared to index-pack and it does not use has_sha1_file, it uses check_and_freshen_file() instead. There are other places where has_sha1_file() may return 0, but I think the number of calls is even smaller to bother (shallow.c, fetch-pack.c, apply.c, buik-checkin.c) Any updates / further thoughts on this? Sorry, I haven't had a chance to look at it further. It still on my todo list. My plan is: 1. Devise some torture to tests to see whether my patch series is in fact racy on Linux. I do not know that we needed further convincing that the patch series to stat() the objects/pack directory was flaky, but just as an extra data point in case somebody tries this approach later: it is indeed flaky. I've been running with it for a week or so, and I noticed that a simple clone of a small repository fails racily: $ git clone --bare --no-local . foo.git Cloning into bare repository 'child.git'... remote: Counting objects: 210, done. remote: Compressing objects: 100% (98/98), done. remote: Total 210 (delta 99), reused 210 (delta 99) Receiving objects: 100% (210/210), 68.00 KiB | 0 bytes/s, done. Resolving deltas: 100% (99/99), done. Checking connectivity... done. error: internal error: refs/heads/master is not a valid packed reference! Sometimes it works, and sometimes not. It looks like we're failing to re-scan the pack directory when we should (presumably because the open() and readdir() operations are not atomic). -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
[PATCH 2/3] trace: add pid to each output line
When there are many git commands running, it can be confusing which process is producing which message. And if one is trying to correlate a specific invocation with a trace-file created using %p, it's rather impossible. Let's print the pid alongside each line. This does extend the already-wide trace lines. We could make it optional, but it's probably not worth the complexity of extra knobs. Looking at trace output is already an exceptional thing to be doing, and verbose output is not going to bother anyone. Signed-off-by: Jeff King p...@peff.net --- I think you can technically get by without this for most cases, as doing GIT_TRACE=base.%p will get you the built-in: ... line for index-pack, pack-objects, etc, which you can then correlate with a matching GIT_TRACE_STDIN filename. trace.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/trace.c b/trace.c index e1d1360..a7ec484 100644 --- a/trace.c +++ b/trace.c @@ -123,12 +123,13 @@ static int prepare_trace_line(const char *file, int line, localtime_r(secs, tm); strbuf_addf(buf, %02d:%02d:%02d.%06ld , tm.tm_hour, tm.tm_min, tm.tm_sec, (long) tv.tv_usec); + strbuf_addf(buf, [pid=%lu] , (unsigned long)getpid()); #ifdef HAVE_VARIADIC_MACROS /* print file:line */ strbuf_addf(buf, %s:%d , file, line); - /* align trace output (column 40 catches most files names in git) */ - while (buf-len 40) + /* align trace output (column 50 catches most files names in git) */ + while (buf-len 50) strbuf_addch(buf, ' '); #endif -- 2.4.3.699.g84b4da7 -- 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 3/3] trace: add GIT_TRACE_STDIN
Sometimes tracing the invocation of git programs with GIT_TRACE is not quite enough to replay a situation; the interesting input to the program often comes over its standard input. For instance, if you want to replay a particular fetch (e.g., for performance analysis or debugging), you would want both the arguments and stdin sent to pack-objects. This patch lets you capture the stdin of any git process. For instance: GIT_TRACE=/tmp/processes.out \ GIT_TRACE_STDIN=/tmp/stdin.%p \ git daemon ... After a fetch, processes.out will contain a line like: 15:19:08.275493 [pid=13196] git.c:348 trace: built-in: git 'pack-objects' '--revs' '--thin' '--stdout' '--progress' '--delta-base-offset' And stdin.13196 (the pid picked from the above line) will contain its stdin. Signed-off-by: Jeff King p...@peff.net --- git.c | 60 1 file changed, 60 insertions(+) diff --git a/git.c b/git.c index 44374b1..e7e58e3 100644 --- a/git.c +++ b/git.c @@ -616,6 +616,65 @@ static void restore_sigpipe_to_default(void) signal(SIGPIPE, SIG_DFL); } +static int copy_stdin(int in, int out, void *data) +{ + struct trace_key *key = data; + while (1) { + char buf[8192]; + ssize_t len = xread(in, buf, sizeof(buf)); + if (!len) + break; + if (len 0) { + warning(error reading stdin trace: %s, + strerror(errno)); + break; + } + + trace_verbatim(key, buf, len); + if (write_in_full(out, buf, len) 0) { + warning(error writing stdin trace: %s, + strerror(errno)); + break; + } + } + close(in); + close(out); + return 0; +} + +static void trace_stdin(void) +{ + static struct trace_key key = TRACE_KEY_INIT(STDIN); + static struct async async; + + if (!trace_want(key)) + return; + + memset(async, 0, sizeof(async)); + async.proc = copy_stdin; + async.data = key; + async.in = dup(0); + async.out = -1; + + if (async.in 0 || start_async(async) 0) { + warning(unable to trace stdin: %s, strerror(errno)); + return ; + } + + /* +* At this point we've handed stdin off to the async process, +* so there we are past the point of no return. +*/ + if (dup2(async.out, 0)) + die_errno(unable to redirect stdin from async process); + close(async.out); + + /* +* leak async; we would know to finish_async() only when we are +* exiting, and there is no point then +*/ +} + int main(int argc, char **av) { const char **argv = (const char **) av; @@ -640,6 +699,7 @@ int main(int argc, char **av) git_setup_gettext(); trace_command_performance(argv); + trace_stdin(); /* * git- is the same as git , but we obviously: -- 2.4.3.699.g84b4da7 -- 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: Visualizing merge conflicts after the fact (using kdiff3)
On 6/16/2015 2:43 AM, Johannes Schindelin wrote: Hi Eric, On 2015-06-16 03:17, Eric Raible wrote: I'm running 1.9.5.msysgit.1, but this is a general git question... Upon returning from a vacation, I was looking at what people had been up to, and discovered on merge in which a colleague had resolved a merge incorrectly. It turns out that he has pushed *many* merges over the past year which had conflicts in my code, and now I don't trust any of them. So naturally I want to check each of them for correctness. I know about git log -p -cc SHA -- path, but it really doesn't show just the conflicts so there's just too much noise in that output. I use kdiff3 to resolve conflicts, so I'm looking for a way to visualize these already-resolved conflicts with that tool. As I said, there are many merges, so the prospect of checking out each sha, doing the merge, and then comparing the results is completely untenable. Can anyone help? Surely other people have wanted to review how conflicts were resolved w/out looking at the noise of unconflicted changes, right? If I was walking in your shoes, I would essentially recreate the merge conflicts and then use git diff merge-commit with the resolved merge in your current history. Something like this: ```bash mergecommit=$1 # probably should verify that the working directory is clean, yadda yadda # recreate merge conflicts on an unnamed branch (Git speak: detached HEAD) git checkout $mergecommit^ git merge $mergecommit^2 || die This merge did not have any problem! # compare to the actual resolution as per the merge commit git diff $mergecommit ``` To list all the merge commits in the current branch, I would use the command-line: ```bash git rev-list --author=My Colleague --parents HEAD | sed -n 's/ .* .*//p' ``` (i.e. listing all the commits with their parents, then filtering just the ones having more than one parent, which would include octopus merges if your history has them.) Hopefully this gives you good ideas how to proceed. Ciao, Johannes . Thanks for the reply, Johannes. That basically the procedure that I did on just the one I stumbled across. But what I really want is just a way to review how each conflicts was resolved w/out having to re-resolve each one myself. gitk (obviously) makes it trivial to view changes in normal commits, but given that git provides such a straightforward conflict resolution model I'm surprised that there isn't a corresponding straightforward way of viewing those resolved conflicts in context. Thanks - Eric -- 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: 'git status -z' missing separators on OSX
On Tue, Jun 16, 2015 at 06:21:56PM -0500, Tad Hardesty wrote: ~/test (master #)$ git status -z | hexdump -C 41 20 20 61 41 20 20 62 |A aA b| 0008 That's really weird. I don't have a Yosemite box available, but I can't reproduce on the Mavericks box I have access to. Still, I'd suspect something weird in your config (e.g., something that is inserting itself on the output pipe of git status). You said you blanked your config, but can you do git config --list to double-check? Also, try running with GIT_TRACE=1, which would show if git is starting a pager between your processes which might be eating the NUL. Can you also double-check that you have no aliases for git (try type git). We have sometimes seen weird behavior when people have aliased git to hub. Is it only git status that is affected? Does git log --oneline -1 -z end in a NUL (it should)? -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: GET PAID DRIVING YOUR CAR BY ROCKSTAR ENERGY DRINK FOR ADVERTISEMENT
Wow! That sounds like a great opportunity!! I would love to work for this Green drink http://www.ivlproducts.com/Superfoods/All-Day-Energy-Greens-174---Original---Hi-Octane-Energy-Drink-For-Health-Life.axd brand. This is really famous brand that provide quality products. I wonder if you can share some more details regarding this work!! -- View this message in context: http://git.661346.n2.nabble.com/GET-PAID-DRIVING-YOUR-CAR-BY-ROCKSTAR-ENERGY-DRINK-FOR-ADVERTISEMENT-tp7603309p7633484.html Sent from the git mailing list archive at Nabble.com. -- 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] notes: Use get_sha1_committish instead of read_ref in init_notes()
Mike Hommey m...@glandium.org writes: init_notes() is essentially the only point of entry to the notes API. It is an arbitrary restriction that all it allows as input is a strict ref name, when callers may want to give an arbitrary committish. While it may be a good idea to allow reading from any note-shaped tree-ish, not just what is at the tip of a ref, I suspect that the use of read_ref() is not an arbitrary restriction, but is an effective way to achieve safety against callers that update notes. That is, you can feed refs/notes/commit@{4.days.ago} to the machinery and show you notes from 4 days ago, but you cannot update that as if it were a ref. Hence, if you are loosening the safety at init_notes() site, you would at least need to add a similar safety in the write codepath, I would think. One thing you would need to be careful about is that you would give users a crappy experience, if an operation reads notes, does its own thing, and then tries to write updated notes (think: rebase that transplants notes from original to rewritten commits), and you fail the operation only at the very last phase of updating. In order to prevent that, in the write codepath above has to be reject any non-ref note, e.g. --notes=refs/notes/commit@{4.days.ago} upfront, if the operation will write updated notes. -- 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] notes: Use get_sha1_committish instead of read_ref in init_notes()
On Wed, Jun 17, 2015 at 10:15:31AM +0900, Mike Hommey wrote: init_notes() is essentially the only point of entry to the notes API. It is an arbitrary restriction that all it allows as input is a strict ref name, when callers may want to give an arbitrary committish. This has the side effect of enabling the use of committish as notes refs in commands allowing them, e.g. git log --notes=foo@{1}, although I haven't checked whether that's the case for all of them. What about expand_notes_ref? We call that on the argument to --notes. I guess it is OK to expand foo@{1} into refs/notes/foo@{1}, but what about other more arcane syntaxes, like :/? In a sense that is weirdly broken already: $ git log --notes=:/foo /dev/null warning: notes ref refs/notes/:/foo is invalid but I wonder if we should be making expand_notes_ref a little more careful as part of the same topic. -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
'git status -z' missing separators on OSX
I have been experiencing a problem where git on OSX prints incorrect output to the `git status -z` command, where instead of NUL separators records are simply not separated. This is causing problems with IDE integration. While I have a workaround involving manually replacing \n with \0, it would be nice to identify and fix the root issue. Here's a terminal session showing the problem: ~$ mkdir test cd test ~/test$ git init Initialized empty Git repository in /Users/thardesty/test/.git/ ~/test (master #)$ touch a b ~/test (master #)$ ls a b ~/test (master #)$ git status -z | hexdump -C 3f 3f 20 61 3f 3f 20 62 |?? a?? b| 0008 ~/test (master #)$ git add a b ~/test (master #)$ git status -z | hexdump -C 41 20 20 61 41 20 20 62 |A aA b| 0008 ~/test (master #)$ git status --porcelain | hexdump -C 41 20 20 61 0a 41 20 20 62 0a|A a.A b.| 000a ~/test (master #)$ git --version git version 2.4.3 ~/test (master #)$ uname -a Darwin HA002070 14.3.0 Darwin Kernel Version 14.3.0: Mon Mar 23 11:59:05 PDT 2015; root:xnu-2782.20.48~5/RELEASE_X86_64 x86_64 As shown, --porcelain prints a newline but -z yields no separator at all. The Mac is running OS X Yosemite 10.10.3. I have tried git 2.3.2 from Apple, git 2.4.2 from brew, and git 2.4.3 from brew, git-scm.org, and self-compiled, and these all exhibit the problem. Some helpful folks on the #git IRC tried the commands for me and didn't see any problems, but I temporarily blanked all my configuration files and that didn't help. I double-checked and git on Linux has the correct behavior. Thanks for any help. -- 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] Documentation on git-checkout --ours/--theirs improved
2015-06-15 22:10 GMT+02:00 Junio C Hamano gits...@pobox.com: Simon A. Eugster simon...@gmail.com writes: --- - Lack of explanation as to why this is a good thing. - Lack of sign-off. Why is there still 1/2, if its effect is wholly annulled by a subsequent step 2/2? Sorry for that, still trying to find out how git send-email works. Documentation/git-checkout.txt | 39 +++ 1 file changed, 35 insertions(+), 4 deletions(-) diff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt index 5c3ef86..ec0be28 100644 --- a/Documentation/git-checkout.txt +++ b/Documentation/git-checkout.txt @@ -116,10 +116,41 @@ entries; instead, unmerged entries are ignored. --theirs:: When checking out paths from the index, check out stage #2 ('ours', HEAD) or #3 ('theirs', MERGE_HEAD) for unmerged paths. -+ -After a `git pull --rebase`, for example, 'ours' points to the remote -version and 'theirs' points to the local version. See linkgit:git-merge[1] -for details about stages #2 and #3. + See linkgit:git-merge[1] for details about stages #2 and #3. ++ +Note that during `git rebase` and `git pull --rebase`, 'theirs' checks out +the local version, and 'ours' the remote version or the history that is rebased +against. ++ +The reason ours/theirs appear to be swapped during a rebase is that we +define the remote history as the canonical history, on top of which our +private commits are applied on, as opposed to normal merging where the +local history is the canonical one. We define sounds a bit strange to me. It is not we who define so. Those who use rebase because they employ a shared central repository workflow are the ones that treat the history of their remote repository (which is their shared central repository) as the canonical one. Yes, that is how it is meant; I checked other parts of the documentation of git-checkout, and they use the same style, e.g.: Let’s look at what happens when we checkout commit b (here we show two ways this may be done) Note that during `git rebase` and `git pull --rebase`, 'ours' and 'theirs' may appear swapped; `--ours` gives the version from the branch the changes are rebased onto, while `--theirs` gives the version from the branch that holds your work that is being rebased. This is because `rebase` is used in a workflow that treats the history at the remote as the shared canonical one, and treat the work done on the branch you are rebasing as the third-party work to be integrated, and while you are rebasing, you are temporarily assuming the role of the keeper of the canonical history. As the keeper of the canonical history, you would view the history from the remote as `ours`, while what you did on your side branch as `theirs`. Could you commit your version? I think that's easier. +During merging, we assume the role of the canonical history’s keeper, +which, in case of a rebase, is the remote history, and our private commits +look to the keeper as “their” commits which need to be integrated on top +of “our” work. ++ +Normal merging: + +local -abC -- canonical history + | git checkout --ours + v +MERGE -abC + ^ + | git checkout --theirs +origin/master ---Xyz + +Rebasing: + +local ---Abc + | git checkout --theirs + v +REBASE xyZ + ^ + | git checkout --ours +origin/master -xyZ-- canonical history + I can see that an arrow with canonical history points at different things between the two pictures, but other than that, I am not sure what these are trying to illustrate. Especially between abc and xyz, why does the former choose abc while the latter choooses xyz? Are these pictures meant to show what happens when the user says checkout --ours during a conflicted integration (whether it is a merge or a rebase)? I tried to create a picture which shows the difference of ours and theirs when merging vs. rebasing, but apparently it did not turn out well, and I will just leave it away. Simon -- 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: GitHub Pull Request merge commands
On 16/06, Florian Lindner wrote: My question is, if davidsblom make further commits to his develop branch (after the pull request was issued) aren't these commits also included in the pull and therefore in the merge? If yes, isn't the idea to merge just the changes that the pull request was about? If not, why? ;-) A pull request is about all commits in the branch, which is why topic-branches should be used for PRs. -- Sincerely, Johannes Löthberg PGP Key ID: 0x50FB9B273A9D0BB5 https://theos.kyriasis.com/~kyrias/ signature.asc Description: PGP signature
[PATCH/RFC] Revert git am/mailinfo: Don't look at in-body headers when rebasing
This reverts commit d25e51596be9271ad833805a3d6f9012dc24ee79, removing git-mailsplit's --no-inbody-headers option. While --no-inbody-headers was introduced to prevent commit messages from being munged by git-mailinfo while rebasing, the need for this option disappeared since 5e835ca (rebase: do not munge commit log message, 2008-04-16), as git-am bypasses git-mailinfo and gets the commit message directly from the commit ID in the patch. git-am is the only user of --no-inbody-headers, and this option is not documented. As such, it should be removed. Signed-off-by: Paul Tan pyoka...@gmail.com --- Notes: The other direction, of course, is to turn --no-inbody-headers into a supported, documented option in both git-mailsplit and git-am. I do also wonder if we should just ensure that git-format-patch does not generate a message that start with From or Date. builtin/mailinfo.c | 12 +--- git-am.sh| 9 + t/t5100-mailinfo.sh | 4 t/t5100/info0015--no-inbody-headers | 5 - t/t5100/info0016--no-inbody-headers | 5 - t/t5100/msg0015--no-inbody-headers | 3 --- t/t5100/msg0016--no-inbody-headers | 4 t/t5100/patch0015--no-inbody-headers | 8 t/t5100/patch0016--no-inbody-headers | 8 9 files changed, 2 insertions(+), 56 deletions(-) delete mode 100644 t/t5100/info0015--no-inbody-headers delete mode 100644 t/t5100/info0016--no-inbody-headers delete mode 100644 t/t5100/msg0015--no-inbody-headers delete mode 100644 t/t5100/msg0016--no-inbody-headers delete mode 100644 t/t5100/patch0015--no-inbody-headers delete mode 100644 t/t5100/patch0016--no-inbody-headers diff --git a/builtin/mailinfo.c b/builtin/mailinfo.c index 999a525..34ea160 100644 --- a/builtin/mailinfo.c +++ b/builtin/mailinfo.c @@ -26,7 +26,6 @@ static int patch_lines; static struct strbuf **p_hdr_data, **s_hdr_data; static int use_scissors; static int add_message_id; -static int use_inbody_headers = 1; #define MAX_HDR_PARSED 10 #define MAX_BOUNDARIES 5 @@ -795,17 +794,10 @@ static int handle_commit_msg(struct strbuf *line) if (still_looking) { if (!line-len || (line-len == 1 line-buf[0] == '\n')) return 0; - } - - if (use_inbody_headers still_looking) { still_looking = check_header(line, s_hdr_data, 0); if (still_looking) return 0; - } else - /* Only trim the first (blank) line of the commit message -* when ignoring in-body headers. -*/ - still_looking = 0; + } /* normalize the log message to UTF-8. */ if (metainfo_charset) @@ -1062,8 +1054,6 @@ int cmd_mailinfo(int argc, const char **argv, const char *prefix) use_scissors = 1; else if (!strcmp(argv[1], --no-scissors)) use_scissors = 0; - else if (!strcmp(argv[1], --no-inbody-headers)) - use_inbody_headers = 0; else usage(mailinfo_usage); argc--; argv++; diff --git a/git-am.sh b/git-am.sh index 761befb..df403b0 100755 --- a/git-am.sh +++ b/git-am.sh @@ -372,7 +372,7 @@ split_patches () { prec=4 dotest=$GIT_DIR/rebase-apply sign= utf8=t keep= keepcr= skip= interactive= resolved= rebasing= abort= -messageid= resolvemsg= resume= scissors= no_inbody_headers= +messageid= resolvemsg= resume= scissors= git_apply_opt= committer_date_is_author_date= ignore_date= @@ -579,7 +579,6 @@ Use \git am --abort\ to remove it.) echo $keep $dotest/keep echo $messageid $dotest/messageid echo $scissors $dotest/scissors - echo $no_inbody_headers $dotest/no_inbody_headers echo $GIT_QUIET $dotest/quiet echo 1 $dotest/next if test -n $rebasing @@ -644,12 +643,6 @@ t) f) scissors=--no-scissors ;; esac -if test $(cat $dotest/no_inbody_headers) = t -then - no_inbody_headers=--no-inbody-headers -else - no_inbody_headers= -fi if test $(cat $dotest/quiet) = t then GIT_QUIET=t diff --git a/t/t5100-mailinfo.sh b/t/t5100-mailinfo.sh index e97cfb2..b2b5be6 100755 --- a/t/t5100-mailinfo.sh +++ b/t/t5100-mailinfo.sh @@ -31,10 +31,6 @@ do then check_mailinfo $mail --scissors fi - if test -f $TEST_DIRECTORY/t5100/msg$mail--no-inbody-headers - then - check_mailinfo $mail --no-inbody-headers - fi if test -f $TEST_DIRECTORY/t5100/msg$mail--message-id then check_mailinfo $mail --message-id diff --git a/t/t5100/info0015--no-inbody-headers b/t/t5100/info0015--no-inbody-headers deleted file mode 100644 index c4d8d77..000 --- a/t/t5100/info0015--no-inbody-headers
GitHub Pull Request merge commands
Hello, GitHub proposes these commands to merge a pull requests (explanations from me, to make sure I got it correctly) # Basically branch develop to davidsblom-develop git checkout -b davidsblom-develop develop # Pull in foreign repos commits from foreign develop branch. git pull git://github.com/davidsblom/precice.git develop # Edit and merge the changes to the main repos develop branch git checkout develop git merge --no-ff davidsblom-develop git push origin develop My question is, if davidsblom make further commits to his develop branch (after the pull request was issued) aren't these commits also included in the pull and therefore in the merge? If yes, isn't the idea to merge just the changes that the pull request was about? If not, why? ;-) Thanks, Florian -- 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
git-describe manual page's one sentence summary is misleading
According to the man of git-describe, http://git-scm.com/docs/git-describe git-describe - Show the most recent tag that is reachable from a commit It means that this command shows the name of the most recent tag. However, actually, it construct a new tag name if the current commit is not tagged. I think, git-describe - Describe a commit using the most recent tag reachable from it is a better summary. -- Best Regards Yours faithfully Albert Netymk -- 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 1/4] implement submodule config API for lookup of .gitmodules values
On Mon, Jun 15, 2015 at 11:06:11PM +0200, Heiko Voigt wrote: In a superproject some commands need to interact with submodules. They need to query values from the .gitmodules file either from the worktree of from certain revisions. At the moment this is quite hard since a caller would need to read the .gitmodules file from the history and then parse the values. We want to provide an API for this so we have one place to get values from .gitmodules from any revision (including the worktree). I just realized that we are talking too much about .gitmodules here, where it probably should be submodule configuration values. For revisions we only read from .gitmodules files but for the worktree we actually overlay those with local configurations from .git/config and friends. Not sure how we can name this though. submodule configuration values is kind of long compared to .gitmodules. Does anyone have a better name? Or is it maybe to confusing, to abstract it too much and we should just keep it .gitmodules, since everyone knows that those values can be overridden by local configuration? Cheers Heiko -- 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: Should the --encoding argument to log/show commands make any guarantees about their output?
On 15.06.2015 18:21, Torsten Bögershausen wrote: On 2015-06-15 10.50, Jan-Philip Gehrcke wrote: Let me describe what I think it currently does: The program attempts to re-code a log message, so it follows the chain raw input - unicode - raw output Not sure what raw input/output means. But there is only one reencode step involved, e.g. input(8859) - output(UTF-8) We surely agree. With raw I meant a sequence of bytes, and with unicode I meant the intermediate state in the process of re-encoding (which can be thought of as decoding and encoding with a transient intermediate state). If the user ignores this warning, how should Git guess the encoding ? I entirely appreciate that there is no satisfying solution to this very problem. If this step fails (if the entry contains a byte sequence that is invalid in the specified/assumed input codec), the procedure is aborted and the data is dumped as is (obviously without applying the requested output encoding). Is that correct? Yes, see above. Thanks! Hence, from my point of view, the rational that git show/log should be able to output *text* information means that they should not emit byte sequences that are invalid in the codec specified via the --encoding argument. In the current situation, the work of dealing with invalid byte sequences is just outsourced to software further below in the tool chain (at some point a replacement character � should be displayed to the user instead of the invalid raw bytes). I am not entirely sure where this discussion should lead to. Yes, until someone writes a patch to improve either the documentation or the code, nothing will be changed. However, I think that if the behavior of the software will not be changed, then the documentation for the --encoding option should be more precise and clarify what actually happens behind the scenes. What do you think? Patches are more than welcome. I'd be willing to contribute, but of course there must be a discussion and an agreement before that, if there is need to change something at all, and what exactly. To this discussion I would like to contribute that I am of the opinion that there should be a command line option to make git show/log/friends emit a byte stream that is guaranteed to be valid in a given codec. That would require detection and treatment of those cases where corrupted text resides in the repository (we cannot prevent it from entering the repository, as discussed). In these cases, one could emit a replacement symbol (e.g. '?') per invalid byte subsequence (this seems to be more established than just swallowing the invalid byte sequence). What do you think? I think the --encoding option would have ideal semantics for described behavior. However, I guess maintaining backwards compatibility is an issue here. On the other hand, I realize that the --encoding option undergoes changes: the docs for git log in release 2.4.3 do not even list the --encoding option anymore. Why is that? I haven't found a corresponding changelog/release notes entry. Thanks, Jan-Philip -- 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/RFC] Revert git am/mailinfo: Don't look at in-body headers when rebasing
Hi Paul, On 2015-06-16 11:03, Paul Tan wrote: This reverts commit d25e51596be9271ad833805a3d6f9012dc24ee79, removing git-mailsplit's --no-inbody-headers option. While --no-inbody-headers was introduced to prevent commit messages from being munged by git-mailinfo while rebasing, the need for this option disappeared since 5e835ca (rebase: do not munge commit log message, 2008-04-16), as git-am bypasses git-mailinfo and gets the commit message directly from the commit ID in the patch. git-am is the only user of --no-inbody-headers, and this option is not documented. As such, it should be removed. Makes sense to me. Notes: The other direction, of course, is to turn --no-inbody-headers into a supported, documented option in both git-mailsplit and git-am. I do also wonder if we should just ensure that git-format-patch does not generate a message that start with From or Date. In case we need this option for anything in the future, we can easily resurrect it from the commit history, so removing the option the appropriate course of action, methinks. Ciao, Dscho -- 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: Visualizing merge conflicts after the fact (using kdiff3)
Hi Eric, On 2015-06-16 03:17, Eric Raible wrote: I'm running 1.9.5.msysgit.1, but this is a general git question... Upon returning from a vacation, I was looking at what people had been up to, and discovered on merge in which a colleague had resolved a merge incorrectly. It turns out that he has pushed *many* merges over the past year which had conflicts in my code, and now I don't trust any of them. So naturally I want to check each of them for correctness. I know about git log -p -cc SHA -- path, but it really doesn't show just the conflicts so there's just too much noise in that output. I use kdiff3 to resolve conflicts, so I'm looking for a way to visualize these already-resolved conflicts with that tool. As I said, there are many merges, so the prospect of checking out each sha, doing the merge, and then comparing the results is completely untenable. Can anyone help? Surely other people have wanted to review how conflicts were resolved w/out looking at the noise of unconflicted changes, right? If I was walking in your shoes, I would essentially recreate the merge conflicts and then use git diff merge-commit with the resolved merge in your current history. Something like this: ```bash mergecommit=$1 # probably should verify that the working directory is clean, yadda yadda # recreate merge conflicts on an unnamed branch (Git speak: detached HEAD) git checkout $mergecommit^ git merge $mergecommit^2 || die This merge did not have any problem! # compare to the actual resolution as per the merge commit git diff $mergecommit ``` To list all the merge commits in the current branch, I would use the command-line: ```bash git rev-list --author=My Colleague --parents HEAD | sed -n 's/ .* .*//p' ``` (i.e. listing all the commits with their parents, then filtering just the ones having more than one parent, which would include octopus merges if your history has them.) Hopefully this gives you good ideas how to proceed. Ciao, Johannes -- 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