GSOC: Unify the commands `git tag -l`, `git brach -l` and `git for-each-ref`

2015-08-22 Thread Karthik Nayak
Hello All,

As part of GSoC I'm working on the Unification of 'for-each-ref', 'tag -l'
and 'branch -l'.

Current Progress:

1. Building ref-filter.{c,h} from for-each-ref.
This is the process of creating an initial library for the unification
by moving most of the code from for-each-ref to ref-filter.{c,h}.
Merged into next

2. Add options to ref-filter.
This includes the porting of --points-at, --contains, --merged,
--no-merged options from builtin/branch.c and builtin/tag.c, Also the
implementation of these options into for-each-ref.
Merged to next

3. Port builtin/tag.c to use ref-filter.
Here we port tag.c to use ref-filter and also port the --format,
--sort and --merged and --no-merged options to builtin/tag.c.
Version 13 can be found here:
* http://thread.gmane.org/gmane.comp.version-control.git/276363
There was a lot of discussion on v12 (especially how %(align)...%(end) should be
handling '--quote') which can be found here:
* http://thread.gmane.org/gmane.comp.version-control.git/276133

4. Port builtin/branch.c to use ref-filter filtering APIs.
Here we make use of the filtering APIs provided by ref-filter to filter out
branches. Also make it learn the '--points-at' command.
version 3 can be found here:
* http://thread.gmane.org/gmane.comp.version-control.git/276377

As my mentors suggested, I'm working on the printing aspects of branch.c now.
This involves supporting %(if)%(then)%(else)%(end). and
making branch.c
use show_ref_array_item() rather than using its own implementation.

As the ground has been laid for %(align)%(end) atom, the work shouldn't be
of much difficulty.

Will push most of my work to github: github.com/KarthikNayak/git

-- 
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: Which branch(es) contain certain commits? (was Re: (unknown))

2015-08-22 Thread Ivan Chernyavsky


22.08.2015, 01:39, Junio C Hamano gits...@pobox.com:
  Ivan Chernyavsky campo...@yandex.ru writes:

   Another problem is that builtin/branch.c currently does not use
   setup_revisions(), so I'll have to hook it there as well.

  Heh, you say problem above, but I do not think it is a problem
  per-se. If you want to teach branch some preprocessing based on the
  revision traversal API, you naturally need to call setup_revisions().

I meant my problem mostly ;) Yes I see git-branch currently has it's own way 
of doing things, I assume it avoids more heavy machinery from git-rev-list and 
friends.

  The outlined steps above all feel sensible; one thing you did not
  mention is that you may probably have to clear object flags after
  you are done with the initial --grep revision traversal, as some
  features of branch may want to use the object flags (e.g. --merged
  would use in_merge_bases()). Other than that, all of it sounds
  easily implementable.

I still need to understand a lot how it all works. Any hint on where to look is 
appreciated. (I started from looking on builtin/shortlog.c just because it's 
one of the shortest ones using setup_revisions().)

One thing I'm worried about is that git-branch already has option --all. So 
we'll get a semantics conflict with setup_revisions() (all branches vs all 
refs). This will have to be treated carefully, e.g. retrace and fix effects of 
--all after setup_revisions() to include just branches but not other refs. Will 
such mangling be ok? Or could I prepare the call of setup_revisions() in a way 
that only branches will be included by --all? Anyway the semantics of --all 
will be different for git-branch and git-rev-list, but I believe more important 
is to keep it unchanged for git-branch.

  Note that branch --list, tag --list and for-each-ref are being
  revamped to share more internal code. If you want to pursue this,
  you probably would want to build on top of that effort once it is
  done. That way, you may get tag --grep=FIX123 for free.


This is interesting. So can I have a look on some repo/branch or just wait 
until it'll be merged?

   That said, do you think the goal is worth such changes?

  That is a dangerous question. As Duy already said,

   Probably because nobody is interested and steps up to do it. The lack
   of response to you mail is a sign.

  apparently not many people thought it is worth; otherwise we would
  already have such a feature.

  If you are asking me personally, I'm sorry but I have to say no.

  The reason why I personally do not think your branch --grep=FIX123
  would be very useful to me is because I would imagine that I would
  be interested in learning the exact commit that mentions FIX123 as
  well as which branches contain it, if I had a need for such a
  feature.

  That is, it would inherently be two step process for me anyway, i.e.

   (1) I'd run log -p --grep to find which commits are about FIX123
   and check that what they did indeed make sense; and

   (2) I'd then run branch --contains to learn which ones are
   already up to date with respect to the fix.

  Your branch --grep=FIX123 that only tells me the names of branches
  would have no use in that workflow, as it would not even give me an
  indication that the request --grep=FIX123 found the right commit in
  the first place.

Yes, I see your point. But let me also explain my use case so it'll be probably 
more clear. Sorry if it's too long.

I work for HP as SCM and build manager for a product 15 years old. One of this 
product's greatest strenghts always was support for many old releases and 
customizations for particular users. So right now we are providing support for 
10 subsequent releases (oldest one released back in 2006), and each release has 
5 to 10 customer branches with their specific enhancements and fixes. 
(Though, as you would expect, all the fixes are generally merged to mainline, 
some customers are reluctant to apply fixes they don't need, so even here we 
must be flexible when preparing a patch.) So basically now I'm managing around 
60 public branches in the repository.

SCM was ClearCase at the beginning, then StarTeam (whoa, complete disaster). At 
this point I joined the team and performed migration to Git last year (took a 
whole year). So now our history is a mixture of ancient baselines from 
ClearCase and artificial branches imported from StarTeam (thanks to my 
colleague who wrote a Git helper for StarTeam, 
https://github.com/AndreyPavlenko/git-st). So many changes which are 100% 
identical are represented by different commits, only identifiable with 
--cherry-pick option to git-log and friends. Even if I spent a great amount of 
time manually cutting and glueing history in Git to have the shortest logs.

And obviously, our workflow is based on cherry-picks, because very very often 
fixes and enhancements have to be merged *back* to many old customer branches. 
And you could immagine that when you port 

Re: Which branch(es) contain certain commits? (was Re: (unknown))

2015-08-22 Thread Ivan Chernyavsky


22.08.2015, 12:19, Ivan Chernyavsky campo...@yandex.ru:
   One thing I'm worried about is that git-branch already has option --all. So 
 we'll get a semantics conflict with setup_revisions() (all branches vs all 
 refs). This will have to be treated carefully, e.g. retrace and fix effects 
 of --all after setup_revisions() to include just branches but not other refs. 
 Will such mangling be ok? Or could I prepare the call of setup_revisions() in 
 a way that only branches will be included by --all? Anyway the semantics of 
 --all will be different for git-branch and git-rev-list, but I believe more 
 important is to keep it unchanged for git-branch.


Please disregard this part. I should just gobble --all with the existing 
parse_options() and do not provide it to setup_revisions() altogether.

-- 
  Ivan
--
To unsubscribe from this list: send the line 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: Which branch(es) contain certain commits? (was Re: (unknown))

2015-08-22 Thread Karthik Nayak
On Sat, Aug 22, 2015 at 2:49 PM, Ivan Chernyavsky campo...@yandex.ru wrote

  Note that branch --list, tag --list and for-each-ref are being
  revamped to share more internal code. If you want to pursue this,
  you probably would want to build on top of that effort once it is
  done. That way, you may get tag --grep=FIX123 for free.


 This is interesting. So can I have a look on some repo/branch or just wait 
 until it'll be merged?


This should be what you're looking for:
http://thread.gmane.org/gmane.comp.version-control.git/276363
http://thread.gmane.org/gmane.comp.version-control.git/276377

This is the current progress, the rest is in git/next.

There is still a series of porting the printing part of branch.c to be done.

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


[PATCH v3 4/8] branch: move 'current' check down to the presentation layer

