Re: [PATCH 0/2] rebase: support --no-autostash

[Matthieu Moy]

John Keeping  writes:

> John Keeping (2):
>   rebase: support --no-autostash
>   Documentation/git-rebase: fix --no-autostash formatting

Looks good to me, thanks.

-- 
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: [Feature Request] git blame showing only revisions from git rev-list --first-parent

[Junio C Hamano]

Jeff King  writes:

> I'm not too familiar with the code, but this _seems_ to work for me:
>
> diff --git a/builtin/blame.c b/builtin/blame.c
> index 21321be..2e03d47 100644
> --- a/builtin/blame.c
> +++ b/builtin/blame.c
> @@ -1375,6 +1375,10 @@ static struct commit_list *first_scapegoat(struct 
> rev_info *revs, struct commit
>  static int num_scapegoats(struct rev_info *revs, struct commit *commit)
>  {
>   struct commit_list *l = first_scapegoat(revs, commit);
> + if (!l)
> + return 0;
> + if (revs->first_parent_only)
> + return 1;
>   return commit_list_count(l);
>  }
>
> I suspect it doesn't work at all with `--reverse`. I also have the
> nagging feeling that this could be handled inside revision.c with
> parent-rewriting, but I don't really know.

I am not sure how well --children, which is what we use from
revision.c, works with --first-parent flag passed to the revision
walking machinery.  If it already does the right thing, then the
revs.children decoration that collects all children of any given
commit should automatically give its "sole child" (in the world
limited to the first-parent chain) from first_scapegoat().

And if it does not, perhaps that is what we would want to fix,
i.e. making sure rev-list --first-parent --children does what
people would expect.

> But "git blame --first-parent " seems to behave sanely in git.git
> with this.

It is intresting to see that the above is the only thing necessary,
as a naive way to try all parents would be to do:

for (sg = first_scapegoat(...); sg; sg = sg->next)
assign blame to sg;
take the remainder ourselves;

in which case, a better place to patch would be first_scapegoat(),
not this function, so that we will always see zero or one element
parent list returned from the function.

But in reality, the code instead does this:

num_sg = num_scapegoats(...);
for (i = 0, sg = first_scapegoat(...);
 i < num_sg && sg;
 i++, sg = sg->next)
assign blame to sg;
take the remainder ourselves;

so you do not have to touch first_scapegoat() at all.

I do not offhand know if this was a sign of foresight in the
original, or just an overly redundant programming ;-).
--
To unsubscribe from this list: send the line "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] Pass graph width to pretty formatting, so '%>|' can work properly

[Junio C Hamano]

This "feels" correct ;-), but let me summon Duy who did the %><
padding at around a5752342 (pretty: support padding placeholders, %<
%> and %><, 2013-04-19) for an extra set of eyes.

Care to add a test or two while at it?

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 v17 00/14] port tag.c to use ref-filter APIs

[Karthik Nayak]

On Fri, Sep 11, 2015 at 11:00 PM, Eric Sunshine  wrote:
>
> Hmm, but what actually changed in the re-sent patches? Without a link
> to the discussion leading up to the re-send of changed-only patches,
> and without an interdiff, the re-send is opaque and less accessible to
> the reviewer; which is at odds with Matthieu's suggestion which was
> intended to make review easier and more streamlined.
>

Should have put an interdiff, my bad!

> In addition to a link to the previous round and an interdiff, it would
> be helpful to reviewers for you to annotate each patch (in the
> commentary are below the "---" line after your sign-off) with a
> description of the changes in that patch since the previous round in
> order to focus the reviewer's attention (where it needs to be) on the
> latest changes.

WIll follow this next time :)

