[RFC/PATCH] Port branch.c to use ref-filter APIs

2015-07-28 Thread Karthik Nayak
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

2015-07-28 Thread Filippo Gatti

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

2015-07-28 Thread Jakub Narębski

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'

2015-07-28 Thread Matthieu Moy
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

2015-07-28 Thread Christian Couder
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

2015-07-28 Thread Jacob Keller
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

2015-07-28 Thread Christian Couder
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

2015-07-28 Thread Matthieu Moy
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

2015-07-28 Thread Karthik Nayak
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

2015-07-28 Thread Karthik Nayak
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

2015-07-28 Thread Jacob Keller
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

2015-07-28 Thread Christian Couder
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

2015-07-28 Thread Matthieu Moy
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

2015-07-28 Thread Matthieu Moy
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

2015-07-28 Thread Ed Avis
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

2015-07-28 Thread Johannes Sixt

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

2015-07-28 Thread Matthieu Moy
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

2015-07-28 Thread Karthik Nayak
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

2015-07-28 Thread Karthik Nayak
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

2015-07-28 Thread Karthik Nayak
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

2015-07-28 Thread Karthik Nayak
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

2015-07-28 Thread Karthik Nayak
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

2015-07-28 Thread Karthik Nayak
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

2015-07-28 Thread Karthik Nayak
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

2015-07-28 Thread Jacob Keller
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

2015-07-28 Thread Jacob Keller
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

2015-07-28 Thread Junio C Hamano
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

2015-07-28 Thread Junio C Hamano
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

2015-07-28 Thread Johan Herland
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

2015-07-28 Thread Johan Herland
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

2015-07-28 Thread Junio C Hamano
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

2015-07-28 Thread Scott Schmit
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

2015-07-28 Thread Johan Herland
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

2015-07-28 Thread Johan Herland
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

2015-07-28 Thread Junio C Hamano
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

2015-07-28 Thread Jacob Keller
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

2015-07-28 Thread Johan Herland
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

2015-07-28 Thread Junio C Hamano
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

2015-07-28 Thread Jacob Keller
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

2015-07-28 Thread Jacob Keller
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

2015-07-28 Thread Johan Herland
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

2015-07-28 Thread Chris Packham
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

2015-07-28 Thread Matthieu Moy
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

2015-07-28 Thread Matthieu Moy
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

2015-07-28 Thread Matthieu Moy
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

2015-07-28 Thread Matthieu Moy
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'

2015-07-28 Thread Duy Nguyen
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

2015-07-28 Thread Karthik Nayak
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

2015-07-28 Thread Karthik Nayak
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

2015-07-28 Thread Christian Couder
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

2015-07-28 Thread Matthieu Moy
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

2015-07-28 Thread Karthik Nayak
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

2015-07-28 Thread 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).

--
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

2015-07-28 Thread Junio C Hamano
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

2015-07-28 Thread Karthik Nayak
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

2015-07-28 Thread Matthieu Moy
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

2015-07-28 Thread Karthik Nayak
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

2015-07-28 Thread Karthik Nayak
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

2015-07-28 Thread Karthik Nayak
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

2015-07-28 Thread Junio C Hamano
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

2015-07-28 Thread Duy Nguyen
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

2015-07-28 Thread Matthieu Moy
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?

2015-07-28 Thread JASMIE LOAN COMPANY
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

2015-07-28 Thread Karthik Nayak
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

2015-07-28 Thread Karthik Nayak
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

2015-07-28 Thread Junio C Hamano
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

2015-07-28 Thread Junio C Hamano
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

2015-07-28 Thread Johan Herland
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'

2015-07-28 Thread Junio C Hamano
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

2015-07-28 Thread Jeff King
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

2015-07-28 Thread Karthik Nayak
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

2015-07-28 Thread Eric Sunshine
[+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

2015-07-28 Thread Junio C Hamano
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

2015-07-28 Thread Paul Tan
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

2015-07-28 Thread Eric Sunshine
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

2015-07-28 Thread Junio C Hamano
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

2015-07-28 Thread Karthik Nayak
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

2015-07-28 Thread Junio C Hamano
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

2015-07-28 Thread Johannes Sixt

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

2015-07-28 Thread Karthik Nayak
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

2015-07-28 Thread Matthieu Moy
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

2015-07-28 Thread Junio C Hamano
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

2015-07-28 Thread David Turner
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

2015-07-28 Thread Thomas Ackermann

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

2015-07-28 Thread Junio C Hamano
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

2015-07-28 Thread David Turner
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

2015-07-28 Thread Matthieu Moy
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

2015-07-28 Thread Junio C Hamano
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

2015-07-28 Thread Junio C Hamano
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

2015-07-28 Thread David Turner
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

2015-07-28 Thread David Turner
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

2015-07-28 Thread David Turner
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

2015-07-28 Thread David Turner
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

2015-07-28 Thread David Turner
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

2015-07-28 Thread Junio C Hamano
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

2015-07-28 Thread David Turner
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

2015-07-28 Thread Junio C Hamano
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

2015-07-28 Thread David Turner
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

2015-07-28 Thread Junio C Hamano
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

2015-07-28 Thread Jeff King
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


  1   2   >