2015-08-22 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 | 22 +++---
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/builtin/branch.c b/builtin/branch.c
index 6ba7a3f..4d9e4d0 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -534,9 +534,10 @@ 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;
+   int current = 0;
int color;
struct strbuf out = STRBUF_INIT, name = STRBUF_INIT;
const char *prefix = ;
@@ -548,15 +549,18 @@ 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))
+   current = 1;
+   else
+   color = BRANCH_COLOR_LOCAL;
break;
case REF_REMOTE_BRANCH:
color = BRANCH_COLOR_REMOTE;
prefix = remote_prefix;
break;
case REF_DETACHED_HEAD:
-   color = BRANCH_COLOR_CURRENT;
desc = to_free = get_head_description();
+   current = 1;
break;
default:
color = BRANCH_COLOR_PLAIN;
@@ -685,21 +689,17 @@ static int print_ref_list(int kinds, int detached, int 
verbose, int abbrev, stru
index = ref_list.index;
 
/* Print detached HEAD 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.5.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


[PATCH v3 5/8] branch: drop non-commit error reporting

2015-08-22 Thread Karthik Nayak
Remove the error reporting variable to make the code easier to port
over to using ref-filter APIs. This variable is not required as in
ref-filter we already check for possible errors and report them.

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 4d9e4d0..8b9da60 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))
@@ -617,7 +614,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;
@@ -642,7 +639,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;
/*
 * First we obtain all regular branch refs and if the HEAD is
 * detached then we insert that ref to the end of the ref_fist
@@ -702,11 +698,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)
@@ -922,15 +913,14 @@ 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;
/*  git branch --local also shows HEAD when it is detached */
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.5.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


[PATCH v3 3/8] branch: roll show_detached HEAD into regular ref_list

2015-08-22 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(). This eliminates the need for an
extra function and helps in easier porting of branch.c to use
ref-filter APIs.

Before show_detached() used to check if the HEAD branch satisfies the
'--contains' option, now that is taken care by append_ref().

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 | 68 +---
 1 file changed, 40 insertions(+), 28 deletions(-)

diff --git a/builtin/branch.c b/builtin/branch.c
index 193296a..6ba7a3f 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)
@@ -535,6 +540,8 @@ 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;
+   char *to_free = NULL;
 
if (item-ignore)
return;
@@ -547,6 +554,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 = to_free = get_head_description();
+   break;
default:
color = BRANCH_COLOR_PLAIN;
break;
@@ -558,7 +569,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),
@@ -581,6 +592,7 @@ static void print_ref_item(struct ref_item *item, int 
maxwidth, int verbose,
}
strbuf_release(name);
strbuf_release(out);
+   free(to_free);
 }
 
 static int calc_maxwidth(struct ref_list *refs, int remote_bonus)
@@ -601,25 +613,9 @@ static int calc_maxwidth(struct ref_list *refs, int 
remote_bonus)
return max;
 }
 