On Fri, Sep 11, 2015 at 11:35 PM, Junio C Hamano  wrote:
> Eric Sunshine  writes:
>
>> In addition to a link to the previous round and an interdiff, it would
>> be helpful to reviewers for you to annotate each patch (in the
>> commentary are below the "---" line after your sign-off) with a
>> description of the changes in that patch since the previous round in
>> order to focus the reviewer's attention (where it needs to be) on the
>> latest changes.
>
> I may have got confused by seeing the same v17 (if they were marked
> as v18 or v17bis, it would have been easier to make sure I didn't
> miss anything), but here is the difference between what I had last
> night and what I queued.  The removal of !body[1] and flipping the
> order of to_free/format are not seen because I already had a local
> fix-up SQUASH??? commits queued in the yesterday's batch.
>
>
> $ git diff --stat -p kn/for-each-tag@{4.hours} kn/for-each-tag
>  ref-filter.c | 5 +
>  1 file changed, 1 insertion(+), 4 deletions(-)
>
> diff --git a/ref-filter.c b/ref-filter.c
> index 226e94d..fd839ac 100644
> --- a/ref-filter.c
> +++ b/ref-filter.c
> @@ -47,9 +47,6 @@ static struct {
> { "subject" },
> { "body" },
> { "contents" },
> -   { "contents:subject" },
> -   { "contents:body" },
> -   { "contents:signature" },
> { "upstream" },
> { "push" },
> { "symref" },
> @@ -58,7 +55,6 @@ static struct {
> { "color" },
> { "align" },
> { "end" },
> -   { "contents:lines" },
>  };
>
>  #define REF_FORMATTING_STATE_INIT  { 0, NULL }
> @@ -899,6 +895,7 @@ static void populate_value(struct ref_array_item *ref)
> align->position = ALIGN_LEFT;
>
> while (*s) {
> +   /*  Strip trailing comma */
> if (s[1])
> strbuf_setlen(s[0], s[0]->len - 1);
> if (!strtoul_ui(s[0]->buf, 10, (unsigned int 
> *)))

The complete interdiff:

diff --git a/builtin/tag.c b/builtin/tag.c
index 081fe84..f2f6e2d 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -43,9 +43,9 @@ static int list_tags(struct ref_filter *filter,
struct ref_sorting *sorting, con

 if (!format) {
 if (filter->lines) {
-format = xstrfmt("%s %%(contents:lines=%d)",
- "%(align:15)%%(refname:short)%%(end)", filter->lines);
-to_free = format;
+to_free = xstrfmt("%s %%(contents:lines=%d)",
+  "%(align:15)%(refname:short)%(end)", filter->lines);
+format = to_free;
 } else
 format = "%(refname:short)";
 }
diff --git a/ref-filter.c b/ref-filter.c
index 59716db..fd839ac 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -47,9 +47,6 @@ static struct {
 { "subject" },
 { "body" },
 { "contents" },
-{ "contents:subject" },
-{ "contents:body" },
-{ "contents:signature" },
 { "upstream" },
 { "push" },
 { "symref" },
@@ -58,7 +55,6 @@ static struct {
 { "color" },
 { "align" },
 { "end" },
-{ "contents:lines" },
 };

 #define REF_FORMATTING_STATE_INIT  { 0, NULL }
@@ -268,7 +264,7 @@ static int match_atom_name(const char *name, const
char *atom_name, const char *

 if (!skip_prefix(name, atom_name, ))
 return 0; /* doesn't even begin with "atom_name" */
-if (!body[0] || !body[1]) {
+if (!body[0]) {
 *val = NULL; /* %(atom_name) and no customization */
 return 1;
 }
@@ -899,6 +895,7 @@ static void populate_value(struct ref_array_item *ref)
 align->position = ALIGN_LEFT;

 while (*s) {
+/*  Strip trailing comma */
 if (s[1])
 strbuf_setlen(s[0], s[0]->len - 1);
 if (!strtoul_ui(s[0]->buf, 10, (unsigned int *)))


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

Re: [PATCH v7 2/3] worktree: move/refactor find_shared_symref from branch.c

[Junio C Hamano]

Michael Rappazzo  writes:

> The code formerly in branch.c was largely the basis of the
> get_worktree_list implementation is now moved to worktree.c,
> and the find_shared_symref implementation has been refactored
> to use get_worktree_list
>
> Signed-off-by: Michael Rappazzo 
> ---




>  branch.c| 79 
> +
>  branch.h|  8 --
>  builtin/notes.c |  1 +
>  worktree.c  | 40 +
>  worktree.h  |  7 +
>  5 files changed, 49 insertions(+), 86 deletions(-)
>
> diff --git a/branch.c b/branch.c
> index d013374..77d7f2a 100644
> --- a/branch.c
> +++ b/branch.c
> @@ -4,6 +4,7 @@
>  #include "refs.h"
>  #include "remote.h"
>  #include "commit.h"
> +#include "worktree.h"
>  
>  struct tracking {
>   struct refspec spec;
> @@ -311,84 +312,6 @@ void remove_branch_state(void)
>   unlink(git_path_squash_msg());
>  }
>  
> -static char *find_linked_symref(const char *symref, const char *branch,
> - const char *id)

The title refers to a function with a different name ;-).

Copying the bulk of the function in 1/3 and then removing the
original here made it somewhat hard to compare what got changed in
the implementation.

I _think_ the code structure in the end result is more or less
right, though.

> diff --git a/worktree.c b/worktree.c
> index 33d2e57..e45b651 100644
> --- a/worktree.c
> +++ b/worktree.c
> @@ -155,3 +155,43 @@ done:
>   return list;
>  }
>  
> +char *find_shared_symref(const char *symref, const char *target)
> +{
> + char *existing = NULL;
> + struct strbuf path = STRBUF_INIT;
> + struct strbuf sb = STRBUF_INIT;
> + struct worktree_list *worktree_list = get_worktree_list();
> + struct worktree_list *orig_list = worktree_list;
> + struct worktree *matched = NULL;
> +
> + while (!matched && worktree_list) {
> + if (strcmp("HEAD", symref)) {
> + strbuf_reset();
> + strbuf_reset();
> + strbuf_addf(, "%s/%s", 
> worktree_list->worktree->git_dir, symref);
> +
> + if (_parse_ref(path.buf, , NULL)) {
> + continue;
> + }

I forgot to say this on 1/3, but this helper function _parse_ref()
is obviously an implementation detail; does it have to be visible
outside the file, or can it become file-scope static?  There may
be other helpers that may not want to be global (I didn't check).

Provided that 1/3 will become accepable, this step looks sensible to
me.

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 v2 2/9] Documentation/git-send-pack.txt: Flow long synopsis line

[Junio C Hamano]

Junio C Hamano  writes:

> Dave Borowitz  writes:
>
>> I produced the patch with "git format-patch --subject-prefix='PATCH
>> v2' --cover-letter @{u}.." and mailed with "git send-email
>> --to=git@vger.kernel.org,gits...@pobox.com 0*.patch"; is there a way
>> that would have preserved whitespace better?
>
> No need to worry, I suspect that this is a local Emacs/GNUS glitch
> on the receiving end.  Sorry for a noise.

PSA, as I figured this out.

It turns out that gnus-treat-fill-long-lines was set to (typep
"text/plain"), which meant that I cannot trust what I see in my MUA
as an exact copy of the patch the sender intended to give me.

Here is what "Describe variable" gave me (after I fixed it, that is).

---
gnus-treat-fill-long-lines's value is nil
Original value was 
(typep "text/plain")


Documentation:
Fill long lines.
Valid values are nil, t, `head', `first', `last', an integer or a
predicate.  See Info node `(gnus)Customizing Articles'.

You can customize this variable.

This variable was introduced, or its default value was changed, in
version 24.1 of Emacs.
---
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

v2.5.2 installation on Windows 10

[Long]

Installation of 64bit could complete, but running Git GUI always prompt the
error below:

Couldn’t read file “C:\Program
Files\Git\cmd\mingw64\libexec\git-core\git-gui”: no such file or 
directoryN�r��yb�X��ǧv�^�)޺{.n�+ا���ܨ}���Ơz�:+v���zZ+��+zf���h���~i���z��w���?�&�)ߢf

error opening "git gui"

[Renato Akaboci]

Hi,

I´m in trouble just after installation of my Git for Windows.
I get a box with error saying:

couldn´t read file "C:\Program
Files\Git\cmd\mingw64\libexec\git-core\git-gui": no such file or
directory

I´ve just installed git as normal. Git Bash works perfectly, and if I
call "git gui" on bash or cmd, it open. I guess that is something
missing on my machine, but I don´t know what.
- My Operational System is Windows 10 x64

Any advice will be appreciated.
Thanks in advance
--
To unsubscribe from this list: send the line "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] tag, update-ref: improve description of option "create-reflog"

[Junio C Hamano]

Ralf Thielow  writes:

> The description of option "create-reflog" is "create_reflog", which
> is neither a good description, nor a sensible string to translate.
> Change it to a more meaningful message.
>
> Signed-off-by: Ralf Thielow 
> ---

This definitely is an improvement.  I will assume i18n team is OK
with a change like this this late in the cycle (I see you CC'ed
Jiang) and will merge before the next -rc.

Thanks.

>  builtin/tag.c| 2 +-
>  builtin/update-ref.c | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/builtin/tag.c b/builtin/tag.c
> index a99..cba0e22 100644
> --- a/builtin/tag.c
> +++ b/builtin/tag.c
> @@ -606,7 +606,7 @@ int cmd_tag(int argc, const char **argv, const char 
> *prefix)
>   OPT_STRING('u', "local-user", , N_("key-id"),
>   N_("use another key to sign the tag")),
>   OPT__FORCE(, N_("replace the tag if exists")),
> - OPT_BOOL(0, "create-reflog", _reflog, 
> N_("create_reflog")),
> + OPT_BOOL(0, "create-reflog", _reflog, N_("create a 
> reflog")),
>  
>   OPT_GROUP(N_("Tag listing options")),
>   OPT_COLUMN(0, "column", , N_("show tag list in 
> columns")),
> diff --git a/builtin/update-ref.c b/builtin/update-ref.c
> index 04dd00f..7f30d3a 100644
> --- a/builtin/update-ref.c
> +++ b/builtin/update-ref.c
> @@ -365,7 +365,7 @@ int cmd_update_ref(int argc, const char **argv, const 
> char *prefix)
>   N_("update  not the one it 
> points to")),
>   OPT_BOOL('z', NULL, _null, N_("stdin has NUL-terminated 
> arguments")),
>   OPT_BOOL( 0 , "stdin", _stdin, N_("read updates from 
> stdin")),
> - OPT_BOOL( 0 , "create-reflog", _reflog, 
> N_("create_reflog")),
> + OPT_BOOL( 0 , "create-reflog", _reflog, N_("create a 
> reflog")),
>   OPT_END(),
>   };
--
To unsubscribe from this list: send the line "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] Add tests for "Pass graph width to pretty formatting, so '%>|' can work properly"

[Josef Kufner]

---
 t/t4205-log-pretty-formats.sh | 23 +++
 1 file changed, 23 insertions(+)

diff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh
index 7398605..3358837 100755
--- a/t/t4205-log-pretty-formats.sh
+++ b/t/t4205-log-pretty-formats.sh
@@ -319,6 +319,18 @@ EOF
test_cmp expected actual
 '
 
+# Note: Space between 'message' and 'two' should be in the same column as in 
previous test.
+test_expect_success 'right alignment formatting at the nth column with 
--graph. i18n.logOutputEncoding' '
+   git -c i18n.logOutputEncoding=$test_encoding log --graph 
--pretty="tformat:%h %>|(40)%s" >actual &&
+   qz_to_tab_space actual &&
cat actual &&
+   cat actual &&
cat 

Re: [PATCH] Pass graph width to pretty formatting, so '%>|' can work properly

[Josef Kufner]

Ok, there are the two tests (see the second e-mail). It should cover two
cases where something may go wrong.


Junio C Hamano wrote, on 11.9.2015 18:54:
> This "feels" correct ;-), but let me summon Duy who did the %><
> padding at around a5752342 (pretty: support padding placeholders, %<
> %> and %><, 2013-04-19) for an extra set of eyes.
> 
> Care to add a test or two while at it?
> 
> Thanks.  
> 



signature.asc
Description: OpenPGP digital signature

[PATCH] tag, update-ref: improve description of option "create-reflog"

[Ralf Thielow]

The description of option "create-reflog" is "create_reflog", which
is neither a good description, nor a sensible string to translate.
Change it to a more meaningful message.

Signed-off-by: Ralf Thielow 
---
 builtin/tag.c| 2 +-
 builtin/update-ref.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/builtin/tag.c b/builtin/tag.c
index a99..cba0e22 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -606,7 +606,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
OPT_STRING('u', "local-user", , N_("key-id"),
N_("use another key to sign the tag")),
OPT__FORCE(, N_("replace the tag if exists")),
-   OPT_BOOL(0, "create-reflog", _reflog, 
N_("create_reflog")),
+   OPT_BOOL(0, "create-reflog", _reflog, N_("create a 
reflog")),
 
OPT_GROUP(N_("Tag listing options")),
OPT_COLUMN(0, "column", , N_("show tag list in 
columns")),
diff --git a/builtin/update-ref.c b/builtin/update-ref.c
index 04dd00f..7f30d3a 100644
--- a/builtin/update-ref.c
+++ b/builtin/update-ref.c
@@ -365,7 +365,7 @@ int cmd_update_ref(int argc, const char **argv, const char 
*prefix)
N_("update  not the one it 
points to")),
OPT_BOOL('z', NULL, _null, N_("stdin has NUL-terminated 
arguments")),
OPT_BOOL( 0 , "stdin", _stdin, N_("read updates from 
stdin")),
-   OPT_BOOL( 0 , "create-reflog", _reflog, 
N_("create_reflog")),
+   OPT_BOOL( 0 , "create-reflog", _reflog, N_("create a 
reflog")),
OPT_END(),
};
 
-- 
2.6.0.rc1.199.g678474c

--
To unsubscribe from this list: send the line "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 v17 00/14] port tag.c to use ref-filter APIs

[Eric Sunshine]

On Fri, Sep 11, 2015 at 11:08 AM, Karthik Nayak  wrote:
> On Thu, Sep 10, 2015 at 10:27 PM, Matthieu Moy
>  wrote:
>> Karthik Nayak  writes:
>>> Changes in this version:
>>> * The arguments of the %(align) atom are interchangeable.
>>> * Small grammatical changes.
>>> * Small changes in the tests to reflect changes in the align
>>> atom code.
>>
>> Clearly, we're almost there. I did a few minor remarks. I suggest
>> (admitedly, Eric suggested of-list to suggest ;-) ) that you reply to
>> them by re-sending only individual patches that changed (replying to the
>> original patch) so that we can check the new patches individually. I
>> think we can do the finishing touches for each patch in a subthread of
>> this patch.
>
> I replied with suggested changes by you and Junio.
> Let me know if any other changes to be made :)

Hmm, but what actually changed in the re-sent patches? Without a link
to the discussion leading up to the re-send of changed-only patches,
and without an interdiff, the re-send is opaque and less accessible to
the reviewer; which is at odds with Matthieu's suggestion which was
intended to make review easier and more streamlined.

In addition to a link to the previous round and an interdiff, it would
be helpful to reviewers for you to annotate each patch (in the
commentary are below the "---" line after your sign-off) with a
description of the changes in that patch since the previous round in
order to focus the reviewer's attention (where it needs to be) on the
latest changes.
--
To unsubscribe from this list: send the line "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: storing cover letter of a patch series?

[Chris Packham]

On Fri, Sep 11, 2015 at 4:28 AM, Jacob Keller  wrote:
> Hey,
>
> does anyone know of any tricks for storing a cover letter for a patch
> series inside of git somehow? I'd guess the only obvious way currently
> is to store it at the top of the series as an empty commit.. but this
> doesn't get emailed *as* the cover letter...
>
> Is there some other way? Would others be interested in such a feature?
>
> I get very annoyed when I've written a nice long patch cover letter in
> vim before an email and then realize I should fix something else up,
> or accidentally cancel it because I didn't use the write "To:" address
> or something..
>
> I really think it should be possible to store something somehow as a
> blob that could be looked up later. Even if this was a slightly more
> manual process that would be helpful to store the message inside git
> itself.
>
> In addition, this would help re-rolls since it would mean if I go back
> to a topic and re-roll it I can just update the message. If it were
> properly stored in my local history that would also mean I could see
> revisions on it.
>
> Any thoughts on how to do this?
>

A bit of a plug for patman[1] which the u-boot project uses (although
there's nothing u-boot specific about it). It lets you put the cover
letter and other meta information in the commit messages as you go
then will extract that information and generate a cover letter and
clean patches. As of fairly recently it's also installable as a
standalone application.

--
[1] - http://git.denx.de/?p=u-boot.git;a=blob;f=tools/patman/README
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] git-svn: parse authors file more leniently

[Michael J Gruber]

Eric Wong venit, vidit, dixit 10.09.2015 20:08:
> Michael J Gruber  wrote:
>> Instead, make git svn uses the perl regex
>>
>> /^(.+?|\(no author\))\s*=\s*(.+?)\s*<(.*)>\s*$/
>>
>> for parsing the authors file so that the same (slightly more lenient)
>> regex is used in both cases.
>>
>> Reported-by: Till Schäfer 
>> Signed-off-by: Michael J Gruber 
> 
> Thanks.
> Signed-off-by: Eric Wong 
> 
> And pushed to master of git://bogomips.org/git-svn
> (commit f7c6de0ea1bd5722a1181c6279676c6831b38a34)
> 
> By the way, I also had some other patches sitting around for you.
> Did you ever have time to revisit them?  (I haven't)
> 
>   t/lib-httpd: load mod_unixd
>   t/lib-git-svn: check same httpd module dirs as lib-httpd
> 

Also "from me".

Short answer: No

If I remember correctly, they were correct bit not complete in the sense
that on a standard Fedora install (with newer apache), svn tests still
wouldn't run over http. But I/we learned that those tests were simply
run over local file protocol instead when svn over http didn't work. On
a standard debian install (which apparantly has non-standard, thus
downwards compatible apache config) everything was fine with or without
those patches.

I still plan to look at them when I find time. (I'll be retiring
sometime between 20 and 30 years from now, so there's hope.)

Michael
--
To unsubscribe from this list: send the line "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 v8 1/2] submodule refactor: use git_pathdup_submodule() in add_submodule_odb()

[Jeff King]

On Fri, Sep 11, 2015 at 12:57:10AM +0300, Max Kirillov wrote:

> diff --git a/submodule.c b/submodule.c
> index 245ed4d..7340069 100644
> --- a/submodule.c
> +++ b/submodule.c
> @@ -119,43 +119,37 @@ void stage_updated_gitmodules(void)
>  
>  static int add_submodule_odb(const char *path)
>  {
> - struct strbuf objects_directory = STRBUF_INIT;
>   struct alternate_object_database *alt_odb;
> + char *objects_directory;

Now that we have strbuf_git_path_submodule(), is there any reason to
switch this away from a strbuf?

That saves us a bunch of strlen calls, and it makes the diff way
shorter. My ulterior motive is that the result also conflicts a lot less
with some patches I'm about to post to harden the malloc and strcpy
calls below.

That would make your patch look something like:

diff --git a/submodule.c b/submodule.c
index 245ed4d..5e5a46f 100644
--- a/submodule.c
+++ b/submodule.c
@@ -122,15 +122,8 @@ static int add_submodule_odb(const char *path)
struct strbuf objects_directory = STRBUF_INIT;
struct alternate_object_database *alt_odb;
int ret = 0;
-   const char *git_dir;
 
-   strbuf_addf(_directory, "%s/.git", path);
-   git_dir = read_gitfile(objects_directory.buf);
-   if (git_dir) {
-   strbuf_reset(_directory);
-   strbuf_addstr(_directory, git_dir);
-   }
-   strbuf_addstr(_directory, "/objects/");
+   strbuf_git_path_submodule(_directory, path, "objects/");
if (!is_directory(objects_directory.buf)) {
ret = -1;
goto done;

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

Re: [PATCH] Add tests for "Pass graph width to pretty formatting, so '%>|' can work properly"

[Eric Sunshine]

On Fri, Sep 11, 2015 at 1:50 PM, Josef Kufner  wrote:
> ---

Missing sign-off. Or is this intended to be concatenated with the
patch you sent earlier?

> diff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh
> index 7398605..3358837 100755
> --- a/t/t4205-log-pretty-formats.sh
> +++ b/t/t4205-log-pretty-formats.sh
> @@ -319,6 +319,18 @@ EOF
> test_cmp expected actual
>  '
>
> +# Note: Space between 'message' and 'two' should be in the same column as in 
> previous test.
> +test_expect_success 'right alignment formatting at the nth column with 
> --graph. i18n.logOutputEncoding' '
> +   git -c i18n.logOutputEncoding=$test_encoding log --graph 
> --pretty="tformat:%h %>|(40)%s" >actual &&
> +   qz_to_tab_space  +* $head1message two
> +* $head2message one
> +* $head3add bar
> +* $head4$(commit_msg)
> +EOF
> +   test_cmp expected actual
> +'
> +
>  test_expect_success 'right alignment formatting with no padding' '
> git log --pretty="tformat:%>(1)%s" >actual &&
> cat  @@ -330,6 +342,17 @@ EOF
> test_cmp expected actual
>  '
>
> +test_expect_success 'right alignment formatting with no padding and with 
> --graph' '
> +   git log --graph --pretty="tformat:%>(1)%s" >actual &&
> +   cat  +* message two
> +* message one
> +* add bar
> +* $(commit_msg)
> +EOF
> +   test_cmp expected actual
> +'
--
To unsubscribe from this list: send the line "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: error opening

[Long]

I'm having the same problem.  In fact, my post was right in front of yours.
 Another thing I notice is that Git commands are not listed in Windows Explorer.

--
To unsubscribe from this list: send the line "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: [Feature Request] git blame showing only revisions from git rev-list --first-parent

[Junio C Hamano]

Junio C Hamano  writes:

> Jeff King  writes:
>
>> I'm not too familiar with the code, but this _seems_ to work for me:
>>
>> diff --git a/builtin/blame.c b/builtin/blame.c
>> index 21321be..2e03d47 100644
>> --- a/builtin/blame.c
>> +++ b/builtin/blame.c
>> @@ -1375,6 +1375,10 @@ static struct commit_list *first_scapegoat(struct 
>> rev_info *revs, struct commit
>>  static int num_scapegoats(struct rev_info *revs, struct commit *commit)
>>  {
>>  struct commit_list *l = first_scapegoat(revs, commit);
>> +if (!l)
>> +return 0;
>> +if (revs->first_parent_only)
>> +return 1;
>>  return commit_list_count(l);
>>  }
>>
>> I suspect it doesn't work at all with `--reverse`. I also have the
>> nagging feeling that this could be handled inside revision.c with
>> parent-rewriting, but I don't really know.
>
> I am not sure how well --children, which is what we use from
> revision.c, works with --first-parent flag passed to the revision
> walking machinery.  If it already does the right thing, then the
> revs.children decoration that collects all children of any given
> commit should automatically give its "sole child" (in the world
> limited to the first-parent chain) from first_scapegoat().
>
> And if it does not, perhaps that is what we would want to fix,
> i.e. making sure rev-list --first-parent --children does what
> people would expect.
> 
>> But "git blame --first-parent " seems to behave sanely in git.git
>> with this.
>
> It is intresting to see that the above is the only thing necessary,
> as a naive way to try all parents would be to do:
>
>   for (sg = first_scapegoat(...); sg; sg = sg->next)
>   assign blame to sg;
>   take the remainder ourselves;
>
> in which case, a better place to patch would be first_scapegoat(),
> not this function, so that we will always see zero or one element
> parent list returned from the function.
>
> But in reality, the code instead does this:
>
>   num_sg = num_scapegoats(...);
>   for (i = 0, sg = first_scapegoat(...);
>  i < num_sg && sg;
>  i++, sg = sg->next)
>   assign blame to sg;
>   take the remainder ourselves;
>
> so you do not have to touch first_scapegoat() at all.
>
> I do not offhand know if this was a sign of foresight in the
> original, or just an overly redundant programming ;-).

So here is an outline of what I had in mind when I was writing the
above.  Instead of trusting that num_scapegoats() is always used to
limit the enumeration of commit->parents, we discard the second and
subsequent parents from the commit objects, and also make sure the
later parents do not participate in the revs.children annotation, so
that the world "blame" sees truly becomes a single strand of pearls.

As usual, not even compile tested, but it feels correct ;-)



 builtin/blame.c | 9 -
 revision.c  | 4 +++-
 2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/builtin/blame.c b/builtin/blame.c
index 4db01c1..bc87d9d 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -1366,8 +1366,15 @@ static void pass_whole_blame(struct scoreboard *sb,
  */
 static struct commit_list *first_scapegoat(struct rev_info *revs, struct 
commit *commit)
 {
-   if (!reverse)
+   if (!reverse) {
+   if (revs->first_parent_only &&
+   commit->parents &&
+   commit->parents->next) {
+   free_commit_list(commit->parents->next);
+   commit->parents->next = NULL;
+   }
return commit->parents;
+   }
return lookup_decoration(>children, >object);
 }
 
diff --git a/revision.c b/revision.c
index af2a18e..a020a42 100644
--- a/revision.c
+++ b/revision.c
@@ -2765,7 +2765,9 @@ static void set_children(struct rev_info *revs)
struct commit *commit = l->item;
struct commit_list *p;
 
-   for (p = commit->parents; p; p = p->next)
+   for (p = commit->parents;
+p && !revs->first_parent_only;
+p = p->next)
add_child(revs, p->item, 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: [PATCH] Add tests for "Pass graph width to pretty formatting, so '%>|' can work properly"

[Josef Kufner]

Eric Sunshine wrote, on 11.9.2015 21:37:
> On Fri, Sep 11, 2015 at 1:50 PM, Josef Kufner  wrote:
>> ---
> 
> Missing sign-off. Or is this intended to be concatenated with the
> patch you sent earlier?

Just forgot to add it. Fixed patch on the way.


>> diff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh
>> index 7398605..3358837 100755
>> --- a/t/t4205-log-pretty-formats.sh
>> +++ b/t/t4205-log-pretty-formats.sh
>> @@ -319,6 +319,18 @@ EOF
>> test_cmp expected actual
>>  '
>>
>> +# Note: Space between 'message' and 'two' should be in the same column as 
>> in previous test.
>> +test_expect_success 'right alignment formatting at the nth column with 
>> --graph. i18n.logOutputEncoding' '
>> +   git -c i18n.logOutputEncoding=$test_encoding log --graph 
>> --pretty="tformat:%h %>|(40)%s" >actual &&
>> +   qz_to_tab_space  
> You don't seem to be taking advantage of qz_to_tab_space's
> transliteration of Q to tab and Z to space, so s/qz_to_tab_space/cat/
> would make the code clearer.

I've copied another test which tests the padding without --graph and
added it to test the new case. I have no idea what qz_to_tab_space can
do. If you wish some clean up, it should be done on older tests too :)



signature.asc
Description: OpenPGP digital signature

[PATCH] Add tests for "Pass graph width to pretty formatting, so '%>|' can work properly"

[Josef Kufner]

Signed-off-by: Josef Kufner 
---
 t/t4205-log-pretty-formats.sh | 23 +++
 1 file changed, 23 insertions(+)

diff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh
index 7398605..3358837 100755
--- a/t/t4205-log-pretty-formats.sh
+++ b/t/t4205-log-pretty-formats.sh
@@ -319,6 +319,18 @@ EOF
test_cmp expected actual
 '
 
+# Note: Space between 'message' and 'two' should be in the same column as in 
previous test.
+test_expect_success 'right alignment formatting at the nth column with 
--graph. i18n.logOutputEncoding' '
+   git -c i18n.logOutputEncoding=$test_encoding log --graph 
--pretty="tformat:%h %>|(40)%s" >actual &&
+   qz_to_tab_space actual &&
cat actual &&
+   cat actual &&
cat 

Re: [PATCH] Add tests for "Pass graph width to pretty formatting, so '%>|' can work properly"

[Junio C Hamano]

Eric Sunshine  writes:

>> +   qz_to_tab_space 
> You don't seem to be taking advantage of qz_to_tab_space's
> transliteration of Q to tab and Z to space, so s/qz_to_tab_space/cat/
> would make the code clearer.

Don't cat into a pipe unless you are concatenating multiple files,
though ;-).
--
To unsubscribe from this list: send the line "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: error opening

[Adrian Ang]

Renato Akaboci  gmail.com> writes:

> 
> Hi,
> 
> I´m in trouble just after installation of my Git for Windows.
> I get a box with error saying:
> 
> couldn´t read file "C:\Program
> Files\Git\cmd\mingw64\libexec\git-core\git-gui": no such file or
> directory
> 
> I´ve just installed git as normal. Git Bash works perfectly, and if I
> call "git gui" on bash or cmd, it open. I guess that is something
> missing on my machine, but I don´t know what.
> - My Operational System is Windows 10 x64
> 
> Any advice will be appreciated.
> Thanks in advance
> 


I'm having the exact same problem. 

Installation proceeds normally, but trying to open Git GUI will always 
fail with the same error message you have.

I tried searching the Registry to see if this is a misconfiguration 
somewhere, but no such setting exists in my registry. I don't know if 
this configuration is baked into the Windows x64 Git GUI executable or 
set in an ini or config file somewhere.

I'm trying to document the install process for my team's implementation 
of git. If this issue can't be resolved, or if previous Windows 
installer versions aren't available, my team's git implementation is 
blocked.

We could try to simply copy the "missing" subdirectory (which actually 
does exist somewhere else) into the "cmd" subdirectory, but that would 
seem really clumsy and it will affect the team members' first 
impressions of git. We're trying to move from SVN to git, and most of my 
team members have not used git before.

Any assistance or reply from the devs would be appreciated.  Thank you.

Re: storing cover letter of a patch series?

[Junio C Hamano]

Junio C Hamano  writes:

> Ideally, I would think that you want that information when the
> series is fully cooked and gets merged to a more permanent place in
> the log message of the merge commit.  At that point, where the
> series started may become more clear from the topology (i.e. the set
> difference X^..X for the resulting merge is what got merged).  One
> possible "hacky" convention could be
>
>  - Developers keep rerolling with the "empty commit with cover
>letter material at the tip".  topic@{upstream}..topic~1 are the
>real changes, topic~0 is an empty "cover letter material".
>
>  - When the series is fully cooked, a new "git merge" option notices
>that the topic is structured in a "strange" way, uncaps its tip
>commit and merges the remainder of the series and adds the cover
>letter material when presenting the editor to record the merge
>commit.  That is
>
>   $ git merge --cover-at-tip topic
>
>would work roughly by doing the following:
>
> - verify that "git rev-parse topic^^{tree} topic^{tree}" shows that
>   they record the same tree; otherwise it will error out, saying
>   the tip is not a pure cover.
>
> - verify that "git rev-list ..topic^" shows that there is
>   something to merge after the tip is removed; otherwise it will
>   error out, saying that there is nothing to merge.
>
> - run "git merge --no-ff --edit topic^1" but with the log
>   message of topic^{commit} in the editor's template.

To complement the above, if we want to pursue this approach, the
following would also help.

 - (obvious) "git pull" would learn the same "--cover-at-tip" option
   and would pass it to "git merge".

 - "git am --cover-at-tip" would make the incoming cover-letter
   material into a non-tree-changing commit at the tip of the
   resulting topic.

 - "git format-patch" would notice a topic branch in such a shape
   and would use the log message of the non-tree-changing commit at
   the tip as part of the cover letter.

Then the overall workflow would become:

 * A developer works on a topic and concludes it by writing a
   summary that should appear in the final merge to the trunk as a
   log message of a non-tree-changing commit at the tip.

 * In "request-to-pull" workflow, the developer requests the topic
   to be pulled.  The integrator uses "git pull --cover-at-tip" and
   the resulting merge commit will carry the summary written by the
   original developer.

 * In e-mail workflow, the developer runs "git format-patch"; the
   cover-letter is populated with the summary the developer wrote.

 * The integrator uses "git am --cover-at-tip" on a new branch,
   which recreates the topic branch the developer created at the
   first step above.

 * The integrator merges the topic with "git merge --cover-at-tip"
   to the trunk, and the resulting merge commit will carry the
   summary written by the original developer.

--
To unsubscribe from this list: send the line "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 PATCHv1 0/2] Parallel git submodule fetching

[Stefan Beller]

This is a new approach to making git submodules faster without using threads.
We'll directly run commands in parallel without an intermediate thread pool.

patch 1 is fixing an error, which was hard to come by in a parallel world,
but is not much of a problem in a single-childed world.

The second patch works fine, (I'd love to hear feedback on the percieved
change of output) though it has still two edges I want to tackle:

* The fcntl call to make the pipes nonblocking is not checked for errors.
* We also need to communicate back if one process cannot spawn or such,
  i.e. the reporting for all subprocesses combined needs to added. I think
  about adding another callback which is called to finish up a child process.

Earlier attempts used a threading pool, which made the conversion very
easy as you don't need to touch lots of existing code as you would just
replace the run_command(..) call by an add_task(..). The threading pool
would abstract away the whole parallelism and its problems. It would also
have the backpressure to the main program, add_task would just not
return if there was no place left in the queue.

[1] http://www.spinics.net/lists/git/msg258435.html

Any feedback welcome!
Thanks,
Stefan

Jonathan Nieder (1):
  Sending "Fetching submodule " output to stderr

Stefan Beller (1):
  fetch: fetch submodules in parallel

 Documentation/fetch-options.txt |   7 ++
 builtin/fetch.c |   6 +-
 builtin/pull.c  |   6 ++
 run-command.c   | 144 
 run-command.h   |  29 
 strbuf.c|  31 +
 strbuf.h|   1 +
 submodule.c |  99 +--
 submodule.h |   2 +-
 t/t0061-run-command.sh  |  16 +
 t/t5526-fetch-submodules.sh |  70 ---
 test-run-command.c  |  23 +++
 12 files changed, 373 insertions(+), 61 deletions(-)

-- 
2.6.0.rc0.131.gf624c3d

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

[PATCH 2/2] fetch: fetch submodules in parallel

[Stefan Beller]

If we run external commands in parallel we cannot pipe the output directly
to the our stdout/err as it would mix up. So each process's output will
flow through a pipe, which we buffer. One subprocess can be directly
piped to out stdout/err for a low latency feedback to the user.

Example:
Let's assume we have 5 submodules A,B,C,D,E and each fetch takes a
different amount of time as the different submodules vary in size, then
the output of fetches in sequential order might look like this:

 time -->
 output: |---A---|   |-B-|   |C---|   |-D-|   |-E-|

When we schedule these submodules into maximal two parallel processes,
a schedule and sample output over time may look like this:

thread 1: |---A---|   |-D-|   |-E-|

thread 2: |-B-|   |C---|

output:   |---A---|B|--C---|DE

So A will be perceived as it would run normally in the single child
version. As B has finished by the time A is done, we can dump its whole
progress buffer on stderr, such that it looks like it finished in no time.
Once that is done, C is determined to be the visible child and its progress
will be reported in real time.

So this way of output is really good for human consumption,
as it only changes the timing, not the actual output.

For machine consumption the output needs to be prepared in
the tasks, by either having a prefix per line or per block
to indicate whose tasks output is displayed.

Signed-off-by: Stefan Beller 
---
 Documentation/fetch-options.txt |   7 ++
 builtin/fetch.c |   6 +-
 builtin/pull.c  |   6 ++
 run-command.c   | 144 
 run-command.h   |  29 
 strbuf.c|  31 +
 strbuf.h|   1 +
 submodule.c |  99 +--
 submodule.h |   2 +-
 t/t0061-run-command.sh  |  16 +
 t/t5526-fetch-submodules.sh |  19 ++
 test-run-command.c  |  23 +++
 12 files changed, 347 insertions(+), 36 deletions(-)

diff --git a/Documentation/fetch-options.txt b/Documentation/fetch-options.txt
index 45583d8..e2a59c3 100644
--- a/Documentation/fetch-options.txt
+++ b/Documentation/fetch-options.txt
@@ -100,6 +100,13 @@ ifndef::git-pull[]
reference to a commit that isn't already in the local submodule
clone.
 
+-j::
+--jobs=::
+   Number of parallel children to be used for fetching submodules.
+   Each will fetch from different submodules, such that fetching many
+   submodules will be faster. By default submodules will be fetched
+   one at a time
+
 --no-recurse-submodules::
Disable recursive fetching of submodules (this has the same effect as
using the '--recurse-submodules=no' option).
diff --git a/builtin/fetch.c b/builtin/fetch.c
index ee1f1a9..09ff837 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -37,6 +37,7 @@ static int prune = -1; /* unspecified */
 static int all, append, dry_run, force, keep, multiple, update_head_ok, 
verbosity;
 static int progress = -1, recurse_submodules = RECURSE_SUBMODULES_DEFAULT;
 static int tags = TAGS_DEFAULT, unshallow, update_shallow;
+static int max_children = 1;
 static const char *depth;
 static const char *upload_pack;
 static struct strbuf default_rla = STRBUF_INIT;
@@ -99,6 +100,8 @@ static struct option builtin_fetch_options[] = {
N_("fetch all tags and associated objects"), TAGS_SET),
OPT_SET_INT('n', NULL, ,
N_("do not fetch all tags (--no-tags)"), TAGS_UNSET),
+   OPT_INTEGER('j', "jobs", _children,
+   N_("number of threads used for fetching")),
OPT_BOOL('p', "prune", ,
 N_("prune remote-tracking branches no longer on remote")),
{ OPTION_CALLBACK, 0, "recurse-submodules", NULL, N_("on-demand"),
@@ -1217,7 +1220,8 @@ int cmd_fetch(int argc, const char **argv, const char 
*prefix)
result = fetch_populated_submodules(,
submodule_prefix,
recurse_submodules,
-   verbosity < 0);
+   verbosity < 0,
+   max_children);
argv_array_clear();
}
 
diff --git a/builtin/pull.c b/builtin/pull.c
index 722a83c..fbbda67 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -94,6 +94,7 @@ static int opt_force;
 static char *opt_tags;
 static char *opt_prune;
 static char *opt_recurse_submodules;
+static char *max_children;
 static int opt_dry_run;
 static char *opt_keep;
 static char *opt_depth;
@@ -177,6 +178,9 @@ static struct option pull_options[] = {
N_("on-demand"),
N_("control recursive fetching of submodules"),

Re: [PATCH v7 2/3] worktree: move/refactor find_shared_symref from branch.c

[Eric Sunshine]

On Fri, Sep 11, 2015 at 5:52 PM, Junio C Hamano  wrote:
> Mike Rappazzo  writes:
>> On Fri, Sep 11, 2015 at 12:16 PM, Junio C Hamano  wrote:
>>> Michael Rappazzo  writes:
 The code formerly in branch.c was largely the basis of the
 get_worktree_list implementation is now moved to worktree.c,
 and the find_shared_symref implementation has been refactored
 to use get_worktree_list
>>>
>>> Copying the bulk of the function in 1/3 and then removing the
>>> original here made it somewhat hard to compare what got changed in
>>> the implementation.
>>>
>>> I _think_ the code structure in the end result is more or less
>>> right, though.
>>
>> Should I squash these first two commits together in the next series?
>
> Mashing the two into one patch would not help anybody, I would
> suspect.
>
> I didn't try this myself, but if I were doing this and if I were
> aiming for perfection, then I would probably try to split it even
> finer.  Refactor the original while both the callers and the helpers
> are still inside branch.c (hence the early steps in the series will
> not benefit worktree.c at all---worktree.c may not even exist in
> them yet), move refactored helpers to worktree.[ch] (and at this
> step you may not even have get_worktree() etc. yet), and then use
> that refactored helper while writing new functions in worktree.c.

Thanks for bringing this up. I haven't found a sufficient block of
time yet to review these patches, however, I had the same thought upon
a quick initial read, and is how I was hoping to see the patch series
constructed based upon my earlier two reviews suggesting refactoring
the existing branch.c functions into a new get_worktrees(). There are
at least a couple important reasons for taking this approach.

First, it keeps the "blame" trail intact, the full context of which
can be helpful to someone trying to understand why the code is the way
it is. The current approach of having get_worktree_list() materialize
out of thin air (even though it may have been patterned after existing
code) doesn't give the same benefit.

Second, it's easier for reviewers to understand and check the code for
correctness if it mutates to the final form in small increments than
it is to have get_worktrees() come into existence fully matured.

A final minor comment: Since all three branch.c functions,
die_if_checked_out(), find_shared_symref(), and find_linked_symref(),
ought to be moved to top-level worktree.c, I'd probably have patch 1
do the relocation (with no functional changes), and subsequent patches
refactor the moved code into a general purpose get_worktrees(). The
final patch would then implement "git worktrees list".
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Bloom filters for have/want negotiation

[Michael Haggerty]

I have been thinking about Wilhelm Bierbaum's talk at the last GitMerge
conference [1] in which he describes a scheme for using Bloom filters to
make the initial reference advertisement less expensive.

In his scheme (if I understand correctly) the client starts off the
conversation by passing the server a Bloom filter that indicates what
(refname, SHA-1) pairs the client already has. This makes it unnecessary
for the server to advertise those references, thereby reducing the cost
of incremental fetches dramatically if the server has very many references.

Because Bloom filters have false positives, this scheme is not 100%
reliable. Therefore I don't think we would want Git to depend on it.

But it got me thinking about how the client could use a Bloom filter in
a later stage of the negotiation, when telling the server what objects
it already has, while preserving 100% reliability.

The idea is to use connectivity information to correct mistakes caused
by reliance on the Bloom-filter:

1. The server advertises the references that it has in the way that it
is currently done.
2. The client advertises the objects that it has (or some subset of
them; see below) via a Bloom filter.
3. The server sends the client the packfile that results from assuming
that the Bloom filter is giving correct answers. (This might mean that
too few objects are sent to the client.)
4. The client does a connectivity check on the packfile. If any objects
are missing, it asks the server for them via a reliable
(non-Bloom-filter-based) request.

How would one construct the Bloom filter for step 2? (Remember that a
properly-configured Bloom filter requires about 5 bits of space per
value stored for each factor of 0.1 in the false-positive rate. So, for
example, to store 5000 values with a 1% false-positive rate, the Bloom
filter would need to be approximately 5000 * 10 bits = 6.2 kB in size.)

Here are some possible schemes:

* Record *all* objects in the Bloom filter. The Git repo has
approximately 200k objects, so, supposing that we could live with a 10%
false-positive rate (see below), the Bloom filter would need to be about
125 kB.

* Record all commit objects in the Bloom filter. For the Git repo that
is about 40k commits, so for a 10% error rate the Bloom filter would
have to be about 25 kB.

* Record some subset of commits; for example, all unique branch and tag
tips, the peeled tags, plus some sparse subsets of commits deeper in the
history. The ls-remote for the Git repo lists 1730 unique SHA-1s, so,
supposing we choose 10x that number with a 1% error rate, the Bloom
filter would be about 20 kB.

* Record only the branch and tag tips and peeled tags. Please note that
for situations where the client has fetched from the server before and
still has the remote-tracking references from that fetch, this scheme
might work surprisingly well. For the Git repository, with a 1% error
rate, this would be about 2 kB.

For the first two schemes, we could tolerate a pretty high error rate
because the server could perform additional consistency checks to reduce
the error rate. For example, if the Bloom filter reports that the client
has commit X, but that the client does *not* have a parent of X, then
the server can assume that the check of X was a false positive and
discard it. Such consistency checks would not be possible with the third
or fourth schemes, so I have chosen lower false-positive rates for those
schemes.

Additional points:

* The client can decide what to include in the Bloom filter. For
example, if it has done a recent fetch from the server, it might want to
send only the remote-tracking branch tips. But if it has never fetched
from this server before, it might want to send all commits.

* A Bloom filter could be computed at repack time rather than at each
fetch. On fetch, the precomputed Bloom filters could be loaded, any
loose objects added to it, and the result sent to the server.

I don't have a gut feeling about the cost of this phase of the
negotiation, so I don't know whether this would be a net savings, let
alone one that is worth the added complexity. But I wanted to document
the idea in case somebody thinks it has promise. (I have no plans to
pursue it.)

Michael

[1] http://git-merge.com/videos/scaling-git-at-twitter-wilhelm-bierbaum.html

-- 
Michael Haggerty
mhag...@alum.mit.edu

--
To unsubscribe from this list: send the line "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 v7 2/3] worktree: move/refactor find_shared_symref from branch.c

[Mike Rappazzo]

On Fri, Sep 11, 2015 at 12:16 PM, Junio C Hamano  wrote:
> Michael Rappazzo  writes:
>
>> The code formerly in branch.c was largely the basis of the
>> get_worktree_list implementation is now moved to worktree.c,
>> and the find_shared_symref implementation has been refactored
>> to use get_worktree_list
>>
>
> Copying the bulk of the function in 1/3 and then removing the
> original here made it somewhat hard to compare what got changed in
> the implementation.
>
> I _think_ the code structure in the end result is more or less
> right, though.

Should I squash these first two commits together in the next series?
--
To unsubscribe from this list: send the line "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 v7 3/3] worktree: add 'list' command

[Junio C Hamano]

Michael Rappazzo  writes:

> diff --git a/builtin/worktree.c b/builtin/worktree.c
> index 71bb770..a0c0fe8 100644
> --- a/builtin/worktree.c
> +++ b/builtin/worktree.c
> @@ -8,10 +8,13 @@
>  #include "run-command.h"
>  #include "sigchain.h"
>  #include "refs.h"
> +#include "utf8.h"
> +#include "worktree.h"
>  
>  static const char * const worktree_usage[] = {
>   N_("git worktree add []  "),
>   N_("git worktree prune []"),
> + N_("git worktree list []"),
>   NULL
>  };
>  
> @@ -359,6 +362,64 @@ static int add(int ac, const char **av, const char 
> *prefix)
>   return add_worktree(path, branch, );
>  }
>  
> +static int list(int ac, const char **av, const char *prefix)
> +{
> + int no_bare = 0;
> +
> + struct option options[] = {
> + OPT_BOOL(0, "skip-bare", _bare,  N_("exclude bare worktrees 
> from the list")),
> + OPT__VERBOSE(, N_("include more worktree details")),
> + OPT_END()
> + };
> +
> + ac = parse_options(ac, av, prefix, options, worktree_usage, 0);
> + if (ac)
> + usage_with_options(worktree_usage, options);
> + else {
> + struct worktree_list *worktree_list = get_worktree_list();

Call that variable just "list"; you are not dealing with any other
kind of list in this code anyway.

> + struct worktree_list *orig = worktree_list;
> + struct strbuf sb = STRBUF_INIT;
> + int path_maxlen = 0;
> +
> + if (verbose) {
> + while (worktree_list) {
> + int cur_len = 
> strlen(worktree_list->worktree->path);
> + if (cur_len > path_maxlen)
> + path_maxlen = cur_len;
> + worktree_list = worktree_list->next ? 
> worktree_list->next : NULL;

Ehh, (A ? A : NULL), when A is a pointer value, is equivalent to A, no?

struct worktree_list *orig = get_worktree_list();
struct worktree_list *list;

for (list = orig; list; list = list->next) {
struct worktree *worktree = list->worktree;
int len = strlen(worktree->path);
if (maxlen < len)
maxlen = len;
}

It seems that "do we really need callback interface?" suggestion
really made the code simple and straight-forward.

> + while (worktree_list) {
> + /* if this work tree is not bare OR if bare repos are 
> to be shown (default) */
> + if (!worktree_list->worktree->is_bare || !no_bare) {
> + strbuf_reset();
> + if (!verbose)
> + strbuf_addstr(, 
> worktree_list->worktree->path);
> + else {
> + int cur_len = 
> strlen(worktree_list->worktree->path);
> + int utf8_adj = cur_len - 
> utf8_strwidth(worktree_list->worktree->path);
> + strbuf_addf(, "%-*s ", 1 + 
> path_maxlen + utf8_adj, worktree_list->worktree->path);
> + if (worktree_list->worktree->is_bare)
> + strbuf_addstr(, "(bare)");
> + else {
> + strbuf_addf(, "%s ", 
> find_unique_abbrev(worktree_list->worktree->head_sha1, DEFAULT_ABBREV));
> + if 
> (!worktree_list->worktree->is_detached)
> + strbuf_addf(, 
> "[%s]", shorten_unambiguous_ref(worktree_list->worktree->head_ref, 0));
> + else
> + strbuf_addstr(, 
> "(detached HEAD)");
> + }
> + }
> + printf("%s\n", sb.buf);
> + }
> + worktree_list = worktree_list->next ? 
> worktree_list->next : NULL;

Would it help reduce the indentation level if you inroduced a helper
function that takes a single worktree object and does all the above?

for (list = orig; list; list = list->next) {
struct worktree *worktree = list->worktree;
if (!no_bare || !worktree->is_bare)
show_worktree(worktree);
}

or something?

--
To unsubscribe from this list: send the line "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 v7 2/3] worktree: move/refactor find_shared_symref from branch.c

[Junio C Hamano]

Mike Rappazzo  writes:

> On Fri, Sep 11, 2015 at 12:16 PM, Junio C Hamano  wrote:
>> Michael Rappazzo  writes:
>>
>>> The code formerly in branch.c was largely the basis of the
>>> get_worktree_list implementation is now moved to worktree.c,
>>> and the find_shared_symref implementation has been refactored
>>> to use get_worktree_list
>>>
>>
>> Copying the bulk of the function in 1/3 and then removing the
>> original here made it somewhat hard to compare what got changed in
>> the implementation.
>>
>> I _think_ the code structure in the end result is more or less
>> right, though.
>
> Should I squash these first two commits together in the next series?

Mashing the two into one patch would not help anybody, I would
suspect.

I didn't try this myself, but if I were doing this and if I were
aiming for perfection, then I would probably try to split it even
finer.  Refactor the original while both the callers and the helpers
are still inside branch.c (hence the early steps in the series will
not benefit worktree.c at all---worktree.c may not even exist in
them yet), move refactored helpers to worktree.[ch] (and at this
step you may not even have get_worktree() etc. yet), and then use
that refactored helper while writing new functions in worktree.c.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: --progress option for git submodule update?

[Stefan Beller]

On Wed, Sep 9, 2015 at 8:06 PM, Vitali Lovich  wrote:
>> Doh! I see what you're missing now after rereading the email closely.
>> You can add a --quiet option,
>> but --verbose or --progress just errors out, but you want that as a
>> possible argument for git clone
>> inside the git submodule update code.
> Yes exactly.

Instead of cloning with submodules, you could also clone only the
superproject and then do a git fetch --recuse-submodules=yes -v
instead soonish.
--
To unsubscribe from this list: send the line "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/2] Sending "Fetching submodule " output to stderr

[Stefan Beller]

From: Jonathan Nieder 

The "Pushing submodule " progress output correctly goes to
stderr, but "Fetching submodule " is going to stdout by mistake.
Fix it to write to stderr.

Noticed while trying to implement a parallel submodule fetch.  When
this particular output line went to a different file descriptor, it
was buffered separately, resulting in wrongly interleaved output if
we copied it to the terminal naively.

Signed-off-by: Jonathan Nieder 
Signed-off-by: Stefan Beller 
---
 submodule.c |  2 +-
 t/t5526-fetch-submodules.sh | 51 +++--
 2 files changed, 27 insertions(+), 26 deletions(-)

diff --git a/submodule.c b/submodule.c
index 9fcc86f..1d64e57 100644
--- a/submodule.c
+++ b/submodule.c
@@ -694,7 +694,7 @@ int fetch_populated_submodules(const struct argv_array 
*options,
git_dir = submodule_git_dir.buf;
if (is_directory(git_dir)) {
if (!quiet)
-   printf("Fetching submodule %s%s\n", prefix, 
ce->name);
+   fprintf(stderr, "Fetching submodule %s%s\n", 
prefix, ce->name);
cp.dir = submodule_path.buf;
argv_array_push(, default_argv);
argv_array_push(, "--submodule-prefix");
diff --git a/t/t5526-fetch-submodules.sh b/t/t5526-fetch-submodules.sh
index a4532b0..17759b1 100755
--- a/t/t5526-fetch-submodules.sh
+++ b/t/t5526-fetch-submodules.sh
@@ -16,7 +16,8 @@ add_upstream_commit() {
git add subfile &&
git commit -m new subfile &&
head2=$(git rev-parse --short HEAD) &&
-   echo "From $pwd/submodule" > ../expect.err &&
+   echo "Fetching submodule submodule" > ../expect.err &&
+   echo "From $pwd/submodule" >> ../expect.err &&
echo "   $head1..$head2  master -> origin/master" >> 
../expect.err
) &&
(
@@ -27,6 +28,7 @@ add_upstream_commit() {
git add deepsubfile &&
git commit -m new deepsubfile &&
head2=$(git rev-parse --short HEAD) &&
+   echo "Fetching submodule submodule/subdir/deepsubmodule" >> 
../expect.err
echo "From $pwd/deepsubmodule" >> ../expect.err &&
echo "   $head1..$head2  master -> origin/master" >> 
../expect.err
)
@@ -56,9 +58,7 @@ test_expect_success setup '
(
cd downstream &&
git submodule update --init --recursive
-   ) &&
-   echo "Fetching submodule submodule" > expect.out &&
-   echo "Fetching submodule submodule/subdir/deepsubmodule" >> expect.out
+   )
 '
 
 test_expect_success "fetch --recurse-submodules recurses into submodules" '
@@ -67,7 +67,7 @@ test_expect_success "fetch --recurse-submodules recurses into 
submodules" '
cd downstream &&
git fetch --recurse-submodules >../actual.out 2>../actual.err
) &&
-   test_i18ncmp expect.out actual.out &&
+   test_must_be_empty actual.out &&
test_i18ncmp expect.err actual.err
 '
 
@@ -96,7 +96,7 @@ test_expect_success "using fetchRecurseSubmodules=true in 
.gitmodules recurses i
git config -f .gitmodules 
submodule.submodule.fetchRecurseSubmodules true &&
git fetch >../actual.out 2>../actual.err
) &&
-   test_i18ncmp expect.out actual.out &&
+   test_must_be_empty actual.out &&
test_i18ncmp expect.err actual.err
 '
 
@@ -127,7 +127,7 @@ test_expect_success "--recurse-submodules overrides 
fetchRecurseSubmodules setti
git config --unset -f .gitmodules 
submodule.submodule.fetchRecurseSubmodules &&
git config --unset submodule.submodule.fetchRecurseSubmodules
) &&
-   test_i18ncmp expect.out actual.out &&
+   test_must_be_empty actual.out &&
test_i18ncmp expect.err actual.err
 '
 
@@ -146,7 +146,7 @@ test_expect_success "--dry-run propagates to submodules" '
cd downstream &&
git fetch --recurse-submodules --dry-run >../actual.out 
2>../actual.err
) &&
-   test_i18ncmp expect.out actual.out &&
+   test_must_be_empty actual.out &&
test_i18ncmp expect.err actual.err
 '
 
@@ -155,7 +155,7 @@ test_expect_success "Without --dry-run propagates to 
submodules" '
cd downstream &&
git fetch --recurse-submodules >../actual.out 2>../actual.err
) &&
-   test_i18ncmp expect.out actual.out &&
+   test_must_be_empty actual.out &&
test_i18ncmp expect.err actual.err
 '
 
@@ -166,7 +166,7 @@ test_expect_success "recurseSubmodules=true propagates into 
submodules" '
git config fetch.recurseSubmodules true
git fetch >../actual.out 2>../actual.err
) &&
-   

Re: Bloom filters for have/want negotiation

[Junio C Hamano]

Michael Haggerty  writes:

> 1. The server advertises the references that it has in the way that it
> is currently done.
> 2. The client advertises the objects that it has (or some subset of
> them; see below) via a Bloom filter.
> 3. The server sends the client the packfile that results from assuming
> that the Bloom filter is giving correct answers. (This might mean that
> too few objects are sent to the client.)

Wouldn't this end up sending objects the server has (perhaps
reachable from a branch that is not advertised to the client) that
is not reachable from the tips the client asked upon a false hit?
If so, that has security ramifications for servers who restrict what
you can download based on who you are (and which branches are
supposed to be visible to you).

Using bloom filter (perhaps invertible ones) to identify what the
receiving end lacks is certainly an intriguing approach, though.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/3] Add tests for "pretty: Pass graph width to pretty formatting for use in '%>|(N)'"

[Josef Kufner]

Signed-off-by: Josef Kufner 
---
 t/t4205-log-pretty-formats.sh | 23 +++
 1 file changed, 23 insertions(+)

diff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh
index 7398605..3358837 100755
--- a/t/t4205-log-pretty-formats.sh
+++ b/t/t4205-log-pretty-formats.sh
@@ -319,6 +319,18 @@ EOF
test_cmp expected actual
 '
 
+# Note: Space between 'message' and 'two' should be in the same column as in 
previous test.
+test_expect_success 'right alignment formatting at the nth column with 
--graph. i18n.logOutputEncoding' '
+   git -c i18n.logOutputEncoding=$test_encoding log --graph 
--pretty="tformat:%h %>|(40)%s" >actual &&
+   qz_to_tab_space actual &&
cat actual &&
+   cat actual &&
cat 

[PATCH 1/3] pretty: Pass graph width to pretty formatting for use in '%>|(N)'

[Josef Kufner]

Pass graph width to pretty formatting, so N in '%>|(N)' includes columns
consumed by graph rendered when --graph option is in use.

Example:
  git log --all --graph --pretty='format: [%>|(20)%h] %ar%d'

  All commit hashes should be aligned at 20th column from edge of the
  terminal, not from the edge of the graph.

Signed-off-by: Josef Kufner 
---
 commit.h   | 1 +
 graph.c| 7 +++
 graph.h| 5 +
 log-tree.c | 2 ++
 pretty.c   | 1 +
 5 files changed, 16 insertions(+)

diff --git a/commit.h b/commit.h
index 5d58be0..0a9a707 100644
--- a/commit.h
+++ b/commit.h
@@ -160,6 +160,7 @@ struct pretty_print_context {
 * should not be counted on by callers.
 */
struct string_list in_body_headers;
+   int graph_width;
 };
 
 struct userformat_want {
diff --git a/graph.c b/graph.c
index c25a09a..4802411 100644
--- a/graph.c
+++ b/graph.c
@@ -671,6 +671,13 @@ static void graph_output_padding_line(struct git_graph 
*graph,
graph_pad_horizontally(graph, sb, graph->num_new_columns * 2);
 }
 
+
+int graph_width(struct git_graph *graph)
+{
+   return graph->width;
+}
+
+
 static void graph_output_skip_line(struct git_graph *graph, struct strbuf *sb)
 {
/*
diff --git a/graph.h b/graph.h
index 0be62bd..3f48c19 100644
--- a/graph.h
+++ b/graph.h
@@ -68,6 +68,11 @@ int graph_next_line(struct git_graph *graph, struct strbuf 
*sb);
 
 
 /*
+ * Return current width of the graph in on-screen characters.
+ */
+int graph_width(struct git_graph *graph);
+
+/*
  * graph_show_*: helper functions for printing to stdout
  */
 
diff --git a/log-tree.c b/log-tree.c
index 7b1b57a..08fd5b6 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -686,6 +686,8 @@ void show_log(struct rev_info *opt)
ctx.output_encoding = get_log_output_encoding();
if (opt->from_ident.mail_begin && opt->from_ident.name_begin)
ctx.from_ident = >from_ident;
+   if (opt->graph)
+   ctx.graph_width = graph_width(opt->graph);
pretty_print_commit(, commit, );
 
if (opt->add_signoff)
diff --git a/pretty.c b/pretty.c
index 151c2ae..f1cf9e2 100644
--- a/pretty.c
+++ b/pretty.c
@@ -1297,6 +1297,7 @@ static size_t format_and_pad_commit(struct strbuf *sb, /* 
in UTF-8 */
if (!start)
start = sb->buf;
occupied = utf8_strnwidth(start, -1, 1);
+   occupied += c->pretty_ctx->graph_width;
padding = (-padding) - occupied;
}
while (1) {
-- 
2.5.1

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

[PATCH 3/3] t4205: remove unnecessary use of qz_to_tab_space

[Josef Kufner]

Many existing tests in this script pipes the result thru
qz_to_tab_space, but the payload does not need any Q or Z to be replaced
to tab or space. Redirect the input directly into iconv or use cat
instead.

Signed-off-by: Josef Kufner 
---
 t/t4205-log-pretty-formats.sh | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh
index 3358837..1f191cf 100755
--- a/t/t4205-log-pretty-formats.sh
+++ b/t/t4205-log-pretty-formats.sh
@@ -299,7 +299,7 @@ EOF
 
 test_expect_success 'right alignment formatting at the nth column' '
git log --pretty="tformat:%h %>|(40)%s" >actual &&
-   qz_to_tab_space actual &&
-   qz_to_tab_space actual &&
-   qz_to_tab_space actual &&
-   cat 

Re: --progress option for git submodule update?

[Vitali Lovich]

& then do a git submodule init?

> On Sep 11, 2015, at 4:05 PM, Stefan Beller  wrote:
> 
> On Wed, Sep 9, 2015 at 8:06 PM, Vitali Lovich  wrote:
>>> Doh! I see what you're missing now after rereading the email closely.
>>> You can add a --quiet option,
>>> but --verbose or --progress just errors out, but you want that as a
>>> possible argument for git clone
>>> inside the git submodule update code.
>> Yes exactly.
> 
> Instead of cloning with submodules, you could also clone only the
> superproject and then do a git fetch --recuse-submodules=yes -v
> instead soonish.

--
To unsubscribe from this list: send the line "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: [Feature Request] git blame showing only revisions from git rev-list --first-parent

[Jeff King]

On Fri, Sep 11, 2015 at 11:47:30AM +0100, Stephen Connolly wrote:

> A command line option to `git blame HEAD -- path` that instructs that
> the revisions of blame be the revisions where the change was applied
> to the current branch not the revision where the change first
> originated (i.e. limit commits to `git rev-list --first-parent HEAD`)
> 
> I can get what I want with the following:
> 
> git rev-list --first-parent HEAD | awk '{print p " " $0}{p=$0}' >
> tmpfile && git blame -b -S tmpfile HEAD -- path && rm tmpfile
> 
> But that is a rather ugly command. Could we have something built in to
> git blame to make this much easier for users?

I agree this would be a useful feature. Though blame takes rev-list
options, it doesn't use the stock rev-list traversal internally, so it
has to handle first-parent itself.

I'm not too familiar with the code, but this _seems_ to work for me:

diff --git a/builtin/blame.c b/builtin/blame.c
index 21321be..2e03d47 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -1375,6 +1375,10 @@ static struct commit_list *first_scapegoat(struct 
rev_info *revs, struct commit
 static int num_scapegoats(struct rev_info *revs, struct commit *commit)
 {
struct commit_list *l = first_scapegoat(revs, commit);
+   if (!l)
+   return 0;
+   if (revs->first_parent_only)
+   return 1;
return commit_list_count(l);
 }
 

I suspect it doesn't work at all with `--reverse`. I also have the
nagging feeling that this could be handled inside revision.c with
parent-rewriting, but I don't really know.

But "git blame --first-parent " seems to behave sanely in git.git
with this.

-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: Bloom filters for have/want negotiation

[Shawn Pearce]

On Fri, Sep 11, 2015 at 2:13 PM, Michael Haggerty  wrote:
> I have been thinking about Wilhelm Bierbaum's talk at the last GitMerge
> conference [1] in which he describes a scheme for using Bloom filters to
> make the initial reference advertisement less expensive.
...
> But it got me thinking about how the client could use a Bloom filter in
> a later stage of the negotiation, when telling the server what objects
> it already has, while preserving 100% reliability.
...
> I don't have a gut feeling about the cost of this phase of the
> negotiation, so I don't know whether this would be a net savings, let
> alone one that is worth the added complexity. But I wanted to document
> the idea in case somebody thinks it has promise. (I have no plans to
> pursue it.)

Maybe I can help... it just so happens that I have Git servers at
$DAY_JOB instrumented in the smart HTTP negotiate code. They do "many"
fetch requests. :)

The average client is only sending 16 have lines; at 50 bytes/line
this is 800 bytes.

The worst case is due to a bug in the negotiation. With nothing
common, the client just goes on forever until it reaches roots
(something is wrong with MAX_IN_VAIN). We saw 56,318 have lines ... a
2.6 MiB section. But smart HTTP gzips, so this may be only 1.3 MiB on
the wire.

But average/worst doesn't tell the entire picture. Bucketing requests
by have lines gives us more:

  %req | have_lines
  -|---
  100%   56,318
   99%   88
   98%   52
   97%   35
   96%   31
   95%   26

95% of requests have <= 26 have lines, or ~650 bytes after gzip on smart HTTP.
99% of requests have <= 88 have lines, like 2.15 KiB after gzip.

The smart HTTP client with nodone extension is allowed to send up to
1024 have lines in a single round trip; if the server can make an
efficient graph cut using only that batch, it can immediately return
the pack in that same round trip.

Ergo, if this is all working correctly on smart HTTP, clients can
fetch from a server they already "know" with decent efficiency, and
smaller than your 2 KiB Bloom filter estimate for git.git at 1% error
rate.


In practice this is not what always happens. The client isn't making
great decisions about what to send on smart HTTP. For example I have
observed a fetch session where the client sends:

  req #1:  16 haves
  req #2:  26 haves
  req #3:  45 haves
  req #4:  70 haves ... and done

So that is 4 round trips. It started out with 16; waited for common
ACKs. I guess 10 were common but no clean graph cut was made so the
client send another 16 to make 26. Again no clean cut so it added more
to send 45, then finally at 70 we found the cut point.

Reading fetch-pack.c, the first flush is at 16 lines. On the 2nd
request my intent was to double to 32 lines, but its not because count
is not reset to 0. So flush_at starts at 16, doubles to 32, but count
stays at 16 and only 16 new lines can be sent in the next packet, when
the intent was to try probing 32 on the second round. IOW fetch-pack.c
is not expanding the have window as quickly as I wanted.

In reality the above session probably sent:

  16 new,  0 state = 16 haves
  16 new, 10 state = 26 haves
  32 new, 13 state = 45 haves
  ?? new, ?? state = 70 haves

Maybe 120 unique have lines.


Looking at my request data and your Bloom filter "budget" of 2 KiB ..
we could just update fetch-pack.c to send 88 have lines in the first
request burst. 90+-ish% of my user's traffic might be served in just 1
round trip on smart HTTP, instead of 4.


This smart HTTP study also applies to the traditional bi-directional
protocol. With multi_ack the sends a 2nd batch while waiting for reply
from the server. With data moving in both directions on the wire total
round-trips becomes less important and its just data volume. But again
we are talking about 120 unique have lines, so 5.9 KiB (no
compression).

The have lines are very verbose in text format. Just using a binary
SHA-1 would nearly halve the negotiation to ~3 KiB. Binary SHA-1 have
line extension to upload-pack extension is far simpler than a Bloom
filter.

I do wonder if we are stuffing the pipe deep enough with multi_ack on
Internet links. Git doesn't need very long to walk 16 commits,
certainly less than the 200 ms RTT a user might have talking to a
server on the other side of the US. It is possible both sides are
spending most of their time waiting for data transfer of the batches
in flight.
--
To unsubscribe from this list: send the line "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 v7 1/3] worktree: add top-level worktree.c

[Mike Rappazzo]

On Thu, Sep 10, 2015 at 4:04 PM, Junio C Hamano  wrote:
> Michael Rappazzo  writes:
>
>> Including functions to get the list of all worktrees, and to get
>> a specific worktree (primary or linked).
>
> Was this meant as a continuation of the sentence started on the
> Subject line, or is s/Including/Include/ necessary?

I think I was continuing the subject line.  I will make it nicer.

>
>> + worktree_list = next;
>> + }
>> +}
>> +
>> +/*
>> + * read 'path_to_ref' into 'ref'.  Also set is_detached to 1 if the ref is 
>> detatched
>> + *
>> + * return 1 if the ref is not a proper ref, 0 otherwise (success)
>
> These lines are overly long.
>
>> + */
>> +int _parse_ref(char *path_to_ref, struct strbuf *ref, int *is_detached)
>> +{
>> + if (!strbuf_readlink(ref, path_to_ref, 0)) {
>> + if (!starts_with(ref->buf, "refs/") || 
>> check_refname_format(ref->buf, 0)) {
>
> An overly long line.  Perhaps
>
> if (!starts_with(ref->buf, "refs/") ||
> check_refname_format(ref->buf, 0)) {
>
> (I see many more "overly long line" after this point, which I won't mention).


Is the limit 80 characters?

>
>> + /* invalid ref - something is awry with this repo */
>> + return 1;
>> + }
>> + } else if (strbuf_read_file(ref, path_to_ref, 0) >= 0) {
>> + if (starts_with(ref->buf, "ref:")) {
>> + strbuf_remove(ref, 0, strlen("ref:"));
>> + strbuf_trim(ref);
>
> We don't do the same "starts_with() and format is sane" check?

The code from this snippet was mostly ripped from branch.c (see the
second commit).
I will add the sanity check.

>
>
> An idiomatic way to extend a singly-linked list at the end in our
> codebase is to use a pointer to the pointer that has the element at
> the end, instead of to use a pointer that points at the element at
> the end or NULL (i.e. the pointer the above code calls current_entry
> is "struct worktree_list **end_of_list").  Would it make the above
> simpler if you followed that pattern?
>

I think I follow what you are saying here.  I will explore this route.
I am also unhappy about
having to separately maintain a point to the head of the list when
using it.  Would it be
"normal" to add a head pointer to the list structure?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Feature Request] git blame showing only revisions from git rev-list --first-parent

[Stephen Connolly]

Background
===

My colleagues and I disagree on how exactly to handle feature
development in branches.
There are approximately two camps:

Camp 1: All feature branches should be squashed after code review to a
single commit before merging.

Camp 2: Leave the history of development alone, being able to trace
why the feature evolved that way is useful information, never squash
commits after code review.

Being somebody who leans towards the second camp I have been trying to
understand the first camps objections...

Those objections seem to boil down to tracing when the feature was introduced.

I have been able to get past some of the objections by pointing out
the `--first-parent` option available to `git log` and `gitk`. With
that command line option they are presented with a single history of
the branch and all merge commits are shown *as if squashed* (or at
least that is what it looks like from my testing)

The only remaining objection is around `git blame`...

You see a problem on one line in the file, you run `git blame
file.txt` and it says  was when it changed...

Now  is not on the main line of the branch but actually is a
commit that was merged from one branch to another before finally being
merged into the current branch in 

That is a complex nest of commits to detangle, especially when we have
had some refactoring branches with multiple experiments that drag on
over the course of a couple of weeks with many commits... all they
want to know is .

Yes I could (and I do try to) teach them how to find the commit where
that SHA was merged into the current branch, but really they just want
to be able to see the current branches SHAs only.

Doing some reading in the git blame sources I discovered that git
blame takes the rev-list command line options... so I thought "hey
--first-parent should work"

Except it doesn't

I suspect that this is because of how blame uses rev-list

Request
===

A command line option to `git blame HEAD -- path` that instructs that
the revisions of blame be the revisions where the change was applied
to the current branch not the revision where the change first
originated (i.e. limit commits to `git rev-list --first-parent HEAD`)

I can get what I want with the following:

git rev-list --first-parent HEAD | awk '{print p " " $0}{p=$0}' >
tmpfile && git blame -b -S tmpfile HEAD -- path && rm tmpfile

But that is a rather ugly command. Could we have something built in to
git blame to make this much easier for users?

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 v7 2/3] worktree: move/refactor find_shared_symref from branch.c

[Mike Rappazzo]

On Fri, Sep 11, 2015 at 7:10 PM, Eric Sunshine  wrote:
> On Fri, Sep 11, 2015 at 5:52 PM, Junio C Hamano  wrote:
>
> Thanks for bringing this up. I haven't found a sufficient block of
> time yet to review these patches, however, I had the same thought upon
> a quick initial read, and is how I was hoping to see the patch series
> constructed based upon my earlier two reviews suggesting refactoring
> the existing branch.c functions into a new get_worktrees(). There are
> at least a couple important reasons for taking this approach.
>
> First, it keeps the "blame" trail intact, the full context of which
> can be helpful to someone trying to understand why the code is the way
> it is. The current approach of having get_worktree_list() materialize
> out of thin air (even though it may have been patterned after existing
> code) doesn't give the same benefit.
>
> Second, it's easier for reviewers to understand and check the code for
> correctness if it mutates to the final form in small increments than
> it is to have get_worktrees() come into existence fully matured.
>
> A final minor comment: Since all three branch.c functions,
> die_if_checked_out(), find_shared_symref(), and find_linked_symref(),
> ought to be moved to top-level worktree.c, I'd probably have patch 1
> do the relocation (with no functional changes), and subsequent patches
> refactor the moved code into a general purpose get_worktrees(). The
> final patch would then implement "git worktrees list".

I like the way this history works out, and I have reworked the history
to follow this idea.  The only thing
I didn't do was move the die_if_checked_out() function from branch.c,
as I feel that this function is more
of a branch feature than a worktree feature.
--
To unsubscribe from this list: send the line "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: [Feature Request] git blame showing only revisions from git rev-list --first-parent

[Jeff King]

On Fri, Sep 11, 2015 at 12:06:27PM -0700, Junio C Hamano wrote:

> So here is an outline of what I had in mind when I was writing the
> above.  Instead of trusting that num_scapegoats() is always used to
> limit the enumeration of commit->parents, we discard the second and
> subsequent parents from the commit objects, and also make sure the
> later parents do not participate in the revs.children annotation, so
> that the world "blame" sees truly becomes a single strand of pearls.

Yeah, I think what is happening in this first hunk:

> diff --git a/builtin/blame.c b/builtin/blame.c
> index 4db01c1..bc87d9d 100644
> --- a/builtin/blame.c
> +++ b/builtin/blame.c
> @@ -1366,8 +1366,15 @@ static void pass_whole_blame(struct scoreboard *sb,
>   */
>  static struct commit_list *first_scapegoat(struct rev_info *revs, struct 
> commit *commit)
>  {
> - if (!reverse)
> + if (!reverse) {
> + if (revs->first_parent_only &&
> + commit->parents &&
> + commit->parents->next) {
> + free_commit_list(commit->parents->next);
> + commit->parents->next = NULL;
> + }
>   return commit->parents;
> + }
>   return lookup_decoration(>children, >object);
>  }

is doing the right thing. It did feel a little weird to me to be munging
the global commit objects themselves, but I guess it is fairly normal
for git code.

I wondered if you could then simplify away the counters used in the
for-loops. I.e., to end up with:

  for (sg = first_scapegoat(revs, commit); sg; sg = sg->next)

in the callers. But no. One of the reasons for the counters is that we
need to know we are on the n'th parent, so we can compare it to parent
n-1, n-2, etc, for sameness.

> diff --git a/revision.c b/revision.c
> index af2a18e..a020a42 100644
> --- a/revision.c
> +++ b/revision.c
> @@ -2765,7 +2765,9 @@ static void set_children(struct rev_info *revs)
>   struct commit *commit = l->item;
>   struct commit_list *p;
>  
> - for (p = commit->parents; p; p = p->next)
> + for (p = commit->parents;
> +  p && !revs->first_parent_only;
> +  p = p->next)
>   add_child(revs, p->item, commit);
>   }
>  }

Yeah, this makes sense to me. Adding all children to the decoration list
and then later choosing only the first child (as my earlier suggestion
did) is not right. You can tell immediately that it is nonsense because
the child you would get depends on the order we visited the commits when
building up the decoration. And that does not necessarily have any real
meaning.

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

[PATCH v17 06/14] ref-filter: implement an `align` atom

[Karthik Nayak]

Implement an `align` atom which left-, middle-, or right-aligns the
content between %(align:...) and %(end).

The "align:" is followed by `` and `` in any order
separated by a comma, where the `` is either left, right or
middle, default being left and `` is the total length of the
content with alignment. If the contents length is more than the width
then no alignment is performed.  e.g. to align a refname atom to the
middle with a total width of 40 we can do:
--format="%(align:middle,40)%(refname)%(end)".

We introduce an `at_end` function for each element of the stack which
is to be called when the `end` atom is encountered. Using this we
implement end_align_handler() for the `align` atom, this aligns the
final strbuf by calling `strbuf_utf8_align()` from utf8.c.

Ensure that quote formatting is performed on the whole of
%(align:...)...%(end) rather than individual atoms inside. We skip
quote formatting for individual atoms when the current stack element
is handling an %(align:...) atom and perform quote formatting at the
end when we encounter the %(end) atom of the second element of then
stack.

Add documentation and tests for the same.

Mentored-by: Christian Couder 
Mentored-by: Matthieu Moy 
Helped-by: Junio C Hamano 
Signed-off-by: Karthik Nayak 
---
 Documentation/git-for-each-ref.txt |  11 
 ref-filter.c   | 110 -
 t/t6302-for-each-ref-filter.sh |  82 +++
 3 files changed, 202 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-for-each-ref.txt 
b/Documentation/git-for-each-ref.txt
index e49d578..3a271bf 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -127,6 +127,17 @@ color::
Change output color.  Followed by `:`, where names
are described in `color.branch.*`.
 
+align::
+   Left-, middle-, or right-align the content between
+   %(align:...) and %(end). The "align:" is followed by ``
+   and `` in any order separated by a comma, where the
+   `` is either left, right or middle, default being
+   left and `` is the total length of the content with
+   alignment. If the contents length is more than the width then
+   no alignment is performed. If used with '--quote' everything
+   in between %(align:...) and %(end) is quoted, but if nested
+   then only the topmost level performs quoting.
+
 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 514de34..c65cd60 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -10,6 +10,7 @@
 #include "quote.h"
 #include "ref-filter.h"
 #include "revision.h"
+#include "utf8.h"
 
 typedef enum { FIELD_STR, FIELD_ULONG, FIELD_TIME } cmp_type;
 
@@ -53,13 +54,22 @@ static struct {
{ "flag" },
{ "HEAD" },
{ "color" },
+   { "align" },
+   { "end" },
 };
 
 #define REF_FORMATTING_STATE_INIT  { 0, NULL }
 
+struct align {
+   align_type position;
+   unsigned int width;
+};
+
 struct ref_formatting_stack {
struct ref_formatting_stack *prev;
struct strbuf output;
+   void (*at_end)(struct ref_formatting_stack *stack);
+   void *at_end_data;
 };
 
 struct ref_formatting_state {
@@ -69,6 +79,9 @@ struct ref_formatting_state {
 
 struct atom_value {
const char *s;
+   union {
+   struct align align;
+   } u;
void (*handler)(struct atom_value *atomv, struct ref_formatting_state 
*state);
unsigned long ul; /* used for sorting when not FIELD_STR */
 };
@@ -165,7 +178,16 @@ static void quote_formatting(struct strbuf *s, const char 
*str, int quote_style)
 
 static void append_atom(struct atom_value *v, struct ref_formatting_state 
*state)
 {
-   quote_formatting(>stack->output, v->s, state->quote_style);
+   /*
+* Quote formatting is only done when the stack has a single
+* element. Otherwise quote formatting is done on the
+* element's entire output strbuf when the %(end) atom is
+* encountered.
+*/
+   if (!state->stack->prev)
+   quote_formatting(>stack->output, v->s, 
state->quote_style);
+   else
+   strbuf_addstr(>stack->output, v->s);
 }
 
 static void push_stack_element(struct ref_formatting_stack **stack)
@@ -189,6 +211,48 @@ static void pop_stack_element(struct ref_formatting_stack 
**stack)
*stack = prev;
 }
 
+static void end_align_handler(struct ref_formatting_stack *stack)
+{
+   struct align *align = (struct align *)stack->at_end_data;
+   struct strbuf s = STRBUF_INIT;
+
+   strbuf_utf8_align(, align->position, align->width, stack->output.buf);
+   strbuf_swap(>output, );
+   strbuf_release();

[PATCH v17 06/14] ref-filter: implement an `align` atom

[Karthik Nayak]

Implement an `align` atom which left-, middle-, or right-aligns the
content between %(align:...) and %(end).

The "align:" is followed by `` and `` in any order
separated by a comma, where the `` is either left, right or
middle, default being left and `` is the total length of the
content with alignment. If the contents length is more than the width
then no alignment is performed.  e.g. to align a refname atom to the
middle with a total width of 40 we can do:
--format="%(align:middle,40)%(refname)%(end)".

We introduce an `at_end` function for each element of the stack which
is to be called when the `end` atom is encountered. Using this we
implement end_align_handler() for the `align` atom, this aligns the
final strbuf by calling `strbuf_utf8_align()` from utf8.c.

Ensure that quote formatting is performed on the whole of
%(align:...)...%(end) rather than individual atoms inside. We skip
quote formatting for individual atoms when the current stack element
is handling an %(align:...) atom and perform quote formatting at the
end when we encounter the %(end) atom of the second element of then
stack.

Add documentation and tests for the same.

Mentored-by: Christian Couder 
Mentored-by: Matthieu Moy 
Helped-by: Junio C Hamano 
Signed-off-by: Karthik Nayak 
---
 Documentation/git-for-each-ref.txt |  11 
 ref-filter.c   | 110 -
 t/t6302-for-each-ref-filter.sh |  82 +++
 3 files changed, 202 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-for-each-ref.txt 
b/Documentation/git-for-each-ref.txt
index e49d578..3a271bf 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -127,6 +127,17 @@ color::
Change output color.  Followed by `:`, where names
are described in `color.branch.*`.
 
+align::
+   Left-, middle-, or right-align the content between
+   %(align:...) and %(end). The "align:" is followed by ``
+   and `` in any order separated by a comma, where the
+   `` is either left, right or middle, default being
+   left and `` is the total length of the content with
+   alignment. If the contents length is more than the width then
+   no alignment is performed. If used with '--quote' everything
+   in between %(align:...) and %(end) is quoted, but if nested
+   then only the topmost level performs quoting.
+
 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 514de34..c65cd60 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -10,6 +10,7 @@
 #include "quote.h"
 #include "ref-filter.h"
 #include "revision.h"
+#include "utf8.h"
 
 typedef enum { FIELD_STR, FIELD_ULONG, FIELD_TIME } cmp_type;
 
@@ -53,13 +54,22 @@ static struct {
{ "flag" },
{ "HEAD" },
{ "color" },
+   { "align" },
+   { "end" },
 };
 
 #define REF_FORMATTING_STATE_INIT  { 0, NULL }
 
+struct align {
+   align_type position;
+   unsigned int width;
+};
+
 struct ref_formatting_stack {
struct ref_formatting_stack *prev;
struct strbuf output;
+   void (*at_end)(struct ref_formatting_stack *stack);
+   void *at_end_data;
 };
 
 struct ref_formatting_state {
@@ -69,6 +79,9 @@ struct ref_formatting_state {
 
 struct atom_value {
const char *s;
+   union {
+   struct align align;
+   } u;
void (*handler)(struct atom_value *atomv, struct ref_formatting_state 
*state);
unsigned long ul; /* used for sorting when not FIELD_STR */
 };
@@ -165,7 +178,16 @@ static void quote_formatting(struct strbuf *s, const char 
*str, int quote_style)
 
 static void append_atom(struct atom_value *v, struct ref_formatting_state 
*state)
 {
-   quote_formatting(>stack->output, v->s, state->quote_style);
+   /*
+* Quote formatting is only done when the stack has a single
+* element. Otherwise quote formatting is done on the
+* element's entire output strbuf when the %(end) atom is
+* encountered.
+*/
+   if (!state->stack->prev)
+   quote_formatting(>stack->output, v->s, 
state->quote_style);
+   else
+   strbuf_addstr(>stack->output, v->s);
 }
 
 static void push_stack_element(struct ref_formatting_stack **stack)
@@ -189,6 +211,48 @@ static void pop_stack_element(struct ref_formatting_stack 
**stack)
*stack = prev;
 }
 
+static void end_align_handler(struct ref_formatting_stack *stack)
+{
+   struct align *align = (struct align *)stack->at_end_data;
+   struct strbuf s = STRBUF_INIT;
+
+   strbuf_utf8_align(, align->position, align->width, stack->output.buf);
+   strbuf_swap(>output, );
+   strbuf_release();

[PATCH v17 05/14] ref-filter: introduce match_atom_name()

[Karthik Nayak]

Introduce match_atom_name() which helps in checking if a particular
atom is the atom we're looking for and if it has a value attached to
it or not.

Use it instead of starts_with() for checking the value of %(color:...)
atom. Write a test for the same.

Mentored-by: Christian Couder 
Mentored-by: Matthieu Moy 
Thanks-to: Junio C Hamano 
Signed-off-by: Karthik Nayak 
---
 ref-filter.c   | 23 +--
 t/t6302-for-each-ref-filter.sh |  4 
 2 files changed, 25 insertions(+), 2 deletions(-)

diff --git a/ref-filter.c b/ref-filter.c
index a993216..514de34 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -189,6 +189,22 @@ static void pop_stack_element(struct ref_formatting_stack 
**stack)
*stack = prev;
 }
 
+static int match_atom_name(const char *name, const char *atom_name, const char 
**val)
+{
+   const char *body;
+
+   if (!skip_prefix(name, atom_name, ))
+   return 0; /* doesn't even begin with "atom_name" */
+   if (!body[0]) {
+   *val = NULL; /* %(atom_name) and no customization */
+   return 1;
+   }
+   if (body[0] != ':')
+   return 0; /* "atom_namefoo" is not "atom_name" or 
"atom_name:..." */
+   *val = body + 1; /* "atom_name:val" */
+   return 1;
+}
+
 /*
  * In a format string, find the next occurrence of %(atom).
  */
@@ -687,6 +703,7 @@ static void populate_value(struct ref_array_item *ref)
int deref = 0;
const char *refname;
const char *formatp;
+   const char *valp;
struct branch *branch = NULL;
 
v->handler = append_atom;
@@ -721,10 +738,12 @@ static void populate_value(struct ref_array_item *ref)
refname = branch_get_push(branch, NULL);
if (!refname)
continue;
-   } else if (starts_with(name, "color:")) {
+   } else if (match_atom_name(name, "color", )) {
char color[COLOR_MAXLEN] = "";
 
-   if (color_parse(name + 6, color) < 0)
+   if (!valp)
+   die(_("expected format: %%(color:)"));
+   if (color_parse(valp, color) < 0)
die(_("unable to parse format"));
v->s = xstrdup(color);
continue;
diff --git a/t/t6302-for-each-ref-filter.sh b/t/t6302-for-each-ref-filter.sh
index 505a360..c4f0378 100755
--- a/t/t6302-for-each-ref-filter.sh
+++ b/t/t6302-for-each-ref-filter.sh
@@ -81,4 +81,8 @@ test_expect_success 'filtering with --contains' '
test_cmp expect actual
 '
 
+test_expect_success '%(color) must fail' '
+   test_must_fail git for-each-ref --format="%(color)%(refname)"
+'
+
 test_done
-- 
2.5.1

--
To unsubscribe from this list: send the line "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 v17 08/14] ref-filter: add support for %(contents:lines=X)

[Karthik Nayak]

In 'tag.c' we can print N lines from the annotation of the tag using
the '-n' option. Copy code from 'tag.c' to 'ref-filter' and
modify it to support appending of N lines from the annotation of tags
to the given strbuf.

Implement %(contents:lines=X) where X lines of the given object are
obtained.

While we're at it, remove unused "contents:" atoms from
the `valid_atom` array.

Add documentation and test for the same.

Mentored-by: Christian Couder 
Mentored-by: Matthieu Moy 
Signed-off-by: Karthik Nayak 
---
 Documentation/git-for-each-ref.txt |  3 ++-
 builtin/tag.c  |  4 +++
 ref-filter.c   | 47 +++---
 ref-filter.h   |  3 ++-
 t/t6302-for-each-ref-filter.sh | 52 ++
 5 files changed, 103 insertions(+), 6 deletions(-)

diff --git a/Documentation/git-for-each-ref.txt 
b/Documentation/git-for-each-ref.txt
index 3a271bf..324ad2c 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -150,7 +150,8 @@ The complete message in a commit and tag object is 
`contents`.
 Its first line is `contents:subject`, where subject is the concatenation
 of all lines of the commit message up to the first blank line.  The next
 line is 'contents:body', where body is all of the lines after the first
-blank line.  Finally, the optional GPG signature is `contents:signature`.
+blank line.  The optional GPG signature is `contents:signature`.  The
+first `N` lines of the message is obtained using `contents:lines=N`.
 
 For sorting purposes, fields with numeric values sort in numeric
 order (`objectsize`, `authordate`, `committerdate`, `taggerdate`).
diff --git a/builtin/tag.c b/builtin/tag.c
index 471d6b1..b0bc1c5 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -185,6 +185,10 @@ static enum contains_result contains(struct commit 
*candidate,
return contains_test(candidate, want);
 }
 
+/*
+ * Currently modified and used in ref-filter as append_lines(), will
+ * eventually be removed as we port tag.c to use ref-filter APIs.
+ */
 static void show_tag_lines(const struct object_id *oid, int lines)
 {
int i;
diff --git a/ref-filter.c b/ref-filter.c
index f046d82..32aab37 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -45,9 +45,6 @@ static struct {
{ "subject" },
{ "body" },
{ "contents" },
-   { "contents:subject" },
-   { "contents:body" },
-   { "contents:signature" },
{ "upstream" },
{ "push" },
{ "symref" },
@@ -65,6 +62,11 @@ struct align {
unsigned int width;
 };
 
+struct contents {
+   unsigned int lines;
+   struct object_id oid;
+};
+
 struct ref_formatting_stack {
struct ref_formatting_stack *prev;
struct strbuf output;
@@ -81,6 +83,7 @@ struct atom_value {
const char *s;
union {
struct align align;
+   struct contents contents;
} u;
void (*handler)(struct atom_value *atomv, struct ref_formatting_state 
*state);
unsigned long ul; /* used for sorting when not FIELD_STR */
@@ -643,6 +646,30 @@ static void find_subpos(const char *buf, unsigned long sz,
*nonsiglen = *sig - buf;
 }
 
+/*
+ * If 'lines' is greater than 0, append that many lines from the given
+ * 'buf' of length 'size' to the given strbuf.
+ */
+static void append_lines(struct strbuf *out, const char *buf, unsigned long 
size, int lines)
+{
+   int i;
+   const char *sp, *eol;
+   size_t len;
+
+   sp = buf;
+
+   for (i = 0; i < lines && sp < buf + size; i++) {
+   if (i)
+   strbuf_addstr(out, "\n");
+   eol = memchr(sp, '\n', size - (sp - buf));
+   len = eol ? eol - sp : size - (sp - buf);
+   strbuf_add(out, sp, len);
+   if (!eol)
+   break;
+   sp = eol + 1;
+   }
+}
+
 /* See grab_values */
 static void grab_sub_body_contents(struct atom_value *val, int deref, struct 
object *obj, void *buf, unsigned long sz)
 {
@@ -653,6 +680,7 @@ static void grab_sub_body_contents(struct atom_value *val, 
int deref, struct obj
for (i = 0; i < used_atom_cnt; i++) {
const char *name = used_atom[i];
struct atom_value *v = [i];
+   const char *valp = NULL;
if (!!deref != (*name == '*'))
continue;
if (deref)
@@ -662,7 +690,8 @@ static void grab_sub_body_contents(struct atom_value *val, 
int deref, struct obj
strcmp(name, "contents") &&
strcmp(name, "contents:subject") &&
strcmp(name, "contents:body") &&
-   strcmp(name, "contents:signature"))
+   strcmp(name, "contents:signature") &&
+   

[PATCH] l10n: de.po: translate 121 new messages

[Ralf Thielow]

Translate 121 new messages came from git.pot update in 47d73eb
(l10n: git.pot: proposed updates for v2.6.0 (+121)).

Signed-off-by: Ralf Thielow 
---
 po/de.po | 384 ++-
 1 file changed, 183 insertions(+), 201 deletions(-)

diff --git a/po/de.po b/po/de.po
index 2874dc5..ae616db 100644
--- a/po/de.po
+++ b/po/de.po
@@ -33,19 +33,18 @@ msgstr ""
 
 #: advice.c:101 builtin/merge.c:1227
 msgid "You have not concluded your merge (MERGE_HEAD exists)."
 msgstr "Sie haben Ihren Merge nicht abgeschlossen (MERGE_HEAD existiert)."
 
 #: advice.c:103
-#, fuzzy
 msgid "Please, commit your changes before you can merge."
-msgstr "Bitte gebe die Versionsbeschreibung für deine Änderungen ein."
+msgstr "Bitte committen Sie Ihre Änderungen, bevor Sie \"merge\" ausführen."
 
 #: advice.c:104
 msgid "Exiting because of unfinished merge."
-msgstr ""
+msgstr "Beende wegen nicht abgeschlossenem Merge."
 
 #: archive.c:12
 msgid "git archive []  [...]"
 msgstr "git archive []  [...]"
 
 #: archive.c:13
@@ -1084,25 +1083,25 @@ msgstr ""
 #: builtin/merge.c:983
 #, c-format
 msgid "Could not open '%s' for writing"
 msgstr "Konnte '%s' nicht zum Schreiben öffnen."
 
 #: refs.c:3001
-#, fuzzy, c-format
+#, c-format
 msgid "could not delete reference %s: %s"
-msgstr "Konnte %s nicht entfernen"
+msgstr "Konnte Referenz %s nicht entfernen: %s"
 
 #: refs.c:3004
-#, fuzzy, c-format
+#, c-format
 msgid "could not delete references: %s"
-msgstr "Konnte %s nicht entfernen"
+msgstr "Konnte Referenzen nicht entfernen: %s"
 
 #: refs.c:3013
-#, fuzzy, c-format
+#, c-format
 msgid "could not remove reference %s"
-msgstr "Konnte Branch %s nicht löschen"
+msgstr "Konnte Referenz %s nicht löschen"
 
 #: ref-filter.c:660
 msgid "unable to parse format"
 msgstr "Konnte Format nicht parsen."
 
 #: remote.c:792
@@ -1250,18 +1249,18 @@ msgstr "Fehler beim Signieren des \"push\"-Zertifikates"
 #: send-pack.c:404
 msgid "the receiving end does not support --signed push"
 msgstr ""
 "die Gegenseite unterstützt keinen signierten Versand (\"--signed push\")"
 
 #: send-pack.c:406
-#, fuzzy
 msgid ""
 "not sending a push certificate since the receiving end does not support --"
 "signed push"
 msgstr ""
-"die Gegenseite unterstützt keinen signierten Versand (\"--signed push\")"
+"kein Versand des \"push\"-Zertifikates, da die Gegenseite keinen signierten\n"
+"Versand (\"--signed push\") unterstützt"
 
 #: send-pack.c:418
 msgid "the receiving end does not support --atomic push"
 msgstr "die Gegenseite unterstützt keinen atomaren Versand (\"--atomic push\")"
 
 #: sequencer.c:183
@@ -1301,13 +1300,13 @@ msgstr ""
 msgid "Your local changes would be overwritten by revert."
 msgstr "Ihre lokalen Änderungen würden von \"revert\" überschrieben werden."
 
 #: sequencer.c:222
 msgid "Commit your changes or stash them to proceed."
 msgstr ""
-"Tragen Sie Ihre Änderungen ein oder benutzen Sie \"stash\", um fortzufahren."
+"Committen Sie Ihre Änderungen oder benutzen Sie \"stash\", um fortzufahren."
 
 #. TRANSLATORS: %s will be "revert" or "cherry-pick"
 #: sequencer.c:309
 #, c-format
 msgid "%s: Unable to write new index file"
 msgstr "%s: Konnte neue Index-Datei nicht schreiben"
@@ -1584,15 +1583,15 @@ msgstr "Konnte Eingabe-Datei '%s' nicht lesen"
 
 #: trailer.c:704
 msgid "could not read from stdin"
 msgstr "konnte nicht von der Standard-Eingabe lesen"
 
 #: transport-helper.c:1025
-#, fuzzy, c-format
+#, c-format
 msgid "Could not read ref %s"
-msgstr "Konnte %s nicht lesen."
+msgstr "Konnte Referenz %s nicht lesen."
 
 #: unpack-trees.c:203
 msgid "Checking out files"
 msgstr "Checke Dateien aus"
 
 #: urlmatch.c:120
@@ -1622,18 +1621,18 @@ msgstr "ungültige Portnummer"
 
 #: urlmatch.c:322
 msgid "invalid '..' path segment"
 msgstr "ungültiges '..' Pfadsegment"
 
 #: wrapper.c:219 wrapper.c:362
-#, fuzzy, c-format
+#, c-format
 msgid "could not open '%s' for reading and writing"
-msgstr "Konnte '%s' nicht zum Lesen öffnen."
+msgstr "Konnte '%s' nicht zum Lesen und Schreiben öffnen."
 
 #: wrapper.c:221 wrapper.c:364
-#, fuzzy, c-format
+#, c-format
 msgid "could not open '%s' for writing"
 msgstr "Konnte '%s' nicht zum Schreiben öffnen."
 
 #: wrapper.c:223 wrapper.c:366 builtin/am.c:337 builtin/commit.c:1688
 #: builtin/merge.c:1076 builtin/pull.c:380
 #, c-format
@@ -1886,44 +1885,41 @@ msgstr "  (benutzen Sie \"git am --skip\", um diesen 
Patch auszulassen)"
 msgid "  (use \"git am --abort\" to restore the original branch)"
 msgstr ""
 "  (benutzen Sie \"git am --abort\", um den ursprünglichen Branch "
 "wiederherzustellen)"
 
 #: wt-status.c:1105
-#, fuzzy
 msgid "No commands done."
-msgstr "Keine Commits geparst."
+msgstr "Keine Kommandos ausgeführt."
 
 #: wt-status.c:1108
 #, c-format
 msgid "Last command done (%d command done):"
 msgid_plural "Last commands done (%d commands done):"
-msgstr[0] ""
-msgstr[1] ""
+msgstr[0] "Letztes Kommando ausgeführt (%d Kommandos 

Re: [PATCH] l10n: de.po: translate 121 new messages

[Ralf Thielow]

Am 11. September 2015 um 17:17 schrieb Ralf Thielow :
> Translate 121 new messages came from git.pot update in 47d73eb
> (l10n: git.pot: proposed updates for v2.6.0 (+121)).
>

Hmpf. Please ignore this as the commit this translation is based on is not the
latest. New patch will follow soon.
--
To unsubscribe from this list: send the line "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 v17 12/14] tag.c: use 'ref-filter' APIs

[Karthik Nayak]

From: Karthik Nayak 

Make 'tag.c' use 'ref-filter' APIs for iterating through refs, sorting
and printing of refs. This removes most of the code used in 'tag.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.

For printing tags we use 'show_ref_array_item()' function provided by
'ref-filter'.

We improve the sorting option provided by 'tag.c' by using the sorting
options provided by 'ref-filter'. This causes the test 'invalid sort
parameter on command line' in t7004 to fail, as 'ref-filter' throws an
error for all sorting fields which are incorrect. The test is changed
to reflect the same.

Modify documentation for the same.

Mentored-by: Christian Couder 
Mentored-by: Matthieu Moy 
Signed-off-by: Karthik Nayak 
---
 Documentation/git-tag.txt |  16 ++-
 builtin/tag.c | 345 ++
 t/t7004-tag.sh|   8 +-
 3 files changed, 53 insertions(+), 316 deletions(-)

diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt
index 84f6496..3ac4a96 100644
--- a/Documentation/git-tag.txt
+++ b/Documentation/git-tag.txt
@@ -13,7 +13,7 @@ SYNOPSIS
 [ | ]
 'git tag' -d ...
 'git tag' [-n[]] -l [--contains ] [--points-at ]
-   [--column[=] | --no-column] [--create-reflog] [...]
+   [--column[=] | --no-column] [--create-reflog] [--sort=] 
[...]
 'git tag' -v ...
 
 DESCRIPTION
@@ -94,14 +94,16 @@ OPTIONS
using fnmatch(3)).  Multiple patterns may be given; if any of
them matches, the tag is shown.
 
---sort=::
-   Sort in a specific order. Supported type is "refname"
-   (lexicographic order), "version:refname" or "v:refname" (tag
+--sort=::
+   Sort based on the key given.  Prefix `-` to sort in
+   descending order of the value. You may use the --sort= option
+   multiple times, in which case the last key becomes the primary
+   key. Also supports "version:refname" or "v:refname" (tag
names are treated as versions). The "version:refname" sort
order can also be affected by the
-   "versionsort.prereleaseSuffix" configuration variable. Prepend
-   "-" to reverse sort order. When this option is not given, the
-   sort order defaults to the value configured for the 'tag.sort'
+   "versionsort.prereleaseSuffix" configuration variable.
+   The keys supported are the same as those in `git for-each-ref`.
+   Sort order defaults to the value configured for the 'tag.sort'
variable if it exists, or lexicographic order otherwise. See
linkgit:git-config[1].
 
diff --git a/builtin/tag.c b/builtin/tag.c
index fe66f7b..977a18c 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -28,278 +28,35 @@ static const char * const git_tag_usage[] = {
NULL
 };
 
-#define STRCMP_SORT 0  /* must be zero */
-#define VERCMP_SORT 1
-#define SORT_MASK   0x7fff
-#define REVERSE_SORT0x8000
-
-static int tag_sort;
-
 static unsigned int colopts;
 
-static int match_pattern(const char **patterns, const char *ref)
-{
-   /* no pattern means match everything */
-   if (!*patterns)
-   return 1;
-   for (; *patterns; patterns++)
-   if (!wildmatch(*patterns, ref, 0, NULL))
-   return 1;
-   return 0;
-}
-
-/*
- * This is currently duplicated in ref-filter.c, and will eventually be
- * removed as we port tag.c to use the ref-filter APIs.
- */
-static const unsigned char *match_points_at(const char *refname,
-   const unsigned char *sha1,
-   struct sha1_array *points_at)
-{
-   const unsigned char *tagged_sha1 = NULL;
-   struct object *obj;
-
-   if (sha1_array_lookup(points_at, sha1) >= 0)
-   return sha1;
-   obj = parse_object(sha1);
-   if (!obj)
-   die(_("malformed object at '%s'"), refname);
-   if (obj->type == OBJ_TAG)
-   tagged_sha1 = ((struct tag *)obj)->tagged->sha1;
-   if (tagged_sha1 && sha1_array_lookup(points_at, tagged_sha1) >= 0)
-   return tagged_sha1;
-   return NULL;
-}
-
-static int in_commit_list(const struct commit_list *want, struct commit *c)
-{
-   for (; want; want = want->next)
-   if (!hashcmp(want->item->object.sha1, c->object.sha1))
-   return 1;
-   return 0;
-}
-
-/*
- * The entire code segment for supporting the --contains option has been
- * copied over to ref-filter.{c,h}. This will be deleted evetually when
- * we port tag.c to use ref-filter APIs.
- */
-enum contains_result {
-   CONTAINS_UNKNOWN = -1,
-   CONTAINS_NO = 0,
-   CONTAINS_YES = 1
-};
-
-/*
- * Test whether the candidate or one of its parents is contained in the 

[PATCH v17 13/14] tag.c: implement '--format' option

[Karthik Nayak]

Implement the '--format' option provided by 'ref-filter'.
This lets the user list tags as per desired format similar
to the implementation in 'git for-each-ref'.

Add tests and documentation for the same.

Mentored-by: Christian Couder 
Mentored-by: Matthieu Moy 
Signed-off-by: Karthik Nayak 
---
 Documentation/git-tag.txt |  8 +++-
 builtin/tag.c | 24 ++--
 t/t7004-tag.sh| 12 
 3 files changed, 33 insertions(+), 11 deletions(-)

diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt
index 3ac4a96..0c7f4e6 100644
--- a/Documentation/git-tag.txt
+++ b/Documentation/git-tag.txt
@@ -13,7 +13,8 @@ SYNOPSIS
 [ | ]
 'git tag' -d ...
 'git tag' [-n[]] -l [--contains ] [--points-at ]
-   [--column[=] | --no-column] [--create-reflog] [--sort=] 
[...]
+   [--column[=] | --no-column] [--create-reflog] [--sort=]
+   [--format=] [...]
 'git tag' -v ...
 
 DESCRIPTION
@@ -158,6 +159,11 @@ This option is only applicable when listing tags without 
annotation lines.
The object that the new tag will refer to, usually a commit.
Defaults to HEAD.
 
+::
+   A string that interpolates `%(fieldname)` from the object
+   pointed at by a ref being shown.  The format is the same as
+   that of linkgit:git-for-each-ref[1].  When unspecified,
+   defaults to `%(refname:short)`.
 
 CONFIGURATION
 -
diff --git a/builtin/tag.c b/builtin/tag.c
index 977a18c..91c17b7 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -23,17 +23,17 @@ static const char * const git_tag_usage[] = {
N_("git tag [-a | -s | -u ] [-f] [-m  | -F ] 
 []"),
N_("git tag -d ..."),
N_("git tag -l [-n[]] [--contains ] [--points-at ]"
-   "\n\t\t[...]"),
+   "\n\t\t[--format=] [...]"),
N_("git tag -v ..."),
NULL
 };
 
 static unsigned int colopts;
 
-static int list_tags(struct ref_filter *filter, struct ref_sorting *sorting)
+static int list_tags(struct ref_filter *filter, struct ref_sorting *sorting, 
const char *format)
 {
struct ref_array array;
-   char *format, *to_free = NULL;
+   char *to_free = NULL;
int i;
 
memset(, 0, sizeof(array));
@@ -41,12 +41,14 @@ static int list_tags(struct ref_filter *filter, struct 
ref_sorting *sorting)
if (filter->lines == -1)
filter->lines = 0;
 
-   if (filter->lines) {
-   to_free = xstrfmt("%s %%(contents:lines=%d)",
-"%(align:15)%(refname:short)%(end)", 
filter->lines);
-   format = to_free;
-   } else
-   format = "%(refname:short)";
+   if (!format) {
+   if (filter->lines) {
+   to_free = xstrfmt("%s %%(contents:lines=%d)",
+ "%(align:15)%(refname:short)%(end)", 
filter->lines);
+   format = to_free;
+   } else
+   format = "%(refname:short)";
+   }
 
verify_ref_format(format);
filter_refs(, filter, FILTER_REFS_TAGS);
@@ -330,6 +332,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
struct strbuf err = STRBUF_INIT;
struct ref_filter filter;
static struct ref_sorting *sorting = NULL, **sorting_tail = 
+   const char *format = NULL;
struct option options[] = {
OPT_CMDMODE('l', "list", , N_("list tag names"), 'l'),
{ OPTION_INTEGER, 'n', NULL, , N_("n"),
@@ -362,6 +365,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
OPTION_CALLBACK, 0, "points-at", _at, 
N_("object"),
N_("print only tags of the object"), 0, 
parse_opt_object_name
},
+   OPT_STRING(  0 , "format", , N_("format"), N_("format to 
use for the output")),
OPT_END()
};
 
@@ -402,7 +406,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
run_column_filter(colopts, );
}
filter.name_patterns = argv;
-   ret = list_tags(, sorting);
+   ret = list_tags(, sorting, format);
if (column_active(colopts))
stop_column_filter();
return ret;
diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index 84153ef..8987fb1 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -1519,4 +1519,16 @@ EOF"
test_cmp expect actual
 '
 
+test_expect_success '--format should list tags as per format given' '
+   cat >expect <<-\EOF &&
+   refname : refs/tags/foo1.10
+   refname : refs/tags/foo1.3
+   refname : refs/tags/foo1.6
+   refname : refs/tags/foo1.6-rc1
+   refname : refs/tags/foo1.6-rc2
+   EOF
+   git tag -l --format="refname : %(refname)" "foo*" >actual &&
+  

Re: [PATCH v17 00/14] port tag.c to use ref-filter APIs

[Karthik Nayak]

On Thu, Sep 10, 2015 at 10:27 PM, Matthieu Moy
 wrote:
> Karthik Nayak  writes:
>
>> This is part of the series of unifying the code used by
>> "git tag -l, git branch -l, git for-each-ref".
>>
>> The previous version can be found here (version 16):
>> article.gmane.org/gmane.comp.version-control.git/277394
>>
>> Changes in this version:
>> * The arguments of the %(align) atom are interchangeable.
>> * Small grammatical changes.
>> * Small changes in the tests to reflect changes in the align
>> atom code.
>
> Clearly, we're almost there. I did a few minor remarks. I suggest
> (admitedly, Eric suggested of-list to suggest ;-) ) that you reply to
> them by re-sending only individual patches that changed (replying to the
> original patch) so that we can check the new patches individually. I
> think we can do the finishing touches for each patch in a subthread of
> this patch.
>

I replied with suggested changes by you and Junio.
Let me know if any other changes to be made :)

-- 
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] Pass graph width to pretty formatting, so '%>|' can work properly

[Josef Kufner]

Pass graph width to pretty formatting function, so it can handle
'%>|(N)' paddings correctly when --graph option is used.

Example:
  git log --all --graph --pretty='format: [%>|(20)%h] %ar%d'

  All commit hashes should be aligned at 20th column from edge of the
  terminal, not from the edge of the graph.

Signed-off-by: Josef Kufner 
---
 commit.h   | 1 +
 graph.c| 7 +++
 graph.h| 5 +
 log-tree.c | 2 ++
 pretty.c   | 1 +
 5 files changed, 16 insertions(+)

diff --git a/commit.h b/commit.h
index 5d58be0..0a9a707 100644
--- a/commit.h
+++ b/commit.h
@@ -160,6 +160,7 @@ struct pretty_print_context {
 * should not be counted on by callers.
 */
struct string_list in_body_headers;
+   int graph_width;
 };
 
 struct userformat_want {
diff --git a/graph.c b/graph.c
index c25a09a..4802411 100644
--- a/graph.c
+++ b/graph.c
@@ -671,6 +671,13 @@ static void graph_output_padding_line(struct git_graph 
*graph,
graph_pad_horizontally(graph, sb, graph->num_new_columns * 2);
 }
 
+
+int graph_width(struct git_graph *graph)
+{
+   return graph->width;
+}
+
+
 static void graph_output_skip_line(struct git_graph *graph, struct strbuf *sb)
 {
/*
diff --git a/graph.h b/graph.h
index 0be62bd..3f48c19 100644
--- a/graph.h
+++ b/graph.h
@@ -68,6 +68,11 @@ int graph_next_line(struct git_graph *graph, struct strbuf 
*sb);
 
 
 /*
+ * Return current width of the graph in on-screen characters.
+ */
+int graph_width(struct git_graph *graph);
+
+/*
  * graph_show_*: helper functions for printing to stdout
  */
 
diff --git a/log-tree.c b/log-tree.c
index 7b1b57a..08fd5b6 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -686,6 +686,8 @@ void show_log(struct rev_info *opt)
ctx.output_encoding = get_log_output_encoding();
if (opt->from_ident.mail_begin && opt->from_ident.name_begin)
ctx.from_ident = >from_ident;
+   if (opt->graph)
+   ctx.graph_width = graph_width(opt->graph);
pretty_print_commit(, commit, );
 
if (opt->add_signoff)
diff --git a/pretty.c b/pretty.c
index 151c2ae..f1cf9e2 100644
--- a/pretty.c
+++ b/pretty.c
@@ -1297,6 +1297,7 @@ static size_t format_and_pad_commit(struct strbuf *sb, /* 
in UTF-8 */
if (!start)
start = sb->buf;
occupied = utf8_strnwidth(start, -1, 1);
+   occupied += c->pretty_ctx->graph_width;
padding = (-padding) - occupied;
}
while (1) {
-- 
2.5.1

--
To unsubscribe from this list: send the line "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 submodule ignores --git-dir

[Jens Lehmann]

Am 10.09.2015 um 22:06 schrieb Filip Gospodinov:

Hi!

I use the `--git-dir` flag in some scripts such that I don't need to `cd` back
and forth. Recently, I've discovered that `--git-dir` does not seem to work
correctly for `git submodule`. Here is a short snippet to reproduce that 
behavior:

mkdir repo1 subm
(cd subm; git init; git commit -m 1 --allow-empty)
(cd repo1; git init; git submodule add ../subm subm; git commit -m "add subm")
git clone repo1 repo2
git --git-dir=$PWD/repo2/.git submodule update --init


which errors with the following output:

No submodule mapping found in .gitmodules for path 'subm'

But this works:
cd repo2; git --git-dir=$PWD/.git submodule update --init


Thanks for your report containing a nice recipe to reproduce your issue!


I know that for this particular use case I can just use `git clone --recursive`
and that other use cases can be worked around by using `cd`. Still, I wonder if
the behavior I discovered is a bug or if it's expected.


I don't think this is a bug. The git submodule command needs a work tree
to read the .gitmodules file from, that's while it fails when using
--git-dir from outside the work tree. But I admit that the error message
"No submodule mapping found in .gitmodules for path ..." could be improved
to clearly state that the .gitmodules file wasn't found.

Unfortunately trying to show git the right work tree:

$ git --git-dir=$PWD/repo2/.git --work-tree=$PWD/repo2 submodule update --init

Didn't work as I expected it to either:

fatal: /home/Sledge/libexec/git-core/git-submodule cannot be used without a 
working tree.

So you'll have to cd into the repo for now.
--
To unsubscribe from this list: send the line "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 submodule ignores --git-dir

[John Keeping]

On Fri, Sep 11, 2015 at 05:15:52PM +0200, Jens Lehmann wrote:
> Am 10.09.2015 um 22:06 schrieb Filip Gospodinov:
> > I know that for this particular use case I can just use `git clone 
> > --recursive`
> > and that other use cases can be worked around by using `cd`. Still, I 
> > wonder if
> > the behavior I discovered is a bug or if it's expected.
> 
> I don't think this is a bug. The git submodule command needs a work tree
> to read the .gitmodules file from, that's while it fails when using
> --git-dir from outside the work tree. But I admit that the error message
> "No submodule mapping found in .gitmodules for path ..." could be improved
> to clearly state that the .gitmodules file wasn't found.
> 
> Unfortunately trying to show git the right work tree:
> 
> $ git --git-dir=$PWD/repo2/.git --work-tree=$PWD/repo2 submodule update --init
> 
> Didn't work as I expected it to either:
> 
> fatal: /home/Sledge/libexec/git-core/git-submodule cannot be used without a 
> working tree.
> 
> So you'll have to cd into the repo for now.

There's also "git -C /path/to/repo" which avoids the need for a separate
"cd".
--
To unsubscribe from this list: send the line "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: [Feature Request] git blame showing only revisions from git rev-list --first-parent

[Stephen Connolly]

On 11 September 2015 at 15:01, Jeff King  wrote:
> On Fri, Sep 11, 2015 at 11:47:30AM +0100, Stephen Connolly wrote:
>
>> A command line option to `git blame HEAD -- path` that instructs that
>> the revisions of blame be the revisions where the change was applied
>> to the current branch not the revision where the change first
>> originated (i.e. limit commits to `git rev-list --first-parent HEAD`)
>>
>> I can get what I want with the following:
>>
>> git rev-list --first-parent HEAD | awk '{print p " " $0}{p=$0}' >
>> tmpfile && git blame -b -S tmpfile HEAD -- path && rm tmpfile
>>
>> But that is a rather ugly command. Could we have something built in to
>> git blame to make this much easier for users?
>
> I agree this would be a useful feature. Though blame takes rev-list
> options, it doesn't use the stock rev-list traversal internally, so it
> has to handle first-parent itself.
>
> I'm not too familiar with the code, but this _seems_ to work for me:
>
> diff --git a/builtin/blame.c b/builtin/blame.c
> index 21321be..2e03d47 100644
> --- a/builtin/blame.c
> +++ b/builtin/blame.c
> @@ -1375,6 +1375,10 @@ static struct commit_list *first_scapegoat(struct 
> rev_info *revs, struct commit
>  static int num_scapegoats(struct rev_info *revs, struct commit *commit)
>  {
> struct commit_list *l = first_scapegoat(revs, commit);
> +   if (!l)
> +   return 0;
> +   if (revs->first_parent_only)
> +   return 1;
> return commit_list_count(l);
>  }
>
>
> I suspect it doesn't work at all with `--reverse`. I also have the
> nagging feeling that this could be handled inside revision.c with
> parent-rewriting, but I don't really know.
>
> But "git blame --first-parent " seems to behave sanely in git.git
> with this.
>
> -Peff

It would help if somebody who knew the code could comment, but I am
impressed with the progress ;-)
--
To unsubscribe from this list: send the line "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] l10n: de.po: translate 123 new messages

[Ralf Thielow]

Am 11. September 2015 um 17:35 schrieb Ralf Thielow :
> Translate 121 new messages came from git.pot update in df0617b

I forgot to change this part of the message. Should be 123, obviously.

> (l10n: git.pot: v2.6.0 round 1 (123 new, 41 removed)).
>
> Signed-off-by: Ralf Thielow 
> ---
>  po/de.po | 391 
> ++-
>  1 file changed, 186 insertions(+), 205 deletions(-)
--
To unsubscribe from this list: send the line "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] pull: don't mark values for option "rebase" for translation

[Ralf Thielow]

"false|true|preserve" are actual values for option "rebase"
of the "git-pull" command and should therefore not be marked
for translation.

Signed-off-by: Ralf Thielow 
---
 builtin/pull.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/pull.c b/builtin/pull.c
index 7e3c11e..a39bb0a 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -112,7 +112,7 @@ static struct option pull_options[] = {
/* Options passed to git-merge or git-rebase */
OPT_GROUP(N_("Options related to merging")),
{ OPTION_CALLBACK, 'r', "rebase", _rebase,
- N_("false|true|preserve"),
+ "false|true|preserve",
  N_("incorporate changes by rebasing rather than merging"),
  PARSE_OPT_OPTARG, parse_opt_rebase },
OPT_PASSTHRU('n', NULL, _diffstat, NULL,
-- 
2.6.0.rc1.199.g678474c

--
To unsubscribe from this list: send the line "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 v17 00/14] port tag.c to use ref-filter APIs

[Junio C Hamano]

Eric Sunshine  writes:

> In addition to a link to the previous round and an interdiff, it would
> be helpful to reviewers for you to annotate each patch (in the
> commentary are below the "---" line after your sign-off) with a
> description of the changes in that patch since the previous round in
> order to focus the reviewer's attention (where it needs to be) on the
> latest changes.

I may have got confused by seeing the same v17 (if they were marked
as v18 or v17bis, it would have been easier to make sure I didn't
miss anything), but here is the difference between what I had last
night and what I queued.  The removal of !body[1] and flipping the
order of to_free/format are not seen because I already had a local
fix-up SQUASH??? commits queued in the yesterday's batch.


$ git diff --stat -p kn/for-each-tag@{4.hours} kn/for-each-tag
 ref-filter.c | 5 +
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/ref-filter.c b/ref-filter.c
index 226e94d..fd839ac 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -47,9 +47,6 @@ static struct {
{ "subject" },
{ "body" },
{ "contents" },
-   { "contents:subject" },
-   { "contents:body" },
-   { "contents:signature" },
{ "upstream" },
{ "push" },
{ "symref" },
@@ -58,7 +55,6 @@ static struct {
{ "color" },
{ "align" },
{ "end" },
-   { "contents:lines" },
 };
 
 #define REF_FORMATTING_STATE_INIT  { 0, NULL }
@@ -899,6 +895,7 @@ static void populate_value(struct ref_array_item *ref)
align->position = ALIGN_LEFT;
 
while (*s) {
+   /*  Strip trailing comma */
if (s[1])
strbuf_setlen(s[0], s[0]->len - 1);
if (!strtoul_ui(s[0]->buf, 10, (unsigned int 
*)))
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html