[RFC/PATCH] Port branch.c to use ref-filter APIs
This is part of my GSoC project to unify git tag -l, git branch -l, git for-each-ref. This patch series is continued from: Gmane (v6) http://article.gmane.org/gmane.comp.version-control.git/274726 This is a RFC version and I'm sending to ensure that I'm on the right path. This version also doesn't use the printing options provided by branch.c. I wanted to discuss how exactly to go about that, because in branch.c, we might need to change the --format based on attributes such as abbrev, verbose and so on. But ref-filter expects us to verify the format before filtering. Any tips or suggestions are welcome, thanks all :) -- 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
default configuration files on cygwin
Hi, I'm currently running git on a cygwin platform. I would like to know how i can set up a sort of configuration file to launch automatically the ssh-agent and get connected to github (for istance) directly. Thanks Filippo -- 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 branch command is incompatible with bash
W dniu 2015-07-28 o 09:28, Johannes Sixt pisze: Am 27.07.2015 um 23:49 schrieb Junio C Hamano: Johannes Sixt j...@kdbg.org writes: Try branchName=$(git rev-parse --abbrev-ref HEAD) Hmm, interesting. $ git checkout --orphan notyet $ git rev-parse --abbrev-ref HEAD $ git symbolic-ref --short HEAD Please don't scare newcomers with these corner cases ;-) :-P Yet another corner case: $ git checkout origin/master # or v1.0.0 $ git rev-parse --abbrev-ref HEAD $ git symbolic-ref --short HEAD I see this: $ git rev-parse --abbrev-ref HEAD HEAD fatal: ambiguous argument 'HEAD': unknown revision or path not in the working tree. Use '--' to separate paths from revisions, like this: 'git command [revision...] -- [file...]' Errr... this error message is errorneous (well, at least somewhat misleading). Git should know that HEAD is not a path, and that --abbrev-ref doe not need no revision. $ git symbolic-ref --short HEAD notyet Are you trying to say that the result of 'rev-parse --abbrev-ref HEAD' is suboptimal and that of 'symbolic-ref --short HEAD' is OK? -- Jakub Narębski -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 01/10] ref-filter: introduce 'ref_formatting_state'
Karthik Nayak karthik@gmail.com writes: -static void print_value(struct atom_value *v, int quote_style) +static void apply_formatting_state(struct ref_formatting_state *state, +struct atom_value *v, struct strbuf *value) { - struct strbuf sb = STRBUF_INIT; - switch (quote_style) { + /* Eventually we'll formatt based on the ref_formatting_state */ s/formatt/format/ Also, we usually use a single space after /*. (Neither is very important since it disapears in the next commit) + /* + * Some (pesudo) atoms have no immediate side effect, but only s/pesudo/pseudo/. But if you are to rename these atoms to modifier atoms, you should get rid of this pseudo here. -- 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: [RFC/PATCH 09/11] branch.c: use 'ref-filter' data structures
On Tue, Jul 28, 2015 at 8:56 AM, Karthik Nayak karthik@gmail.com wrote: diff --git a/ref-filter.h b/ref-filter.h index 7d1871d..3458595 100644 --- a/ref-filter.h +++ b/ref-filter.h @@ -5,6 +5,7 @@ #include refs.h #include commit.h #include parse-options.h +#include revision.h /* Quoting styles */ #define QUOTE_NONE 0 @@ -48,6 +49,7 @@ struct ref_sorting { struct ref_array_item { unsigned char objectname[20]; int flag, kind; + int ignore : 1; Maybe you could add a comment to say that this will be removed in the next patch. const char *symref; struct commit *commit; struct atom_value *value; -- 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: [RFC/PATCH 10/11] branch.c: use 'ref-filter' APIs
On Tue, Jul 28, 2015 at 12:11 AM, Karthik Nayak karthik@gmail.com wrote: Make 'branch.c' use 'ref-filter' APIs for iterating through refs sorting. This removes most of the code used in 'branch.c' replacing it with calls to the 'ref-filter' library. Make 'tag.c' use the 'filter_refs()' function provided by 'ref-filter' to filter out tags based on the options set. This patch doesn't do anything to tag.c, so you should update the commit message to remove this or replace it with s/tag/branch if it was intended to be about branch.c? Regards, Jake -- 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: [RFC/PATCH 10/11] branch.c: use 'ref-filter' APIs
On Tue, Jul 28, 2015 at 9:11 AM, Karthik Nayak karthik@gmail.com wrote: diff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt index a67138a..897cd81 100644 --- a/Documentation/git-branch.txt +++ b/Documentation/git-branch.txt @@ -11,7 +11,7 @@ SYNOPSIS 'git branch' [--color[=when] | --no-color] [-r | -a] [--list] [-v [--abbrev=length | --no-abbrev]] [--column[=options] | --no-column] - [(--merged | --no-merged | --contains) [commit]] [pattern...] + [(--merged | --no-merged | --contains) [commit]] [--sort=key] [pattern...] Maybe: [(--[no-]merged | --contains) [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
Re: [RFC/PATCH 02/11] ref-filter: add 'colornext' atom
Karthik Nayak karthik@gmail.com writes: --- a/t/t6302-for-each-ref-filter.sh +++ b/t/t6302-for-each-ref-filter.sh @@ -133,4 +133,20 @@ test_expect_success 'reverse version sort' ' test_cmp expect actual ' +get_color () +{ + git config --get-color no.such.slot $1 +} + +cat expect EOF +$(get_color green)foo1.10$(get_color reset)|| +$(get_color green)foo1.3$(get_color reset)|| +$(get_color green)foo1.6$(get_color reset)|| +EOF + +test_expect_success 'check `colornext` format option' ' + git for-each-ref --format=%(colornext:green)%(refname:short)|| | grep foo actual + test_cmp expect actual +' This is not a very good test: you're not checking that colornext applies to the next and only this one. Similarly to what I suggested for padright, I'd suggest --format=%(refname:short)%(colornext:green)|%(refname:short)|%(refname:short)| -- 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
[RFC/PATCH 09/11] branch.c: use 'ref-filter' data structures
Make 'branch.c' use 'ref-filter' data structures and make changes to support the new data structures. This is a part of the process of porting 'branch.c' to use 'ref-filter' APIs. This is a temporary step before porting 'branch.c' to use 'ref-filter' completely. As this is a temporary step, most of the code introduced here will be removed when 'branch.c' is ported over to use 'ref-filter' APIs 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 | 289 +-- ref-filter.h | 7 +- 2 files changed, 116 insertions(+), 180 deletions(-) diff --git a/builtin/branch.c b/builtin/branch.c index 8d0521e..df74527 100644 --- a/builtin/branch.c +++ b/builtin/branch.c @@ -19,6 +19,7 @@ #include column.h #include utf8.h #include wt-status.h +#include ref-filter.h static const char * const builtin_branch_usage[] = { N_(git branch [options] [-r | -a] [--merged | --no-merged]), @@ -28,10 +29,6 @@ static const char * const builtin_branch_usage[] = { NULL }; -#define REF_LOCAL_BRANCH0x01 -#define REF_REMOTE_BRANCH 0x02 -#define REF_DETACHED_HEAD 0x04 - static const char *head; static unsigned char head_sha1[20]; @@ -53,13 +50,6 @@ enum color_branch { BRANCH_COLOR_UPSTREAM = 5 }; -static enum merge_filter { - NO_FILTER = 0, - SHOW_NOT_MERGED, - SHOW_MERGED -} merge_filter; -static unsigned char merge_filter_ref[20]; - static struct string_list output = STRING_LIST_INIT_DUP; static unsigned int colopts; @@ -280,22 +270,6 @@ static int delete_branches(int argc, const char **argv, int force, int kinds, return(ret); } -struct ref_item { - char *name; - char *dest; - unsigned int kind; - struct commit *commit; - int ignore; -}; - -struct ref_list { - struct rev_info revs; - int index, alloc, verbose, abbrev; - struct ref_item *list; - struct commit_list *with_commit; - int kinds; -}; - static char *resolve_symref(const char *src, const char *prefix) { unsigned char sha1[20]; @@ -310,11 +284,6 @@ static char *resolve_symref(const char *src, const char *prefix) return xstrdup(dst); } -struct append_ref_cb { - struct ref_list *ref_list; - const char **pattern; -}; - static int match_patterns(const char **pattern, const char *refname) { if (!*pattern) @@ -327,11 +296,22 @@ static int match_patterns(const char **pattern, const char *refname) return 0; } +static void ref_array_append(struct ref_array *array, const char *refname) +{ + size_t len = strlen(refname); + struct ref_array_item *ref = xcalloc(1, sizeof(struct ref_array_item) + len + 1); + memcpy(ref-refname, refname, len); + ref-refname[len] = '\0'; + REALLOC_ARRAY(array-items, array-nr + 1); + array-items[array-nr++] = ref; +} + static int append_ref(const char *refname, const struct object_id *oid, int flags, void *cb_data) { - struct append_ref_cb *cb = (struct append_ref_cb *)(cb_data); - struct ref_list *ref_list = cb-ref_list; - struct ref_item *newitem; + struct ref_filter_cbdata *cb = (struct ref_filter_cbdata *)(cb_data); + struct ref_filter *filter = cb-filter; + struct ref_array *array = cb-array; + struct ref_array_item *item; struct commit *commit; int kind, i; const char *prefix, *orig_refname = refname; @@ -360,59 +340,47 @@ static int append_ref(const char *refname, const struct object_id *oid, int flag } /* Don't add types the caller doesn't want */ - if ((kind ref_list-kinds) == 0) + if ((kind filter-branch_kind) == 0) return 0; - if (!match_patterns(cb-pattern, refname)) + if (!match_patterns(filter-name_patterns, refname)) return 0; commit = NULL; - if (ref_list-verbose || ref_list-with_commit || merge_filter != NO_FILTER) { + if (filter-verbose || filter-with_commit || filter-merge != REF_FILTER_MERGED_NONE) { commit = lookup_commit_reference_gently(oid-hash, 1); if (!commit) return 0; /* Filter with with_commit if specified */ - if (!is_descendant_of(commit, ref_list-with_commit)) + if (!is_descendant_of(commit, filter-with_commit)) return 0; - if (merge_filter != NO_FILTER) - add_pending_object(ref_list-revs, + if (filter-merge != REF_FILTER_MERGED_NONE) + add_pending_object(array-revs, (struct object *)commit, refname); } - ALLOC_GROW(ref_list-list, ref_list-index + 1, ref_list-alloc); +
[RFC/PATCH 05/11] branch: fix width computation
From: Karthik Nayak karthik@gmail.com Remove unnecessary variables from ref_list and ref_item which were used for width computation. Make other changes accordingly. This patch is a precursor for porting 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 | 61 +--- 1 file changed, 32 insertions(+), 29 deletions(-) diff --git a/builtin/branch.c b/builtin/branch.c index 4fc8beb..b058b74 100644 --- a/builtin/branch.c +++ b/builtin/branch.c @@ -282,14 +282,14 @@ static int delete_branches(int argc, const char **argv, int force, int kinds, struct ref_item { char *name; char *dest; - unsigned int kind, width; + unsigned int kind; struct commit *commit; int ignore; }; struct ref_list { struct rev_info revs; - int index, alloc, maxwidth, verbose, abbrev; + int index, alloc, verbose, abbrev; struct ref_item *list; struct commit_list *with_commit; int kinds; @@ -386,15 +386,8 @@ static int append_ref(const char *refname, const struct object_id *oid, int flag newitem-name = xstrdup(refname); newitem-kind = kind; newitem-commit = commit; - newitem-width = utf8_strwidth(refname); newitem-dest = resolve_symref(orig_refname, prefix); newitem-ignore = 0; - /* adjust for remotes/ */ - if (newitem-kind == REF_REMOTE_BRANCH - ref_list-kinds != REF_REMOTE_BRANCH) - newitem-width += 8; - if (newitem-width ref_list-maxwidth) - ref_list-maxwidth = newitem-width; return 0; } @@ -505,11 +498,12 @@ static void add_verbose_info(struct strbuf *out, struct ref_item *item, } static void print_ref_item(struct ref_item *item, int maxwidth, int verbose, - int abbrev, int current, char *prefix) + int abbrev, int current, const char *remote_prefix) { char c; int color; struct strbuf out = STRBUF_INIT, name = STRBUF_INIT; + const char *prefix = ; if (item-ignore) return; @@ -520,6 +514,7 @@ static void print_ref_item(struct ref_item *item, int maxwidth, int verbose, break; case REF_REMOTE_BRANCH: color = BRANCH_COLOR_REMOTE; + prefix = remote_prefix; break; default: color = BRANCH_COLOR_PLAIN; @@ -557,16 +552,21 @@ static void print_ref_item(struct ref_item *item, int maxwidth, int verbose, strbuf_release(out); } -static int calc_maxwidth(struct ref_list *refs) +static int calc_maxwidth(struct ref_list *refs, int remote_bonus) { - int i, w = 0; + int i, max = 0; for (i = 0; i refs-index; i++) { + struct ref_item *it = refs-list[i]; + int w = utf8_strwidth(it-name); + if (refs-list[i].ignore) continue; - if (refs-list[i].width w) - w = refs-list[i].width; + if (it-kind == REF_REMOTE_BRANCH) + w += remote_bonus; + if (w max) + max = w; } - return w; + return max; } static char *get_head_description(void) @@ -600,21 +600,18 @@ static char *get_head_description(void) return strbuf_detach(desc, NULL); } -static void show_detached(struct ref_list *ref_list) +static void show_detached(struct ref_list *ref_list, int maxwidth) { struct commit *head_commit = lookup_commit_reference_gently(head_sha1, 1); if (head_commit is_descendant_of(head_commit, ref_list-with_commit)) { struct ref_item item; item.name = get_head_description(); - item.width = utf8_strwidth(item.name); item.kind = REF_LOCAL_BRANCH; item.dest = NULL; item.commit = head_commit; item.ignore = 0; - if (item.width ref_list-maxwidth) - ref_list-maxwidth = item.width; - print_ref_item(item, ref_list-maxwidth, ref_list-verbose, ref_list-abbrev, 1, ); + print_ref_item(item, maxwidth, ref_list-verbose, ref_list-abbrev, 1, ); free(item.name); } } @@ -624,6 +621,16 @@ static int print_ref_list(int kinds, int detached, int verbose, int abbrev, stru int i; struct append_ref_cb cb; struct ref_list ref_list; + int maxwidth = 0; + const char *remote_prefix = ; + + /* +* If we are listing more than just remote branches, +* then remote branches will have a remotes/ prefix. +* We need to account for
Re: [RFC/PATCH 04/11] ref-filter: add 'ifexists' atom
On Mon, Jul 27, 2015 at 11:56 PM, Karthik Nayak karthik@gmail.com wrote: The 'ifexists' atom allows us to print a required format if the preceeding atom has a value. If the preceeding atom has no value then Don't you mean following atom here? since you do document it as the next atom below you should fix the commit message as well to match. In any respect, this is a useful formatting atom :) %(ifexists:[%s])%(atom) where the contents of atom will be placed into %s? the format given is not printed. e.g. to print [refname] we can now use the format %(ifexists:[%s])%(refname). Add documentation and test 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 | 8 ref-filter.c | 37 ++--- ref-filter.h | 5 +++-- t/t6302-for-each-ref-filter.sh | 21 + 4 files changed, 66 insertions(+), 5 deletions(-) diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt index 9dc02aa..4424020 100644 --- a/Documentation/git-for-each-ref.txt +++ b/Documentation/git-for-each-ref.txt @@ -138,6 +138,14 @@ colornext:: `:colorname`. Not compatible with `padright` and resets any previous `color`, if set. +ifexists:: + Print required string only if the next atom specified in the + '--format' option exists. + e.g. --format=%(ifexists:[%s])%(symref) prints the symref + like [symref] only if the ref has a symref. This was + incorporated to simulate the output of 'git branch -vv', where + we need to display the upstream branch in square brackets. + I suggest documenting that the atom will be placed into the contents of ifexists via the %s indicator, as you do show an example but don't explicitely say %s is the formatting character. In addition to the above, for commit and tag objects, the header field names (`tree`, `parent`, `object`, `type`, and `tag`) can be used to specify the value in the header field. diff --git a/ref-filter.c b/ref-filter.c index 3f40144..ff5a16b 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -58,6 +58,7 @@ static struct { { color }, { padright }, { colornext }, + { ifexists }, }; /* @@ -722,6 +723,13 @@ static void populate_value(struct ref_array_item *ref) v-modifier_atom = 1; v-color_next = 1; continue; + } else if (starts_with(name, ifexists:)) { + skip_prefix(name, ifexists:, v-s); + if (!*v-s) + die(_(no string given with 'ifexists:')); + v-modifier_atom = 1; + v-ifexists = 1; + continue; } else continue; @@ -1315,11 +1323,32 @@ static void apply_formatting_state(struct ref_formatting_state *state, { if (state-color_next state-pad_to_right) die(_(cannot use `colornext` and `padright` together)); - if (state-color_next) { + if (state-pad_to_right state-ifexists) + die(_(cannot use 'align' and 'ifexists' together)); + if (state-color_next !state-ifexists) { strbuf_addf(value, %s%s%s, state-color_next, v-s, GIT_COLOR_RESET); return; - } - else if (state-pad_to_right) { + } else if (state-ifexists) { + const char *sp = state-ifexists; + + while (*sp) { + if (*sp != '%') { + strbuf_addch(value, *sp++); + continue; + } else if (sp[1] == '%') { + strbuf_addch(value, *sp++); + continue; + } else if (sp[1] == 's') { + if (state-color_next) + strbuf_addf(value, %s%s%s, state-color_next, v-s, GIT_COLOR_RESET); + else + strbuf_addstr(value, v-s); + sp += 2; + } + } + + return; + } else if (state-pad_to_right) { if (!is_utf8(v-s)) strbuf_addf(value, %-*s, state-pad_to_right, v-s); else { @@ -1413,6 +1442,8 @@ static void store_formatting_state(struct ref_formatting_state *state, state-color_next = atomv-s; if (atomv-pad_to_right) state-pad_to_right = atomv-ul; + if (atomv-ifexists) + state-ifexists =
Re: [RFC/PATCH 09/11] branch.c: use 'ref-filter' data structures
On Tue, Jul 28, 2015 at 8:56 AM, Karthik Nayak karthik@gmail.com wrote: +static void ref_array_append(struct ref_array *array, const char *refname) +{ + size_t len = strlen(refname); + struct ref_array_item *ref = xcalloc(1, sizeof(struct ref_array_item) + len + 1); + memcpy(ref-refname, refname, len); + ref-refname[len] = '\0'; + REALLOC_ARRAY(array-items, array-nr + 1); + array-items[array-nr++] = ref; +} This function belongs more to ref-filter.{c,h}... [...] - ALLOC_GROW(ref_list-list, ref_list-index + 1, ref_list-alloc); + ref_array_append(array, refname); + item = array-items[array-nr - 1]; ...and the above is a bit ugly. -- 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: [RFC/PATCH 04/11] ref-filter: add 'ifexists' atom
Karthik Nayak karthik@gmail.com writes: --- a/t/t6302-for-each-ref-filter.sh +++ b/t/t6302-for-each-ref-filter.sh @@ -149,4 +149,25 @@ test_expect_success 'check `colornext` format option' ' test_cmp expect actual ' +test_expect_success 'check `ifexists` format option' ' + cat expect -\EOF + [foo1.10] + [foo1.3] + [foo1.6] + EOF + git for-each-ref --format=%(ifexists:[%s])%(refname:short) | grep foo actual + test_cmp expect actual +' You're testing only the positive case of ifexists, right? You need a test for the negative case (i.e. the attribute does not exist) too. Ideally, check that the ifexist actually applies to the right attribute, like --format '%(ifexist:%s)%(attribute1) %(ifexist:[%s])%(attribute2)' and manage to get an output like foo [bar] foobar [barfoo] +cat expect EOF +[$(get_color green)foo1.10$(get_color reset)]||foo1.10 +[$(get_color green)foo1.3$(get_color reset)]||foo1.3 +[$(get_color green)foo1.6$(get_color reset)]||foo1.6 +EOF + +test_expect_success 'check `ifexists` with `colornext` format option' ' + git for-each-ref --format=%(ifexists:[%s])%(colornext:green)%(refname:short)||%(refname:short) | grep foo actual + test_cmp expect actual +' You have a double || that is not useful. Not really harmful either, but it may distract the reader. You may want to s/||/|/. -- 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: [RFC/PATCH 05/11] branch: fix width computation
Karthik Nayak karthik@gmail.com writes: From: Karthik Nayak karthik@gmail.com Why did send-email add this From: header? Strange, it has the same content as your actual From: field. Remove unnecessary variables from ref_list and ref_item which were used for width computation. Make other changes accordingly. This patch is a precursor for porting branch.c to use ref-filter APIs. You can explain better why this is needed. I guess something like we're making struct ref_item similar to ref-filter's ref_array_item, but the reader shouldn't have to guess. You should adujst the subject like BTW, I don't think you are fixing anything. On a side note: on the tag series, see how explaining better and splitting patches led not only to a better history, but also to better final code, and to finding a actual bugs (the %(color) thing and the absence of reset on the state variable) even after sereval rounds of review? I'm being picky and demanding, but don't see that as a complain from me, just hints on getting even better ;-). @@ -386,15 +386,8 @@ static int append_ref(const char *refname, const struct object_id *oid, int flag newitem-name = xstrdup(refname); newitem-kind = kind; newitem-commit = commit; - newitem-width = utf8_strwidth(refname); newitem-dest = resolve_symref(orig_refname, prefix); newitem-ignore = 0; - /* adjust for remotes/ */ - if (newitem-kind == REF_REMOTE_BRANCH - ref_list-kinds != REF_REMOTE_BRANCH) - newitem-width += 8; - if (newitem-width ref_list-maxwidth) - ref_list-maxwidth = newitem-width; return 0; } OK, in the old code, the width computation is done when inserting the ref into the array. In the new code, you build the array and then do the width computation. You can explain this better in the commit message I think (instead of Make other changes accordingly which doesn't bring much). @@ -505,11 +498,12 @@ static void add_verbose_info(struct strbuf *out, struct ref_item *item, } static void print_ref_item(struct ref_item *item, int maxwidth, int verbose, -int abbrev, int current, char *prefix) +int abbrev, int current, const char *remote_prefix) { char c; int color; struct strbuf out = STRBUF_INIT, name = STRBUF_INIT; + const char *prefix = ; if (item-ignore) return; @@ -520,6 +514,7 @@ static void print_ref_item(struct ref_item *item, int maxwidth, int verbose, break; case REF_REMOTE_BRANCH: color = BRANCH_COLOR_REMOTE; + prefix = remote_prefix; break; Why do you need these two hunks? I did not investigate in details, but it seems you're calling print_ref_item either with prefix= or with prefix=remote_prefix so it would seem that keeping prefix as argument would work. I guess I missed something. -static int calc_maxwidth(struct ref_list *refs) +static int calc_maxwidth(struct ref_list *refs, int remote_bonus) { - int i, w = 0; + int i, max = 0; for (i = 0; i refs-index; i++) { + struct ref_item *it = refs-list[i]; + int w = utf8_strwidth(it-name); + if (refs-list[i].ignore) continue; - if (refs-list[i].width w) - w = refs-list[i].width; + if (it-kind == REF_REMOTE_BRANCH) + w += remote_bonus; + if (w max) + max = w; } - return w; + return max; } The old code was using 'w' for the max and no variable for the value at the current iteration. You're using 'max' for the max and 'w' at the current iteration. Using the same name 'w' for different things in the pre- and post-image of the patch distracts the reader. It may make sense to s/w/max/ in a separate preparatory patch. Or maybe it's overkill. @@ -600,21 +600,18 @@ static char *get_head_description(void) return strbuf_detach(desc, NULL); } -static void show_detached(struct ref_list *ref_list) +static void show_detached(struct ref_list *ref_list, int maxwidth) { struct commit *head_commit = lookup_commit_reference_gently(head_sha1, 1); if (head_commit is_descendant_of(head_commit, ref_list-with_commit)) { struct ref_item item; item.name = get_head_description(); - item.width = utf8_strwidth(item.name); item.kind = REF_LOCAL_BRANCH; item.dest = NULL; item.commit = head_commit; item.ignore = 0; - if (item.width ref_list-maxwidth) - ref_list-maxwidth = item.width; - print_ref_item(item, ref_list-maxwidth, ref_list-verbose, ref_list-abbrev, 1, ); + print_ref_item(item, maxwidth, ref_list-verbose, ref_list-abbrev, 1, );
Re: Log messages beginning # and git rebase -i
Eric Sunshine sunshine at sunshineco.com writes: the editing for the combined log message treats lines beginning with # as comments. This means that if you are not careful the commit message can get lost on rebasing. I suggest that git rebase should add an extra space at the start 'git rebase --interactive' respects the core.commentChar configuration variable, which you can set to some value other than '#'. I was thinking of the default configuration. But you are right, this applies to whatever the comment character is - so if commentChar is set to * for example, then log lines beginning with * should get an extra space prepended in git rebase --interactive so that they don't get lost. -- Ed Avis e...@waniasset.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: git branch command is incompatible with bash
Am 27.07.2015 um 23:49 schrieb Junio C Hamano: Johannes Sixt j...@kdbg.org writes: Try branchName=$(git rev-parse --abbrev-ref HEAD) Hmm, interesting. $ git checkout --orphan notyet $ git rev-parse --abbrev-ref HEAD $ git symbolic-ref --short HEAD Please don't scare newcomers with these corner cases ;-) I see this: $ git rev-parse --abbrev-ref HEAD HEAD fatal: ambiguous argument 'HEAD': unknown revision or path not in the working tree. Use '--' to separate paths from revisions, like this: 'git command [revision...] -- [file...]' $ git symbolic-ref --short HEAD notyet Are you trying to say that the result of 'rev-parse --abbrev-ref HEAD' is suboptimal and that of 'symbolic-ref --short HEAD' is OK? -- Hannes -- 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: [RFC/PATCH 01/11] ref-filter: add %(objectname:size=X) option
Karthik Nayak karthik@gmail.com writes: + if (skip_prefix(name, objectname:size=, p)) { + unsigned int size = atoi(p); You have the same problem as for tag.c: don't use atoi, and make accurate error checking (absence of value, negative value, non-integer value). If you have other higher-priorities task, leave a temporary comment /* FIXME: ... */ or /* TODO: ... */ and make sure you have no such comment when you drop the RFC in the subject of your emails. -- 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
[RFC/PATCH 02/11] ref-filter: add 'colornext' atom
The 'colornext' atom allows us to color only the succeeding atom with a particular color. This resets any previous color preferences set. It is not compatible with `padright`. This is required for printing formats like `git branch -vv` where the upstream is colored in blue. 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 + ref-filter.c | 20 +++- ref-filter.h | 4 +++- t/t6302-for-each-ref-filter.sh | 16 4 files changed, 43 insertions(+), 2 deletions(-) diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt index 2b60aee..9dc02aa 100644 --- a/Documentation/git-for-each-ref.txt +++ b/Documentation/git-for-each-ref.txt @@ -133,6 +133,11 @@ padright:: padding. If the `value` is greater than the atom length, then no padding is performed. +colornext:: + Change output color only for the next atom. Followed by + `:colorname`. Not compatible with `padright` and resets any + previous `color`, if set. + In addition to the above, for commit and tag objects, the header field names (`tree`, `parent`, `object`, `type`, and `tag`) can be used to specify the value in the header field. diff --git a/ref-filter.c b/ref-filter.c index 4c5e3f8..3ab4ab9 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -57,6 +57,7 @@ static struct { { HEAD }, { color }, { padright }, + { colornext }, }; /* @@ -712,6 +713,15 @@ static void populate_value(struct ref_array_item *ref) v-modifier_atom = 1; v-pad_to_right = 1; continue; + } else if (starts_with(name, colornext:)) { + char color[COLOR_MAXLEN] = ; + + if (color_parse(name + 10, color) 0) + die(_(unable to parse format)); + v-s = xstrdup(color); + v-modifier_atom = 1; + v-color_next = 1; + continue; } else continue; @@ -1256,7 +1266,13 @@ void ref_array_sort(struct ref_sorting *sorting, struct ref_array *array) static void apply_formatting_state(struct ref_formatting_state *state, struct atom_value *v, struct strbuf *value) { - if (state-pad_to_right) { + if (state-color_next state-pad_to_right) + die(_(cannot use `colornext` and `padright` together)); + if (state-color_next) { + strbuf_addf(value, %s%s%s, state-color_next, v-s, GIT_COLOR_RESET); + return; + } + else if (state-pad_to_right) { if (!is_utf8(v-s)) strbuf_addf(value, %-*s, state-pad_to_right, v-s); else { @@ -1346,6 +1362,8 @@ static void emit(const char *cp, const char *ep) static void store_formatting_state(struct ref_formatting_state *state, struct atom_value *atomv) { + if (atomv-color_next) + state-color_next = atomv-s; if (atomv-pad_to_right) state-pad_to_right = atomv-ul; } diff --git a/ref-filter.h b/ref-filter.h index fcf469e..8c82fd1 100644 --- a/ref-filter.h +++ b/ref-filter.h @@ -21,12 +21,14 @@ struct atom_value { const char *s; unsigned long ul; /* used for sorting when not FIELD_STR */ unsigned int modifier_atom : 1, /* atoms which act as modifiers for the next atom */ - pad_to_right : 1; + pad_to_right : 1, + color_next : 1; }; struct ref_formatting_state { int quote_style; unsigned int pad_to_right; + const char *color_next; }; struct ref_sorting { diff --git a/t/t6302-for-each-ref-filter.sh b/t/t6302-for-each-ref-filter.sh index 68688a9..6aad069 100755 --- a/t/t6302-for-each-ref-filter.sh +++ b/t/t6302-for-each-ref-filter.sh @@ -133,4 +133,20 @@ test_expect_success 'reverse version sort' ' test_cmp expect actual ' +get_color () +{ + git config --get-color no.such.slot $1 +} + +cat expect EOF +$(get_color green)foo1.10$(get_color reset)|| +$(get_color green)foo1.3$(get_color reset)|| +$(get_color green)foo1.6$(get_color reset)|| +EOF + +test_expect_success 'check `colornext` format option' ' + git for-each-ref --format=%(colornext:green)%(refname:short)|| | grep foo actual + test_cmp expect actual +' + test_done -- 2.4.6 -- 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
[RFC/PATCH 06/11] branch: roll show_detached HEAD into regular ref_list
Remove show_detached() and make detached HEAD to be rolled into regular ref_list by adding REF_DETACHED_HEAD as a kind of branch and supporting the same in append_ref(). Bump get_head_description() to the top. 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 | 122 --- 1 file changed, 63 insertions(+), 59 deletions(-) diff --git a/builtin/branch.c b/builtin/branch.c index b058b74..f3d83d0 100644 --- a/builtin/branch.c +++ b/builtin/branch.c @@ -30,6 +30,7 @@ static const char * const builtin_branch_usage[] = { #define REF_LOCAL_BRANCH0x01 #define REF_REMOTE_BRANCH 0x02 +#define REF_DETACHED_HEAD 0x04 static const char *head; static unsigned char head_sha1[20]; @@ -352,8 +353,12 @@ static int append_ref(const char *refname, const struct object_id *oid, int flag break; } } - if (ARRAY_SIZE(ref_kind) = i) - return 0; + if (ARRAY_SIZE(ref_kind) = i) { + if (!strcmp(refname, HEAD)) + kind = REF_DETACHED_HEAD; + else + return 0; + } /* Don't add types the caller doesn't want */ if ((kind ref_list-kinds) == 0) @@ -497,6 +502,37 @@ static void add_verbose_info(struct strbuf *out, struct ref_item *item, strbuf_release(subject); } +static char *get_head_description(void) +{ + struct strbuf desc = STRBUF_INIT; + struct wt_status_state state; + memset(state, 0, sizeof(state)); + wt_status_get_state(state, 1); + if (state.rebase_in_progress || + state.rebase_interactive_in_progress) + strbuf_addf(desc, _((no branch, rebasing %s)), + state.branch); + else if (state.bisect_in_progress) + strbuf_addf(desc, _((no branch, bisect started on %s)), + state.branch); + else if (state.detached_from) { + /* TRANSLATORS: make sure these match _(HEAD detached at ) + and _(HEAD detached from ) in wt-status.c */ + if (state.detached_at) + strbuf_addf(desc, _((HEAD detached at %s)), + state.detached_from); + else + strbuf_addf(desc, _((HEAD detached from %s)), + state.detached_from); + } + else + strbuf_addstr(desc, _((no branch))); + free(state.branch); + free(state.onto); + free(state.detached_from); + return strbuf_detach(desc, NULL); +} + static void print_ref_item(struct ref_item *item, int maxwidth, int verbose, int abbrev, int current, const char *remote_prefix) { @@ -504,6 +540,7 @@ static void print_ref_item(struct ref_item *item, int maxwidth, int verbose, int color; struct strbuf out = STRBUF_INIT, name = STRBUF_INIT; const char *prefix = ; + const char *desc = item-name; if (item-ignore) return; @@ -516,6 +553,10 @@ static void print_ref_item(struct ref_item *item, int maxwidth, int verbose, color = BRANCH_COLOR_REMOTE; prefix = remote_prefix; break; + case REF_DETACHED_HEAD: + color = BRANCH_COLOR_CURRENT; + desc = get_head_description(); + break; default: color = BRANCH_COLOR_PLAIN; break; @@ -527,7 +568,7 @@ static void print_ref_item(struct ref_item *item, int maxwidth, int verbose, color = BRANCH_COLOR_CURRENT; } - strbuf_addf(name, %s%s, prefix, item-name); + strbuf_addf(name, %s%s, prefix, desc); if (verbose) { int utf8_compensation = strlen(name.buf) - utf8_strwidth(name.buf); strbuf_addf(out, %c %s%-*s%s, c, branch_get_color(color), @@ -569,56 +610,9 @@ static int calc_maxwidth(struct ref_list *refs, int remote_bonus) return max; } -static char *get_head_description(void) -{ - struct strbuf desc = STRBUF_INIT; - struct wt_status_state state; - memset(state, 0, sizeof(state)); - wt_status_get_state(state, 1); - if (state.rebase_in_progress || - state.rebase_interactive_in_progress) - strbuf_addf(desc, _((no branch, rebasing %s)), - state.branch); - else if (state.bisect_in_progress) - strbuf_addf(desc, _((no branch, bisect started on %s)), - state.branch); - else if (state.detached_from) { - /* TRANSLATORS: make sure these match _(HEAD detached at ) - and _(HEAD detached from ) in
[RFC/PATCH 04/11] ref-filter: add 'ifexists' atom
The 'ifexists' atom allows us to print a required format if the preceeding atom has a value. If the preceeding atom has no value then the format given is not printed. e.g. to print [refname] we can now use the format %(ifexists:[%s])%(refname). Add documentation and test 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 | 8 ref-filter.c | 37 ++--- ref-filter.h | 5 +++-- t/t6302-for-each-ref-filter.sh | 21 + 4 files changed, 66 insertions(+), 5 deletions(-) diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt index 9dc02aa..4424020 100644 --- a/Documentation/git-for-each-ref.txt +++ b/Documentation/git-for-each-ref.txt @@ -138,6 +138,14 @@ colornext:: `:colorname`. Not compatible with `padright` and resets any previous `color`, if set. +ifexists:: + Print required string only if the next atom specified in the + '--format' option exists. + e.g. --format=%(ifexists:[%s])%(symref) prints the symref + like [symref] only if the ref has a symref. This was + incorporated to simulate the output of 'git branch -vv', where + we need to display the upstream branch in square brackets. + In addition to the above, for commit and tag objects, the header field names (`tree`, `parent`, `object`, `type`, and `tag`) can be used to specify the value in the header field. diff --git a/ref-filter.c b/ref-filter.c index 3f40144..ff5a16b 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -58,6 +58,7 @@ static struct { { color }, { padright }, { colornext }, + { ifexists }, }; /* @@ -722,6 +723,13 @@ static void populate_value(struct ref_array_item *ref) v-modifier_atom = 1; v-color_next = 1; continue; + } else if (starts_with(name, ifexists:)) { + skip_prefix(name, ifexists:, v-s); + if (!*v-s) + die(_(no string given with 'ifexists:')); + v-modifier_atom = 1; + v-ifexists = 1; + continue; } else continue; @@ -1315,11 +1323,32 @@ static void apply_formatting_state(struct ref_formatting_state *state, { if (state-color_next state-pad_to_right) die(_(cannot use `colornext` and `padright` together)); - if (state-color_next) { + if (state-pad_to_right state-ifexists) + die(_(cannot use 'align' and 'ifexists' together)); + if (state-color_next !state-ifexists) { strbuf_addf(value, %s%s%s, state-color_next, v-s, GIT_COLOR_RESET); return; - } - else if (state-pad_to_right) { + } else if (state-ifexists) { + const char *sp = state-ifexists; + + while (*sp) { + if (*sp != '%') { + strbuf_addch(value, *sp++); + continue; + } else if (sp[1] == '%') { + strbuf_addch(value, *sp++); + continue; + } else if (sp[1] == 's') { + if (state-color_next) + strbuf_addf(value, %s%s%s, state-color_next, v-s, GIT_COLOR_RESET); + else + strbuf_addstr(value, v-s); + sp += 2; + } + } + + return; + } else if (state-pad_to_right) { if (!is_utf8(v-s)) strbuf_addf(value, %-*s, state-pad_to_right, v-s); else { @@ -1413,6 +1442,8 @@ static void store_formatting_state(struct ref_formatting_state *state, state-color_next = atomv-s; if (atomv-pad_to_right) state-pad_to_right = atomv-ul; + if (atomv-ifexists) + state-ifexists = atomv-s; } static void reset_formatting_state(struct ref_formatting_state *state) diff --git a/ref-filter.h b/ref-filter.h index a021b04..7d1871d 100644 --- a/ref-filter.h +++ b/ref-filter.h @@ -28,13 +28,14 @@ struct atom_value { unsigned long ul; /* used for sorting when not FIELD_STR */ unsigned int modifier_atom : 1, /* atoms which act as modifiers for the next atom */ pad_to_right : 1, - color_next : 1; + color_next : 1, + ifexists : 1; }; struct ref_formatting_state { int quote_style; unsigned int pad_to_right; - const
[RFC/PATCH 03/11] ref-filter: add option to filter only branches
From: Karthik Nayak karthik@gmail.com Add an option in 'filter_refs()' to use 'for_each_branch_ref()' and filter refs. This type checking is done by adding a 'FILTER_REFS_BRANCHES' in 'ref-filter.h'. Add an option in 'ref_filter_handler()' to filter different types of branches by calling 'filter_branch_kind()' which checks for the type of branch needed. 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 | 47 +++ ref-filter.h | 10 -- 2 files changed, 55 insertions(+), 2 deletions(-) diff --git a/ref-filter.c b/ref-filter.c index 3ab4ab9..3f40144 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -1054,6 +1054,46 @@ static const unsigned char *match_points_at(struct sha1_array *points_at, return NULL; } +/* + * Checks if a given refname is a branch and returns the kind of + * branch it is. If not a branch, 0 is returned. + */ +static int filter_branch_kind(struct ref_filter *filter, const char *refname) +{ + int kind, i; + + static struct { + int kind; + const char *prefix; + } ref_kind[] = { + { REF_LOCAL_BRANCH, refs/heads/ }, + { REF_REMOTE_BRANCH, refs/remotes/ }, + }; + + /* If no kind is specified, no need to filter */ + if (!filter-branch_kind) + return REF_NO_BRANCH_FILTERING; + + for (i = 0; i ARRAY_SIZE(ref_kind); i++) { + if (starts_with(refname, ref_kind[i].prefix)) { + kind = ref_kind[i].kind; + break; + } + } + + if (ARRAY_SIZE(ref_kind) = i) { + if (!strcmp(refname, HEAD)) + kind = REF_DETACHED_HEAD; + else + return 0; + } + + if ((filter-branch_kind kind) == 0) + return 0; + + return kind; +} + /* 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, @@ -1079,6 +1119,7 @@ static int ref_filter_handler(const char *refname, const struct object_id *oid, struct ref_filter *filter = ref_cbdata-filter; struct ref_array_item *ref; struct commit *commit = NULL; + unsigned int kind; if (flag REF_BAD_NAME) { warning(ignoring ref with broken name %s, refname); @@ -1090,6 +1131,9 @@ static int ref_filter_handler(const char *refname, const struct object_id *oid, return 0; } + if (!(kind = filter_branch_kind(filter, refname))) + return 0; + if (!filter_pattern_match(filter, refname)) return 0; @@ -1118,6 +1162,7 @@ static int ref_filter_handler(const char *refname, const struct object_id *oid, */ ref = new_ref_array_item(refname, oid-hash, flag); ref-commit = commit; + ref-kind = kind; REALLOC_ARRAY(ref_cbdata-array-items, ref_cbdata-array-nr + 1); ref_cbdata-array-items[ref_cbdata-array-nr++] = ref; @@ -1208,6 +1253,8 @@ int filter_refs(struct ref_array *array, struct ref_filter *filter, unsigned int ret = for_each_ref(ref_filter_handler, ref_cbdata); else if (type FILTER_REFS_TAGS) ret = for_each_tag_ref_fullpath(ref_filter_handler, ref_cbdata); + else if (type FILTER_REFS_BRANCHES) + ret = for_each_rawref(ref_filter_handler, ref_cbdata); else if (type) die(filter_refs: invalid type); diff --git a/ref-filter.h b/ref-filter.h index 8c82fd1..a021b04 100644 --- a/ref-filter.h +++ b/ref-filter.h @@ -16,6 +16,12 @@ #define FILTER_REFS_INCLUDE_BROKEN 0x1 #define FILTER_REFS_ALL 0x2 #define FILTER_REFS_TAGS 0x4 +#define FILTER_REFS_BRANCHES 0x8 + +#define REF_DETACHED_HEAD 0x01 +#define REF_LOCAL_BRANCH0x02 +#define REF_REMOTE_BRANCH 0x04 +#define REF_NO_BRANCH_FILTERING 0x08 struct atom_value { const char *s; @@ -40,7 +46,7 @@ struct ref_sorting { struct ref_array_item { unsigned char objectname[20]; - int flag; + int flag, kind; const char *symref; struct commit *commit; struct atom_value *value; @@ -66,7 +72,7 @@ struct ref_filter { unsigned int with_commit_tag_algo : 1, match_as_path : 1; - unsigned int lines; + unsigned int lines, branch_kind; }; struct ref_filter_cbdata { -- 2.4.6 -- 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
[RFC/PATCH 07/11] branch: move 'current' check down to the presentation layer
We check if given ref is the current branch in print_ref_list(). Move this check to print_ref_item() where it is checked right before printing. 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 | 29 - 1 file changed, 12 insertions(+), 17 deletions(-) diff --git a/builtin/branch.c b/builtin/branch.c index f3d83d0..ba9cbad 100644 --- a/builtin/branch.c +++ b/builtin/branch.c @@ -534,9 +534,9 @@ static char *get_head_description(void) } static void print_ref_item(struct ref_item *item, int maxwidth, int verbose, - int abbrev, int current, const char *remote_prefix) + int abbrev, int detached, const char *remote_prefix) { - char c; + char c = ' '; int color; struct strbuf out = STRBUF_INIT, name = STRBUF_INIT; const char *prefix = ; @@ -547,7 +547,11 @@ static void print_ref_item(struct ref_item *item, int maxwidth, int verbose, switch (item-kind) { case REF_LOCAL_BRANCH: - color = BRANCH_COLOR_LOCAL; + if (!detached !strcmp(item-name, head)) { + color = BRANCH_COLOR_CURRENT; + c = '*'; + } else + color = BRANCH_COLOR_LOCAL; break; case REF_REMOTE_BRANCH: color = BRANCH_COLOR_REMOTE; @@ -556,18 +560,13 @@ static void print_ref_item(struct ref_item *item, int maxwidth, int verbose, case REF_DETACHED_HEAD: color = BRANCH_COLOR_CURRENT; desc = get_head_description(); + c = '*'; break; default: color = BRANCH_COLOR_PLAIN; break; } - c = ' '; - if (current) { - c = '*'; - color = BRANCH_COLOR_CURRENT; - } - strbuf_addf(name, %s%s, prefix, desc); if (verbose) { int utf8_compensation = strlen(name.buf) - utf8_strwidth(name.buf); @@ -677,21 +676,17 @@ static int print_ref_list(int kinds, int detached, int verbose, int abbrev, stru index = ref_list.index; /* Print detached heads before sorting and printing the rest */ - if (detached (ref_list.list[index - 1].kind == REF_DETACHED_HEAD) - !strcmp(ref_list.list[index - 1].name, head)) { + if (detached) { print_ref_item(ref_list.list[index - 1], maxwidth, verbose, abbrev, - 1, remote_prefix); + detached, remote_prefix); index -= 1; } qsort(ref_list.list, index, sizeof(struct ref_item), ref_cmp); - for (i = 0; i index; i++) { - int current = !detached (ref_list.list[i].kind == REF_LOCAL_BRANCH) - !strcmp(ref_list.list[i].name, head); + for (i = 0; i index; i++) print_ref_item(ref_list.list[i], maxwidth, verbose, - abbrev, current, remote_prefix); - } + abbrev, detached, remote_prefix); free_ref_list(ref_list); -- 2.4.6 -- 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
[RFC/PATCH 08/11] branch: drop non-commit error reporting
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 | 18 -- 1 file changed, 4 insertions(+), 14 deletions(-) diff --git a/builtin/branch.c b/builtin/branch.c index ba9cbad..8d0521e 100644 --- a/builtin/branch.c +++ b/builtin/branch.c @@ -313,7 +313,6 @@ static char *resolve_symref(const char *src, const char *prefix) struct append_ref_cb { struct ref_list *ref_list; const char **pattern; - int ret; }; static int match_patterns(const char **pattern, const char *refname) @@ -370,10 +369,8 @@ static int append_ref(const char *refname, const struct object_id *oid, int flag commit = NULL; if (ref_list-verbose || ref_list-with_commit || merge_filter != NO_FILTER) { commit = lookup_commit_reference_gently(oid-hash, 1); - if (!commit) { - cb-ret = error(_(branch '%s' does not point at a commit), refname); + if (!commit) return 0; - } /* Filter with with_commit if specified */ if (!is_descendant_of(commit, ref_list-with_commit)) @@ -609,7 +606,7 @@ static int calc_maxwidth(struct ref_list *refs, int remote_bonus) return max; } -static int print_ref_list(int kinds, int detached, int verbose, int abbrev, struct commit_list *with_commit, const char **pattern) +static void print_ref_list(int kinds, int detached, int verbose, int abbrev, struct commit_list *with_commit, const char **pattern) { int i, index; struct append_ref_cb cb; @@ -634,7 +631,6 @@ static int print_ref_list(int kinds, int detached, int verbose, int abbrev, stru init_revisions(ref_list.revs, NULL); cb.ref_list = ref_list; cb.pattern = pattern; - cb.ret = 0; for_each_rawref(append_ref, cb); if (detached) head_ref(append_ref, cb); @@ -689,11 +685,6 @@ static int print_ref_list(int kinds, int detached, int verbose, int abbrev, stru abbrev, detached, remote_prefix); free_ref_list(ref_list); - - if (cb.ret) - error(_(some refs could not be read)); - - return cb.ret; } static void rename_branch(const char *oldname, const char *newname, int force) @@ -909,14 +900,13 @@ int cmd_branch(int argc, const char **argv, const char *prefix) die(_(branch name required)); return delete_branches(argc, argv, delete 1, kinds, quiet); } else if (list) { - int ret; if (kinds REF_LOCAL_BRANCH) kinds |= REF_DETACHED_HEAD; - ret = print_ref_list(kinds, detached, verbose, abbrev, + print_ref_list(kinds, detached, verbose, abbrev, with_commit, argv); print_columns(output, colopts, NULL); string_list_clear(output, 0); - return ret; + return 0; } else if (edit_description) { const char *branch_name; -- 2.4.6 -- 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
[RFC/PATCH 01/11] ref-filter: add %(objectname:size=X) option
From: Karthik Nayak karthik@gmail.com Add support for %(objectname:size=X) where X is a number. This will print the first X characters of an objectname. The minimum value for X is 5. Hence any value lesser than 5 will default to 5 characters. 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 | 9 + 1 file changed, 9 insertions(+) diff --git a/ref-filter.c b/ref-filter.c index 0a34924..4c5e3f8 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -196,6 +196,8 @@ static void *get_obj(const unsigned char *sha1, struct object **obj, unsigned lo static int grab_objectname(const char *name, const unsigned char *sha1, struct atom_value *v) { + const char *p; + if (!strcmp(name, objectname)) { char *s = xmalloc(41); strcpy(s, sha1_to_hex(sha1)); @@ -206,6 +208,13 @@ static int grab_objectname(const char *name, const unsigned char *sha1, v-s = xstrdup(find_unique_abbrev(sha1, DEFAULT_ABBREV)); return 1; } + if (skip_prefix(name, objectname:size=, p)) { + unsigned int size = atoi(p); + if (size 5) + size = 5; + v-s = xstrdup(find_unique_abbrev(sha1, size)); + return 1; + } return 0; } -- 2.4.6 -- 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: [RFC/PATCH 11/11] branch: add '--points-at' option
On Tue, Jul 28, 2015 at 12:11 AM, Karthik Nayak karthik@gmail.com wrote: Add the '--points-at' option provided by 'ref-filter'. The option lets the user to list only branches which points at the given object. 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-branch.txt | 6 +- builtin/branch.c | 7 ++- t/t3203-branch-output.sh | 9 + 3 files changed, 20 insertions(+), 2 deletions(-) diff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt index 897cd81..efa23a5 100644 --- a/Documentation/git-branch.txt +++ b/Documentation/git-branch.txt @@ -11,7 +11,8 @@ SYNOPSIS 'git branch' [--color[=when] | --no-color] [-r | -a] [--list] [-v [--abbrev=length | --no-abbrev]] [--column[=options] | --no-column] - [(--merged | --no-merged | --contains) [commit]] [--sort=key] [pattern...] + [(--merged | --no-merged | --contains) [commit]] [--sort=key] + [--points-at object] [pattern...] 'git branch' [--set-upstream | --track | --no-track] [-l] [-f] branchname [start-point] 'git branch' (--set-upstream-to=upstream | -u upstream) [branchname] 'git branch' --unset-upstream [branchname] @@ -237,6 +238,9 @@ start-point is either a local or remote-tracking branch. for-each-ref`. Sort order defaults to sorting based on branch type. +--points-at object:: + Only list tags of the given object. + s/tags/branches/ ?? Since this is for the branch version, I think this is just a copy-paste oversight. Examples diff --git a/builtin/branch.c b/builtin/branch.c index 75d8bfd..d25f43b 100644 --- a/builtin/branch.c +++ b/builtin/branch.c @@ -26,6 +26,7 @@ static const char * const builtin_branch_usage[] = { N_(git branch [options] [-l] [-f] branch-name [start-point]), N_(git branch [options] [-r] (-d | -D) branch-name...), N_(git branch [options] (-m | -M) [old-branch] new-branch), + N_(git branch [options] [-r | -a] [--points-at]), NULL }; @@ -647,6 +648,10 @@ int cmd_branch(int argc, const char **argv, const char *prefix) OPT_COLUMN(0, column, colopts, N_(list branches in columns)), OPT_CALLBACK(0 , sort, sorting_tail, N_(key), N_(field name to sort on), parse_opt_ref_sorting), + { + OPTION_CALLBACK, 0, points-at, filter.points_at, N_(object), + N_(print only tags of the object), 0, parse_opt_object_name + }, Same as above. s/tags/branches/ OPT_END(), }; @@ -675,7 +680,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix) if (!delete !rename !edit_description !new_upstream !unset_upstream argc == 0) list = 1; - if (filter.with_commit || filter.merge != REF_FILTER_MERGED_NONE) + if (filter.with_commit || filter.merge != REF_FILTER_MERGED_NONE || filter.points_at.nr) list = 1; if (!!delete + !!rename + !!new_upstream + diff --git a/t/t3203-branch-output.sh b/t/t3203-branch-output.sh index 38c68bd..1deb7cb 100755 --- a/t/t3203-branch-output.sh +++ b/t/t3203-branch-output.sh @@ -154,4 +154,13 @@ EOF test_i18ncmp expect actual ' +test_expect_success 'git branch --points-at option' ' + cat expect EOF + master + branch-one +EOF + git branch --points-at=branch-one actual + test_cmp expect actual +' + test_done -- 2.4.6 -- 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 -- 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: builtin/tag.c issue with sort option
On Tue, Jul 28, 2015 at 3:27 PM, Jacob Keller jacob.kel...@gmail.com wrote: When passing -n on the command line, if you have configured sort manually, you get an error as it thinks you passed --sort and -n. It should automatically disable tag_sort if it wasn't passed from the command line, as you probably know what you are doing when passing -n I'm attempting to work up a patch for this fix, but I am having a little bit of trouble and want to make sure it's right to fix this behavior. Regards, Jake Actually, the sort and -n imcompatability is going away with the unification of tag using the ref-filter API, so I think this is actually not worth it to bother trying to fix, since it will be removed soon anyways. Regards, Jake -- 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 2/6] notes: replace pseudorefs with real refs
Johan Herland jo...@herland.net writes: In fact, you can easily do a notes merge in a _bare_ repo... You keep repeating that but I do not think it is relevant at all. If you have a bare repository, you either (1) do not have any worktree associated with it; or (2) have worktrees associated with it elsewhere, but its parent directory is *not* the root of any worktree. If (1), what are found outside GIT_COMMON_DIR (e.g. HEAD) is found inside GIT_DIR, so if we leave NOTES_MERGE_REF and friends outside GIT_COMMON_DIR and create the NOTES_MERGE_WORKTREE in GIT_DIR, that would work just as fine as it currently does. If (2), what are found outside GIT_COMMON_DIR (e.g. HEAD) is still found inside GIT_DIR (that is now per worktree, but for your bare repository, that is the repository directory itself). Again, if we leave NOTES_MERGE_REF and friends outside GIT_COMMON_DIR and create the NOTES_MERGE_WORKTREE in GIT_DIR, that would work just as fine as it currently does. And as long as NOTES_MERGE_REF is made per $GIT_DIR (per worktree is the phrase I am refraining deliberately here, because all the worktrees have their own private area, and in addition, your bare repository has one, too) that is protected against multiple checkout, all worktrees and your bare repository can perform independent notes-merges. Perhaps you meant by per repo to mean per $GIT_DIR in this message, and if that is the case, then I think we are in agreement. -- 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 2/6] notes: replace pseudorefs with real refs
Michael Haggerty mhag...@alum.mit.edu writes: It sounds like what a notes merge really wants is a new linked worktree that has branch refs/notes/foo checked out: * This would allow multiple notes merges to take place at the same time provided they target different merge references. * This would prevent multiple notes merges to the same notes reference at the same time by the same mechanism that prevents the same branch from being checked out in two linked worktrees at the same time. It's just a thought; I have no idea whether it is practical... That was certainly one of the possibilities that crossed my mind. In any case, the primary thing I am interested in at this point is to unblock David's prepare things so that we can put primary refs in a different ref backends more easily topic, and I've already made my point a few messages ago upstream: I think it is OK for us to admit that the notes subsystem is not quite ready to work well with multiple working tree world yet [*1*], and move this series forward without worrying about them. So doing the absolute minimum, leaving the now what can we do to improve notes-merge process? outside the scope of the topic. Improving the notes-merge process is also an interesting topic, but it is clear that people started thinking about it today ;-), so it can wait without blocking David's work. The refs/notes/* hierarchy will be handled exactly the same way as regular branches in the pluggable ref-backends, and how the ephemeral REF_NOTES_REF and friends are represented (some are refs, some may be pseudo-refs, some may be just a filesystem entity) would need to be revamped if we really do the improving notes-merge thing anyway, so David's topic shouldn't be blocked by the possible future tweak of the current design that hasn't happened yet. Instead, improving notes-merge can be done on top, potentially undoing and redoing Davi'd topic. -- 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 2/6] notes: replace pseudorefs with real refs
On Wed, Jul 29, 2015 at 4:17 AM, Junio C Hamano gits...@pobox.com wrote: Perhaps you meant by per repo to mean per $GIT_DIR in this message, and if that is the case, then I think we are in agreement. No, by per repo I mean per $GIT_COMMON_DIR (I haven't followed the linked worktree effort, so this language is new to me. My apologies). ...Johan -- Johan Herland, jo...@herland.net www.herland.net -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] notes: handle multiple worktrees
On Wed, Jul 29, 2015 at 12:12 AM, Junio C Hamano gits...@pobox.com wrote: David Turner dtur...@twopensource.com writes: Prevent merges to the same notes branch from different worktrees. Before creating NOTES_MERGE_REF, check NOTES_MERGE_REF using the same code we use to check that two HEADs in different worktrees don't point to the same branch. Modify that code, die_if_checked_out, to take a head ref to examine; previously, it just looked at HEAD. Reported-by: Junio C Hamano gits...@pobox.com Signed-off-by: David Turner dtur...@twopensource.com --- Thanks for following through. As I didn't report anything, I do not deserve that label, but it's OK ;-) I know that it is a requirement to protect NOTES_MERGE_REF from being used by multiple places for notes merge to play well with the multi-worktree world order, but because I do not know if that is sufficient, I'm asking a few people for further review. As just stated in a related thread, I don't think it makes sense to have NOTES_MERGE_REF per worktree, as the notes merge is always completely unrelated to the current worktree (or the current branch for that matter). AFAICS this patch is all about handling per-worktree NOTES_MERGE_REFs, and as such I'm NAK on this patch. Instead, there should only be one NOTES_MERGE_REF per _repo_, and although we might want to expand to allow multiple concurrent notes merges in the future, that is still a per-repo, and not a per-worktree thing, hence completely unrelated to David's current effort. ...Johan -- 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 2/6] notes: replace pseudorefs with real refs
Johan Herland jo...@herland.net writes: Johan Herland jo...@herland.net writes: However, in any case, notes merges are always per _repo_ and never per _worktree_, so this is all unrelated to the current patch/discussion AFAICS. Thanks for chiming in, but I actually think you are confused. git merge is always per _repo_ in the sense that the result of a merge of a topic to the 'master' is recorded in the 'master' which is per-repo. In the multi-worktree world order, that does not change. What changes is that you could have different worktrees that check out different branches. Worktree A may have 'master' checked out and do the merge there to update the tip of 'master'. But while worktree A is doing that, worktree B may have 'next' checked out and do an unrelated merge there. Once worktree A leaves 'master' by checking out another branch, worktree B is free to check out 'master' and do further merges there. Merging into 'master' is per _repo_, but the act of merging is per worktree. I think merges of refs/notes/commits and refs/notes/someotherthing works exactly the same way. In worktree A, you may decide to merge a notes tree refs/notes/commits with somebody else's. It may conflict and you may need to lock refs/notes/commits from being touched by other worktrees while resolving that, but that does not mean other worktrees cannot do a merge of refs/notes/someotherthing at all. The temporary area you use for merging notes, i.e. the working tree as far as notes merge is concerned, is private to worktree A and does not need to be seen by other worktrees. So while you are working on merging and resolving, that intermediate state is *NOT* per _repo_ at all. It is at most per worktree (Yes you could extend and have one notes_merge_ref per each refs/notes/* ref to make it even finer grained to allow more than one notes merge going on inside a single worktree, but I do not think it is worth it). -- 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 branch command is incompatible with bash
On Tue, Jul 28, 2015 at 08:23:40AM -0700, Junio C Hamano wrote: Johannes Sixt j...@kdbg.org writes: Are you trying to say that the result of 'rev-parse --abbrev-ref HEAD' is suboptimal and that of 'symbolic-ref --short HEAD' is OK? My Interesting was primarily about that I wasn't aware of the --abbrev-ref option. Yes, I am sure some time ago I accepted a patch to add it, but I simply do not see the point, especially because the --short option to symbolic-ref feels much more superiour. What branch am I on? is about symbolic refs, rev-parse is about revisions. Sometimes my question is what branch am I on? -- in which case symbolic-ref is adequate. Other times, my question is where am I/what did I check out? which is usually a branch, sometimes a tag, and sometimes a commit hash. We don't have a one-stop-shop command to answer that question in all cases, which is unfortunate. git status answers, but that's not plumbing. git-prompt manages to do a fairly good job, but the logic is by no means straightforward. -- Scott Schmit -- 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 2/6] notes: replace pseudorefs with real refs
On Wed, Jul 29, 2015 at 2:56 AM, Michael Haggerty mhag...@alum.mit.edu wrote: Johan Herland jo...@herland.net writes: Here is where we start to differ. I would say that starting a notes merge is completely unrelated to your worktree. Consider this: It sounds like what a notes merge really wants is a new linked worktree that has branch refs/notes/foo checked out: Yes, almost. There are some complications with the concept of checking out a notes tree: - The notes tree fanout must be flattened (so that when merging two note trees with different fanout, conflicting notes (e.g. deadbeef... and de/adbeef) are turned into a file-level conflict in the notes merge worktree (i.e. contents with conflict markers in .git/NOTES_MERGE_WORKTREE/deadbeef...). - Notes trees may be very large (e.g. one note per commit for the entire project history), so we want to avoid checking out as many notes as possible. Currently we only checkout the notes that actually do conflict, and keep the rest referenced from NOTES_MERGE_PARTIAL. * This would allow multiple notes merges to take place at the same time provided they target different merge references. * This would prevent multiple notes merges to the same notes reference at the same time by the same mechanism that prevents the same branch from being checked out in two linked worktrees at the same time. It's just a thought; I have no idea whether it is practical... I'm not sure whether it's worth trying to reuse the same linked worktree mechanism for notes trees, when (a) the concept of checking out a notes tree is so different, as explained above, and (b) currently the only use case for a notes worktree is during a notes merge. ...Johan Michael -- Michael Haggerty mhag...@alum.mit.edu -- Johan Herland, jo...@herland.net www.herland.net -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 2/6] notes: replace pseudorefs with real refs
On Tue, Jul 28, 2015 at 9:44 PM, Junio C Hamano gits...@pobox.com wrote: Junio C Hamano gits...@pobox.com writes: David Turner dtur...@twopensource.com writes: All-caps files like NOTES_MERGE_REF are pseudorefs, and thus are per-worktree. We don't want multiple notes merges happening at once (in different worktrees), so we want to make these refs true refs. ... The two rev-parse --verify looks semi-sensible [*1*]; notes_merge_partial is all lowercase and it refers to $GIT_DIR/notes_merge_partial, because they are shared across working tree. But then why are $GIT_DIR/notes_merge_worktree/* still checked with test -f? If they are not refs or ref-like things, why should they be downcased? If they are, why not rev-parse --verify? [Footnote] *1* I say semi- sensible, because it looks ugly. All ref-like things immediately below $GIT_DIR/ are all-caps by convention. Perhaps it is a better idea to move it under refs/; everything under refs/ is shared across working trees is probably a much better rule than all caps but HEAD is special. I am not sure if we don't want multiple notes merges at once is a valid cause for this inconsistency in the first place. When you try to start merging a notes tree in one place (leaving NOTES_MERGE_REF and friends in the ref storage shared across worktrees), should you be prevented from merging a different notes tree in another? Isn't the whole point of having refs/notes/ hierarchy to allow different notes to hang underneath there, and isn't NOTES_MERGE_REF symref about keeping track of which one of them is being worked on in this working tree? I believe the current M.O. for notes merge is to allow only one simultaneous notes merge per _repo_. AFAIR, the notes merge code is completely unrelated to the work tree (you could probably even do a notes merge in a _bare_ repo), so IINM, the NOTES_MERGE_* refs should be bound to the _repo_, NOT to the worktree. So if I understand the pseudoref concept correctly, that should make NOTES_MERGE_* regular refs, and not pseudorefs. We don't want multiple usual merges at once in different worktrees, either; rather, we don't want conflicting updates to the same branch, be it done by a merge or a single-parent commit, from different worktrees. And the way we protect ourselves is by forbidding two checkout of the same branch by controlling what HEAD can point at. Making NOTES_MERGE_REF shared across working trees feels like solving that no multiple checkouts problem by making HEAD shared across working trees, ensuring that only one of them can have usable HEAD. Instead, shouldn't we be solving the issue by allowing NOTES_MERGE_REF in multiple working trees point at different refs but ensuring that there is at most one working tree that updates one particular ref in refs/notes/ hierarchy? Can't we learn from (and even better, reuse) the logic that controls HEAD for this purpose? I think it is OK for us to admit that the notes subsystem is not quite ready to work well with multiple working tree world yet [*1*], and move this series forward without worrying about them. [Footnote] *1* I am not saying that the notes subsystem can forever stay to be second class citizen that does not know about multiple worktrees or pluggable ref backends. But my brief scan of builtin/notes.c seems to indicate that there are quite a lot of work to be done to prepare it for these two big changes. My gut feeling is that: - NOTES_MERGE_REF symref is like HEAD, that is per working tree but is controlled across working trees---no two working trees can checkout the same refs/notes/* tree for conflict resolution at the same time. This sentence does not make sense to me. IMHO, a notes merge has nothing to do with the worktree at all. Instead, the notes merge creates its _own_ worktree (under .git/NOTES_MERGE_WORKTREE) when needed (i.e. when there are notes conflicts to be resolved). It should be all-caps and live directly under $GIT_DIR/. It should definitely live under $GIT_DIR (and not inside each worktree's .git/). Whether or not it's all-caps is not that important to me. - NOTES_MERGE_PARTIAL is like MERGE_HEAD or the index in that it is merely a state of in-progress merge that is private to a single working tree. $GIT_DIR/NOTES_MERGE_PARTIAL is just fine, and we do not have to change it. Agreed. - NOTES_MERGE_WORKTREE is a temporary area in the filesystem that houses bunch of blob files (i.e. notes contents). It should be per-working tree but it is not even refs. $GIT_DIR/NOTES_MERGE_WORKTREE is just fine, and we do not have to change it. From the notes merge perspective, NOTES_MERGE_WORKTREE _is_ the worktree. There is no other worktree that is relevant to the notes merge code. So I'm not quite sure how to handle notes merges in the multiple worktree
Re: [PATCH v3 2/6] notes: replace pseudorefs with real refs
Johan Herland jo...@herland.net writes: Yes, almost. There are some complications with the concept of checking out a notes tree: - The notes tree fanout must be flattened (so that when merging two note trees with different fanout, conflicting notes (e.g. deadbeef... and de/adbeef) are turned into a file-level conflict in the notes merge worktree (i.e. contents with conflict markers in .git/NOTES_MERGE_WORKTREE/deadbeef...). True. I however think Michael was envisioning further into the future, where the tree-level merges would take care of that detail and populate the index to express conflicts using canonicalised paths, removing these fan-out slashes, before externalising the conflicted paths that need user's attention on the filesystem. - Notes trees may be very large (e.g. one note per commit for the entire project history), so we want to avoid checking out as many notes as possible. Currently we only checkout the notes that actually do conflict, and keep the rest referenced from NOTES_MERGE_PARTIAL. This is a valid point, but nobody forces us to do a full checkout to perform a merge. From day one, our merge machinery is prepared to merge in an empty working tree, only checking out paths that need to be modified by the merge. * This would allow multiple notes merges to take place at the same time provided they target different merge references. * This would prevent multiple notes merges to the same notes reference at the same time by the same mechanism that prevents the same branch from being checked out in two linked worktrees at the same time. It's just a thought; I have no idea whether it is practical... I'm not sure whether it's worth trying to reuse the same linked worktree mechanism for notes trees, when (a) the concept of checking out a notes tree is so different, as explained above, and (b) currently the only use case for a notes worktree is during a notes merge. I have a very similar feeling except that I'd replace linked worktree mechanism with checkout mechanism. If we were to introduce such a new checkout mechanism that flattens a fan-out paths, with sparse checkout-like behaviour, the current checkout mechanism would not be reused, but the resulting system would benefit from linked worktree mechanism just as much as the normal multiple worktree layout benefits from it. You'd want to make sure only one such worktree has the checkout of one refs/notes/* ref, everything in refs/* is shared across the repository and its worktrees, and certain things like MERGE_HEAD and the index inside $GIT_DIR/ are not shared, which are what the linked worktree mechanism gives us. -- 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
builtin/tag.c issue with sort option
When passing -n on the command line, if you have configured sort manually, you get an error as it thinks you passed --sort and -n. It should automatically disable tag_sort if it wasn't passed from the command line, as you probably know what you are doing when passing -n I'm attempting to work up a patch for this fix, but I am having a little bit of trouble and want to make sure it's right to fix this behavior. Regards, Jake -- 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 2/6] notes: replace pseudorefs with real refs
On Wed, Jul 29, 2015 at 12:52 AM, Junio C Hamano gits...@pobox.com wrote: Johan Herland jo...@herland.net writes: However, in any case, notes merges are always per _repo_ and never per _worktree_, so this is all unrelated to the current patch/discussion AFAICS. Thanks for chiming in, but I actually think you are confused. git merge is always per _repo_ in the sense that the result of a merge of a topic to the 'master' is recorded in the 'master' which is per-repo. In the multi-worktree world order, that does not change. What changes is that you could have different worktrees that check out different branches. Worktree A may have 'master' checked out and do the merge there to update the tip of 'master'. But while worktree A is doing that, worktree B may have 'next' checked out and do an unrelated merge there. Once worktree A leaves 'master' by checking out another branch, worktree B is free to check out 'master' and do further merges there. Merging into 'master' is per _repo_, but the act of merging is per worktree. Agreed thus far. I think merges of refs/notes/commits and refs/notes/someotherthing works exactly the same way. In worktree A, you may decide to merge a notes tree refs/notes/commits with somebody else's. Here is where we start to differ. I would say that starting a notes merge is completely unrelated to your worktree. Consider this: When you start a regular (non-notes) merge in worktree A, that merge will occupy worktree A for the purpose of completing the merge, e.g. conflicting files will be checked out inside worktree A, and it is obvious that you cannot do other/unrelated things in worktree A until you have resolved the conflicts and completed the merge. As such, a regular merge is inextricably bound to a specific worktree. This is not the case for notes merges. If I start a notes merge from worktree A, there is no occupation of that worktree. Before the notes merge is resolved, I can do whatever I want in worktree A (including checking out a different branch, performing a rebase, whatever...). Instead, the notes merge creates its own worktree (that is occupied until the notes merge is completed), which is completely unrelated to worktree A. It may conflict and you may need to lock refs/notes/commits from being touched by other worktrees while resolving that, but that does not mean other worktrees cannot do a merge of refs/notes/someotherthing at all. In principle, I agree that an ongoing notes-merge into refs/notes/someotherthing should be able to coexist with an ongoing notes-merge into refs/notes/commits. However, it does not make sense to bind those notes-merges to a specific worktree. Let's say I have two worktrees, A and B, and from worktree A, I have started a notes-merge into refs/notes/commits. Now: - From worktree B I should NOT be able to start another notes-merge into refs/notes/commits. - From worktree B I SHOULD be able to start another notes-merge into refs/notes/someotherthing But this doesn't really have anything to do with worktree B. I can just as easily say: - From worktree A I should NOT be able to start another notes-merge into refs/notes/commits. - From worktree A I SHOULD be able to start another notes-merge into refs/notes/someotherthing My conclusion is therefore that binding a notes merge to a specific worktree does not make any sense. Preventing simultaneous notes merges into the same notes ref is something that must be solved per _repo_ (and not per worktree), and since the worktree plays no part in the resolution/completion of the notes merge, it makes more sense to place all the notes-merge-related refs/files inside the _repo_, and not inside the worktree. Now, we do not yet support simultaneous notes merges at all, but my follow-on point is that the addition of such support is wholly independent of the multi-worktree support. For now, it would make more sense to only allow a single notes-merge across all worktrees. I.e. when starting a notes-merge from ANY worktree, it should simply fail if there is an existing unresolved notes merge (no matter which worktree started that unresolved notes merge). The temporary area you use for merging notes, i.e. the working tree as far as notes merge is concerned, is private to worktree A and does not need to be seen by other worktrees. Disagree. The private area used to resolve notes merges should be per _repo_, not per worktree. That is IMHO the only sane way to (in the future) prevent simultaneous notes merges going into the same notes ref. So while you are working on merging and resolving, that intermediate state is *NOT* per _repo_ at all. It is at most per worktree (Yes you could extend and have one notes_merge_ref per each refs/notes/* ref to make it even finer grained to allow more than one notes merge going on inside a single worktree, but I do not think it is worth it). As stated above, my position is that while you are resolving a notes merge, the
Re: [PATCH v3 2/6] notes: replace pseudorefs with real refs
Johan Herland jo...@herland.net writes: Here is where we start to differ. I would say that starting a notes merge is completely unrelated to your worktree. Consider this: ... This is not the case for notes merges. If I start a notes merge from worktree A, there is no occupation of that worktree. Before the notes merge is resolved, I can do whatever I want in worktree A (including checking out a different branch, performing a rebase, whatever...). Instead, the notes merge creates its own worktree (that is occupied until the notes merge is completed), which is completely unrelated to worktree A. That does not mean the notes merge that you started when you were sitting in worktree A has to be shared with worktree B, by say doing vi .git/NOTES_MERGE_WORKTREE/$commit git notes merge --commit. Also the above does not explain why it is sensible for you to forbid worktree B from doing an unrelated notes merge of a different ref under refs/notes/* while your worktree A is doing a notes merge. In principle, I agree that an ongoing notes-merge into refs/notes/someotherthing should be able to coexist with an ongoing notes-merge into refs/notes/commits. However, it does not make sense to bind those notes-merges to a specific worktree. The thing is, the choice is between per worktree or per repository. Taking the latter would mean you can only be doing one notes merge at a time, even though you prepared multiple worktrees so that you can work on different things at a time. It is true that there is nothing inherent that ties a note merge to a worktree (a worktree is tied to a branch that is checed out, and there is no tie between a branch and a notes tree), but not inherantly tied to does not automatically mean has to be one per repository. You'd ideally want to allow N workspaces for N refs/notes/* refs. But people work in worktrees, and that is their unit of working space. From that point of view, unless you are proposing a completely different design where the primary worktree can be used only for manipulating notes (hence, you can have worktrees for resolving refs/notes/A and refs/notes/B, in addition to the other worktrees you use to advance branches), treating NOTES_MERGE_REF as a per-worktree entity just like HEAD and the index is, would be the most sensible comporise. Let's say I have two worktrees, A and B, and from worktree A, I have started a notes-merge into refs/notes/commits. Now: - From worktree B I should NOT be able to start another notes-merge into refs/notes/commits. Correct. - From worktree B I SHOULD be able to start another notes-merge into refs/notes/someotherthing Correct. But this doesn't really have anything to do with worktree B. Correct. There is nothing inherent that ties someotherthing to B and commits to A. The only reason why I think per worktree is vastly superiour than only one per repository is because the end user did set up two worktrees so that he can do two things at once independently. I can just as easily say: - From worktree A I should NOT be able to start another notes-merge into refs/notes/commits. Correct. - From worktree A I SHOULD be able to start another notes-merge into refs/notes/someotherthing Irrelevant and moving the goalpost. We are not talking about extending capability of the system that does not use multi-worktree. We do not do two regular merges in a single worktree at a time, and I do not think it is sensible to claim I SHOULD be able to. Now, we do not yet support simultaneous notes merges at all, but my follow-on point is that the addition of such support is wholly independent of the multi-worktree support. Now we are getting somewhere. So is there more that is needed than separating NOTES_MERGE_REF per worktree to make this work (remember, multiple notes-merge in a single worktree is a non-goal, just like multiple merge in a single worktree is not supported today and will not be)? Is there some other state that is not captured by NOTES_MERGE_REF and friends that you would end up recording a wrong merge result, if two worktrees that have NOTES_MERGE_REF pointing at a different ref in refs/notes/* try to do the notes-merge at the same time? For now, it would make more sense to only allow a single notes-merge across all worktrees. I.e. when starting a notes-merge from ANY worktree, it should simply fail if there is an existing unresolved notes merge (no matter which worktree started that unresolved notes merge). I do not understand why we need to have such a restriction. It is like saying you may have two worktrees but you cannot merge into master in worktree A if you are doing a merge into next in worktree B. I'd need a clear answer to the question in the previous paragraph to say something like ah, ok, that limitation (either imposed by the current code design, or perhaps reliance of the filesystem, or whatever) I didn't think about, and now what you say makes sense to me. Disagree. The
Re: [RFC/PATCH 07/11] branch: move 'current' check down to the presentation layer
On Tue, Jul 28, 2015 at 1:12 PM, Karthik Nayak karthik@gmail.com wrote: On Tue, Jul 28, 2015 at 6:39 PM, Matthieu Moy matthieu@grenoble-inp.fr wrote: Karthik Nayak karthik@gmail.com writes: We check if given ref is the current branch in print_ref_list(). Move this check to print_ref_item() where it is checked right before printing. This means that the '*' and the different color are coded in C, hence it's not possible to mimick this using git for-each-ref --format I do not consider this as blocking, but I think the ultimate goal should be to allow this, so that all the goodies of git branch can be made available to other ref-listing commands. Not sure what you mean here. He means to make sure that any feature that was in tag, branch, for-each-ref, etc should be available as part of ref-filter and in all of them Maybe he misunderstood code or maybe you misunderstood the comment here? Regards, Jake -- 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 2/6] notes: replace pseudorefs with real refs
On Tue, Jul 28, 2015 at 5:56 PM, Michael Haggerty mhag...@alum.mit.edu wrote: Johan Herland jo...@herland.net writes: Here is where we start to differ. I would say that starting a notes merge is completely unrelated to your worktree. Consider this: It sounds like what a notes merge really wants is a new linked worktree that has branch refs/notes/foo checked out: * This would allow multiple notes merges to take place at the same time provided they target different merge references. * This would prevent multiple notes merges to the same notes reference at the same time by the same mechanism that prevents the same branch from being checked out in two linked worktrees at the same time. It's just a thought; I have no idea whether it is practical... Michael That seems far more flexible and robust to me. Regards, Jake -- 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 2/6] notes: replace pseudorefs with real refs
On Wed, Jul 29, 2015 at 2:33 AM, Junio C Hamano gits...@pobox.com wrote: Johan Herland jo...@herland.net writes: Here is where we start to differ. I would say that starting a notes merge is completely unrelated to your worktree. Consider this: ... This is not the case for notes merges. If I start a notes merge from worktree A, there is no occupation of that worktree. Before the notes merge is resolved, I can do whatever I want in worktree A (including checking out a different branch, performing a rebase, whatever...). Instead, the notes merge creates its own worktree (that is occupied until the notes merge is completed), which is completely unrelated to worktree A. That does not mean the notes merge that you started when you were sitting in worktree A has to be shared with worktree B, by say doing vi .git/NOTES_MERGE_WORKTREE/$commit git notes merge --commit. Also the above does not explain why it is sensible for you to forbid worktree B from doing an unrelated notes merge of a different ref under refs/notes/* while your worktree A is doing a notes merge. I do not argue that it is sensible to forbid concurrent unrelated notes merges. I argue that using linked worktrees is a poor solution for concurrent unrelated notes merges. A better solution does not concern itself with worktrees at all, and does not need to add nonsensical conditions like: die_if_checked_out(NOTES_MERGE_REF, default_notes_ref()); A better solution does not need to add complexity to the branch or linked worktree code to deal with notes merges. Instead, it simply organizes the notes merge worktrees in such a manner that the correct semantics naturally emerge. Again, let's compare the two approaches (against the current situation): Current situation: - A single $GIT_COMMON_DIR/NOTES_MERGE_* - Concurrent (unrelated or not) notes merges are simply not supported Proposal A (please correct me where I have misunderstood what's proposed): - Each worktree has its own $GIT_DIR/NOTES_MERGE_* - Concurrent unrelated notes merges are supported, provided that you create an additional linked worktree to host each concurrent notes merge. - Logic must be added to ensure unrelated-ness, i.e. make sure that the NOTES_MERGE_REF in worktree X is different from all other worktrees' NOTES_MERGE_REF. Proposal B: - The repo has a $GIT_COMMON_DIR/notes-merge/$ref/* hierarchy for organizing concurrent notes merges - Concurrent unrelated notes merges are supported, independently of whether you have zero, one, or several worktrees. - The notes merge code must be adjusted to work with the above hierarchy, and must naturally fail if the user attempts to start a new notes merge that would clobber an in-progress notes merge (only notes merges to the same notes ref will clobber). I obviously feel proposal B is superior to A, so I wonder what I have missed about A that makes it preferable. In principle, I agree that an ongoing notes-merge into refs/notes/someotherthing should be able to coexist with an ongoing notes-merge into refs/notes/commits. However, it does not make sense to bind those notes-merges to a specific worktree. The thing is, the choice is between per worktree or per repository. Taking the latter would mean you can only be doing one notes merge at a time, even though you prepared multiple worktrees so that you can work on different things at a time. It is true that there is nothing inherent that ties a note merge to a worktree (a worktree is tied to a branch that is checed out, and there is no tie between a branch and a notes tree), but not inherantly tied to does not automatically mean has to be one per repository. You'd ideally want to allow N workspaces for N refs/notes/* refs. But people work in worktrees, and that is their unit of working space. From that point of view, unless you are proposing a completely different design where the primary worktree can be used only for manipulating notes (hence, you can have worktrees for resolving refs/notes/A and refs/notes/B, in addition to the other worktrees you use to advance branches), treating NOTES_MERGE_REF as a per-worktree entity just like HEAD and the index is, would be the most sensible comporise. I believe it is a bad compromise. It complicates the code, and it provides a concurrent notes merges that is unnecessarily tied to (and dependent on) worktrees. For example, if I wanted to perform N concurrent notes merges in a project that happens to have a huge worktree, I would now have to create N copies of the huge worktree... ...Johan -- Johan Herland, jo...@herland.net www.herland.net -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: default configuration files on cygwin
On Tue, Jul 28, 2015 at 7:10 PM, Filippo Gatti filippo.ga...@centralesupelec.fr wrote: Hi, I'm currently running git on a cygwin platform. I would like to know how i can set up a sort of configuration file to launch automatically the ssh-agent and get connected to github (for istance) directly. I'm not a regular cygwin user so I can't give you a direct answer, but perhaps you need to ask the right question (or at least explain your use-case). The point with git (or any other DVCS) is that it does not need to connect to anything until you want to publish your changes (i.e. git push) or incorporate changes someone else has published (i.e. git pull). These are the (main) cases where git will actually connect to a remote the rest of the time everything is happening locally on your local copy of the repository. -- 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: [RFC/PATCH 03/11] ref-filter: add option to filter only branches
Karthik Nayak karthik@gmail.com writes: +static int filter_branch_kind(struct ref_filter *filter, const char *refname) +{ + int kind, i; + + static struct { + int kind; + const char *prefix; + } ref_kind[] = { + { REF_LOCAL_BRANCH, refs/heads/ }, + { REF_REMOTE_BRANCH, refs/remotes/ }, + }; Nit: I would swap the order of fields, to make it a bit clearer that this is a kind of dictionary key - value (I think it's more common to write it in this order than value - key). -- 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: [RFC/PATCH 09/11] branch.c: use 'ref-filter' data structures
Christian Couder christian.cou...@gmail.com writes: On Tue, Jul 28, 2015 at 8:56 AM, Karthik Nayak karthik@gmail.com wrote: +static void ref_array_append(struct ref_array *array, const char *refname) +{ + size_t len = strlen(refname); + struct ref_array_item *ref = xcalloc(1, sizeof(struct ref_array_item) + len + 1); + memcpy(ref-refname, refname, len); + ref-refname[len] = '\0'; This looks very much like new_ref_array_item, except that the later also takes an objectname parameter. I find it suspicious that you leave the objectname field uninitialized. Why is this code not calling new_ref_array_item? A detail: you could return a pointer to the newly allocated object to write item = ref_array_append(array, refname); instead of ref_array_append(array, refname); item = array-items[array-nr - 1]; + REALLOC_ARRAY(array-items, array-nr + 1); + array-items[array-nr++] = ref; +} This function belongs more to ref-filter.{c,h}... The function disapears in the next commit, but I also think that this function deserves to exist in ref-filter.{c,h} and remain after the end of the series. -- 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: [RFC/PATCH] Port branch.c to use ref-filter APIs
Karthik Nayak karthik@gmail.com writes: This version also doesn't use the printing options provided by branch.c. Do you mean provided by ref-filter.{c,h}? I wanted to discuss how exactly to go about that, because in branch.c, we might need to change the --format based on attributes such as abbrev, verbose and so on. But ref-filter expects us to verify the format before filtering. I took time to understand the problem, but here's my understanding: ref-filter expects the code to look like format = ...; verify_ref_format(format); filter_refs(...); for (...) show_ref_array_item(..., format, ...); and in the case of git branch -v, you need to align the sha1s based on the longest branch name, i.e. use %(padright:N+1) where N is the longest branch name. And you can get N only after calling filter_refs, while you need it for verify_ref_format(). Is my understanding correct? If so, what prevents swapping the order of verify_ref_format and filter_refs? I understand that verify_ref_format() builds used_atom and other data-structures, hence it has to be called before show_ref_array_item() and before sorting, but I don't think you need it before filter_refs (I may have missed something though). So, the code could look like: filter_refs(...) if (--format is given) format = the argument of --format else format = STRBUF_INIT; strbuf_add(format, %(some_directive_to_display '*' if needed)); if (verbose) strbuf_addf(format, %(padright:%d), max_width); ... verify_ref_format(format); ref_array_sort(...); for (...) show_ref_array_item(...); (BTW, a trivial helper function to display the whole ref_array could help. It would avoid having each caller write the 'for' loop) Ideally, you could also have a modifier atom %(alignleft) that would not need an argument and that would go through the ref_array to find the maxwidth, and do the alignment. That would give even more flexibility to the end users of for-each-ref --format. -- 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: [RFC/PATCH 10/11] branch.c: use 'ref-filter' APIs
Karthik Nayak karthik@gmail.com writes: @@ -458,7 +345,7 @@ static void add_verbose_info(struct strbuf *out, struct ref_array_item *item, } if (item-kind == REF_LOCAL_BRANCH) - fill_tracking_info(stat, item-refname, filter-verbose 1); + fill_tracking_info(stat, refname, filter-verbose 1); Why can't you continue using item-refname? (It's a real question) @@ -635,14 +495,21 @@ static void print_ref_list(struct ref_filter *filter) /* Print detached heads before sorting and printing the rest */ if (filter-detached) { print_ref_item(array.items[index - 1], maxwidth, filter, remote_prefix); - index -= 1; + array.nr--; } - qsort(array.items, index, sizeof(struct ref_array_item *), ref_cmp); + if (!sorting) { + def_sorting.next = NULL; + def_sorting.atom = parse_ref_filter_atom(sort_type, + sort_type + strlen(sort_type)); + sorting = def_sorting; + } + ref_array_sort(sorting, array); Does this belong to print_ref_list()? Is it not possible to extract it to get a code closer to the simple: filter_refs(...); ref_array_sort(...); print_ref_list(...); ? - for (i = 0; i index; i++) + for (i = 0; i array.nr; i++) print_ref_item(array.items[i], maxwidth, filter, remote_prefix); Now that we have show_ref_array_item, it may make sense to rename print_ref_item to something that make the difference between these functions more explicit. Well, ideally, you'd get rid of it an actually use show_ref_array_item, but if you are to keep it, maybe print_ref_item_default_branch_format (or something shorter)? --- a/ref-filter.h +++ b/ref-filter.h @@ -49,7 +49,6 @@ struct ref_sorting { struct ref_array_item { unsigned char objectname[20]; int flag, kind; - int ignore : 1; You should explain why you needed it and why you don't need it anymore (I guess, because it was used to implement --merge and you now get it from ref-filter). --- a/t/t1430-bad-ref-name.sh +++ b/t/t1430-bad-ref-name.sh @@ -38,7 +38,7 @@ test_expect_success 'fast-import: fail on invalid branch name bad[branch]name' test_must_fail git fast-import input ' -test_expect_success 'git branch shows badly named ref' ' +test_expect_failure 'git branch does not show badly named ref' ' I'm not sure what's the convention, but I think the test description should give the expected behavior even with test_expect_failure. And please help the reviewers by saying what's the status wrt this test (any plan on how to fix it?). -- 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
Fwd: Bug#793884: git: allows nonsensical command 'git checkout -b HEAD'
This seems like a good thing to fix (i.e. make sure XX is not ambiguous before creating it with git checkout -b XX) -- Forwarded message -- From: Andreas Beckmann a...@debian.org Date: Tue, Jul 28, 2015 at 9:18 PM Subject: Bug#793884: git: allows nonsensical command 'git checkout -b HEAD' To: Debian Bug Tracking System sub...@bugs.debian.org Package: git Version: 1:2.1.4-2.1 Severity: normal Tags: upstream $ git branch HEAD fatal: it does not make sense to create 'HEAD' manually # OK, special casing prevents this $ git checkout -b HEAD Switched to a new branch 'HEAD' # but not this :-P $ git checkout master Switched to branch 'master' $ git checkout HEAD warning: refname 'HEAD' is ambiguous. Switched to branch 'HEAD' # oops ;-) $ git checkout master Switched to branch 'master' $ git branch -d HEAD Deleted branch HEAD (was 6e54945). # OK, we can easily cleanup this mess again The same works in 1:2.4.6-1 in sid. If there is some special casing for HEAD in git branch, the same should probably be done for git checkout -b, too. Andreas -- Duy -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC/PATCH 10/11] branch.c: use 'ref-filter' APIs
Make 'branch.c' use 'ref-filter' APIs for iterating through refs sorting. This removes most of the code used in 'branch.c' replacing it with calls to the 'ref-filter' library. Make 'tag.c' use the 'filter_refs()' function provided by 'ref-filter' to filter out tags based on the options set. We provide a sorting option provided for 'branch.c' by using the sorting options provided by 'ref-filter'. Modify documentation and add 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-branch.txt | 9 +- builtin/branch.c | 200 --- ref-filter.c | 8 +- ref-filter.h | 1 - t/t1430-bad-ref-name.sh | 2 +- t/t3203-branch-output.sh | 11 +++ 6 files changed, 60 insertions(+), 171 deletions(-) diff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt index a67138a..897cd81 100644 --- a/Documentation/git-branch.txt +++ b/Documentation/git-branch.txt @@ -11,7 +11,7 @@ SYNOPSIS 'git branch' [--color[=when] | --no-color] [-r | -a] [--list] [-v [--abbrev=length | --no-abbrev]] [--column[=options] | --no-column] - [(--merged | --no-merged | --contains) [commit]] [pattern...] + [(--merged | --no-merged | --contains) [commit]] [--sort=key] [pattern...] 'git branch' [--set-upstream | --track | --no-track] [-l] [-f] branchname [start-point] 'git branch' (--set-upstream-to=upstream | -u upstream) [branchname] 'git branch' --unset-upstream [branchname] @@ -229,6 +229,13 @@ start-point is either a local or remote-tracking branch. The new name for an existing branch. The same restrictions as for branchname apply. +--sort=key:: + Sort based on the key given. Prefix `-` to sort in descending + order of the value. You may use the --sort=key option + multiple times, in which case the last key becomes the primary + key. The keys supported are the same as those in `git + for-each-ref`. Sort order defaults to sorting based on branch + type. Examples diff --git a/builtin/branch.c b/builtin/branch.c index df74527..75d8bfd 100644 --- a/builtin/branch.c +++ b/builtin/branch.c @@ -270,119 +270,6 @@ static int delete_branches(int argc, const char **argv, int force, int kinds, return(ret); } -static char *resolve_symref(const char *src, const char *prefix) -{ - unsigned char sha1[20]; - int flag; - const char *dst; - - dst = resolve_ref_unsafe(src, 0, sha1, flag); - if (!(dst (flag REF_ISSYMREF))) - return NULL; - if (prefix) - skip_prefix(dst, prefix, dst); - return xstrdup(dst); -} - -static int match_patterns(const char **pattern, const char *refname) -{ - if (!*pattern) - return 1; /* no pattern always matches */ - while (*pattern) { - if (!wildmatch(*pattern, refname, 0, NULL)) - return 1; - pattern++; - } - return 0; -} - -static void ref_array_append(struct ref_array *array, const char *refname) -{ - size_t len = strlen(refname); - struct ref_array_item *ref = xcalloc(1, sizeof(struct ref_array_item) + len + 1); - memcpy(ref-refname, refname, len); - ref-refname[len] = '\0'; - REALLOC_ARRAY(array-items, array-nr + 1); - array-items[array-nr++] = ref; -} - -static int append_ref(const char *refname, const struct object_id *oid, int flags, void *cb_data) -{ - struct ref_filter_cbdata *cb = (struct ref_filter_cbdata *)(cb_data); - struct ref_filter *filter = cb-filter; - struct ref_array *array = cb-array; - struct ref_array_item *item; - struct commit *commit; - int kind, i; - const char *prefix, *orig_refname = refname; - - static struct { - int kind; - const char *prefix; - } ref_kind[] = { - { REF_LOCAL_BRANCH, refs/heads/ }, - { REF_REMOTE_BRANCH, refs/remotes/ }, - }; - - /* Detect kind */ - for (i = 0; i ARRAY_SIZE(ref_kind); i++) { - prefix = ref_kind[i].prefix; - if (skip_prefix(refname, prefix, refname)) { - kind = ref_kind[i].kind; - break; - } - } - if (ARRAY_SIZE(ref_kind) = i) { - if (!strcmp(refname, HEAD)) - kind = REF_DETACHED_HEAD; - else - return 0; - } - - /* Don't add types the caller doesn't want */ - if ((kind filter-branch_kind) == 0) - return 0; - - if (!match_patterns(filter-name_patterns, refname)) - return 0; - - commit = NULL; - if (filter-verbose || filter-with_commit ||
[RFC/PATCH 11/11] branch: add '--points-at' option
Add the '--points-at' option provided by 'ref-filter'. The option lets the user to list only branches which points at the given object. 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-branch.txt | 6 +- builtin/branch.c | 7 ++- t/t3203-branch-output.sh | 9 + 3 files changed, 20 insertions(+), 2 deletions(-) diff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt index 897cd81..efa23a5 100644 --- a/Documentation/git-branch.txt +++ b/Documentation/git-branch.txt @@ -11,7 +11,8 @@ SYNOPSIS 'git branch' [--color[=when] | --no-color] [-r | -a] [--list] [-v [--abbrev=length | --no-abbrev]] [--column[=options] | --no-column] - [(--merged | --no-merged | --contains) [commit]] [--sort=key] [pattern...] + [(--merged | --no-merged | --contains) [commit]] [--sort=key] + [--points-at object] [pattern...] 'git branch' [--set-upstream | --track | --no-track] [-l] [-f] branchname [start-point] 'git branch' (--set-upstream-to=upstream | -u upstream) [branchname] 'git branch' --unset-upstream [branchname] @@ -237,6 +238,9 @@ start-point is either a local or remote-tracking branch. for-each-ref`. Sort order defaults to sorting based on branch type. +--points-at object:: + Only list tags of the given object. + Examples diff --git a/builtin/branch.c b/builtin/branch.c index 75d8bfd..d25f43b 100644 --- a/builtin/branch.c +++ b/builtin/branch.c @@ -26,6 +26,7 @@ static const char * const builtin_branch_usage[] = { N_(git branch [options] [-l] [-f] branch-name [start-point]), N_(git branch [options] [-r] (-d | -D) branch-name...), N_(git branch [options] (-m | -M) [old-branch] new-branch), + N_(git branch [options] [-r | -a] [--points-at]), NULL }; @@ -647,6 +648,10 @@ int cmd_branch(int argc, const char **argv, const char *prefix) OPT_COLUMN(0, column, colopts, N_(list branches in columns)), OPT_CALLBACK(0 , sort, sorting_tail, N_(key), N_(field name to sort on), parse_opt_ref_sorting), + { + OPTION_CALLBACK, 0, points-at, filter.points_at, N_(object), + N_(print only tags of the object), 0, parse_opt_object_name + }, OPT_END(), }; @@ -675,7 +680,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix) if (!delete !rename !edit_description !new_upstream !unset_upstream argc == 0) list = 1; - if (filter.with_commit || filter.merge != REF_FILTER_MERGED_NONE) + if (filter.with_commit || filter.merge != REF_FILTER_MERGED_NONE || filter.points_at.nr) list = 1; if (!!delete + !!rename + !!new_upstream + diff --git a/t/t3203-branch-output.sh b/t/t3203-branch-output.sh index 38c68bd..1deb7cb 100755 --- a/t/t3203-branch-output.sh +++ b/t/t3203-branch-output.sh @@ -154,4 +154,13 @@ EOF test_i18ncmp expect actual ' +test_expect_success 'git branch --points-at option' ' + cat expect EOF + master + branch-one +EOF + git branch --points-at=branch-one actual + test_cmp expect actual +' + test_done -- 2.4.6 -- 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: [RFC/PATCH 02/11] ref-filter: add 'colornext' atom
On Tue, Jul 28, 2015 at 8:56 AM, Karthik Nayak karthik@gmail.com wrote: @@ -712,6 +713,15 @@ static void populate_value(struct ref_array_item *ref) v-modifier_atom = 1; v-pad_to_right = 1; continue; + } else if (starts_with(name, colornext:)) { + char color[COLOR_MAXLEN] = ; + + if (color_parse(name + 10, color) 0) + die(_(unable to parse format)); Maybe use skip_prefix(), and die() with a more helpful message. + v-s = xstrdup(color); + v-modifier_atom = 1; + v-color_next = 1; + continue; } else continue; -- 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: [RFC/PATCH 07/11] branch: move 'current' check down to the presentation layer
Karthik Nayak karthik@gmail.com writes: We check if given ref is the current branch in print_ref_list(). Move this check to print_ref_item() where it is checked right before printing. This means that the '*' and the different color are coded in C, hence it's not possible to mimick this using git for-each-ref --format I do not consider this as blocking, but I think the ultimate goal should be to allow this, so that all the goodies of git branch can be made available to other ref-listing commands. --- a/builtin/branch.c +++ b/builtin/branch.c @@ -534,9 +534,9 @@ static char *get_head_description(void) } static void print_ref_item(struct ref_item *item, int maxwidth, int verbose, -int abbrev, int current, const char *remote_prefix) +int abbrev, int detached, const char *remote_prefix) { - char c; + char c = ' '; int color; struct strbuf out = STRBUF_INIT, name = STRBUF_INIT; const char *prefix = ; @@ -547,7 +547,11 @@ static void print_ref_item(struct ref_item *item, int maxwidth, int verbose, switch (item-kind) { case REF_LOCAL_BRANCH: - color = BRANCH_COLOR_LOCAL; + if (!detached !strcmp(item-name, head)) { + color = BRANCH_COLOR_CURRENT; + c = '*'; + } else + color = BRANCH_COLOR_LOCAL; break; case REF_REMOTE_BRANCH: color = BRANCH_COLOR_REMOTE; @@ -556,18 +560,13 @@ static void print_ref_item(struct ref_item *item, int maxwidth, int verbose, case REF_DETACHED_HEAD: color = BRANCH_COLOR_CURRENT; desc = get_head_description(); + c = '*'; break; default: color = BRANCH_COLOR_PLAIN; break; } - c = ' '; - if (current) { - c = '*'; - color = BRANCH_COLOR_CURRENT; - } I actually prefered the old way: current is a Boolean that says semantically this is the current branch, and the Boolean is turned into presentation directives in a separate piece of code. I'd write char c; int current = 0; ... if (...) current = 1; ... case REF_DETACHED_HEAD: current = 1; ... and keep the last hunk. (IOW, turn current from a parameter into a local variable) -- 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: [RFC/PATCH] Port branch.c to use ref-filter APIs
On Tue, Jul 28, 2015 at 7:05 PM, Matthieu Moy matthieu@grenoble-inp.fr wrote: Karthik Nayak karthik@gmail.com writes: This version also doesn't use the printing options provided by branch.c. Do you mean provided by ref-filter.{c,h}? Yes! my bad. I wanted to discuss how exactly to go about that, because in branch.c, we might need to change the --format based on attributes such as abbrev, verbose and so on. But ref-filter expects us to verify the format before filtering. I took time to understand the problem, but here's my understanding: ref-filter expects the code to look like format = ...; verify_ref_format(format); filter_refs(...); for (...) show_ref_array_item(..., format, ...); and in the case of git branch -v, you need to align the sha1s based on the longest branch name, i.e. use %(padright:N+1) where N is the longest branch name. And you can get N only after calling filter_refs, while you need it for verify_ref_format(). Is my understanding correct? Absolutely correct :) If so, what prevents swapping the order of verify_ref_format and filter_refs? I understand that verify_ref_format() builds used_atom and other data-structures, hence it has to be called before show_ref_array_item() and before sorting, but I don't think you need it before filter_refs (I may have missed something though). Nope! This is exactly what I'm trying :D So, the code could look like: filter_refs(...) if (--format is given) format = the argument of --format else format = STRBUF_INIT; strbuf_add(format, %(some_directive_to_display '*' if needed)); if (verbose) strbuf_addf(format, %(padright:%d), max_width); ... verify_ref_format(format); ref_array_sort(...); for (...) show_ref_array_item(...); (BTW, a trivial helper function to display the whole ref_array could help. It would avoid having each caller write the 'for' loop) This I gotta try! Have been just keeping the flow the same and trying to mess around with how formatting works instead. Ideally, you could also have a modifier atom %(alignleft) that would not need an argument and that would go through the ref_array to find the maxwidth, and do the alignment. That would give even more flexibility to the end users of for-each-ref --format. This could work for refname, as each ref_array_item holds the refname, But other atoms are only filled in after a call to verify_ref_format(). What we could do would be after filling in all atom values, have a loop through all items with their atom values and maybe implement this. But I don't see this as priority for now, so will mark it off for later. 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: git branch command is incompatible with bash
Johannes Sixt j...@kdbg.org writes: Are you trying to say that the result of 'rev-parse --abbrev-ref HEAD' is suboptimal and that of 'symbolic-ref --short HEAD' is OK? My Interesting was primarily about that I wasn't aware of the --abbrev-ref option. Yes, I am sure some time ago I accepted a patch to add it, but I simply do not see the point, especially because the --short option to symbolic-ref feels much more superiour. What branch am I on? is about symbolic refs, rev-parse is about revisions. I can see that symbolic-ref --short is much newer than the other one, so addition of --abbrev-ref to rev-parse may have been a mistake made while being desperate (i.e. not having a way to do so with plumbing, we wanted some way to do so and chose poorly). -- 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: [RFC/PATCH 01/11] ref-filter: add %(objectname:size=X) option
Karthik Nayak karthik@gmail.com writes: From: Karthik Nayak karthik@gmail.com Add support for %(objectname:size=X) where X is a number. This will print the first X characters of an objectname. The minimum value for X is 5. Hence any value lesser than 5 will default to 5 characters. Is the reason why you do not call this abbrev because you are allowing it to truncate to a non-unique prefix? If so, wouldn't it make more sense to treat this kind of string function in a way very similar to your earlier padright? I.e. %(maxwidth:5)%(objectname) or something? If not, and if this is really an abbrev, then it makes sense to make it specific to the objectname, e.g. %(objectname:abbrev=7). -- 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: [RFC/PATCH 01/11] ref-filter: add %(objectname:size=X) option
On Tue, Jul 28, 2015 at 9:13 PM, Junio C Hamano gits...@pobox.com wrote: Karthik Nayak karthik@gmail.com writes: From: Karthik Nayak karthik@gmail.com Add support for %(objectname:size=X) where X is a number. This will print the first X characters of an objectname. The minimum value for X is 5. Hence any value lesser than 5 will default to 5 characters. Is the reason why you do not call this abbrev because you are allowing it to truncate to a non-unique prefix? If so, wouldn't it make more sense to treat this kind of string function in a way very similar to your earlier padright? I.e. %(maxwidth:5)%(objectname) or something? If not, and if this is really an abbrev, then it makes sense to make it specific to the objectname, e.g. %(objectname:abbrev=7). It is finding unique abbrev, I just named size as it was smaller. But abbrev seems better, will rename, 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: Log messages beginning # and git rebase -i
Ed Avis e...@waniasset.com writes: Eric Sunshine sunshine at sunshineco.com writes: the editing for the combined log message treats lines beginning with # as comments. This means that if you are not careful the commit message can get lost on rebasing. I suggest that git rebase should add an extra space at the start 'git rebase --interactive' respects the core.commentChar configuration variable, which you can set to some value other than '#'. I was thinking of the default configuration. But you are right, this applies to whatever the comment character is - so if commentChar is set to * for example, then log lines beginning with * should get an extra space prepended in git rebase --interactive so that they don't get lost. Actually, is there any reason why we do not allow a simple escaping like \# this is a line starting with # \\ this is a line starting with \ # this is a comment ? -- 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: [RFC/PATCH 02/11] ref-filter: add 'colornext' atom
On Tue, Jul 28, 2015 at 2:43 PM, Christian Couder christian.cou...@gmail.com wrote: On Tue, Jul 28, 2015 at 8:56 AM, Karthik Nayak karthik@gmail.com wrote: @@ -712,6 +713,15 @@ static void populate_value(struct ref_array_item *ref) v-modifier_atom = 1; v-pad_to_right = 1; continue; + } else if (starts_with(name, colornext:)) { + char color[COLOR_MAXLEN] = ; + + if (color_parse(name + 10, color) 0) + die(_(unable to parse format)); Maybe use skip_prefix(), and die() with a more helpful message. Okay will do. 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: [RFC/PATCH 01/11] ref-filter: add %(objectname:size=X) option
On Tue, Jul 28, 2015 at 2:12 PM, Matthieu Moy matthieu@grenoble-inp.fr wrote: Karthik Nayak karthik@gmail.com writes: + if (skip_prefix(name, objectname:size=, p)) { + unsigned int size = atoi(p); You have the same problem as for tag.c: don't use atoi, and make accurate error checking (absence of value, negative value, non-integer value). If you have other higher-priorities task, leave a temporary comment /* FIXME: ... */ or /* TODO: ... */ and make sure you have no such comment when you drop the RFC in the subject of your emails. It's a small change, will fix it with the drop in RFC :) -- 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: [RFC/PATCH 02/11] ref-filter: add 'colornext' atom
On Tue, Jul 28, 2015 at 2:15 PM, Matthieu Moy matthieu@grenoble-inp.fr wrote: Karthik Nayak karthik@gmail.com writes: --- a/t/t6302-for-each-ref-filter.sh +++ b/t/t6302-for-each-ref-filter.sh @@ -133,4 +133,20 @@ test_expect_success 'reverse version sort' ' test_cmp expect actual ' +get_color () +{ + git config --get-color no.such.slot $1 +} + +cat expect EOF +$(get_color green)foo1.10$(get_color reset)|| +$(get_color green)foo1.3$(get_color reset)|| +$(get_color green)foo1.6$(get_color reset)|| +EOF + +test_expect_success 'check `colornext` format option' ' + git for-each-ref --format=%(colornext:green)%(refname:short)|| | grep foo actual + test_cmp expect actual +' This is not a very good test: you're not checking that colornext applies to the next and only this one. Similarly to what I suggested for padright, I'd suggest --format=%(refname:short)%(colornext:green)|%(refname:short)|%(refname:short)| That was the purpose of the || but that doesn't check the color of next atom, Thanks for the example will use that :) -- 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: [RFC/PATCH 01/11] ref-filter: add %(objectname:size=X) option
Karthik Nayak karthik@gmail.com writes: From: Karthik Nayak karthik@gmail.com Add support for %(objectname:size=X) where X is a number. This will print the first X characters of an objectname. The minimum value for X is 5. Hence any value lesser than 5 will default to 5 characters. Where does this hardcoded 5 come from? I'd agree that we would want some minimum for sanity (not safety), but I do not think we want random callers of find-unique-abbrev arbitrarily imposing their own minimum, making different codepaths behave inconsistently without a good reason. It seems that the minimum we use for sanity at the core level is MINIMUM_ABBREV. Is there a reason why that value is inappropriate for this codepath? 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 | 9 + 1 file changed, 9 insertions(+) diff --git a/ref-filter.c b/ref-filter.c index 0a34924..4c5e3f8 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -196,6 +196,8 @@ static void *get_obj(const unsigned char *sha1, struct object **obj, unsigned lo static int grab_objectname(const char *name, const unsigned char *sha1, struct atom_value *v) { + const char *p; + if (!strcmp(name, objectname)) { char *s = xmalloc(41); strcpy(s, sha1_to_hex(sha1)); @@ -206,6 +208,13 @@ static int grab_objectname(const char *name, const unsigned char *sha1, v-s = xstrdup(find_unique_abbrev(sha1, DEFAULT_ABBREV)); return 1; } + if (skip_prefix(name, objectname:size=, p)) { + unsigned int size = atoi(p); + if (size 5) + size = 5; + v-s = xstrdup(find_unique_abbrev(sha1, size)); + return 1; + } return 0; } -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 01/10] ref-filter: add option to align atoms to the left
On Mon, Jul 27, 2015 at 5:18 PM, Duy Nguyen pclo...@gmail.com wrote: On Mon, Jul 27, 2015 at 2:39 PM, Jacob Keller jacob.kel...@gmail.com wrote: On Sun, Jul 26, 2015 at 5:39 PM, Duy Nguyen pclo...@gmail.com wrote: On Sun, Jul 26, 2015 at 11:08 AM, Eric Sunshine sunsh...@sunshineco.com wrote: You can generate an interdiff with git diff branchname-v4 branchname-v5, for instance. Off topic. But what stops me from doing this often is it creates a big mess in git tag -l. Do we have an option to hide away some insignificant: tags? reflog might be an option if we have something like foo@{/v2} to quickly retrieve the reflog entry whose message contains v2. ... But maybe we're abusing reflog.. Actually a good place for this stuff is git branch --edit-description. A lot of manual steps to save old refs, do inter-diff.. but it's probably good enough. -- Duy -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 06/11] branch: roll show_detached HEAD into regular ref_list
Karthik Nayak karthik@gmail.com writes: Remove show_detached() and make detached HEAD to be rolled into regular ref_list by adding REF_DETACHED_HEAD as a kind of branch and supporting the same in append_ref(). Again, this lacks the why? explanation. Bump get_head_description() to the top. Here also: why? And this could easily go to a separate patch to let the reviewer focus on actual changes. @@ -504,6 +540,7 @@ static void print_ref_item(struct ref_item *item, int maxwidth, int verbose, int color; struct strbuf out = STRBUF_INIT, name = STRBUF_INIT; const char *prefix = ; + const char *desc = item-name; if (item-ignore) return; @@ -516,6 +553,10 @@ static void print_ref_item(struct ref_item *item, int maxwidth, int verbose, color = BRANCH_COLOR_REMOTE; prefix = remote_prefix; break; + case REF_DETACHED_HEAD: + color = BRANCH_COLOR_CURRENT; + desc = get_head_description(); I think you're leaking a string here: get_head_description() builds an strbuf and returns the dynamically allocated string, which you never free. -static void show_detached(struct ref_list *ref_list, int maxwidth) -{ - struct commit *head_commit = lookup_commit_reference_gently(head_sha1, 1); - - if (head_commit is_descendant_of(head_commit, ref_list-with_commit)) { I'm not sure what this if was doing, and why you can get rid of it. My understanding is that with_commit comes from --contains, and in the previous code the filtering was done at display time (detached HEAD was not shown if it was not contained in commits specified with --contains). Eventually, you'll use ref-filter to do this filtering so you won't need this check at display time. But am I correct that for a few commits, you ignore --contains on detached HEAD? @@ -678,15 +674,20 @@ static int print_ref_list(int kinds, int detached, int verbose, int abbrev, stru if (verbose) maxwidth = calc_maxwidth(ref_list, strlen(remote_prefix)); - qsort(ref_list.list, ref_list.index, sizeof(struct ref_item), ref_cmp); + index = ref_list.index; + + /* Print detached heads before sorting and printing the rest */ I think you mean head (no s) or HEAD. It's not Mercurial ;-). + if (detached (ref_list.list[index - 1].kind == REF_DETACHED_HEAD) + !strcmp(ref_list.list[index - 1].name, head)) { + print_ref_item(ref_list.list[index - 1], maxwidth, verbose, abbrev, +1, remote_prefix); + index -= 1; + } This relies on the assumption that HEAD has to be the last in the array. The assumption is correct since you call head_ref(append_ref, cb) after for_each_rawref(append_ref, cb). I think this deserves a comment to remind the reader that HEAD is always the last (here or at the call to head_ref to make sure nobody try to change the order between head_ref and for_each_rawref). It may be more natural to do it the other way around: call head_ref first and get HEAD first, as you are going to display it first (but in any case, you'll have to call qsort on a subset of the array so it doesn't change much). @@ -913,7 +914,10 @@ int cmd_branch(int argc, const char **argv, const char *prefix) die(_(branch name required)); return delete_branches(argc, argv, delete 1, kinds, quiet); } else if (list) { - int ret = print_ref_list(kinds, detached, verbose, abbrev, + int ret; + if (kinds REF_LOCAL_BRANCH) + kinds |= REF_DETACHED_HEAD; Perhaps add /* git branch --local also shows HEAD when it is detached */ -- 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: Do you need a loan?
Welcome to JASMIE LOAN COMPANY. We give out loans of all kinds. If you are in need of urgent loan kindly contact us instant approval for just 0.5% interest rate. APPLICATION FORM NAME... COUNTRY AMOUNT NUMBER. ADDRESS DURATION... PURPOSE Email:.. Mr J.S.Ravi Kumar C.E.O JASMIE LOAN COMPANY -- 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: [RFC/PATCH 10/11] branch.c: use 'ref-filter' APIs
On Tue, Jul 28, 2015 at 1:39 PM, Christian Couder christian.cou...@gmail.com wrote: On Tue, Jul 28, 2015 at 9:11 AM, Karthik Nayak karthik@gmail.com wrote: diff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt index a67138a..897cd81 100644 --- a/Documentation/git-branch.txt +++ b/Documentation/git-branch.txt @@ -11,7 +11,7 @@ SYNOPSIS 'git branch' [--color[=when] | --no-color] [-r | -a] [--list] [-v [--abbrev=length | --no-abbrev]] [--column[=options] | --no-column] - [(--merged | --no-merged | --contains) [commit]] [pattern...] + [(--merged | --no-merged | --contains) [commit]] [--sort=key] [pattern...] Maybe: [(--[no-]merged | --contains) [commit]] Thats an existing string. So not sure if this patch is the right place to make this change. -- 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: [RFC/PATCH 10/11] branch.c: use 'ref-filter' APIs
On Tue, Jul 28, 2015 at 1:27 PM, Jacob Keller jacob.kel...@gmail.com wrote: On Tue, Jul 28, 2015 at 12:11 AM, Karthik Nayak karthik@gmail.com wrote: Make 'branch.c' use 'ref-filter' APIs for iterating through refs sorting. This removes most of the code used in 'branch.c' replacing it with calls to the 'ref-filter' library. Make 'tag.c' use the 'filter_refs()' function provided by 'ref-filter' to filter out tags based on the options set. This patch doesn't do anything to tag.c, so you should update the commit message to remove this or replace it with s/tag/branch if it was intended to be about branch.c? Regards, Copy-paste error, thanks for pointing out. -- 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 2/6] notes: replace pseudorefs with real refs
Johan Herland jo...@herland.net writes: On Wed, Jul 29, 2015 at 4:00 AM, Junio C Hamano gits...@pobox.com wrote: So doing the absolute minimum, leaving the now what can we do to improve notes-merge process? outside the scope of the topic. That's exactly what I was also trying to do: David's topic should not have to deal with NOTES_MERGE_* at all. Simply leave it all as something that belongs in $GIT_COMMON_DIR, and let's solve concurrent unrelated notes merges as a wholly independent, separate topic. Here, it perhaps is showing that you are unfamiliar with the linked worktree topic. Things directly under .git/, like HEAD, MERGE_HEAD, etc., are per worktree (i.e. not shared across working trees). You have to work to make them shared, i.e. per repo. David's earlier one downcased them while still keeing them directly under .git/ but I think it is ugly and in general a wrong way to mark what is shared and what is per worktree. E.g. index is not shared, even though the name is all lowercase. You will be updating that mechanism when you truly make the notes-merge a per refs/notes/* ref thing (not per repo, not per worktree). Perhaps you will use refs/notes-merge/$foo that is shared across worktrees as the replacement for NOTES_MERGE_REF that currently is used to merge into refs/notes/$foo, or something like that, to pursue your per ref goal. At that point, NOTES_MERGE_REF based design needs to be scrapped anyway; it should not matter if NOTES_MERGE_REF is per worktree (not shared) or per repo (shared) in the meantime. Doing absolute minimum means that $GIT_DIR/NOTES_MERGE_REF should be left per worktree, like MERGE_HEAD, etc., without doing anything like downcasing or moving it under refs/notes-merge/ (which I think is a better way to make it shared, if we wanted to), which is additional special casing that will be a wasted effort that would not matter in the end result. -- 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 2/6] notes: replace pseudorefs with real refs
Johan Herland jo...@herland.net writes: I believe it is a bad compromise. It complicates the code, and it provides a concurrent notes merges that is unnecessarily tied to (and dependent on) worktrees. For example, if I wanted to perform N concurrent notes merges in a project that happens to have a huge worktree, I would now have to create N copies of the huge worktree... Who said worktree has to be populated? You should be able to have an absolutely empty checkout just like clone --no-checkout. -- 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 2/6] notes: replace pseudorefs with real refs
On Wed, Jul 29, 2015 at 4:00 AM, Junio C Hamano gits...@pobox.com wrote: Michael Haggerty mhag...@alum.mit.edu writes: It sounds like what a notes merge really wants is a new linked worktree that has branch refs/notes/foo checked out: * This would allow multiple notes merges to take place at the same time provided they target different merge references. * This would prevent multiple notes merges to the same notes reference at the same time by the same mechanism that prevents the same branch from being checked out in two linked worktrees at the same time. It's just a thought; I have no idea whether it is practical... That was certainly one of the possibilities that crossed my mind. In any case, the primary thing I am interested in at this point is to unblock David's prepare things so that we can put primary refs in a different ref backends more easily topic, and I've already made my point a few messages ago upstream: I think it is OK for us to admit that the notes subsystem is not quite ready to work well with multiple working tree world yet [*1*], and move this series forward without worrying about them. So doing the absolute minimum, leaving the now what can we do to improve notes-merge process? outside the scope of the topic. That's exactly what I was also trying to do: David's topic should not have to deal with NOTES_MERGE_* at all. Simply leave it all as something that belongs in $GIT_COMMON_DIR, and let's solve concurrent unrelated notes merges as a wholly independent, separate topic. ...Johan -- Johan Herland, jo...@herland.net www.herland.net -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Fwd: Bug#793884: git: allows nonsensical command 'git checkout -b HEAD'
Duy Nguyen pclo...@gmail.com writes: This seems like a good thing to fix (i.e. make sure XX is not ambiguous before creating it with git checkout -b XX) Yeah, sounds like an easy lunch-time hack low-hanging fruit :-). -- 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 branch command is incompatible with bash
On Tue, Jul 28, 2015 at 08:23:40AM -0700, Junio C Hamano wrote: I can see that symbolic-ref --short is much newer than the other one, so addition of --abbrev-ref to rev-parse may have been a mistake made while being desperate (i.e. not having a way to do so with plumbing, we wanted some way to do so and chose poorly). I think --abbrev-ref can handle much more than just shortening symrefs, though: E.g. resolving other symbolic names: $ git rev-parse --abbrev-ref @{u} origin/master Or resolving any arbitrary name you happen to have: $ git rev-parse --abbrev-ref refs/heads/master master Even ones that aren't fully qualified (which would be tough to do with simple pattern substitution): $ git rev-parse --abbrev-ref remotes/origin/master origin/master Or handling ambiguities during abbreviation: $ git tag master $ git rev-parse --abbrev-ref HEAD heads/master -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: [RFC/PATCH 03/11] ref-filter: add option to filter only branches
On Tue, Jul 28, 2015 at 7:08 PM, Matthieu Moy matthieu@grenoble-inp.fr wrote: Karthik Nayak karthik@gmail.com writes: +static int filter_branch_kind(struct ref_filter *filter, const char *refname) +{ + int kind, i; + + static struct { + int kind; + const char *prefix; + } ref_kind[] = { + { REF_LOCAL_BRANCH, refs/heads/ }, + { REF_REMOTE_BRANCH, refs/remotes/ }, + }; Nit: I would swap the order of fields, to make it a bit clearer that this is a kind of dictionary key - value (I think it's more common to write it in this order than value - key). This was borrowed from branch.c, but ok will change it! 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: [PATCH 2/6] Documentation/config: mention now and never for 'expire' settings
[+cc:Paul Tan] On Sun, Jul 26, 2015 at 9:41 PM, Michael Haggerty mhag...@alum.mit.edu wrote: On 07/23/2015 09:00 PM, Eric Sunshine wrote: In addition to approxidate-style values (2.months.ago, yesterday), consumers of 'gc.*expire*' configuration variables also accept and respect 'now'/'all' (do it immediately) and 'never'/'false' (suppress entirely). Suggested-by: Michael Haggerty mhag...@alum.mit.edu Signed-off-by: Eric Sunshine sunsh...@sunshineco.com --- gc.pruneExpire:: When 'git gc' is run, it will call 'prune --expire 2.weeks.ago'. Override the grace period with this config variable. The value - now may be used to disable this grace period and always prune - unreachable objects immediately. + now may be used to disable this grace period and always prune + unreachable objects immediately; or never to suppress pruning. A semicolon should be used without a conjunction, and the parts of a sentence joined by a semicolon should be independent clauses. So this should probably be I was just getting ready to re-roll this series[1] to address Michael's comments[2] and noticed that the add-on patch 7/6 which I sent later[3] seems to have been botched when Junio applied it to 'pu'. It's currently at 36598db (Documentation/git-tools: drop references to defunct tools, 2015-07-24) in es/doc-clean-outdated-tools and it appears that the --scissors option didn't cut off the leading cruft from the email conversation, thus the commit has the wrong subject plus a bunch of email conversation gunk in the commit message which doesn't belong. I understand that Junio uses a relatively bleeding-edge version of Git for his day-to-day work and was wondering if this is possible fallout from the git-am rewrite in C? [1]: http://thread.gmane.org/gmane.comp.version-control.git/274537 [2]: http://article.gmane.org/gmane.comp.version-control.git/274647 [3]: http://article.gmane.org/gmane.comp.version-control.git/274602 -- 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: Log messages beginning # and git rebase -i
Matthieu Moy matthieu@grenoble-inp.fr writes: Ed Avis e...@waniasset.com writes: Eric Sunshine sunshine at sunshineco.com writes: the editing for the combined log message treats lines beginning with # as comments. This means that if you are not careful the commit message can get lost on rebasing. I suggest that git rebase should add an extra space at the start 'git rebase --interactive' respects the core.commentChar configuration variable, which you can set to some value other than '#'. I was thinking of the default configuration. But you are right, this applies to whatever the comment character is - so if commentChar is set to * for example, then log lines beginning with * should get an extra space prepended in git rebase --interactive so that they don't get lost. Actually, is there any reason why we do not allow a simple escaping like \# this is a line starting with # \\ this is a line starting with \ # this is a comment What are we trying to achieve? Munging the original # I want this line intact to any other form like # I want this... is as bad as losing it. If the user wants whatever she types in the resulting commit literally, there is the --cleanup=choice option, no? -- 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] am: let command-line options override saved options
When resuming, git-am ignores command-line options. For instance, when a patch fails to apply with git am patch, subsequently running git am --3way patch would not cause git-am to fall back on attempting a threeway merge. This occurs because by default the --3way option is saved as false, and the saved am options are loaded after the command-line options are parsed, thus overwriting the command-line options when resuming. Fix this by moving the am_load() function call before parse_options(), so that command-line options will override the saved am options. Reported-by: Junio C Hamano gits...@pobox.com Helped-by: Jeff King p...@peff.net Signed-off-by: Paul Tan pyoka...@gmail.com --- builtin/am.c | 9 ++- t/t4153-am-resume-override-opts.sh | 144 + 2 files changed, 150 insertions(+), 3 deletions(-) create mode 100755 t/t4153-am-resume-override-opts.sh diff --git a/builtin/am.c b/builtin/am.c index 1116304..8a0b0e4 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -2131,6 +2131,7 @@ int cmd_am(int argc, const char **argv, const char *prefix) int keep_cr = -1; int patch_format = PATCH_FORMAT_UNKNOWN; enum resume_mode resume = RESUME_FALSE; + int in_progress; const char * const usage[] = { N_(git am [options] [(mbox|Maildir)...]), @@ -2226,6 +2227,10 @@ int cmd_am(int argc, const char **argv, const char *prefix) am_state_init(state, git_path(rebase-apply)); + in_progress = am_in_progress(state); + if (in_progress) + am_load(state); + argc = parse_options(argc, argv, prefix, options, usage, 0); if (binary = 0) @@ -2238,7 +2243,7 @@ int cmd_am(int argc, const char **argv, const char *prefix) if (read_index_preload(the_index, NULL) 0) die(_(failed to read the index)); - if (am_in_progress(state)) { + if (in_progress) { /* * Catch user error to feed us patches when there is a session * in progress: @@ -2256,8 +2261,6 @@ int cmd_am(int argc, const char **argv, const char *prefix) if (resume == RESUME_FALSE) resume = RESUME_APPLY; - - am_load(state); } else { struct argv_array paths = ARGV_ARRAY_INIT; int i; diff --git a/t/t4153-am-resume-override-opts.sh b/t/t4153-am-resume-override-opts.sh new file mode 100755 index 000..c49457c --- /dev/null +++ b/t/t4153-am-resume-override-opts.sh @@ -0,0 +1,144 @@ +#!/bin/sh + +test_description='git-am command-line options override saved options' + +. ./test-lib.sh + +test_expect_success 'setup' ' + test_commit initial file + test_commit first file + + git checkout -b side initial + test_commit side-first file + test_commit side-second file + + { + echo Message-Id: side-fi...@example.com + git format-patch --stdout -1 side-first | sed -e 1d + } side-first.patch + { + sed -ne 1,/^\$/p side-first.patch + echo -- 8 -- + sed -e 1,/^\$/d side-first.patch + } side-first.scissors + + { + echo Message-Id: side-sec...@example.com + git format-patch --stdout -1 side-second | sed -e 1d + } side-second.patch + { + sed -ne 1,/^\$/p side-second.patch + echo -- 8 -- + sed -e 1,/^\$/d side-second.patch + } side-second.scissors +' + +test_expect_success '--3way, --no-3way' ' + rm -fr .git/rebase-apply + git reset --hard + git checkout first + test_must_fail git am --3way side-first.patch side-second.patch + test -n $(git ls-files -u) + echo will-conflict file + git add file + test_must_fail git am --no-3way --continue + test -z $(git ls-files -u) +' + +test_expect_success '--no-quiet, --quiet' ' + rm -fr .git/rebase-apply + git reset --hard + git checkout first + test_must_fail git am --no-quiet side-first.patch side-second.patch + test_must_be_empty out + echo side-first file + git add file + git am --quiet --continue out + test_must_be_empty out +' + +test_expect_success '--signoff, --no-signoff' ' + rm -fr .git/rebase-apply + git reset --hard + git checkout first + test_must_fail git am --signoff side-first.patch side-second.patch + echo side-first file + git add file + git am --no-signoff --continue + + # applied side-first will be signed off + echo Signed-off-by: $GIT_COMMITTER_NAME $GIT_COMMITTER_EMAIL expected + git cat-file commit HEAD^ | grep Signed-off-by: actual + test_cmp expected actual + + # applied side-second will not be signed off + test $(git cat-file commit HEAD | grep -c
Re: [PATCH 1/5] refs: Introduce pseudoref and per-worktree ref concepts
On Mon, Jul 27, 2015 at 4:08 PM, David Turner dtur...@twopensource.com wrote: refs: Introduce pseudoref and per-worktree ref concepts Add glossary entries for both concepts. Based upon the above, I thought this was going to be a documentation-only patch and was mildly surprised to find that it also changed code. Perhaps: Describe these concepts in the glossary and introduce is_per_worktree_ref() to distinguish such files. or something. Of course the and in there suggests that this might be better off split into two patches... More below. Pseudorefs and per-worktree refs do not yet have special handling, because the files refs backend already handles them correctly. Later, we will make the LMDB backend call out to the files backend to handle per-worktree refs. Signed-off-by: David Turner dtur...@twopensource.com --- diff --git a/Documentation/glossary-content.txt b/Documentation/glossary-content.txt index ab18f4b..67952f3 100644 --- a/Documentation/glossary-content.txt +++ b/Documentation/glossary-content.txt @@ -411,6 +411,27 @@ exclude;; core Git. Porcelains expose more of a def_SCM,SCM interface than the def_plumbing,plumbing. +[[def_per_worktree_ref]]per-worktree ref:: + Refs that are per-def_worktree,worktree, rather than + global. This is presently only def_HEAD,HEAD, but might + later include other unusual refs. + +[[def_pseudoref]]pseudoref:: + Pseudorefs are a class of files under `$GIT_DIR` which behave + like refs for the purposes of rev-parse, but which are treated + specially by git. Psuedorefs both have names are all-caps, s/names/ that/ + and always start with a line consisting of a + def_sha1,SHA-1 followed by whitespace. So, HEAD is not a + pseudoref, because it is sometimes a symbolic ref. They might + optionally some additional data. `MERGE_HEAD` and s/optionally/ contain/ + `CHERRY_PICK_HEAD` are examples. Unlike + def_per_worktree_ref,per-worktree refs, these files cannot + be symbolic refs, and never have reflogs. They also cannot be + updated through the normal ref update machinery. Instead, + they are updated by directly writing to the files. However, + they can be read as if they were refs, so `git rev-parse + MERGE_HEAD` will work. + [[def_pull]]pull:: Pulling a def_branch,branch means to def_fetch,fetch it and def_merge,merge it. See also linkgit:git-pull[1]. diff --git a/refs.c b/refs.c index 0b96ece..0d10b7b 100644 --- a/refs.c +++ b/refs.c @@ -2848,6 +2848,29 @@ static int delete_ref_loose(struct ref_lock *lock, int flag, struct strbuf *err) return 0; } +int is_per_worktree_ref(const char *refname) +{ + return !strcmp(refname, HEAD); +} + +static int is_pseudoref(const char *refname) +{ + const char *c; + + if (strchr(refname, '/')) + return 0; + + if (is_per_worktree_ref(refname)) + return 0; + + for (c = refname; *c; ++c) { + if (!isupper(*c) *c != '-' *c != '_') + return 0; + } + + return 1; +} This static function doesn't seem to have any callers, thus seems out of place in this patch. + int delete_ref(const char *refname, const unsigned char *old_sha1, unsigned int flags) { -- 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] am: let command-line options override saved options
Paul Tan pyoka...@gmail.com writes: When resuming, git-am ignores command-line options. For instance, when a patch fails to apply with git am patch, subsequently running git am --3way patch would not cause git-am to fall back on attempting a The second one goes without any file argument, i.e. git am -3. threeway merge. This occurs because by default the --3way option is saved as false, and the saved am options are loaded after the command-line options are parsed, thus overwriting the command-line options when resuming. Fix this by moving the am_load() function call before parse_options(), so that command-line options will override the saved am options. Makes sense. -- 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: [RFC/PATCH 04/11] ref-filter: add 'ifexists' atom
On Tue, Jul 28, 2015 at 1:24 PM, Jacob Keller jacob.kel...@gmail.com wrote: On Mon, Jul 27, 2015 at 11:56 PM, Karthik Nayak karthik@gmail.com wrote: The 'ifexists' atom allows us to print a required format if the preceeding atom has a value. If the preceeding atom has no value then Don't you mean following atom here? since you do document it as the next atom below you should fix the commit message as well to match. In any respect, this is a useful formatting atom :) That should have been `succeeding` atom. My bad! thanks :) %(ifexists:[%s])%(atom) where the contents of atom will be placed into %s? I suggest documenting that the atom will be placed into the contents of ifexists via the %s indicator, as you do show an example but don't explicitely say %s is the formatting character. Yeah! I should have explicitly mentioned that, 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: [PATCH] am: let command-line options override saved options
Paul Tan pyoka...@gmail.com writes: diff --git a/t/t4153-am-resume-override-opts.sh b/t/t4153-am-resume-override-opts.sh new file mode 100755 index 000..c49457c --- /dev/null +++ b/t/t4153-am-resume-override-opts.sh @@ -0,0 +1,144 @@ +#!/bin/sh + +test_description='git-am command-line options override saved options' + +. ./test-lib.sh + +test_expect_success 'setup' ' + test_commit initial file + test_commit first file + + git checkout -b side initial + test_commit side-first file + test_commit side-second file + + { + echo Message-Id: side-fi...@example.com + git format-patch --stdout -1 side-first | sed -e 1d + } side-first.patch Hmm, puzzled... Ah, you want to make sure Message-Id comes at the very beginning, and you are going to use a single e-mail per mailbox so it is easier to strip the Beginning of Message marker than to insert Message-Id after it. I can understand what is going on. + { + sed -ne 1,/^\$/p side-first.patch sed -e /^\$/q would work just as well here + echo -- 8 -- + sed -e 1,/^\$/d side-first.patch + } side-first.scissors So *.scissors version has -- 8 -- inserted at the beginning of the body. + { + echo Message-Id: side-sec...@example.com + git format-patch --stdout -1 side-second | sed -e 1d + } side-second.patch + { + sed -ne 1,/^\$/p side-second.patch + echo -- 8 -- + sed -e 1,/^\$/d side-second.patch + } side-second.scissors +' A helper function that takes the branch name may be a good idea, not just to consolidate the implementation but as a place to document how these pairs of files are constructed and why. +test_expect_success '--3way, --no-3way' ' + rm -fr .git/rebase-apply + git reset --hard + git checkout first + test_must_fail git am --3way side-first.patch side-second.patch + test -n $(git ls-files -u) + echo will-conflict file + git add file + test_must_fail git am --no-3way --continue + test -z $(git ls-files -u) +' + +test_expect_success '--no-quiet, --quiet' ' + rm -fr .git/rebase-apply + git reset --hard + git checkout first + test_must_fail git am --no-quiet side-first.patch side-second.patch + test_must_be_empty out Where did this 'out' come from? + echo side-first file + git add file + git am --quiet --continue out + test_must_be_empty out I can see this one, though I am not sure if it is sensible to see what the command says under --quiet option, especially if you are making sure it does not fail, which you already have checked for its exit status. +' + +test_expect_success '--signoff, --no-signoff' ' + rm -fr .git/rebase-apply + git reset --hard + git checkout first + test_must_fail git am --signoff side-first.patch side-second.patch + echo side-first file + git add file + git am --no-signoff --continue + + # applied side-first will be signed off + echo Signed-off-by: $GIT_COMMITTER_NAME $GIT_COMMITTER_EMAIL expected + git cat-file commit HEAD^ | grep Signed-off-by: actual + test_cmp expected actual + + # applied side-second will not be signed off + test $(git cat-file commit HEAD | grep -c Signed-off-by:) -eq 0 +' Hmm, the command was run with --signoff at the start, first gets applied with am --no-signoff --resolved so I would expect it does not get signed off, but the second one will apply cleanly on top, so shouldn't it get signed off? Or perhaps somehow I misread Peff's idea to make these override one-shot in $gmane/274635? -- 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 branch command is incompatible with bash
Am 28.07.2015 um 17:23 schrieb Junio C Hamano: Johannes Sixt j...@kdbg.org writes: Are you trying to say that the result of 'rev-parse --abbrev-ref HEAD' is suboptimal and that of 'symbolic-ref --short HEAD' is OK? My Interesting was primarily about that I wasn't aware of the --abbrev-ref option. Yes, I am sure some time ago I accepted a patch to add it, but I simply do not see the point, especially because the --short option to symbolic-ref feels much more superiour. What branch am I on? is about symbolic refs, rev-parse is about revisions. I can see that symbolic-ref --short is much newer than the other one, so addition of --abbrev-ref to rev-parse may have been a mistake made while being desperate (i.e. not having a way to do so with plumbing, we wanted some way to do so and chose poorly). Heh. Originially, I was about to suggest git symbolic-ref -q --short HEAD || git rev-parse --abbrev HEAD in order to handle the detached HEAD case by printing the abbreviated commit name, only to learn that this doesn't do what I expected. Looking at the man page of rev-parse, I discovered --abbrev-ref. And learned that I actually should have used --short instead of --abbrev in the above rev-parse command. Confusing... -- Hannes -- 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: [RFC/PATCH 04/11] ref-filter: add 'ifexists' atom
On Tue, Jul 28, 2015 at 2:20 PM, Matthieu Moy matthieu@grenoble-inp.fr wrote: Karthik Nayak karthik@gmail.com writes: --- a/t/t6302-for-each-ref-filter.sh +++ b/t/t6302-for-each-ref-filter.sh @@ -149,4 +149,25 @@ test_expect_success 'check `colornext` format option' ' test_cmp expect actual ' +test_expect_success 'check `ifexists` format option' ' + cat expect -\EOF + [foo1.10] + [foo1.3] + [foo1.6] + EOF + git for-each-ref --format=%(ifexists:[%s])%(refname:short) | grep foo actual + test_cmp expect actual +' You're testing only the positive case of ifexists, right? You need a test for the negative case (i.e. the attribute does not exist) too. Ideally, check that the ifexist actually applies to the right attribute, like --format '%(ifexist:%s)%(attribute1) %(ifexist:[%s])%(attribute2)' and manage to get an output like foo [bar] foobar [barfoo] Yes! this seems like an important test, thanks :) You have a double || that is not useful. Not really harmful either, but it may distract the reader. You may want to s/||/|/. Will change! -- 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: [RFC/PATCH] Port branch.c to use ref-filter APIs
Karthik Nayak karthik@gmail.com writes: On Tue, Jul 28, 2015 at 7:05 PM, Matthieu Moy matthieu@grenoble-inp.fr wrote: Ideally, you could also have a modifier atom %(alignleft) [...] This could work for refname, as each ref_array_item holds the refname, But other atoms are only filled in after a call to verify_ref_format(). If you swap the order and call verify_ref_format() after filter_refs(), then you could detect %(alignleft) and iterate over the list as needed in verify_ref_format(). (You would actually force swapping the order and you would need to adapt other callers in for-each-ref and tag) But I don't see this as priority for now, so will mark it off for later. Absolutely, that's what I meant by Ideally. I'm just thinking aloud. -- 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: [RFC/PATCH 04/11] ref-filter: add 'ifexists' atom
Karthik Nayak karthik@gmail.com writes: The 'ifexists' atom allows us to print a required format if the preceeding atom has a value. If the preceeding atom has no value then the format given is not printed. e.g. to print [refname] we can now use the format %(ifexists:[%s])%(refname). A handful of huh? on the design. - The atom says if *exists* and explanation says has a value. How are they related? Does an atom whose value is an empty string has a value? Or is ifexists meant to be used only to ignore meaningless atom, e.g. %(*objectname) applied to a ref that refers to an object that is not an annotated tag? - That %s looks ugly. Are there cases where a user may want to say %(ifexists:[%i]) or something other than 's' after that per-cent? . Is it allowed to have more than one %s there? . Is it allowed to have no %s there? - The syntax makes the reader wonder if [] is part of the construct, or just an example of any arbitrary string, i.e. is %(ifexists:the %s can be part of arbitrary string) valid? - If an arbitrary string is allowed, is there any quoting mechanism to allow ) to be part of that arbitrary string? - What, if anything, is allowed to come between %(ifexists...) and the next atom like %(refname)? For example, are these valid constructs? . %(ifexists...)%(padright:20)%(refname) . %(ifexists...) %(refname) [%(subject)] - This syntax does not seem to allow switching on an attribute to show or not to show another, e.g. if %(*objectname) makes sense, then show '%(padright:20)%(refname:short) %(*subject)' for it. -- 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 6/6] sequencer: replace write_cherry_pick_head with update_ref
Now update_ref (via write_pseudoref) does almost exactly what write_cherry_pick_head did, so we can remove write_cherry_pick_head and just use update_ref. Signed-off-by: David Turner dtur...@twopensource.com --- sequencer.c | 23 --- 1 file changed, 4 insertions(+), 19 deletions(-) diff --git a/sequencer.c b/sequencer.c index c4f4b7d..554a704 100644 --- a/sequencer.c +++ b/sequencer.c @@ -158,23 +158,6 @@ static void free_message(struct commit *commit, struct commit_message *msg) unuse_commit_buffer(commit, msg-message); } -static void write_cherry_pick_head(struct commit *commit, const char *pseudoref) -{ - const char *filename; - int fd; - struct strbuf buf = STRBUF_INIT; - - strbuf_addf(buf, %s\n, sha1_to_hex(commit-object.sha1)); - - filename = git_path(%s, pseudoref); - fd = open(filename, O_WRONLY | O_CREAT, 0666); - if (fd 0) - die_errno(_(Could not open '%s' for writing), filename); - if (write_in_full(fd, buf.buf, buf.len) != buf.len || close(fd)) - die_errno(_(Could not write to '%s'), filename); - strbuf_release(buf); -} - static void print_advice(int show_hint, struct replay_opts *opts) { char *msg = getenv(GIT_CHERRY_PICK_HELP); @@ -607,9 +590,11 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts) * write it at all. */ if (opts-action == REPLAY_PICK !opts-no_commit (res == 0 || res == 1)) - write_cherry_pick_head(commit, CHERRY_PICK_HEAD); + update_ref(NULL, CHERRY_PICK_HEAD, commit-object.sha1, NULL, + REF_NODEREF, UPDATE_REFS_DIE_ON_ERR); if (opts-action == REPLAY_REVERT ((opts-no_commit res == 0) || res == 1)) - write_cherry_pick_head(commit, REVERT_HEAD); + update_ref(NULL, REVERT_HEAD, commit-object.sha1, NULL, + REF_NODEREF, UPDATE_REFS_DIE_ON_ERR); if (res) { error(opts-action == REPLAY_REVERT -- 2.0.4.315.gad8727a-twtrsrc -- 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] typofix for index-format.txt
Signed-off-by: Thomas Ackermann th.ac...@arcor.de --- Documentation/technical/index-format.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/technical/index-format.txt b/Documentation/technical/index-format.txt index b7093af..7392ff6 100644 --- a/Documentation/technical/index-format.txt +++ b/Documentation/technical/index-format.txt @@ -275,7 +275,7 @@ Git index format - The directory name terminated by NUL. -- A number of untrached file/dir names terminated by NUL. +- A number of untracked file/dir names terminated by NUL. The remaining data of each directory block is grouped by type: -- 1.9.5.msysgit.0 -- 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 0/6] pseudorefs
On top of what work is this series expected to be applied? -- 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 0/6] pseudorefs
On Tue, 2015-07-28 at 12:01 -0700, Junio C Hamano wrote: On top of what work is this series expected to be applied? I think I started from 'next' as of a few days ago: commit df7aaa5e3454bbcbb1f142dd6b95b214d0b8efad Author: Zoë Blade z...@bytenoise.co.uk Date: Tue Jul 21 14:22:46 2015 +0100 userdiff: add support for Fountain documents Let me know if you need me to rebase on something else. -- 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: Log messages beginning # and git rebase -i
Junio C Hamano gits...@pobox.com writes: Matthieu Moy matthieu@grenoble-inp.fr writes: Actually, is there any reason why we do not allow a simple escaping like \# this is a line starting with # \\ this is a line starting with \ # this is a comment What are we trying to achieve? What I would like would be a simple way to: 1) Allow any commit message to be typed. If I want to talk about #include in my commit message, I should have an easy way to do so. 2) Allow commands that pop an editor on an existing message to preserve the original message, whatever it is. Well, actually Git even strips # lines even if it doesn't pop the editor. Currently, I can for example: $ git commit -m #include -a [detached HEAD 0f36ec9] #include 1 file changed, 1 insertion(+), 1 deletion(-) $ GIT_EDITOR=touch git commit --amend Aborting commit due to empty commit message. A simple escaping scheme like the above can solve both points: 1) If I want to talk about #include in my commit message, I can spell it \#include and Git would remove the \. The same way, if I want to tell my shell about a inside a string, I can write double-quote:\. and get a litteral double-quote. 2) A command that pops an editor could add the escaping where needed, pop the editor, and then unescape. A command like pick in rebase -i could escape the message, and feed it to git commit which would unescape it. Munging the original # I want this line intact to any other form like # I want this... is as bad as losing it. It would modify it only when shown in the text editor. The object database would contain unescaped message, hence git log would show it unescaped for example. backslash-escaping special characters seems very natural to me, and I guess it would be for most computer-scientists. If I have problem with a special character, the first thing I would try would be to add a backslash in front of it. If the user wants whatever she types in the resulting commit literally, there is the --cleanup=choice option, no? $ GIT_EDITOR=touch git commit --cleanup=verbatim [detached HEAD 1b136a7] # Please enter the commit message for your changes. Lines starting # with '#' will be kept; you may remove them yourself if you want to. # An empty message aborts the commit. # HEAD detached from 5e70007 # Changes to be committed: # modified: foo.txt # # Changes not staged for commit : # modified: foo.txt # # Untracked files: # last-synchro.txt # 1 file changed, 1 insertion(+), 1 deletion(-) You really don't want that in day-to-day use. -- 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 4/5] rebase: use update_ref
David Turner dtur...@twopensource.com writes: Instead of manually writing a pseudoref (in one case) and shelling out to git update-ref (in another), use the update_ref function. This is much simpler. Signed-off-by: David Turner dtur...@twopensource.com --- bisect.c | 37 - 1 file changed, 8 insertions(+), 29 deletions(-) Mistitled? I can do s/rebase/bisect/ at my end if that is all needed. diff --git a/bisect.c b/bisect.c index 857cf59..33ac88d 100644 --- a/bisect.c +++ b/bisect.c @@ -19,7 +19,6 @@ static struct object_id *current_bad_oid; static const char *argv_checkout[] = {checkout, -q, NULL, --, NULL}; static const char *argv_show_branch[] = {show-branch, NULL, NULL}; -static const char *argv_update_ref[] = {update-ref, --no-deref, BISECT_HEAD, NULL, NULL}; static const char *term_bad; static const char *term_good; @@ -675,34 +674,16 @@ static int is_expected_rev(const struct object_id *oid) return res; } -static void mark_expected_rev(char *bisect_rev_hex) -{ - int len = strlen(bisect_rev_hex); - const char *filename = git_path(BISECT_EXPECTED_REV); - int fd = open(filename, O_CREAT | O_TRUNC | O_WRONLY, 0600); - - if (fd 0) - die_errno(could not create file '%s', filename); - - bisect_rev_hex[len] = '\n'; - write_or_die(fd, bisect_rev_hex, len + 1); - bisect_rev_hex[len] = '\0'; - - if (close(fd) 0) - die(closing file %s: %s, filename, strerror(errno)); -} - -static int bisect_checkout(char *bisect_rev_hex, int no_checkout) +static int bisect_checkout(const unsigned char *bisect_rev, int no_checkout) { + char bisect_rev_hex[GIT_SHA1_HEXSZ + 1]; - mark_expected_rev(bisect_rev_hex); + memcpy(bisect_rev_hex, sha1_to_hex(bisect_rev), GIT_SHA1_HEXSZ + 1); + update_ref(NULL, BISECT_EXPECTED_REV, bisect_rev, NULL, 0, UPDATE_REFS_DIE_ON_ERR); argv_checkout[2] = bisect_rev_hex; if (no_checkout) { - argv_update_ref[3] = bisect_rev_hex; - if (run_command_v_opt(argv_update_ref, RUN_GIT_CMD)) - die(update-ref --no-deref HEAD failed on %s, - bisect_rev_hex); + update_ref(NULL, BISECT_HEAD, bisect_rev, NULL, 0, UPDATE_REFS_DIE_ON_ERR); } else { int res; res = run_command_v_opt(argv_checkout, RUN_GIT_CMD); @@ -804,7 +785,7 @@ static void check_merge_bases(int no_checkout) handle_skipped_merge_base(mb); } else { printf(Bisecting: a merge base must be tested\n); - exit(bisect_checkout(sha1_to_hex(mb), no_checkout)); + exit(bisect_checkout(mb, no_checkout)); } } @@ -948,7 +929,6 @@ int bisect_next_all(const char *prefix, int no_checkout) struct commit_list *tried; int reaches = 0, all = 0, nr, steps; const unsigned char *bisect_rev; - char bisect_rev_hex[GIT_SHA1_HEXSZ + 1]; read_bisect_terms(term_bad, term_good); if (read_bisect_refs()) @@ -986,11 +966,10 @@ int bisect_next_all(const char *prefix, int no_checkout) } bisect_rev = revs.commits-item-object.sha1; - memcpy(bisect_rev_hex, sha1_to_hex(bisect_rev), GIT_SHA1_HEXSZ + 1); if (!hashcmp(bisect_rev, current_bad_oid-hash)) { exit_if_skipped_commits(tried, current_bad_oid); - printf(%s is the first %s commit\n, bisect_rev_hex, + printf(%s is the first %s commit\n, sha1_to_hex(bisect_rev), term_bad); show_diff_tree(prefix, revs.commits-item); /* This means the bisection process succeeded. */ @@ -1003,7 +982,7 @@ int bisect_next_all(const char *prefix, int no_checkout) (roughly %d step%s)\n, nr, (nr == 1 ? : s), steps, (steps == 1 ? : s)); - return bisect_checkout(bisect_rev_hex, no_checkout); + return bisect_checkout(bisect_rev, no_checkout); } static inline int log2i(int n) -- 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/6] Documentation/config: mention now and never for 'expire' settings
Eric Sunshine sunsh...@sunshineco.com writes: I was just getting ready to re-roll this series[1] to address Michael's comments[2] and noticed that the add-on patch 7/6 which I sent later[3] seems to have been botched when Junio applied it to 'pu'. It's currently at 36598db (Documentation/git-tools: drop references to defunct tools, 2015-07-24) in es/doc-clean-outdated-tools and it appears that the --scissors option didn't cut off the leading cruft from the email conversation, thus the commit has the wrong subject plus a bunch of email conversation gunk in the commit message which doesn't belong. I understand that Junio uses a relatively bleeding-edge version of Git for his day-to-day work and was wondering if this is possible fallout from the git-am rewrite in C? It is more likely that I was just lazy, knowing that the patch [3] would not hit next and I'll have a more relaxed time to amend it after the release was done, and let am -s without -c take it. I just tried to re-apply the patch with am -sc on es/doc-clean-outdated-tools^ and the built-in one takes it just fine, so we should be 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
[PATCH v3 4/6] pseudorefs: create and use pseudoref update and delete functions
Pseudorefs should not be updated through the ref transaction API, because alternate ref backends still need to store pseudorefs in GIT_DIR (instead of wherever they store refs). Instead, change update_ref and delete_ref to call pseudoref-specific functions. Signed-off-by: David Turner dtur...@twopensource.com --- refs.c | 100 +++-- 1 file changed, 92 insertions(+), 8 deletions(-) diff --git a/refs.c b/refs.c index 553ae8b..2bd6aa6 100644 --- a/refs.c +++ b/refs.c @@ -2877,12 +2877,87 @@ enum ref_type ref_type(const char *refname) return REF_TYPE_NORMAL; } +static int write_pseudoref(const char *pseudoref, const unsigned char *sha1, + const unsigned char *old_sha1, struct strbuf *err) +{ + const char *filename; + int fd; + static struct lock_file lock; + struct strbuf buf = STRBUF_INIT; + int ret = -1; + + strbuf_addf(buf, %s\n, sha1_to_hex(sha1)); + + filename = git_path(%s, pseudoref); + fd = hold_lock_file_for_update(lock, filename, LOCK_DIE_ON_ERROR); + if (fd 0) { + strbuf_addf(err, Could not open '%s' for writing: %s, + filename, strerror(errno)); + return -1; + } + + if (old_sha1) { + unsigned char actual_old_sha1[20]; + read_ref(pseudoref, actual_old_sha1); + if (hashcmp(actual_old_sha1, old_sha1)) { + strbuf_addf(err, Unexpected sha1 when writing %s, pseudoref); + rollback_lock_file(lock); + goto done; + } + } + + if (write_in_full(fd, buf.buf, buf.len) != buf.len) { + strbuf_addf(err, Could not write to '%s', filename); + rollback_lock_file(lock); + goto done; + } + + commit_lock_file(lock); + ret = 0; +done: + strbuf_release(buf); + return ret; +} + +static int delete_pseudoref(const char *pseudoref, const unsigned char *old_sha1) +{ + static struct lock_file lock; + const char *filename; + + filename = git_path(%s, pseudoref); + + if (old_sha1 !is_null_sha1(old_sha1)) { + int fd; + unsigned char actual_old_sha1[20]; + + fd = hold_lock_file_for_update(lock, filename, + LOCK_DIE_ON_ERROR); + if (fd 0) + die_errno(_(Could not open '%s' for writing), filename); + read_ref(pseudoref, actual_old_sha1); + if (hashcmp(actual_old_sha1, old_sha1)) { + warning(Unexpected sha1 when deleting %s, pseudoref); + return -1; + } + + unlink(filename); + rollback_lock_file(lock); + } else { + unlink(filename); + } + + return 0; +} + int delete_ref(const char *refname, const unsigned char *old_sha1, unsigned int flags) { struct ref_transaction *transaction; struct strbuf err = STRBUF_INIT; + if (ref_type(refname) == REF_TYPE_PSEUDOREF) + return delete_pseudoref(refname, old_sha1); + transaction = ref_transaction_begin(err); if (!transaction || ref_transaction_delete(transaction, refname, old_sha1, @@ -3976,17 +4051,25 @@ int update_ref(const char *msg, const char *refname, const unsigned char *new_sha1, const unsigned char *old_sha1, unsigned int flags, enum action_on_err onerr) { - struct ref_transaction *t; + struct ref_transaction *t = NULL; struct strbuf err = STRBUF_INIT; + int ret = 0; - t = ref_transaction_begin(err); - if (!t || - ref_transaction_update(t, refname, new_sha1, old_sha1, - flags, msg, err) || - ref_transaction_commit(t, err)) { + if (ref_type(refname) == REF_TYPE_PSEUDOREF) { + ret = write_pseudoref(refname, new_sha1, old_sha1, err); + } else { + t = ref_transaction_begin(err); + if (!t || + ref_transaction_update(t, refname, new_sha1, old_sha1, + flags, msg, err) || + ref_transaction_commit(t, err)) { + ret = 1; + ref_transaction_free(t); + } + } + if (ret) { const char *str = update_ref failed for ref '%s': %s; - ref_transaction_free(t); switch (onerr) { case UPDATE_REFS_MSG_ON_ERR: error(str, refname, err.buf); @@ -4001,7 +4084,8 @@ int update_ref(const char *msg, const char *refname, return 1; } strbuf_release(err); - ref_transaction_free(t); + if
[PATCH v3 5/6] rebase: use update_ref
Instead of manually writing a pseudoref (in one case) and shelling out to git update-ref (in another), use the update_ref function. This is much simpler. Signed-off-by: David Turner dtur...@twopensource.com --- bisect.c | 37 - 1 file changed, 8 insertions(+), 29 deletions(-) diff --git a/bisect.c b/bisect.c index 857cf59..33ac88d 100644 --- a/bisect.c +++ b/bisect.c @@ -19,7 +19,6 @@ static struct object_id *current_bad_oid; static const char *argv_checkout[] = {checkout, -q, NULL, --, NULL}; static const char *argv_show_branch[] = {show-branch, NULL, NULL}; -static const char *argv_update_ref[] = {update-ref, --no-deref, BISECT_HEAD, NULL, NULL}; static const char *term_bad; static const char *term_good; @@ -675,34 +674,16 @@ static int is_expected_rev(const struct object_id *oid) return res; } -static void mark_expected_rev(char *bisect_rev_hex) -{ - int len = strlen(bisect_rev_hex); - const char *filename = git_path(BISECT_EXPECTED_REV); - int fd = open(filename, O_CREAT | O_TRUNC | O_WRONLY, 0600); - - if (fd 0) - die_errno(could not create file '%s', filename); - - bisect_rev_hex[len] = '\n'; - write_or_die(fd, bisect_rev_hex, len + 1); - bisect_rev_hex[len] = '\0'; - - if (close(fd) 0) - die(closing file %s: %s, filename, strerror(errno)); -} - -static int bisect_checkout(char *bisect_rev_hex, int no_checkout) +static int bisect_checkout(const unsigned char *bisect_rev, int no_checkout) { + char bisect_rev_hex[GIT_SHA1_HEXSZ + 1]; - mark_expected_rev(bisect_rev_hex); + memcpy(bisect_rev_hex, sha1_to_hex(bisect_rev), GIT_SHA1_HEXSZ + 1); + update_ref(NULL, BISECT_EXPECTED_REV, bisect_rev, NULL, 0, UPDATE_REFS_DIE_ON_ERR); argv_checkout[2] = bisect_rev_hex; if (no_checkout) { - argv_update_ref[3] = bisect_rev_hex; - if (run_command_v_opt(argv_update_ref, RUN_GIT_CMD)) - die(update-ref --no-deref HEAD failed on %s, - bisect_rev_hex); + update_ref(NULL, BISECT_HEAD, bisect_rev, NULL, 0, UPDATE_REFS_DIE_ON_ERR); } else { int res; res = run_command_v_opt(argv_checkout, RUN_GIT_CMD); @@ -804,7 +785,7 @@ static void check_merge_bases(int no_checkout) handle_skipped_merge_base(mb); } else { printf(Bisecting: a merge base must be tested\n); - exit(bisect_checkout(sha1_to_hex(mb), no_checkout)); + exit(bisect_checkout(mb, no_checkout)); } } @@ -948,7 +929,6 @@ int bisect_next_all(const char *prefix, int no_checkout) struct commit_list *tried; int reaches = 0, all = 0, nr, steps; const unsigned char *bisect_rev; - char bisect_rev_hex[GIT_SHA1_HEXSZ + 1]; read_bisect_terms(term_bad, term_good); if (read_bisect_refs()) @@ -986,11 +966,10 @@ int bisect_next_all(const char *prefix, int no_checkout) } bisect_rev = revs.commits-item-object.sha1; - memcpy(bisect_rev_hex, sha1_to_hex(bisect_rev), GIT_SHA1_HEXSZ + 1); if (!hashcmp(bisect_rev, current_bad_oid-hash)) { exit_if_skipped_commits(tried, current_bad_oid); - printf(%s is the first %s commit\n, bisect_rev_hex, + printf(%s is the first %s commit\n, sha1_to_hex(bisect_rev), term_bad); show_diff_tree(prefix, revs.commits-item); /* This means the bisection process succeeded. */ @@ -1003,7 +982,7 @@ int bisect_next_all(const char *prefix, int no_checkout) (roughly %d step%s)\n, nr, (nr == 1 ? : s), steps, (steps == 1 ? : s)); - return bisect_checkout(bisect_rev_hex, no_checkout); + return bisect_checkout(bisect_rev, no_checkout); } static inline int log2i(int n) -- 2.0.4.315.gad8727a-twtrsrc -- 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 2/6] notes: replace pseudorefs with real refs
All-caps files like NOTES_MERGE_REF are pseudorefs, and thus are per-worktree. We don't want multiple notes merges happening at once (in different worktrees), so we want to make these refs true refs. So, we lowercase NOTES_MERGE_REF and friends. That way, backends that distinguish between pseudorefs and real refs can correctly handle notes merges. This will also enable us to prevent pseudorefs from being updated by the ref update machinery e.g. git update-ref. Signed-off-by: David Turner dtur...@twopensource.com --- Documentation/git-notes.txt | 12 ++--- builtin/notes.c | 38 notes-merge.c | 10 ++-- notes-merge.h | 8 ++-- t/t3310-notes-merge-manual-resolve.sh | 86 +-- t/t3311-notes-merge-fanout.sh | 6 +-- 6 files changed, 80 insertions(+), 80 deletions(-) diff --git a/Documentation/git-notes.txt b/Documentation/git-notes.txt index 851518d..82fa3fd 100644 --- a/Documentation/git-notes.txt +++ b/Documentation/git-notes.txt @@ -103,7 +103,7 @@ merge:: If conflicts arise and a strategy for automatically resolving conflicting notes (see the -s/--strategy option) is not given, the manual resolver is used. This resolver checks out the -conflicting notes in a special worktree (`.git/NOTES_MERGE_WORKTREE`), +conflicting notes in a special worktree (`.git/notes_merge_worktree`), and instructs the user to manually resolve the conflicts there. When done, the user can either finalize the merge with 'git notes merge --commit', or abort the merge with @@ -189,11 +189,11 @@ OPTIONS --commit:: Finalize an in-progress 'git notes merge'. Use this option when you have resolved the conflicts that 'git notes merge' - stored in .git/NOTES_MERGE_WORKTREE. This amends the partial + stored in .git/notes_merge_worktree. This amends the partial merge commit created by 'git notes merge' (stored in - .git/NOTES_MERGE_PARTIAL) by adding the notes in - .git/NOTES_MERGE_WORKTREE. The notes ref stored in the - .git/NOTES_MERGE_REF symref is updated to the resulting commit. + .git/notes_merge_partial) by adding the notes in + .git/notes_merge_worktree. The notes ref stored in the + .git/notes_merge_ref symref is updated to the resulting commit. --abort:: Abort/reset a in-progress 'git notes merge', i.e. a notes merge @@ -241,7 +241,7 @@ NOTES MERGE STRATEGIES The default notes merge strategy is manual, which checks out conflicting notes in a special work tree for resolving notes conflicts -(`.git/NOTES_MERGE_WORKTREE`), and instructs the user to resolve the +(`.git/notes_merge_worktree`), and instructs the user to resolve the conflicts in that work tree. When done, the user can either finalize the merge with 'git notes merge --commit', or abort the merge with diff --git a/builtin/notes.c b/builtin/notes.c index 63f95fc..e0b8a02 100644 --- a/builtin/notes.c +++ b/builtin/notes.c @@ -670,14 +670,14 @@ static int merge_abort(struct notes_merge_options *o) int ret = 0; /* -* Remove .git/NOTES_MERGE_PARTIAL and .git/NOTES_MERGE_REF, and call -* notes_merge_abort() to remove .git/NOTES_MERGE_WORKTREE. +* Remove .git/notes_merge_partial and .git/notes_merge_ref, and call +* notes_merge_abort() to remove .git/notes_merge_worktree. */ - if (delete_ref(NOTES_MERGE_PARTIAL, NULL, 0)) - ret += error(Failed to delete ref NOTES_MERGE_PARTIAL); - if (delete_ref(NOTES_MERGE_REF, NULL, REF_NODEREF)) - ret += error(Failed to delete ref NOTES_MERGE_REF); + if (delete_ref(notes_merge_partial, NULL, 0)) + ret += error(Failed to delete ref notes_merge_partial); + if (delete_ref(notes_merge_ref, NULL, REF_NODEREF)) + ret += error(Failed to delete ref notes_merge_ref); if (notes_merge_abort(o)) ret += error(Failed to remove 'git notes merge' worktree); return ret; @@ -694,16 +694,16 @@ static int merge_commit(struct notes_merge_options *o) int ret; /* -* Read partial merge result from .git/NOTES_MERGE_PARTIAL, -* and target notes ref from .git/NOTES_MERGE_REF. +* Read partial merge result from .git/notes_merge_partial, +* and target notes ref from .git/notes_merge_ref. */ - if (get_sha1(NOTES_MERGE_PARTIAL, sha1)) - die(Failed to read ref NOTES_MERGE_PARTIAL); + if (get_sha1(notes_merge_partial, sha1)) + die(Failed to read ref notes_merge_partial); else if (!(partial = lookup_commit_reference(sha1))) - die(Could not find commit from NOTES_MERGE_PARTIAL.); + die(Could not find commit from notes_merge_partial.); else if (parse_commit(partial)) - die(Could not parse commit from
[PATCH v3 1/6] refs: Introduce pseudoref and per-worktree ref concepts
Add glossary entries for both concepts. Pseudorefs and per-worktree refs do not yet have special handling, because the files refs backend already handles them correctly. Later, we will make the LMDB backend call out to the files backend to handle per-worktree refs. Signed-off-by: David Turner dtur...@twopensource.com --- Documentation/glossary-content.txt | 21 + 1 file changed, 21 insertions(+) diff --git a/Documentation/glossary-content.txt b/Documentation/glossary-content.txt index ab18f4b..ff14079 100644 --- a/Documentation/glossary-content.txt +++ b/Documentation/glossary-content.txt @@ -411,6 +411,27 @@ exclude;; core Git. Porcelains expose more of a def_SCM,SCM interface than the def_plumbing,plumbing. +[[def_per_worktree_ref]]per-worktree ref:: + Refs that are per-def_worktree,worktree, rather than + global. This is presently only def_HEAD,HEAD, but might + later include other unusual refs. + +[[def_pseudoref]]pseudoref:: + Pseudorefs are a class of files under `$GIT_DIR` which behave + like refs for the purposes of rev-parse, but which are treated + specially by git. Psuedorefs both have names that are all-caps, + and always start with a line consisting of a + def_sha1,SHA-1 followed by whitespace. So, HEAD is not a + pseudoref, because it is sometimes a symbolic ref. They might + optionally contain some additional data. `MERGE_HEAD` and + `CHERRY_PICK_HEAD` are examples. Unlike + def_per_worktree_ref,per-worktree refs, these files cannot + be symbolic refs, and never have reflogs. They also cannot be + updated through the normal ref update machinery. Instead, + they are updated by directly writing to the files. However, + they can be read as if they were refs, so `git rev-parse + MERGE_HEAD` will work. + [[def_pull]]pull:: Pulling a def_branch,branch means to def_fetch,fetch it and def_merge,merge it. See also linkgit:git-pull[1]. -- 2.0.4.315.gad8727a-twtrsrc -- 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 0/6] pseudorefs
This version fixes documentation issues found by Eric Sunshine. It also adds a new patch so as not to create static functions that aren't immediately used; Eric also noticed that issue. I refactored the functions to classify a ref into a single public ref_type function. This makes it easy for backends that want to treat all non-normal refs the same; in previous patch versions, backends would have had to say is_pseudoref || is_per_worktree_ref, and that would have caused the strcmp in is_per_worktree_ref to be called twice. Now they can just say ref_type(ref) != REF_TYPE_NORMAL. I also fixed another issues that I noticed myself: I removed a stray debugging 0 condition. -- 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: Log messages beginning # and git rebase -i
Matthieu Moy matthieu@grenoble-inp.fr writes: A simple escaping scheme like the above can solve both points: 1) If I want to talk about #include in my commit message, I can spell it \#include and Git would remove the \. The same way, if I want to tell my shell about a inside a string, I can write double-quote:\. and get a litteral double-quote. 2) A command that pops an editor could add the escaping where needed, pop the editor, and then unescape. A command like pick in rebase -i could escape the message, and feed it to git commit which would unescape it. ... backslash-escaping special characters seems very natural to me,... OK. So the proposal on the table is that a backslash at the beginning of a line is stripped. Stripping part should look like this. To make it work for things like git commit --amend, you would need to prefix any line that comes from the payload that begins with the core.commentchar or a backslash with a backslash. diff --git a/builtin/stripspace.c b/builtin/stripspace.c index 1259ed7..39ecb92 100644 --- a/builtin/stripspace.c +++ b/builtin/stripspace.c @@ -52,6 +52,11 @@ void stripspace(struct strbuf *sb, int skip_comments) } newlen = cleanup(sb-buf + i, len); + if (newlen sb-buf[i] == '\\') { + i++; + newlen--; + } + /* Not just an empty line? */ if (newlen) { if (empties 0 j 0) -- 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 4/5] rebase: use update_ref
On Tue, 2015-07-28 at 11:18 -0700, Junio C Hamano wrote: David Turner dtur...@twopensource.com writes: Instead of manually writing a pseudoref (in one case) and shelling out to git update-ref (in another), use the update_ref function. This is much simpler. Signed-off-by: David Turner dtur...@twopensource.com --- bisect.c | 37 - 1 file changed, 8 insertions(+), 29 deletions(-) Mistitled? I can do s/rebase/bisect/ at my end if that is all needed. Apparently, yes. 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 v3 2/6] notes: replace pseudorefs with real refs
David Turner dtur...@twopensource.com writes: All-caps files like NOTES_MERGE_REF are pseudorefs, and thus are per-worktree. We don't want multiple notes merges happening at once (in different worktrees), so we want to make these refs true refs. So, we lowercase NOTES_MERGE_REF and friends. That way, backends that distinguish between pseudorefs and real refs can correctly handle notes merges. This will also enable us to prevent pseudorefs from being updated by the ref update machinery e.g. git update-ref. Signed-off-by: David Turner dtur...@twopensource.com --- This seems to do a bit more than what it claims to do. As this kind of changes are very error-prone to review, I did a bulk replace of the three all-caps NOTES_*THING* and compared the result with what this patch gives to spot this: # Fail to finalize merge test_must_fail git notes merge --commit output 21 - # .git/NOTES_MERGE_* must remain - test -f .git/NOTES_MERGE_PARTIAL - test -f .git/NOTES_MERGE_REF - test -f .git/NOTES_MERGE_WORKTREE/$commit_sha1 - test -f .git/NOTES_MERGE_WORKTREE/$commit_sha2 - test -f .git/NOTES_MERGE_WORKTREE/$commit_sha3 - test -f .git/NOTES_MERGE_WORKTREE/$commit_sha4 + # .git/notes_merge_* must remain + git rev-parse --verify notes_merge_partial + git rev-parse --verify notes_merge_ref + test -f .git/notes_merge_worktree/$commit_sha1 + test -f .git/notes_merge_worktree/$commit_sha2 + test -f .git/notes_merge_worktree/$commit_sha3 + test -f .git/notes_merge_worktree/$commit_sha4 The two rev-parse --verify looks semi-sensible [*1*]; notes_merge_partial is all lowercase and it refers to $GIT_DIR/notes_merge_partial, because they are shared across working tree. But then why are $GIT_DIR/notes_merge_worktree/* still checked with test -f? If they are not refs or ref-like things, why should they be downcased? If they are, why not rev-parse --verify? [Footnote] *1* I say semi- sensible, because it looks ugly. All ref-like things immediately below $GIT_DIR/ are all-caps by convention. Perhaps it is a better idea to move it under refs/; everything under refs/ is shared across working trees is probably a much better rule than all caps but HEAD is special. -- 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] cache-tree: populate cache-tree on successful merge
When we unpack trees into an existing index, we discard the old index and replace it with the new, merged index. Ensure that this index has its cache-tree populated. This will make subsequent git status and commit commands faster. Signed-off-by: David Turner dtur...@twopensource.com Signed-off-by: Brian Degenhardt b...@bmdhacks.com --- This patch is by my colleague, Brian Degenhardt (as part of his work on git at Twitter). I'm sending it with his and Twitter's approval. --- t/t0090-cache-tree.sh | 24 unpack-trees.c| 7 +++ 2 files changed, 31 insertions(+) diff --git a/t/t0090-cache-tree.sh b/t/t0090-cache-tree.sh index 601d02d..055cc19 100755 --- a/t/t0090-cache-tree.sh +++ b/t/t0090-cache-tree.sh @@ -199,6 +199,30 @@ test_expect_success 'checkout -B gives cache-tree' ' test_cache_tree ' +test_expect_success 'merge --ff-only maintains cache-tree' ' + git checkout current + git checkout -b changes + test_commit llamas + test_commit pachyderm + test_cache_tree + git checkout current + test_cache_tree + git merge --ff-only changes + test_cache_tree +' + +test_expect_success 'merge maintains cache-tree' ' + git checkout current + git checkout -b changes2 + test_commit alpacas + test_cache_tree + git checkout current + test_commit struthio + test_cache_tree + git merge changes2 + test_cache_tree +' + test_expect_success 'partial commit gives cache-tree' ' git checkout -b partial no-children test_commit one diff --git a/unpack-trees.c b/unpack-trees.c index 2927660..befc247 100644 --- a/unpack-trees.c +++ b/unpack-trees.c @@ -1156,6 +1156,13 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options o-src_index = NULL; ret = check_updates(o) ? (-2) : 0; if (o-dst_index) { + if (!o-result.cache_tree) + o-result.cache_tree = cache_tree(); + + if (!cache_tree_fully_valid(o-result.cache_tree)) { + cache_tree_update(o-result, WRITE_TREE_SILENT | WRITE_TREE_REPAIR); + } + discard_index(o-dst_index); *o-dst_index = o-result; } else { -- 2.0.4.315.gad8727a-twtrsrc -- 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] cache-tree: populate cache-tree on successful merge
David Turner dtur...@twopensource.com writes: The work done to produce the cache-tree is work that the commit would otherwise have to do. So we're spending extra time in one place to eliminate that work in a different place. Good point. 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 0/2] negative hideRefs for overriding
At GitHub, we keep some information in a private part of the refs namespace. We use transfer.hideRefs so that users do not see it when fetching or pushing, and that works well. However, sometimes we want to expose part of the hidden namespace (e.g., for an internal fetch or push), and that cannot be done with the existing code. Usually config variables are last-one-wins, so you can simply overwrite any existing values with git -c But for multi-valued config like hideRefs, that just appends to the list, and the ref remains hidden. This series adds negation to hideRefs, which gives us a reasonable last-one-wins setup. That allows git -c to unhide a specific set of refs (though note you have to use --upload-pack='git -c ...' since upload-pack does not share our environment). It also allows more complex config, like hiding all of refs/foo except for refs/foo/bar. We don't use that ourselves, but it might come in handy (and logically falls out of the last-one-wins setup). The first patch is some documentation cleanup. The interesting bit is in the second one. [1/2]: docs/config.txt: reorder hideRefs config [2/2]: refs: support negative transfer.hideRefs -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