-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.kind = REF_LOCAL_BRANCH;
-   item.dest = NULL;
-   item.commit = head_commit;
-   item.ignore = 0;
-   print_ref_item(item, maxwidth, ref_list-verbose, 
ref_list-abbrev, 1, );
-   free(item.name);
-   }
-}
-
 static int print_ref_list(int kinds, int detached, int verbose, int abbrev, 
struct commit_list *with_commit, const char **pattern)
 {
-   int i;
+   int i, index;
struct append_ref_cb cb;
struct ref_list ref_list;
int maxwidth = 0;
@@ -643,7 +639,14 @@ static int print_ref_list(int kinds, int detached, int 
verbose, int abbrev, stru
cb.ref_list = ref_list;
cb.pattern = pattern;
cb.ret = 0;
+   /*
+* First we obtain all regular branch refs and if the HEAD is
+* detached then we insert that ref to the end of the ref_fist
+* so that it can be printed and removed first.
+*/
for_each_rawref(append_ref, cb);
+   if (detached)
+   head_ref(append_ref, cb);
/*
 * The following implementation is currently duplicated in ref-filter. 
It
 * will eventually be removed when we port branch.c to use ref-filter 
APIs.
@@ -679,15 +682,20 @@ static int print_ref_list(int kinds, int detached, int 
verbose, int abbrev, stru
if (verbose)
maxwidth = calc_maxwidth(ref_list, 

[PATCH v3 0/8] port the filtering part of ref-filter to branch.c

2015-08-22 Thread Karthik Nayak
This is a follow up to porting tag.c to use ref-fitler APIs.

v2 of this patch series can be found here:
http://thread.gmane.org/gmane.comp.version-control.git/276147

Changes made in this series:
* Improve comment in 3/8 and fix grammar in 5/8.
* Fix the test in t1430 to check stderr for the broken ref warning.
* Instead of showing the detached head, reducing the no of array elements
and displaying all of the other refs and then free'ing all of the refs. We
now free the detached head ref immediately after displaying so we don't have
to bother about decrementing and incrementing the no of array elements.
* 

Karthik Nayak (8):
  branch: refactor width computation
  branch: bump get_head_description() to the top
  branch: roll show_detached HEAD into regular ref_list
  branch: move 'current' check down to the presentation layer
  branch: drop non-commit error reporting
  branch.c: use 'ref-filter' data structures
  branch.c: use 'ref-filter' APIs
  branch: add '--points-at' option

 Documentation/git-branch.txt |  13 +-
 builtin/branch.c | 506 +--
 ref-filter.c |   4 +-
 ref-filter.h |   8 +-
 t/t1430-bad-ref-name.sh  |   2 +-
 t/t3203-branch-output.sh |  20 ++
 6 files changed, 197 insertions(+), 356 deletions(-)

Interdiff:

diff --git a/builtin/branch.c b/builtin/branch.c
index dd2fdbe..32a0d11 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -475,7 +475,7 @@ static int calc_maxwidth(struct ref_array *refs, int 
remote_bonus)
 
 static void print_ref_list(struct ref_filter *filter, struct ref_sorting 
*sorting)
 {
-   int i, index;
+   int i;
struct ref_array array;
int maxwidth = 0;
const char *remote_prefix = ;
@@ -493,17 +493,16 @@ static void print_ref_list(struct ref_filter *filter, 
struct ref_sorting *sortin
memset(array, 0, sizeof(array));
 
verify_ref_format(%(refname)%(symref));
-   filter_refs(array, filter, filter-kind);
+   filter_refs(array, filter, filter-kind | FILTER_REFS_INCLUDE_BROKEN);
 
if (filter-verbose)
maxwidth = calc_maxwidth(array, strlen(remote_prefix));
 
-   index = array.nr;
-
/* Print detached HEAD before sorting and printing the rest */
if (filter-kind  FILTER_REFS_DETACHED_HEAD) {
-   format_and_print_ref_item(array.items[index - 1], maxwidth, 
filter, remote_prefix);
-   array.nr -= 1;
+   format_and_print_ref_item(array.items[array.nr - 1], maxwidth, 
filter, remote_prefix);
+   free_array_item(array.items[array.nr - 1]);
+   array.nr--;
}
 
if (!sorting) {
@@ -517,7 +516,6 @@ static void print_ref_list(struct ref_filter *filter, 
struct ref_sorting *sortin
for (i = 0; i  array.nr; i++)
format_and_print_ref_item(array.items[i], maxwidth, filter, 
remote_prefix);
 
-   array.nr = index;
ref_array_clear(array);
 }
 
diff --git a/ref-filter.c b/ref-filter.c
index 112feaa..3cd0c00 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1251,7 +1251,7 @@ static int ref_filter_handler(const char *refname, const 
struct object_id *oid,
 }
 
 /*  Free memory allocated for a ref_array_item */
-static void free_array_item(struct ref_array_item *item)
+void free_array_item(struct ref_array_item *item)
 {
free((char *)item-symref);
free(item);
diff --git a/ref-filter.h b/ref-filter.h
index 3e29e5d..3e25d84 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -89,6 +89,8 @@ struct ref_filter_cbdata {
  * filtered refs in the ref_array structure.
  */
 int filter_refs(struct ref_array *array, struct ref_filter *filter, unsigned 
int type);
+/*  Clear memory allocated to a ref_array_item */
+void free_array_item(struct ref_array_item *item);
 /*  Clear all memory allocated to ref_array */
 void ref_array_clear(struct ref_array *array);
 /*  Parse format string and sort specifiers */
diff --git a/t/t1430-bad-ref-name.sh b/t/t1430-bad-ref-name.sh
index dcf2931..db3627e 100755
--- a/t/t1430-bad-ref-name.sh
+++ b/t/t1430-bad-ref-name.sh
@@ -38,11 +38,11 @@ test_expect_success 'fast-import: fail on invalid branch 
name bad[branch]name'
test_must_fail git fast-import input
 '
 
-test_expect_failure 'git branch shows badly named ref' '
-   cp .git/refs/heads/master .git/refs/heads/broken...ref 
-   test_when_finished rm -f .git/refs/heads/broken...ref 
-   git branch output 
-   grep -e broken\.\.\.ref output
+test_expect_success 'git branch shows badly named ref as warning' '
+   cp .git/refs/heads/master .git/refs/heads/broken...ref 
+   test_when_finished rm -f .git/refs/heads/broken...ref 
+   git branch 2output 
+   grep -e broken\.\.\.ref output
 '
 
 test_expect_success 'branch -d can delete badly named ref' '
diff --git a/t/t3203-branch-output.sh b/t/t3203-branch-output.sh
index 1deb7cb..c819f3e 100755
--- a/t/t3203-branch-output.sh

[PATCH v3 2/8] branch: bump get_head_description() to the top

2015-08-22 Thread Karthik Nayak
This is a preperatory patch for 'roll show_detached HEAD into regular
ref_list'. This patch moves get_head_description() to the top so that
it can be used in print_ref_item().

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 | 62 
 1 file changed, 31 insertions(+), 31 deletions(-)

diff --git a/builtin/branch.c b/builtin/branch.c
index 28a10d6..193296a 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -497,6 +497,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)
 {
@@ -570,37 +601,6 @@ 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 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 show_detached(struct ref_list *ref_list, int maxwidth)
 {
struct commit *head_commit = lookup_commit_reference_gently(head_sha1, 
1);
-- 
2.5.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


[PATCH v3 8/8] branch: add '--points-at' option

2015-08-22 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..211cfc3 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 branches of the given object.
+
 Examples
 
 
diff --git a/builtin/branch.c b/builtin/branch.c
index e0aa44c..32a0d11 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
 };
 
@@ -652,6 +653,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 branches of the object), 0, 
parse_opt_object_name
+   },
OPT_END(),
};
 
@@ -680,7 +685,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 53d166d..c819f3e 100755
--- a/t/t3203-branch-output.sh
+++ b/t/t3203-branch-output.sh
@@ -154,4 +154,13 @@ test_expect_success 'git branch `--sort` option' '
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.5.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


[PATCH v3 6/8] branch.c: use 'ref-filter' data structures

2015-08-22 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.

Make 'free_array_item()' of ref-filter.c a non static function. This
is used to free memory allocated to a detached head ref, before we
print other refs.

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 | 328 ++-
 ref-filter.c |   2 +-
 ref-filter.h |   9 +-
 3 files changed, 141 insertions(+), 198 deletions(-)

diff --git a/builtin/branch.c b/builtin/branch.c
index 8b9da60..5cb7ef0 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;
 
@@ -122,7 +112,7 @@ static int branch_merged(int kind, const char *name,
void *reference_name_to_free = NULL;
int merged;
 
-   if (kind == REF_LOCAL_BRANCH) {
+   if (kind == FILTER_REFS_BRANCHES) {
struct branch *branch = branch_get(name);
const char *upstream = branch_get_upstream(branch, NULL);
unsigned char sha1[20];
@@ -200,14 +190,14 @@ static int delete_branches(int argc, const char **argv, 
int force, int kinds,
struct strbuf bname = STRBUF_INIT;
 
switch (kinds) {
-   case REF_REMOTE_BRANCH:
+   case FILTER_REFS_REMOTES:
fmt = refs/remotes/%s;
/* For subsequent UI messages */
remote_branch = 1;
 
force = 1;
break;
-   case REF_LOCAL_BRANCH:
+   case FILTER_REFS_BRANCHES:
fmt = refs/heads/%s;
break;
default:
@@ -224,7 +214,7 @@ static int delete_branches(int argc, const char **argv, int 
force, int kinds,
int flags = 0;
 
strbuf_branchname(bname, argv[i]);
-   if (kinds == REF_LOCAL_BRANCH  !strcmp(head, bname.buf)) {
+   if (kinds == FILTER_REFS_BRANCHES  !strcmp(head, bname.buf)) {
error(_(Cannot delete the branch '%s' 
  which you are currently on.), bname.buf);
ret = 1;
@@ -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,29 @@ static int match_patterns(const char **pattern, const 
char *refname)
return 0;
 }
 
+/*
+ * Allocate memory for a new ref_array_item and insert that into the
+ * given ref_array. Doesn't take the objectname unlike
+ * new_ref_array_item(). This is a temporary function which will be
+ * removed when we port branch.c to use ref-filter APIs.
+ */
+static struct ref_array_item *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;
+   return ref;
+}
+
 static int append_ref(const 

[PATCH v3 7/8] branch.c: use 'ref-filter' APIs

2015-08-22 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 'branch.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'.

Also remove the 'ignore' variable from ref_array_item as it was
previously used for the '--merged' option and now that is handled by
ref-filter.

The test t1430 'git branch shows badly named ref' has been changed to
check the stderr for the warning regarding the broken ref. This is
done as ref-filter throws a warning for broken refs rather than
directly printing them.

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 | 213 +++
 ref-filter.c |   2 +-
 ref-filter.h |   1 -
 t/t1430-bad-ref-name.sh  |   2 +-
 t/t3203-branch-output.sh |  11 +++
 6 files changed, 56 insertions(+), 182 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 5cb7ef0..e0aa44c 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -270,125 +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;
-}
-
-/*
- * Allocate memory for a new ref_array_item and insert that into the
- * given ref_array. Doesn't take the objectname unlike
- * new_ref_array_item(). This is a temporary function which will be
- * removed when we port branch.c to use ref-filter APIs.
- */
-static struct ref_array_item *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;
-   return 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[] = {
-   { FILTER_REFS_BRANCHES, refs/heads/ },
-   { FILTER_REFS_REMOTES, refs/remotes/ },
-   };
-
-   /* Detect kind */
-   for (i = 0; i  ARRAY_SIZE(ref_kind); i++) {
-   prefix = 

[PATCH v3 1/8] branch: refactor width computation

2015-08-22 Thread Karthik Nayak
From: Karthik Nayak karthik@gmail.com

Remove unnecessary variables from ref_list and ref_item which were
used for width computation. This is to make ref_item similar to
ref-filter's ref_array_item. This will ensure a smooth port of
branch.c to use ref-filter APIs in further patches.

Previously the maxwidth was computed when inserting the refs into the
ref_list. Now, we obtain the entire ref_list and then compute
maxwidth.

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 | 64 ++--
 1 file changed, 34 insertions(+), 30 deletions(-)

diff --git a/builtin/branch.c b/builtin/branch.c
index 4fc8beb..28a10d6 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,22 @@ 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++) {
-   if (refs-list[i].ignore)
+   struct ref_item *it = refs-list[i];
+   int w;
+
+   if (it-ignore)
continue;
-   if (refs-list[i].width  w)
-   w = refs-list[i].width;
+   w = utf8_strwidth(it-name);
+   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 +601,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 +622,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 

Re: [PATCH v4] git-p4: fix faulty paths for case insensitive systems

2015-08-22 Thread Luke Diamand
Lars - thanks for persisting with this!

I'm still trying to fully understand what's going on here - can you
point out where I've got it wrong below please!

The server is on Linux, and is case-sensitive. For whatever reason
(probably people committing changes on Windows in the first place)
we've ended up with a P4 repo that has differences in path case that
need to be ignored, with all upper-case paths being mapped to
lower-case.

e.g. the *real* depot might be:

//depot/path/File1
//depot/PATH/File2

git-p4 clone should return this case-folded on Windows (and Linux?)

$GIT_DIR/path/File1
$GIT_DIR/path/File2

(Am I right in thinking that this change folds the directory, but not
the filename?)

I don't really understand why a dictionary is required to do this
though - is there a reason we can't just lowercase all incoming paths?
Which is what the existing code in p4StartWith() is trying to do. That
code lowercases the *entire* path including the filename; this change
seems to leave the filename portion unchanged. I guess though that the
answer may be to do with your finding that P4 makes up the case of
directories based on lexicographic ordering - is that the basic
problem?

For a large repo, this approach is surely going to be really slow.

Additionally, could you update your patch to add some words to
Documentation/git-p4.txt please?

A few other comments inline.

Thanks!
Luke



On 21 August 2015 at 18:19,  larsxschnei...@gmail.com wrote:
 From: Lars Schneider larsxschnei...@gmail.com

 PROBLEM:
 We run P4 servers on Linux and P4 clients on Windows. For an unknown
 reason the file path for a number of files in P4 does not match the
 directory path with respect to case sensitivity.

 E.g. `p4 files` might return
 //depot/path/to/file1
 //depot/PATH/to/file2

 If you use P4/P4V then these files end up in the same directory, e.g.
 //depot/path/to/file1
 //depot/path/to/file2

 If you use git-p4 then all files not matching the correct file path
 (e.g. `file2`) will be ignored.

I'd like to think that the existing code that checks core.ignorecase
should handle this. I haven't tried it myself though.



 SOLUTION:
 Identify path names that are different with respect to case sensitivity.
 If there are any then run `p4 dirs` to build up a dictionary
 containing the correct cases for each path. It looks like P4
 interprets correct here as the existing path of the first file in a
 directory. The path dictionary is used later on to fix all paths.

 This is only applied if the parameter --ignore-path-case is passed to
 the git-p4 clone command.

 Signed-off-by: Lars Schneider larsxschnei...@gmail.com
 ---
  git-p4.py |  82 ++--
  t/t9821-git-p4-path-variations.sh | 109 
 ++
  2 files changed, 187 insertions(+), 4 deletions(-)
  create mode 100755 t/t9821-git-p4-path-variations.sh

 diff --git a/git-p4.py b/git-p4.py
 index 073f87b..61a587b 100755
 --- a/git-p4.py
 +++ b/git-p4.py
 @@ -1931,7 +1931,7 @@ class View(object):
  (self.client_prefix, clientFile))
  return clientFile[len(self.client_prefix):]

 -def update_client_spec_path_cache(self, files):
 +def update_client_spec_path_cache(self, files, fixPathCase = None):
   Caching file paths by p4 where batch query 

  # List depot file paths exclude that already cached
 @@ -1950,6 +1950,8 @@ class View(object):
  if unmap in res:
  # it will list all of them, but only one not unmap-ped
  continue
 +if fixPathCase:
 +res['depotFile'] = fixPathCase(res['depotFile'])
  self.client_spec_path_cache[res['depotFile']] = 
 self.convert_client_path(res[clientFile])

  # not found files or unmap files set to 
 @@ -1987,6 +1989,7 @@ class P4Sync(Command, P4UserMap):
   help=Maximum number of changes to 
 import),
  optparse.make_option(--changes-block-size, 
 dest=changes_block_size, type=int,
   help=Internal block size to use when 
 iteratively calling p4 changes),
 +optparse.make_option(--ignore-path-case, 
 dest=ignorePathCase, action=store_true),
  optparse.make_option(--keep-path, dest=keepRepoPath, 
 action='store_true',
   help=Keep entire BRANCH/DIR/SUBDIR 
 prefix during import),
  optparse.make_option(--use-client-spec, 
 dest=useClientSpec, action='store_true',
 @@ -2017,6 +2020,7 @@ class P4Sync(Command, P4UserMap):
  self.maxChanges = 
  self.changes_block_size = None
  self.keepRepoPath = False
 +self.ignorePathCase = False
  self.depotPaths = None
  self.p4BranchesInGit = []
  self.cloneExclude = []
 @@ -2049,7 +2053,8 @@ class P4Sync(Command, P4UserMap):
  files = []
  fnum = 0
  while 

[PATCH 3/3] archive-zip: support more than 65535 entries

2015-08-22 Thread René Scharfe
Support more than 65535 entries cleanly by writing a zip64 end of
central directory record (with a 64-bit field for the number of
entries) before the usual end of central directory record (which
contains only a 16-bit field).  InfoZIP's zip does the same.
Archives with 65535 or less entries are not affected.

Programs that extract all files like InfoZIP's zip and 7-Zip
ignored the field and could extract all files already.  Software
that relies on the ZIP file directory to show a list of contained
files quickly to simulate to normal directory like Windows'
built-in ZIP functionality only saw a subset of the included files.

Windows supports ZIP64 since Vista according to
https://en.wikipedia.org/wiki/Zip_%28file_format%29#ZIP64.

Suggested-by: Johannes Schauer jo...@debian.org
Signed-off-by: Rene Scharfe l@web.de
---
 archive-zip.c   | 93 +++--
 t/t5004-archive-corner-cases.sh |  2 +-
 2 files changed, 91 insertions(+), 4 deletions(-)

diff --git a/archive-zip.c b/archive-zip.c
index 2a76156..9db4735 100644
--- a/archive-zip.c
+++ b/archive-zip.c
@@ -16,7 +16,9 @@ static unsigned int zip_dir_size;
 
 static unsigned int zip_offset;
 static unsigned int zip_dir_offset;
-static unsigned int zip_dir_entries;
+static uint64_t zip_dir_entries;
+
+static unsigned int max_creator_version;
 
 #define ZIP_DIRECTORY_MIN_SIZE (1024 * 1024)
 #define ZIP_STREAM (1   3)
@@ -86,6 +88,28 @@ struct zip_extra_mtime {
unsigned char _end[1];
 };
 
+struct zip64_dir_trailer {
+   unsigned char magic[4];
+   unsigned char record_size[8];
+   unsigned char creator_version[2];
+   unsigned char version[2];
+   unsigned char disk[4];
+   unsigned char directory_start_disk[4];
+   unsigned char entries_on_this_disk[8];
+   unsigned char entries[8];
+   unsigned char size[8];
+   unsigned char offset[8];
+   unsigned char _end[1];
+};
+
+struct zip64_dir_trailer_locator {
+   unsigned char magic[4];
+   unsigned char disk[4];
+   unsigned char offset[8];
+   unsigned char number_of_disks[4];
+   unsigned char _end[1];
+};
+
 /*
  * On ARM, padding is added at the end of the struct, so a simple
  * sizeof(struct ...) reports two bytes more than the payload size
@@ -98,6 +122,12 @@ struct zip_extra_mtime {
 #define ZIP_EXTRA_MTIME_SIZE   offsetof(struct zip_extra_mtime, _end)
 #define ZIP_EXTRA_MTIME_PAYLOAD_SIZE \
(ZIP_EXTRA_MTIME_SIZE - offsetof(struct zip_extra_mtime, flags))
+#define ZIP64_DIR_TRAILER_SIZE offsetof(struct zip64_dir_trailer, _end)
+#define ZIP64_DIR_TRAILER_RECORD_SIZE \
+   (ZIP64_DIR_TRAILER_SIZE - \
+offsetof(struct zip64_dir_trailer, creator_version))
+#define ZIP64_DIR_TRAILER_LOCATOR_SIZE \
+   offsetof(struct zip64_dir_trailer_locator, _end)
 
 static void copy_le16(unsigned char *dest, unsigned int n)
 {
@@ -113,6 +143,31 @@ static void copy_le32(unsigned char *dest, unsigned int n)
dest[3] = 0xff  (n  030);
 }
 
+static void copy_le64(unsigned char *dest, uint64_t n)
+{
+   dest[0] = 0xff  n;
+   dest[1] = 0xff  (n  010);
+   dest[2] = 0xff  (n  020);
+   dest[3] = 0xff  (n  030);
+   dest[4] = 0xff  (n  040);
+   dest[5] = 0xff  (n  050);
+   dest[6] = 0xff  (n  060);
+   dest[7] = 0xff  (n  070);
+}
+
+static uint64_t clamp_max(uint64_t n, uint64_t max, int *clamped)
+{
+   if (n = max)
+   return n;
+   *clamped = 1;
+   return max;
+}
+
+static void copy_le16_clamp(unsigned char *dest, uint64_t n, int *clamped)
+{
+   copy_le16(dest, clamp_max(n, 0x, clamped));
+}
+
 static void *zlib_deflate_raw(void *data, unsigned long size,
  int compression_level,
  unsigned long *compressed_size)
@@ -282,6 +337,9 @@ static int write_zip_entry(struct archiver_args *args,
sha1_to_hex(sha1));
}
 
+   if (creator_version  max_creator_version)
+   max_creator_version = creator_version;
+
if (buffer  method == 8) {
out = deflated = zlib_deflate_raw(buffer, size,
  args-compression_level,
@@ -439,20 +497,49 @@ static int write_zip_entry(struct archiver_args *args,
return 0;
 }
 
+static void write_zip64_trailer(void)
+{
+   struct zip64_dir_trailer trailer64;
+   struct zip64_dir_trailer_locator locator64;
+
+   copy_le32(trailer64.magic, 0x06064b50);
+   copy_le64(trailer64.record_size, ZIP64_DIR_TRAILER_RECORD_SIZE);
+   copy_le16(trailer64.creator_version, max_creator_version);
+   copy_le16(trailer64.version, 45);
+   copy_le32(trailer64.disk, 0);
+   copy_le32(trailer64.directory_start_disk, 0);
+   copy_le64(trailer64.entries_on_this_disk, zip_dir_entries);
+   copy_le64(trailer64.entries, zip_dir_entries);
+   copy_le64(trailer64.size, zip_dir_offset);
+

[PATCH 2/3] archive-zip: use a local variable to store the creator version

2015-08-22 Thread René Scharfe
Use a simpler conditional right next to the code which makes a higher
creator version necessary -- namely symlink handling and support for
executable files -- instead of a long line with a ternary operator.
The resulting code has more lines but is simpler and allows reuse of
the value easily.

Signed-off-by: Rene Scharfe l@web.de
---
 archive-zip.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/archive-zip.c b/archive-zip.c
index ae3d67f..2a76156 100644
--- a/archive-zip.c
+++ b/archive-zip.c
@@ -223,6 +223,7 @@ static int write_zip_entry(struct archiver_args *args,
unsigned long size;
int is_binary = -1;
const char *path_without_prefix = path + args-baselen;
+   unsigned int creator_version = 0;
 
crc = crc32(0, NULL, 0);
 
@@ -251,6 +252,8 @@ static int write_zip_entry(struct archiver_args *args,
method = 0;
attr2 = S_ISLNK(mode) ? ((mode | 0777)  16) :
(mode  0111) ? ((mode)  16) : 0;
+   if (S_ISLNK(mode) || (mode  0111))
+   creator_version = 0x0317;
if (S_ISREG(mode)  args-compression_level != 0  size  0)
method = 8;
 
@@ -303,8 +306,7 @@ static int write_zip_entry(struct archiver_args *args,
}
 
copy_le32(dirent.magic, 0x02014b50);
-   copy_le16(dirent.creator_version,
-   S_ISLNK(mode) || (S_ISREG(mode)  (mode  0111)) ? 0x0317 : 0);
+   copy_le16(dirent.creator_version, creator_version);
copy_le16(dirent.version, 10);
copy_le16(dirent.flags, flags);
copy_le16(dirent.compression_method, method);
-- 
2.5.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


[PATCH 1/3] t5004: test ZIP archives with many entries

2015-08-22 Thread René Scharfe
A ZIP file directory has a 16-bit field for the number of entries it
contains.  There are 64-bit extensions to deal with that.  Demonstrate
that git archive --format=zip currently doesn't use them and instead
overflows the field.

InfoZIP's unzip doesn't care about this field and extracts all files
anyway.  Software that uses the directory for presenting a filesystem
like view quickly -- notably Windows -- depends on it, but doesn't
lend itself to an automatic test case easily.  Use InfoZIP's zipinfo,
which probably isn't available everywhere but at least can provides
*some* way to check this field.

To speed things up a bit create and commit only a subset of the files
and build a fake tree out of duplicates and pass that to git archive.

Signed-off-by: Rene Scharfe l@web.de
---
 t/t5004-archive-corner-cases.sh | 40 
 1 file changed, 40 insertions(+)

diff --git a/t/t5004-archive-corner-cases.sh b/t/t5004-archive-corner-cases.sh
index 654adda..c6bd729 100755
--- a/t/t5004-archive-corner-cases.sh
+++ b/t/t5004-archive-corner-cases.sh
@@ -115,4 +115,44 @@ test_expect_success 'archive empty subtree by direct 
pathspec' '
check_dir extract sub
 '
 
+ZIPINFO=zipinfo
+
+test_lazy_prereq ZIPINFO '
+   n=$($ZIPINFO $TEST_DIRECTORY/t5004/empty.zip | sed -n 2s/.* //p)
+   test x$n = x0
+'
+
+test_expect_failure ZIPINFO 'zip archive with many entries' '
+   # add a directory with 256 files
+   mkdir 00 
+   for a in 0 1 2 3 4 5 6 7 8 9 a b c d e f
+   do
+   for b in 0 1 2 3 4 5 6 7 8 9 a b c d e f
+   do
+   : 00/$a$b
+   done
+   done 
+   git add 00 
+   git commit -m 256 files in 1 directory 
+
+   # duplicate it to get 65536 files in 256 directories
+   subtree=$(git write-tree --prefix=00/) 
+   for c in 0 1 2 3 4 5 6 7 8 9 a b c d e f
+   do
+   for d in 0 1 2 3 4 5 6 7 8 9 a b c d e f
+   do
+   echo 04 tree $subtree  $c$d
+   done
+   done tree 
+   tree=$(git mktree tree) 
+
+   # zip them
+   git archive -o many.zip $tree 
+
+   # check the number of entries in the ZIP file directory
+   expr 65536 + 256 expect 
+   $ZIPINFO many.zip | head -2 | sed -n 2s/.* //p actual 
+   test_cmp expect actual
+'
+
 test_done
-- 
2.5.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


[PATCH v5 0/2] Worktree: for-each function and list command

2015-08-22 Thread Michael Rappazzo
Changes since v4:
 - reduced memory usage by reusing string buffer variables
 - re-scoped variables in the for-each function
 - added tests for 'worktree list' with bare repos

Notes from previous discussion:
 - The following snippet:

 + /* If the common_dir DOES NOT end with '/.git', then it is bare */
 + main_is_bare = !strbuf_strip_suffix(worktree_path, /.git);

This code is run when the current dir is in a linked worktree (not the primary 
worktree).  In that context, the git config _does_ report that 'core.bare' is
true when the primary repo is bare, however `is_bare_repository()` returns
false.  The worktree_path also needs to the /.git removed from it.  
Therefore, I opted to keep the code like this.


Michael Rappazzo (2):
  worktree: add 'for_each_worktree' function
  worktree: add 'list' command

 Documentation/git-worktree.txt |  11 +++-
 builtin/worktree.c | 138 +
 t/t2027-worktree-list.sh   |  81 
 3 files changed, 229 insertions(+), 1 deletion(-)
 create mode 100755 t/t2027-worktree-list.sh

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


[PATCH v5 2/2] worktree: add 'list' command

2015-08-22 Thread Michael Rappazzo
'git worktree list' uses the for_each_worktree function to iterate,
and outputs in the format: 'worktree  (short-ref)'

Signed-off-by: Michael Rappazzo rappa...@gmail.com
---
 Documentation/git-worktree.txt | 11 +-
 builtin/worktree.c | 55 
 t/t2027-worktree-list.sh   | 81 ++
 3 files changed, 146 insertions(+), 1 deletion(-)
 create mode 100755 t/t2027-worktree-list.sh

diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
index fb68156..e953b4e 100644
--- a/Documentation/git-worktree.txt
+++ b/Documentation/git-worktree.txt
@@ -11,6 +11,7 @@ SYNOPSIS
 [verse]
 'git worktree add' [-f] [--detach] [-b new-branch] path [branch]
 'git worktree prune' [-n] [-v] [--expire expire]
+'git worktree list' [--path-only]
 
 DESCRIPTION
 ---
@@ -59,6 +60,12 @@ prune::
 
 Prune working tree information in $GIT_DIR/worktrees.
 
+list::
+
+List the main worktree followed by all of the linked worktrees.  The default
+format of the list includes the full path to the worktree and the branch or
+revision that the head of that worktree is currently pointing to.
+
 OPTIONS
 ---
 
@@ -93,6 +100,9 @@ OPTIONS
 --expire time::
With `prune`, only expire unused working trees older than time.
 
+--path-only
+   With `list`, only show the worktree path
+
 DETAILS
 ---
 Each linked working tree has a private sub-directory in the repository's
@@ -167,7 +177,6 @@ performed manually, such as:
 - `remove` to remove a linked working tree and its administrative files (and
   warn if the working tree is dirty)
 - `mv` to move or rename a working tree and update its administrative files
-- `list` to list linked working trees
 - `lock` to prevent automatic pruning of administrative files (for instance,
   for a working tree on a portable device)
 
diff --git a/builtin/worktree.c b/builtin/worktree.c
index 7b3cb96..673f292 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -12,6 +12,7 @@
 static const char * const worktree_usage[] = {
N_(git worktree add [options] path branch),
N_(git worktree prune [options]),
+   N_(git worktree list [options]),
NULL
 };
 
@@ -442,6 +443,58 @@ done:
return ret;
 }
 
+/* list callback data */
+struct list_opts {
+   int path_only;
+};
+
+static int print_worktree_details(const char *path, const char *git_dir, void 
*cb_data)
+{
+   struct strbuf head_file = STRBUF_INIT;
+   struct strbuf head_ref = STRBUF_INIT;
+   struct stat st;
+   struct list_opts *opts = cb_data;
+   const char *ref_prefix = ref: refs/heads/;
+
+   strbuf_addf(head_file, %s/HEAD, git_dir);
+   if (!opts-path_only  !stat(head_file.buf, st)) {
+   strbuf_read_file(head_ref, head_file.buf, st.st_size);
+   strbuf_strip_suffix(head_ref, \n);
+
+   if (starts_with(head_ref.buf, ref_prefix)) {
+   /* branch checked out */
+   strbuf_remove(head_ref, 0, strlen(ref_prefix));
+   /* } else {
+*  headless -- no-op
+*/
+   }
+   printf(%s  (%s)\n, path, head_ref.buf);
+   } else {
+   printf(%s\n, path);
+   }
+
+   strbuf_release(head_ref);
+   strbuf_release(head_file);
+   return 0;
+}
+
+static int list(int ac, const char **av, const char *prefix)
+{
+   struct list_opts opts;
+   struct option options[] = {
+   OPT_BOOL(0, path-only, opts.path_only, N_(only show the 
path of the worktree)),
+   OPT_END()
+   };
+
+   opts.path_only = 0;
+
+   ac = parse_options(ac, av, prefix, options, worktree_usage, 0);
+   if (ac)
+   usage_with_options(worktree_usage, options);
+
+   return for_each_worktree(print_worktree_details, opts);
+}
+
 int cmd_worktree(int ac, const char **av, const char *prefix)
 {
struct option options[] = {
@@ -454,5 +507,7 @@ int cmd_worktree(int ac, const char **av, const char 
*prefix)
return add(ac - 1, av + 1, prefix);
if (!strcmp(av[1], prune))
return prune(ac - 1, av + 1, prefix);
+   if (!strcmp(av[1], list))
+   return list(ac - 1, av + 1, prefix);
usage_with_options(worktree_usage, options);
 }
diff --git a/t/t2027-worktree-list.sh b/t/t2027-worktree-list.sh
new file mode 100755
index 000..66a635a
--- /dev/null
+++ b/t/t2027-worktree-list.sh
@@ -0,0 +1,81 @@
+#!/bin/sh
+
+test_description='test git worktree list'
+
+. ./test-lib.sh
+
+test_expect_success 'setup' '
+   test_commit init
+'
+
+test_expect_success 'list all worktrees from main' '
+   echo $(git rev-parse --show-toplevel)  ($(git symbolic-ref --short 
HEAD)) expect 
+   git worktree add --detach here master 
+   echo $(git -C here rev-parse --show-toplevel)  ($(git -C here 
rev-parse HEAD)) expect 
+   git worktree 

[PATCH v5 1/2] worktree: add 'for_each_worktree' function

2015-08-22 Thread Michael Rappazzo
for_each_worktree iterates through each worktree and invokes a callback
function.  The main worktree (if not bare) is always encountered first,
followed by worktrees created by `git worktree add`.

If the callback function returns a non-zero value, iteration stops, and
the callback's return value is returned from the for_each_worktree
function.

Signed-off-by: Michael Rappazzo rappa...@gmail.com
---
 builtin/worktree.c | 83 ++
 1 file changed, 83 insertions(+)

diff --git a/builtin/worktree.c b/builtin/worktree.c
index 430b51e..7b3cb96 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -26,6 +26,14 @@ static int show_only;
 static int verbose;
 static unsigned long expire;
 
+/*
+ * The signature for the callback function for the for_each_worktree()
+ * function below.  The memory pointed to by the callback arguments
+ * is only guaranteed to be valid for the duration of a single
+ * callback invocation.
+ */
+typedef int each_worktree_fn(const char *path, const char *git_dir, void 
*cb_data);
+
 static int prune_worktree(const char *id, struct strbuf *reason)
 {
struct stat st;
@@ -359,6 +367,81 @@ static int add(int ac, const char **av, const char *prefix)
return add_worktree(path, branch, opts);
 }
 
+/*
+ * Iterate through each worktree and invoke the callback function.  If
+ * the callback function returns non-zero, the iteration stops, and
+ * this function returns that return value
+ */
+static int for_each_worktree(each_worktree_fn fn, void *cb_data)
+{
+   struct strbuf worktree_path = STRBUF_INIT;
+   struct strbuf worktree_git = STRBUF_INIT;
+   const char *common_dir;
+   int main_is_bare = 0;
+   int ret = 0;
+
+   common_dir = get_git_common_dir();
+   if (!strcmp(common_dir, get_git_dir())) {
+   /* simple case - this is the main repo */
+   main_is_bare = is_bare_repository();
+   if (!main_is_bare) {
+   strbuf_addstr(worktree_path, get_git_work_tree());
+   } else {
+   strbuf_addstr(worktree_path, common_dir);
+   }
+   } else {
+   strbuf_addstr(worktree_path, common_dir);
+   /* If the common_dir DOES NOT end with '/.git', then it is bare 
*/
+   main_is_bare = !strbuf_strip_suffix(worktree_path, /.git);
+   }
+   strbuf_addstr(worktree_git, worktree_path.buf);
+
+   if (!main_is_bare) {
+   strbuf_addstr(worktree_git, /.git);
+
+   ret = fn(worktree_path.buf, worktree_git.buf, cb_data);
+   if (ret)
+   goto done;
+   }
+   strbuf_addstr(worktree_git, /worktrees);
+
+   if (is_directory(worktree_git.buf)) {
+   DIR *dir = opendir(worktree_git.buf);
+   if (dir) {
+   struct stat st;
+   struct dirent *d;
+   size_t base_path_len = worktree_git.len;
+
+   while ((d = readdir(dir)) != NULL) {
+   if (!strcmp(d-d_name, .) || 
!strcmp(d-d_name, ..))
+   continue;
+
+   strbuf_setlen(worktree_git, base_path_len);
+   strbuf_addf(worktree_git, /%s/gitdir, 
d-d_name);
+
+   if (stat(worktree_git.buf, st)) {
+   fprintf(stderr, Unable to read 
worktree git dir: %s\n, worktree_git.buf);
+   continue;
+   }
+
+   strbuf_reset(worktree_path);
+   strbuf_read_file(worktree_path, 
worktree_git.buf, st.st_size);
+   strbuf_strip_suffix(worktree_path, /.git\n);
+
+   strbuf_strip_suffix(worktree_git, /gitdir);
+   ret = fn(worktree_path.buf, worktree_git.buf, 
cb_data);
+   if (ret)
+   break;
+   }
+   }
+   closedir(dir);
+   }
+done:
+   strbuf_release(worktree_git);
+   strbuf_release(worktree_path);
+   return ret;
+}
+
 int cmd_worktree(int ac, const char **av, const char *prefix)
 {
struct option options[] = {
-- 
2.5.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 1/3] t5004: test ZIP archives with many entries

2015-08-22 Thread Eric Sunshine
On Sat, Aug 22, 2015 at 3:06 PM, René Scharfe l@web.de wrote:
 A ZIP file directory has a 16-bit field for the number of entries it
 contains.  There are 64-bit extensions to deal with that.  Demonstrate
 that git archive --format=zip currently doesn't use them and instead
 overflows the field.

 InfoZIP's unzip doesn't care about this field and extracts all files
 anyway.  Software that uses the directory for presenting a filesystem
 like view quickly -- notably Windows -- depends on it, but doesn't
 lend itself to an automatic test case easily.  Use InfoZIP's zipinfo,
 which probably isn't available everywhere but at least can provides
 *some* way to check this field.

 To speed things up a bit create and commit only a subset of the files
 and build a fake tree out of duplicates and pass that to git archive.

 Signed-off-by: Rene Scharfe l@web.de
 ---
 diff --git a/t/t5004-archive-corner-cases.sh b/t/t5004-archive-corner-cases.sh
 index 654adda..c6bd729 100755
 --- a/t/t5004-archive-corner-cases.sh
 +++ b/t/t5004-archive-corner-cases.sh
 @@ -115,4 +115,44 @@ test_expect_success 'archive empty subtree by direct 
 pathspec' '
 check_dir extract sub
  '

 +ZIPINFO=zipinfo
 +
 +test_lazy_prereq ZIPINFO '
 +   n=$($ZIPINFO $TEST_DIRECTORY/t5004/empty.zip | sed -n 2s/.* //p)
 +   test x$n = x0
 +'

Unfortunately, this sed expression isn't portable due to dissimilar
output of various zipinfo implementations. On Linux, the output of
zipinfo is:

$ zipinfo t/t5004/empty.zip
Archive:  t/t5004/empty.zip
Zip file size: 62 bytes, number of entries: 0
Empty zipfile.
$

however, on Mac OS X:

$ zipinfo t/t5004/empty.zip
Archive:  t/t5004/empty.zip   62 bytes   0 files
Empty zipfile.
$

and on FreeBSD, the zipinfo command seems to have been removed
altogether in favor of unzip -Z (emulate zipinfo).

One might hope that unzip -Z would be a reasonable replacement for
zipinfo, however, it is apparently only partially implemented on
FreeBSD, and requires that -1 be passed, as well. Even with unzip -Z
-1, there are issues. The output on Linux and Mac OS X is:

$ unzip -Z -1 t/t5004/empty.zip
Empty zipfile.
$

but FreeBSD differs:

$ unzip -Z -1 t/t5004/empty.zip
$

With a non-empty zip file, the output is identical on all platforms:

$ unzip -Z -1 twofiles.zip
file1
file2
$

So, if you combine that with wc -l or test_line_count, you may have
a portable and reliable entry counter.

More below...

 +test_expect_failure ZIPINFO 'zip archive with many entries' '
 +   # add a directory with 256 files
 +   mkdir 00 
 +   for a in 0 1 2 3 4 5 6 7 8 9 a b c d e f
 +   do
 +   for b in 0 1 2 3 4 5 6 7 8 9 a b c d e f
 +   do
 +   : 00/$a$b
 +   done
 +   done 
 +   git add 00 
 +   git commit -m 256 files in 1 directory 
 +
 +   # duplicate it to get 65536 files in 256 directories
 +   subtree=$(git write-tree --prefix=00/) 
 +   for c in 0 1 2 3 4 5 6 7 8 9 a b c d e f
 +   do
 +   for d in 0 1 2 3 4 5 6 7 8 9 a b c d e f
 +   do
 +   echo 04 tree $subtree  $c$d
 +   done
 +   done tree 
 +   tree=$(git mktree tree) 
 +
 +   # zip them
 +   git archive -o many.zip $tree 
 +
 +   # check the number of entries in the ZIP file directory
 +   expr 65536 + 256 expect 
 +   $ZIPINFO many.zip | head -2 | sed -n 2s/.* //p actual 

With these three patches applied, Mac OS X has trouble with 'many.zip':

$ unzip -Z -1 many.zip
warning [many.zip]:  76 extra bytes at beginning or within zipfile
  (attempting to process anyway)
error [many.zip]:  reported length of central directory is
  -76 bytes too long (Atari STZip zipfile?  J.H.Holm ZIPSPLIT 1.1
  zipfile?).  Compensating...
00/
00/00
...
ff/ff
error: expected central file header signature not found (file
  #65793). (please check that you have transferred or created the
  zipfile in the appropriate BINARY mode and that you have compiled
  UnZip properly)

And FreeBSD doesn't like it either:

$ unzip -Z -1 many.zip
unzip: Invalid central directory signature
$

 +   test_cmp expect actual
 +'
 +
  test_done
 --
 2.5.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


[PATCH] am: terminate state files with a newline

2015-08-22 Thread Paul Tan
On Thu, Aug 20, 2015 at 11:40:20AM -0700, Junio C Hamano wrote:
 SZEDER Gábor sze...@ira.uka.de writes:
 
  The format of the files '.git/rebase-apply/{next,last}' changed slightly
  with the recent builtin 'git am' conversion: while these files were
  newline-terminated when written by the scripted version, the ones written
  by the builtin are not.
 
 Thanks for noticing; that should be corrected, I think.

Okay then, this patch should correct this.

Did we ever explictly allow external programs to poke around the
contents of the .git/rebase-apply directory? I think it may not be so
good, as it means that it may not be possible to switch the storage
format in the future (e.g. to allow atomic modifications, maybe?) :-/ .

Regards,
Paul

-- 8 --
Subject: [PATCH] am: terminate state files with a newline

Since builtin/am.c replaced git-am.sh in 783d7e8 (builtin-am: remove
redirection to git-am.sh, 2015-08-04), the state files written by git-am
did not terminate with a newline.

This is because the code in builtin/am.c did not write the newline to
the state files.

While the git codebase has no problems with the missing newline,
external software which read the contents of the state directory may be
strict about the existence of the terminating newline, and would thus
break.

Fix this by correcting the relevant calls to write_file() to ensure that
the state files written terminate with a newline, matching how git-am.sh
behaves.

While we are fixing the write_file() calls, fix the writing of the
dirtyindex file as well -- we should be creating an empty file to
match the behavior of git-am.sh.

Reported-by: SZEDER Gábor sze...@ira.uka.de
Signed-off-by: Paul Tan pyoka...@gmail.com
---
 builtin/am.c | 30 +++---
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/builtin/am.c b/builtin/am.c
index 1399c8d..2e57fad 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -994,13 +994,13 @@ static void am_setup(struct am_state *state, enum 
patch_format patch_format,
if (state-rebasing)
state-threeway = 1;
 
-   write_file(am_path(state, threeway), 1, state-threeway ? t : f);
+   write_file(am_path(state, threeway), 1, %s\n, state-threeway ? t 
: f);
 
-   write_file(am_path(state, quiet), 1, state-quiet ? t : f);
+   write_file(am_path(state, quiet), 1, %s\n, state-quiet ? t : 
f);
 
-   write_file(am_path(state, sign), 1, state-signoff ? t : f);
+   write_file(am_path(state, sign), 1, %s\n, state-signoff ? t : 
f);
 
-   write_file(am_path(state, utf8), 1, state-utf8 ? t : f);
+   write_file(am_path(state, utf8), 1, %s\n, state-utf8 ? t : f);
 
switch (state-keep) {
case KEEP_FALSE:
@@ -1016,9 +1016,9 @@ static void am_setup(struct am_state *state, enum 
patch_format patch_format,
die(BUG: invalid value for state-keep);
}
 
-   write_file(am_path(state, keep), 1, %s, str);
+   write_file(am_path(state, keep), 1, %s\n, str);
 
-   write_file(am_path(state, messageid), 1, state-message_id ? t : 
f);
+   write_file(am_path(state, messageid), 1, %s\n, state-message_id ? 
t : f);
 
switch (state-scissors) {
case SCISSORS_UNSET:
@@ -1034,10 +1034,10 @@ static void am_setup(struct am_state *state, enum 
patch_format patch_format,
die(BUG: invalid value for state-scissors);
}
 
-   write_file(am_path(state, scissors), 1, %s, str);
+   write_file(am_path(state, scissors), 1, %s\n, str);
 
sq_quote_argv(sb, state-git_apply_opts.argv, 0);
-   write_file(am_path(state, apply-opt), 1, %s, sb.buf);
+   write_file(am_path(state, apply-opt), 1, %s\n, sb.buf);
 
if (state-rebasing)
write_file(am_path(state, rebasing), 1, %s, );
@@ -1045,7 +1045,7 @@ static void am_setup(struct am_state *state, enum 
patch_format patch_format,
write_file(am_path(state, applying), 1, %s, );
 
if (!get_sha1(HEAD, curr_head)) {
-   write_file(am_path(state, abort-safety), 1, %s, 
sha1_to_hex(curr_head));
+   write_file(am_path(state, abort-safety), 1, %s\n, 
sha1_to_hex(curr_head));
if (!state-rebasing)
update_ref(am, ORIG_HEAD, curr_head, NULL, 0,
UPDATE_REFS_DIE_ON_ERR);
@@ -1060,9 +1060,9 @@ static void am_setup(struct am_state *state, enum 
patch_format patch_format,
 * session is in progress, they should be written last.
 */
 
-   write_file(am_path(state, next), 1, %d, state-cur);
+   write_file(am_path(state, next), 1, %d\n, state-cur);
 
-   write_file(am_path(state, last), 1, %d, state-last);
+   write_file(am_path(state, last), 1, %d\n, state-last);
 
strbuf_release(sb);
 }
@@ -1095,12 +1095,12 @@ static void am_next(struct am_state *state)
unlink(am_path(state, original-commit));
 
if (!get_sha1(HEAD, head))
-   

Re: [PATCH v3 7/8] branch.c: use 'ref-filter' APIs

2015-08-22 Thread Karthik Nayak
On Sat, Aug 22, 2015 at 9:21 PM, Christian Couder
christian.cou...@gmail.com wrote:
 On Sat, Aug 22, 2015 at 8:51 AM, Karthik Nayak karthik@gmail.com wrote:

 The test t1430 'git branch shows badly named ref' has been changed to
 check the stderr for the warning regarding the broken ref. This is
 done as ref-filter throws a warning for broken refs rather than
 directly printing them.

 [...]

 diff --git a/t/t1430-bad-ref-name.sh b/t/t1430-bad-ref-name.sh
 index 16d0b8b..db3627e 100755
 --- a/t/t1430-bad-ref-name.sh
 +++ b/t/t1430-bad-ref-name.sh
 @@ -41,7 +41,7 @@ test_expect_success 'fast-import: fail on invalid branch 
 name bad[branch]name'
  test_expect_success 'git branch shows badly named ref' '
 cp .git/refs/heads/master .git/refs/heads/broken...ref 
 test_when_finished rm -f .git/refs/heads/broken...ref 
 -   git branch output 
 +   git branch 2output 
 grep -e broken\.\.\.ref output
  '

 Maybe the test could be renamed to 'git branch warns about badly named
 ref' and maybe you could also check that broken\.\.\.ref is not
 printed on stdout.


The name change sounds reasonable, do we really need to check for it in the
stdout?

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


[PATCH] Fix `git rev-list --show-notes HEAD` when there are no notes

2015-08-22 Thread Johannes Schindelin
The `format_display_notes()` function clearly assumes that the data
structure holding the notes has been initialized already, i.e. that the
`display_notes_trees` variable is no longer `NULL`.

However, when there are no notes whatsoever, this variable is still
`NULL`, even after initialization.

So let's be graceful and just return if that data structure is `NULL`.

Reported in https://github.com/msysgit/git/issues/363.

Signed-off-by: Johannes Schindelin johannes.schinde...@gmx.de
---
 notes.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/notes.c b/notes.c
index df08209..24a335a 100644
--- a/notes.c
+++ b/notes.c
@@ -1266,7 +1266,10 @@ void format_display_notes(const unsigned char 
*object_sha1,
  struct strbuf *sb, const char *output_encoding, int 
raw)
 {
int i;
-   assert(display_notes_trees);
+
+   if (!display_notes_trees)
+   return;
+
for (i = 0; display_notes_trees[i]; i++)
format_note(display_notes_trees[i], object_sha1, sb,
output_encoding, raw);
-- 
2.3.1.windows.1.9.g8c01ab4


--
To unsubscribe from this list: send the line 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 7/8] branch.c: use 'ref-filter' APIs

2015-08-22 Thread Christian Couder
On Sat, Aug 22, 2015 at 8:51 AM, Karthik Nayak karthik@gmail.com wrote:

 The test t1430 'git branch shows badly named ref' has been changed to
 check the stderr for the warning regarding the broken ref. This is
 done as ref-filter throws a warning for broken refs rather than
 directly printing them.

[...]

 diff --git a/t/t1430-bad-ref-name.sh b/t/t1430-bad-ref-name.sh
 index 16d0b8b..db3627e 100755
 --- a/t/t1430-bad-ref-name.sh
 +++ b/t/t1430-bad-ref-name.sh
 @@ -41,7 +41,7 @@ test_expect_success 'fast-import: fail on invalid branch 
 name bad[branch]name'
  test_expect_success 'git branch shows badly named ref' '
 cp .git/refs/heads/master .git/refs/heads/broken...ref 
 test_when_finished rm -f .git/refs/heads/broken...ref 
 -   git branch output 
 +   git branch 2output 
 grep -e broken\.\.\.ref output
  '

Maybe the test could be renamed to 'git branch warns about badly named
ref' and maybe you could also check that broken\.\.\.ref is not
printed on stdout.

Thanks,
Christian.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] Fix `git rev-list --show-notes HEAD` when there are no notes

2015-08-22 Thread Johan Herland
On Sat, Aug 22, 2015 at 5:14 PM, Johannes Schindelin
johannes.schinde...@gmx.de wrote:
 The `format_display_notes()` function clearly assumes that the data
 structure holding the notes has been initialized already, i.e. that the
 `display_notes_trees` variable is no longer `NULL`.

 However, when there are no notes whatsoever, this variable is still
 `NULL`, even after initialization.

 So let's be graceful and just return if that data structure is `NULL`.

 Reported in https://github.com/msysgit/git/issues/363.

 Signed-off-by: Johannes Schindelin johannes.schinde...@gmx.de

Acked-by: Johan Herland jo...@herland.net

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