Re: [PATCH 1/2] archive-tar: write extended headers for file sizes >= 8GB

[René Scharfe]

Am 21.06.2016 um 23:04 schrieb Jeff King:
> On Tue, Jun 21, 2016 at 10:57:43PM +0200, René Scharfe wrote:
> 
>> Am 21.06.2016 um 22:42 schrieb René Scharfe:
>>> The value 120 is magic; we need it to pass the tests.  That's
>>> because prepare_header() is used for building extended header
>>> records as well and we don't create extended headers for extended
>>> headers (not sure if that would work anyway), so they simply
>>> vanish when they're over the limit as their size field is set to
>>> zero.
>>
>> So how about something like this to make sure extended headers are
>> only written for regular files and not for other extended headers?
> 
> This is quite similar to what I wrote originally, but I moved to the
> ustar_size() format to better match the mtime code (which needs
> something like that, because we pass around args->time).
> 
> I think you could drop ustar_size() completely here and just put the
> "if" into write_tar_entry().

Which would look like this:

---
 archive-tar.c | 24 ++--
 1 file changed, 22 insertions(+), 2 deletions(-)

diff --git a/archive-tar.c b/archive-tar.c
index cb99df2..274bdfa 100644
--- a/archive-tar.c
+++ b/archive-tar.c
@@ -137,6 +137,20 @@ static void strbuf_append_ext_header(struct strbuf *sb, 
const char *keyword,
strbuf_addch(sb, '\n');
 }
 
+/*
+ * Like strbuf_append_ext_header, but for numeric values.
+ */
+static void strbuf_append_ext_header_uint(struct strbuf *sb,
+ const char *keyword,
+ uintmax_t value)
+{
+   char buf[40]; /* big enough for 2^128 in decimal, plus NUL */
+   int len;
+
+   len = xsnprintf(buf, sizeof(buf), "%"PRIuMAX, value);
+   strbuf_append_ext_header(sb, keyword, buf, len);
+}
+
 static unsigned int ustar_header_chksum(const struct ustar_header *header)
 {
const unsigned char *p = (const unsigned char *)header;
@@ -208,7 +222,7 @@ static int write_tar_entry(struct archiver_args *args,
struct ustar_header header;
struct strbuf ext_header = STRBUF_INIT;
unsigned int old_mode = mode;
-   unsigned long size;
+   unsigned long size, size_in_header;
void *buffer;
int err = 0;
 
@@ -267,7 +281,13 @@ static int write_tar_entry(struct archiver_args *args,
memcpy(header.linkname, buffer, size);
}
 
-   prepare_header(args, , mode, size);
+   size_in_header = size;
+   if (S_ISREG(mode) && size > 0777UL) {
+   size_in_header = 0;
+   strbuf_append_ext_header_uint(_header, "size", size);
+   }
+
+   prepare_header(args, , mode, size_in_header);
 
if (ext_header.len > 0) {
err = write_extended_header(args, sha1, ext_header.buf,
-- 
2.9.0
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] archive-tar: write extended headers for far-future mtime

[René Scharfe]

Am 21.06.2016 um 00:54 schrieb René Scharfe:

Am 16.06.2016 um 06:37 schrieb Jeff King:

   2. System tars that cannot handle pax headers.


In t5000 there is a simple interpreter for path headers for systems
whose tar doesn't handle them.  Adding one for mtime headers may be
feasible.

It's just a bit complicated to link a pax header file to the file it
applies to when it doesn't also contain a path header.  But we know that
the mtime of all entries in tar files created by git archive are is the
same, so we could simply read one header and then adjust the mtime of
all files accordingly.


This brings me to the idea of using a single global pax header for mtime 
instead of one per entry.  It reduces the size of the archive and allows 
for slightly easier testing -- it just fits better since we know that 
all our mtimes are the same.


René

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

Re: [PATCH 1/2] archive-tar: write extended headers for file sizes >= 8GB

[René Scharfe]

Am 21.06.2016 um 23:02 schrieb Jeff King:

On Tue, Jun 21, 2016 at 10:42:52PM +0200, René Scharfe wrote:


If we could set the limit to a lower value than 8GB for testing then we
could at least check if the extended header is written, e.g. if ustar_size()
could be convinced to return 0 every time using a hidden command line
parameter or an environment variable or something better.


Yes, we could do that, though I think it loses most of the value of the
test. We can check that if we hit an arbitrary value we generate the pax
header, but I think what we _really_ care about is: did we generate an
output that somebody else's tar implementation can handle.


I agree with the last point, but don't see how that diminishes the
value of such a test.  If we provide file sizes only through extended
headers (the normal header field being set to 0) and we can extract
files with correct sizes then tar must have interpreted those header
as intended, right?


The diminished value is:

   1. This is a situation that doesn't actually happen in real life.

   2. Now we're carrying extra code inside git only for the sake of
  testing (which can have its own bugs, etc).

Still, it may be better than nothing.


-- >8 --
Subject: archive-tar: test creation of pax extended size headers

---
The value 120 is magic; we need it to pass the tests.  That's
because prepare_header() is used for building extended header
records as well and we don't create extended headers for extended
headers (not sure if that would work anyway), so they simply
vanish when they're over the limit as their size field is set to
zero.


Right, so this is sort of what I meant in (2). Now we have a
tar.ustarsizemax setting shipped in git that is totally broken if you
set it to "1".

I can live with it as a tradeoff, but it is definitely a negative IMHO.


Yes, it's only useful as a debug flag, but the fact that it breaks 
highlights a (admittedly mostly theoretical) shortcoming: The code 
doesn't handle extended headers that are over the size limit as nicely 
as it could.  So the test was already worth it, even if won't land in 
master. :)


René

--
To unsubscribe from this list: send the line "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] git-p4: Fix verbose printing

[Ben Widawsky]

The current logic seems to invert the error message when using verbose messages.
The inversion seems to be indicated by the caller:
if self.inClientSpec(f['path']) and self.hasBranchPrefix(f['path'])]

This behavior has existed since the original commit:
commit 4ae048e67e5f0d786b9febc438433d95f18e5937
Author: Lars Schneider 
Date:   Tue Dec 8 10:36:22 2015 +0100

git-p4: add option to keep empty commits

Note that there are no behavioral changes with this patch since the code did the
right thing, just with the wrong messages.

Cc: Lars Schneider 
Signed-off-by: Ben Widawsky 
---
 git-p4.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/git-p4.py b/git-p4.py
index b6593cf..b123aa2 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -2674,7 +2674,7 @@ class P4Sync(Command, P4UserMap):
 return True
 hasPrefix = [p for p in self.branchPrefixes
 if p4PathStartsWith(path, p)]
-if hasPrefix and self.verbose:
+if not hasPrefix and self.verbose:
 print('Ignoring file outside of prefix: {0}'.format(path))
 return hasPrefix
 
-- 
2.9.0

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

[PATCH v3] Refactor recv_sideband()

[Lukas Fleischer]

Before this patch, we used character buffer manipulations to split
messages from the sideband at line breaks and insert "remote: " at the
beginning of each line, using the packet size to determine the end of a
message. However, since it is safe to assume that diagnostic messages
from the sideband never contain NUL characters, we can also
NUL-terminate the buffer, use strpbrk() for splitting lines and use
format strings to insert the prefix.

A static strbuf is used for constructing the output which is then
printed using a single write() call, such that the atomicity of the
output is preserved. See 9ac13ec (atomic write for sideband remote
messages, 2006-10-11) for details.

Helped-by: Nicolas Pitre 
Signed-off-by: Lukas Fleischer 
---
Now using a static strbuf for the output buffer such that the number of
memory allocations is kept to a minimum.

 sideband.c | 98 ++
 1 file changed, 34 insertions(+), 64 deletions(-)

diff --git a/sideband.c b/sideband.c
index fde8adc..08b75e2 100644
--- a/sideband.c
+++ b/sideband.c
@@ -13,103 +13,73 @@
  * the remote died unexpectedly.  A flush() concludes the stream.
  */
 
-#define PREFIX "remote:"
+#define PREFIX "remote: "
 
 #define ANSI_SUFFIX "\033[K"
 #define DUMB_SUFFIX ""
 
-#define FIX_SIZE 10  /* large enough for any of the above */
-
 int recv_sideband(const char *me, int in_stream, int out)
 {
-   unsigned pf = strlen(PREFIX);
-   unsigned sf;
-   char buf[LARGE_PACKET_MAX + 2*FIX_SIZE];
-   char *suffix, *term;
-   int skip_pf = 0;
+   const char *term, *suffix;
+   char buf[LARGE_PACKET_MAX + 1];
+   static struct strbuf outbuf = STRBUF_INIT;
+   const char *b, *brk;
 
-   memcpy(buf, PREFIX, pf);
+   strbuf_reset();
+   strbuf_addf(, "%s", PREFIX);
term = getenv("TERM");
if (isatty(2) && term && strcmp(term, "dumb"))
suffix = ANSI_SUFFIX;
else
suffix = DUMB_SUFFIX;
-   sf = strlen(suffix);
 
while (1) {
int band, len;
-   len = packet_read(in_stream, NULL, NULL, buf + pf, 
LARGE_PACKET_MAX, 0);
+   len = packet_read(in_stream, NULL, NULL, buf, LARGE_PACKET_MAX, 
0);
if (len == 0)
break;
if (len < 1) {
fprintf(stderr, "%s: protocol error: no band 
designator\n", me);
return SIDEBAND_PROTOCOL_ERROR;
}
-   band = buf[pf] & 0xff;
+   band = buf[0] & 0xff;
+   buf[len] = '\0';
len--;
switch (band) {
case 3:
-   buf[pf] = ' ';
-   buf[pf+1+len] = '\0';
-   fprintf(stderr, "%s\n", buf);
+   fprintf(stderr, "%s%s\n", PREFIX, buf + 1);
return SIDEBAND_REMOTE_ERROR;
case 2:
-   buf[pf] = ' ';
-   do {
-   char *b = buf;
-   int brk = 0;
-
-   /*
-* If the last buffer didn't end with a line
-* break then we should not print a prefix
-* this time around.
-*/
-   if (skip_pf) {
-   b += pf+1;
-   } else {
-   len += pf+1;
-   brk += pf+1;
-   }
+   b = buf + 1;
 
-   /* Look for a line break. */
-   for (;;) {
-   brk++;
-   if (brk > len) {
-   brk = 0;
-   break;
-   }
-   if (b[brk-1] == '\n' ||
-   b[brk-1] == '\r')
-   break;
-   }
+   /*
+* Append a suffix to each nonempty line to clear the
+* end of the screen line.
+*/
+   while ((brk = strpbrk(b, "\n\r"))) {
+   int linelen = brk - b;
 
-   /*
-* Let's insert a suffix to clear the end
-* of the screen line if a line break was
-* found.  Also, if we don't skip the
-* prefix, then a non-empty 

Re: [PATCH 11/11] i18n: difftool: mark warnings for translation

[David Aguilar]

On Tue, Jun 21, 2016 at 11:44:13AM +, Vasco Almeida wrote:
> --- a/git-difftool.perl
> +++ b/git-difftool.perl
> @@ -451,11 +452,11 @@ sub dir_diff
>   }
>  
>   if (exists $wt_modified{$file} and exists $tmp_modified{$file}) 
> {
> - my $errmsg = "warning: Both files modified: ";
> - $errmsg .= "'$workdir/$file' and '$b/$file'.\n";
> - $errmsg .= "warning: Working tree file has been 
> left.\n";
> - $errmsg .= "warning:\n";
> - warn $errmsg;
> + warn sprintf __(
> +"warning: Both files modified:
> +'%s/%s' and '%s/%s'.
> +warning: Working tree file has been left.
> +warning:\n"), $workdir, $file, $b, $file;


Sorry for the nit, but this seems a little hard to read.

It's unfortunate that we have to lose the flow of the code here.
Perhaps we can do something like this to make it flow more like
the original instead?

+   warn sprintf(__(
+   "warning: Both files modified:\n" .
+   "'%s/%s' and '%s/%s'.\n" .
+   "warning: Working tree file has been left.\n" .
+   "warning:\n"), $workdir, $file, $b, $file);

I tend to also prefer parens on the sprintf so that the
parameter grouping is easier to see.

> @@ -467,8 +468,9 @@ sub dir_diff
>   }
>   }
>   if ($error) {
> - warn "warning: Temporary files exist in '$tmpdir'.\n";
> - warn "warning: You may want to cleanup or recover these.\n";
> + warn sprintf __(
> +"warning: Temporary files exist in '%s'.
> +warning: You may want to cleanup or recover these.\n"), $tmpdir;

Ditto, perhaps something like...

+   warn sprintf(__(
+   "warning: Temporary files exist in '%s'.\n" .
+   "warning: You may want to cleanup or recover these.\n"
+   ), $tmpdir);

I'm assuming that the i18n infrastructure is prepared to deal
with perl's . dot syntax for string concatenation, though, and
I don't know whether that's true.  Does that work?

What do you think?

ciao,
-- 
David
--
To unsubscribe from this list: send the line "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] doc: git-htmldocs.googlecode.com is no more

[Jonathan Nieder]

http://git-htmldocs.googlecode.com/git/git.html says

 There was no service found for the uri requested.

Link to the HTML documentation on repo.or.cz instead.

I'd rather use an up-to-date https link for the rendered
documentation, but I wasn't able to find one.  According to the 'todo'
branch, prebuilt documentation is pushed to

 1. https://kernel.googlesource.com/pub/scm/git/git-htmldocs
 2. git://repo.or.cz/git-htmldocs
 3. somewhere on git.sourceforge.jp and git.sourceforge.net?
 4. https://github.com/gitster/git-htmldocs
 5. https://github.com/git/htmldocs

Of these, (1) and (4) don't provide a raw view with content-type
text/html.  (5) might be available as HTML through Jekyll, but I
wasn't able to find it --- e.g., https://git.github.io/htmldocs does
not show those pages.  I wasn't able to find (3) at all.  That leaves
(2).

Reported-by: Andrea Stacchiotti 
Signed-off-by: Jonathan Nieder 
---
Hi Andrea,

Andrea Stacchiotti wrote[1]:

> In the git manual (`man git`), the documentation link:
> > http://git-htmldocs.googlecode.com/git/git.html
> is broken.

Thanks for reporting.  How about this patch?

Thanks,
Jonathan

[1] http://bugs.debian.org/827844

-- >8 --
Subject: doc: git-htmldocs.googlecode.com is no more

 Documentation/git.txt | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/git.txt b/Documentation/git.txt
index 5490d3c..de923db 100644
--- a/Documentation/git.txt
+++ b/Documentation/git.txt
@@ -31,8 +31,8 @@ page to learn what commands Git offers.  You can learn more 
about
 individual Git commands with "git help command".  linkgit:gitcli[7]
 manual page gives you an overview of the command-line command syntax.
 
-Formatted and hyperlinked version of the latest Git documentation
-can be viewed at `http://git-htmldocs.googlecode.com/git/git.html`.
+A formatted and hyperlinked copy of the latest Git documentation
+can be viewed at `http://repo.or.cz/git-htmldocs.git/blob_plain/:/git.html`.
 
 ifdef::stalenotes[]
 [NOTE]
-- 
--
To unsubscribe from this list: send the line "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/2] grep: fix grepping for "intent to add" files

[Eric Sunshine]

On Tue, Jun 21, 2016 at 5:14 PM, Charles Bailey  wrote:
> From: Charles Bailey 
>
> This reverts commit 4d552005323034c1d6311796ac1074e9a4b4b57e and adds an
> alternative fix to maintain the -L --cached behavior.

It is common to provide some context along with the (shortened) commit
ID. For instance:

This reverts 4d55200 (grep: make it clear i-t-a entries are
ignored, 2015-12-27) and adds ...

> 4d5520053 caused 'git grep' to no longer find matches in new files in
> the working tree where the corresponding index entry had the "intent to
> add" bit set, despite the fact that these files are tracked.
> [...]
> Helped-by: Nguyễn Thái Ngọc Duy 
> Signed-off-by: Charles Bailey 
> ---
>
> Is "Helped-by" an appropriate attribution in this case?

Very much so.

> diff --git a/t/t7810-grep.sh b/t/t7810-grep.sh
> @@ -1364,4 +1364,42 @@ test_expect_success 'grep --color -e A --and -e B -p 
> with context' '
> +test_expect_success 'grep can find things only in the work tree' '
> +   touch work-tree-only &&

Avoid 'touch' if the timestamp of the file has no significance. Use '>' instead:

>work-tree-only &&

> +   git add work-tree-only &&
> +   echo "find in work tree" >work-tree-only &&
> +   git grep --quiet "find in work tree" &&
> +   test_must_fail git grep --quiet --cached "find in work tree" &&
> +   test_must_fail git grep --quiet "find in work tree" HEAD &&
> +   git rm -f work-tree-only

If any statement before this cleanup code fails, then the cleanup will
never take place (due to the &&-chain). To ensure cleanup regardless
of test outcome, instead use test_when_finished() at the beginning of
the test:

test_when_finished "git rm -f work-tree-only" &&

Same applies to other added tests.

> +'
> +
> +test_expect_success 'grep can find things only in the work tree (i-t-a)' '
> +   echo "intend to add this" >intend-to-add &&
> +   git add -N intend-to-add &&
> +   git grep --quiet "intend to add this" &&
> +   test_must_fail git grep --quiet --cached "intend to add this" &&
> +   test_must_fail git grep --quiet "intend to add this" HEAD &&
> +   git rm -f intend-to-add
> +'
> +
> +test_expect_success 'grep can find things only in the index' '
> +   echo "only in the index" >cache-this &&
> +   git add cache-this &&
> +   rm cache-this &&
> +   test_must_fail git grep --quiet "only in the index" &&
> +   git grep --quiet --cached "only in the index" &&
> +   test_must_fail git grep --quiet "only in the index" HEAD &&
> +   git rm --cached cache-this
> +'
> +
> +test_expect_success 'grep does not report i-t-a with -L --cached' '
> +   echo "intend to add this" >intend-to-add &&
> +   git add -N intend-to-add &&
> +   git ls-files | grep -v "^intend-to-add\$" >expected &&
> +   git grep -L --cached "nonexistent_string" >actual &&
> +   test_cmp expected actual &&
> +   git rm -f intend-to-add
> +'
> +
>  test_done
--
To unsubscribe from this list: send the line "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: Managing sub-projects

[Stefan Beller]

Hi Michael,

On Tue, Jun 21, 2016 at 4:06 PM, Michael Eager  wrote:
> Hi Stephan!
>
> On 06/19/2016 07:01 PM, Stefan Beller wrote:
>>
>> On Sat, Jun 18, 2016 at 4:20 PM, Michael Eager  wrote:
>>>
>>>
>>> Any other ways to do what I want without creating a separate forked
>>> repo for each of the sub-projects?  Or have I misunderstood one of
>>> these schemes?
>>
>>
>> I think forking is the way to go here, as you want to have new code
>> and maintain that.
>
>
> This was my conclusion.
>
> What I originally wanted was a repo with two origins, the upstream for
> the master and public branches, and my repo for my branches.  Git may
> be able to do all kinds of magic, but this two-origin scheme sounded
> strange after I thought about it for a while.

Well, 2 origins sound strange indeed, but "origin" is just a name to point at
a remote place. You could have ["origin" , "private"].

Once upon a time, I used ["mainline", "origin", "", ...],
which I confused myself with, so now I am down to
["origin", "private", "].

The difference for my work flow is that I have read permissions only
on all but one remote.

>
>> Personally I would try out submodules.
>
>
> I've used submodules on another project.  There are some odd quirks,
> and lots of web pages which say to avoid submodules like the plague, but I
> didn't have lots of trouble.  (After an initial bit of confusion while
> getting familiar with submodules, which is what I can say about every
> feature in Git.)
>
>>> Git submodule:  Branches created in the sub-projects are pushed to the
>>> upstream repo, not to my repo.  I tried to change origin and created an
>>> upstream reference, but was not able to get changes pushed to my repo.
>>
>>
>> Beware that there are 2 areas you need to look at. First the submodule
>> repo
>> needs to have a remote that points away from the projects origin (to your
>> private fork).
>
>
> I'll create an "upstream" remote to the project repo, so I can pull/rebase
> from the upstream into my forked repo.  The "origin" will point to my repo.

That is similar to the "mainline" I mention above. :)

In your work flow is there such a thing of an upstream of the
superpoject containing
all these subprojects? I thought that was a collection you are
ultimately creating,
such that the superproject has only one remote (your authoritative copy), while
each submodule has 2 remotes "upstream" (that I assume to be read only for you)
and an "origin" (your maintained version, which then contains stuff that is
referenced by your superproject).

>
>
>> Then you have to look at the superproject that
>> 1) records the sha1 for the submodules internally
>> 2) all other information except the tracking sha1s must be user provided,
>>  where the .gitmodules file contains recommendations (i.e. the url
>> where to
>>  obtain the submodule from, whether to clone it shallowly,
>>  if we have a specific branch in mind). The contents of that file
>>  are not binding, e.g. if the url provided in the .gitmodules file
>> becomes
>>  outdated later, it is still possible to setup the
>> submodule/superproject correctly.
>>
>> However for your business purpose, you would put the url of the private
>> forks
>> in the recommended URL of the submodules.
>>
>> As the superproject only tracks the sha1, and has this recommended pointer
>> where to get the submodule repository from, you need to take special care
>> in a rebase workflow, because the old rebased commits fall out of the
>> reachability
>> of the graph of objects, e.g.:
>>
>> Say you have a version `abc` in a submodule that is one commit on top of
>> canonical projects history, and `abc` is recorded as the sha1 in the
>> superproject.
>>
>> Then you rebase the commit in the submodule to a newer version of the
>> upstream,
>> which then becomes a new commit `def` and `abc` is not referenced any
>> more,
>> so it can be garbage collected.
>>
>> This is bad for the history of the superproject as it then points to
>> an unreachable
>> commit in its history.
>>
>> To preserve the historic non-rebased `abc` commit, you could have a
>> set of branches
>> (or tags) that maintain all the old non rebased versions.
>
>
> Sounds like every time I rebase, I should tag the repo to annotate this,
> and (as a side effect) retain the history.
>
>> This problem comes up with submodules with any workflow that requires
>> non fast forward changes (forced pushes), I think.
>>
>> So maybe you need to have an alias in the submodule for rebasing, that
>> is roughly:
>>
>> rebase:
>>  if rebased history is published
>>  create a tag, e.g.: "$(date -I)-${sha1}"
>>  (and push that tag here or later?)
>>  rebase as normal
>>  carry on with life
>
>
> What do you mean "if rebased history is published".

bad wording.

"If the commits that you are going to rebase are published:"



>
> Generally I'd apply a tag after the rebase was completed successfully,

Re: Managing sub-projects

[Michael Eager]

Hi Stephan!

On 06/19/2016 07:01 PM, Stefan Beller wrote:

On Sat, Jun 18, 2016 at 4:20 PM, Michael Eager  wrote:


Any other ways to do what I want without creating a separate forked
repo for each of the sub-projects?  Or have I misunderstood one of
these schemes?


I think forking is the way to go here, as you want to have new code
and maintain that.


This was my conclusion.

What I originally wanted was a repo with two origins, the upstream for
the master and public branches, and my repo for my branches.  Git may
be able to do all kinds of magic, but this two-origin scheme sounded
strange after I thought about it for a while.


Personally I would try out submodules.


I've used submodules on another project.  There are some odd quirks,
and lots of web pages which say to avoid submodules like the plague, but I
didn't have lots of trouble.  (After an initial bit of confusion while
getting familiar with submodules, which is what I can say about every
feature in Git.)


Git submodule:  Branches created in the sub-projects are pushed to the
upstream repo, not to my repo.  I tried to change origin and created an
upstream reference, but was not able to get changes pushed to my repo.


Beware that there are 2 areas you need to look at. First the submodule repo
needs to have a remote that points away from the projects origin (to your
private fork).


I'll create an "upstream" remote to the project repo, so I can pull/rebase
from the upstream into my forked repo.  The "origin" will point to my repo.


Then you have to look at the superproject that
1) records the sha1 for the submodules internally
2) all other information except the tracking sha1s must be user provided,
 where the .gitmodules file contains recommendations (i.e. the url where to
 obtain the submodule from, whether to clone it shallowly,
 if we have a specific branch in mind). The contents of that file
 are not binding, e.g. if the url provided in the .gitmodules file becomes
 outdated later, it is still possible to setup the
submodule/superproject correctly.

However for your business purpose, you would put the url of the private forks
in the recommended URL of the submodules.

As the superproject only tracks the sha1, and has this recommended pointer
where to get the submodule repository from, you need to take special care
in a rebase workflow, because the old rebased commits fall out of the
reachability
of the graph of objects, e.g.:

Say you have a version `abc` in a submodule that is one commit on top of
canonical projects history, and `abc` is recorded as the sha1 in the
superproject.

Then you rebase the commit in the submodule to a newer version of the upstream,
which then becomes a new commit `def` and `abc` is not referenced any more,
so it can be garbage collected.

This is bad for the history of the superproject as it then points to
an unreachable
commit in its history.

To preserve the historic non-rebased `abc` commit, you could have a
set of branches
(or tags) that maintain all the old non rebased versions.


Sounds like every time I rebase, I should tag the repo to annotate this,
and (as a side effect) retain the history.


This problem comes up with submodules with any workflow that requires
non fast forward changes (forced pushes), I think.

So maybe you need to have an alias in the submodule for rebasing, that
is roughly:

rebase:
 if rebased history is published
 create a tag, e.g.: "$(date -I)-${sha1}"
 (and push that tag here or later?)
 rebase as normal
 carry on with life


What do you mean "if rebased history is published".

Generally I'd apply a tag after the rebase was completed successfully,
then push both the updated branch and tags to my repo.


To get back to your complaint:


  I tried to change origin and created an
upstream reference, but was not able to get changes pushed to my repo.


I would imagine this to be

  (cd submodule && git remote set-url origin  && git push origin)

for plain pushing in the submodule and then

 $EDIT .gitmodules
 # edit submodule..url to point at 

to get the superproject correct.


Thanks.


--
Michael Eagerea...@eagercon.com
1960 Park Blvd., Palo Alto, CA 94306  650-325-8077
--
To unsubscribe from this list: send the line "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/2] grep: fix grepping for "intent to add" files

[Junio C Hamano]

Charles Bailey  writes:

> Is "Helped-by" an appropriate attribution in this case?

Sure.

> diff --git a/builtin/grep.c b/builtin/grep.c
> index 462e607..ae73831 100644
> --- a/builtin/grep.c
> +++ b/builtin/grep.c
> @@ -386,7 +386,7 @@ static int grep_cache(struct grep_opt *opt, const struct 
> pathspec *pathspec, int
>  
>   for (nr = 0; nr < active_nr; nr++) {
>   const struct cache_entry *ce = active_cache[nr];
> - if (!S_ISREG(ce->ce_mode) || ce_intent_to_add(ce))
> + if (!S_ISREG(ce->ce_mode))
>   continue;
>   if (!ce_path_match(ce, pathspec, NULL))
>   continue;
> @@ -396,7 +396,7 @@ static int grep_cache(struct grep_opt *opt, const struct 
> pathspec *pathspec, int
>* cache version instead
>*/
>   if (cached || (ce->ce_flags & CE_VALID) || 
> ce_skip_worktree(ce)) {
> - if (ce_stage(ce))
> + if (ce_stage(ce) || ce_intent_to_add(ce))
>   continue;
>   hit |= grep_sha1(opt, ce->sha1, ce->name, 0, ce->name);
>   }

OK, so this function handles searching in either the index or the
working tree.

The first hunk used to unconditionally discard paths marked as
i-t-a, even when we are looking at the working tree, which is
clearly useless, and we stop rejecting i-t-a paths too early, which
is good.

The second hunk is for "grep --cached" but also covers two other
cases.  What are these?

CE_VALID is used by "Assume unchanged".  Because the user promised
that s/he will take responsibility of keeping the working tree
contents in sync with what is in the index by not modifying it, even
when we are not doing "grep --cached", we pick up the contents from
the index and look for the string in there, instead of going to the
working tree.  In other words, even though at the mechanical level
we are looking into the index, logically we are searching in the
working tree.  Is it sensible to skip i-t-a entries in that case?

I think the same discussion would apply to CE_SKIP_WORKTREE (see
"Skip-worktree bit" in Documentation/git-update-index.txt).

So I wonder if a better change would be more like

for (...) {
if (!S_ISREG(ce->ce_mode))
continue; /* not a regular file */
if (!ce_path_match(ce, pathspec, NULL)
continue; /* uninteresting */
+   if (cached && ce_intent_to_add(ce))
+   continue; /* path not yet in the index */

if (cached || ...)
UNCHANGED FROM THE ORIGINAL

perhaps?

I actually think (ce->ce_flags & CE_VALID) case should go to working
tree for performance, but that is a separate topic.

> +test_expect_success 'grep can find things only in the work tree' '
> + touch work-tree-only &&

Please do not use "touch" if the reason to use the command is *not*
to muck with the timestamp, e.g. to create an empty file.

> + git add work-tree-only &&
> + echo "find in work tree" >work-tree-only &&
> + git grep --quiet "find in work tree" &&
> + test_must_fail git grep --quiet --cached "find in work tree" &&
> + test_must_fail git grep --quiet "find in work tree" HEAD &&
> + git rm -f work-tree-only
> +'
> +
> +test_expect_success 'grep can find things only in the work tree (i-t-a)' '
> + echo "intend to add this" >intend-to-add &&
> + git add -N intend-to-add &&
> + git grep --quiet "intend to add this" &&
> + test_must_fail git grep --quiet --cached "intend to add this" &&
> + test_must_fail git grep --quiet "intend to add this" HEAD &&
> + git rm -f intend-to-add
> +'
> +
> +test_expect_success 'grep can find things only in the index' '
> + echo "only in the index" >cache-this &&
> + git add cache-this &&
> + rm cache-this &&
> + test_must_fail git grep --quiet "only in the index" &&
> + git grep --quiet --cached "only in the index" &&
> + test_must_fail git grep --quiet "only in the index" HEAD &&
> + git rm --cached cache-this
> +'
> +
> +test_expect_success 'grep does not report i-t-a with -L --cached' '
> + echo "intend to add this" >intend-to-add &&
> + git add -N intend-to-add &&
> + git ls-files | grep -v "^intend-to-add\$" >expected &&
> + git grep -L --cached "nonexistent_string" >actual &&
> + test_cmp expected actual &&
> + git rm -f intend-to-add
> +'
> +
>  test_done
--
To unsubscribe from this list: send the line "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 3/8] Convert struct diff_filespec to struct object_id

[Junio C Hamano]

"brian m. carlson"  writes:

I was trying to make sure there is no misconversion, but some lines
that got wrapped were distracting.  For example:

> @@ -2721,7 +2722,8 @@ static int diff_populate_gitlink(struct diff_filespec 
> *s, int size_only)
>   if (s->dirty_submodule)
>   dirty = "-dirty";
>  
> - strbuf_addf(, "Subproject commit %s%s\n", sha1_to_hex(s->sha1), 
> dirty);
> + strbuf_addf(, "Subproject commit %s%s\n",
> + oid_to_hex(>oid), dirty);

This would have been

> - strbuf_addf(, "Subproject commit %s%s\n", sha1_to_hex(s->sha1), 
> dirty);
> + strbuf_addf(, "Subproject commit %s%s\n", oid_to_hex(>oid), 
> dirty);

which the conversion made the line _shorter_.  If the original's
line length was acceptable, there is no reason to wrap the result.

> - die("unable to read %s", sha1_to_hex(s->sha1));
> + die("unable to read %s",
> + oid_to_hex(>oid));

Likewise.

> @@ -2937,7 +2940,7 @@ static struct diff_tempfile *prepare_temp_file(const 
> char *name,
>   if (!one->sha1_valid)
>   sha1_to_hex_r(temp->hex, null_sha1);
>   else
> - sha1_to_hex_r(temp->hex, one->sha1);
> + sha1_to_hex_r(temp->hex, one->oid.hash);

This suggests that oid_to_hex_r() is needed, perhaps?

> @@ -2952,7 +2955,7 @@ static struct diff_tempfile *prepare_temp_file(const 
> char *name,
>   if (diff_populate_filespec(one, 0))
>   die("cannot read data blob for %s", one->path);
>   prep_temp_blob(name, temp, one->data, one->size,
> -one->sha1, one->mode);
> +one->oid.hash, one->mode);

prep_temp_blob() is a file-scope static with only two callers.  It
probably would not take too much effort to make it take >oid
instead?

> @@ -3075,8 +3078,8 @@ static void fill_metainfo(struct strbuf *msg,
>   abbrev = 40;
>   }
>   strbuf_addf(msg, "%s%sindex %s..", line_prefix, set,
> - find_unique_abbrev(one->sha1, abbrev));
> - strbuf_addstr(msg, find_unique_abbrev(two->sha1, abbrev));
> + find_unique_abbrev(one->oid.hash, abbrev));
> + strbuf_addstr(msg, find_unique_abbrev(two->oid.hash, abbrev));

Good.  As more and more callers of find_unique_abbrev() is converted
to pass oid.hash to it, eventually we will have only a handful of
callers that have a raw "const unsigned char sha1[20]" to pass it,
and we can eventually make the function take   It seems we are
not quite there yet, by the looks of 'git grep find_unique_abbrev'
output, but we are getting closer.

> @@ -3134,17 +3137,17 @@ static void diff_fill_sha1_info(struct diff_filespec 
> *one)
>   if (!one->sha1_valid) {
>   struct stat st;
>   if (one->is_stdin) {
> - hashcpy(one->sha1, null_sha1);
> + hashcpy(one->oid.hash, null_sha1);
>   return;
>   }

oidclr()?

Perhaps a preparatory step of

unsigned char *E1;

-hashcpy(E1, null_sha1);
+hashclr(E1)

would help?

> @@ -902,13 +904,13 @@ static struct merge_file_info merge_file_1(struct 
> merge_options *o,
>   result.clean = 0;
>   if (S_ISREG(a->mode)) {
>   result.mode = a->mode;
> - hashcpy(result.sha, a->sha1);
> + hashcpy(result.sha, a->oid.hash);
>   } else {
>   result.mode = b->mode;
> - hashcpy(result.sha, b->sha1);
> + hashcpy(result.sha, b->oid.hash);

merge_file_info is a file-scope-static type, and it shouldn't take
too much effort to replace its sha1 with an oid, I would think.

> - if (!sha_eq(a->sha1, one->sha1) && !sha_eq(b->sha1, one->sha1))
> + if (!sha_eq(a->oid.hash, one->oid.hash) && !sha_eq(b->oid.hash, 
> one->oid.hash))

sha_eq() knows that either of its two parameters could be NULL, but
a->sha1 is diff_filespec.sha1 which cannot be NULL.

So !sha_eq() here can become oidcmp().  There are some calls to
sha_eq() that could pass NULL (e.g. a_sha could be NULL in
blob_unchanged()), but many other sha_eq() can become !oidcmp().

Am I reading the conversion correctly?

I'll stop here for now and will come back to the remainder of this
patch sometime later.  Thanks.

> diff --git a/notes-merge.c b/notes-merge.c
> index 34bfac0c..62c23d8a 100644
> --- a/notes-merge.c
> +++ b/notes-merge.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  

Re: [PATCH v2 2/8] Apply object_id Coccinelle transformations.

[Junio C Hamano]

"brian m. carlson"  writes:

> Apply the set of semantic patches from contrib/examples/coccinelle to
> convert some leftover places using struct object_id's hash member to
> instead use the wrapper functions that take struct object_id natively.

It is somewhat curious how these 'some leftover places' are chosen.

Especially, this hunk was interesting.

> diff --git a/refs/files-backend.c b/refs/files-backend.c
> index 1f380764..dac3a222 100644
> --- a/refs/files-backend.c
> +++ b/refs/files-backend.c
> @@ -1725,14 +1725,14 @@ static int verify_lock(struct ref_lock *lock,
>   errno = save_errno;
>   return -1;
>   } else {
> - hashclr(lock->old_oid.hash);
> + oidclr(>old_oid);
>   return 0;
>   }
>   }
>   if (old_sha1 && hashcmp(lock->old_oid.hash, old_sha1)) {
>   strbuf_addf(err, "ref %s is at %s but expected %s",
>   lock->ref_name,
> - sha1_to_hex(lock->old_oid.hash),
> + oid_to_hex(>old_oid),
>   sha1_to_hex(old_sha1));
>   errno = EBUSY;
>   return -1;

The function gets old_sha1 which is the old-world "const unsigned
char *" that is passed via lock_ref_sha1_basic() from public entry
points like rename_ref() and ref_transaction_commit().  As this
codepath and the functions involved have not be converted to oid,
we end up seeing oid_to_hex() next to sha1_to_hex() ;-)

Not a complaint; this is how we make progress incrementally.

It was just I noticed this function is left in a somewhat funny
intermediate state after this step.

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

[PATCH v2 1/2] Fix duplicated test name

[Charles Bailey]

Signed-off-by: Charles Bailey 
---

Spotted while testing t7810-grep and grep "i-t-a" fixes.

 t/t7810-grep.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/t7810-grep.sh b/t/t7810-grep.sh
index 1e72971..c4302ed 100755
--- a/t/t7810-grep.sh
+++ b/t/t7810-grep.sh
@@ -353,7 +353,7 @@ test_expect_success 'grep -l -C' '
 cat >expected 

[PATCH v2 2/2] grep: fix grepping for "intent to add" files

[Charles Bailey]

From: Charles Bailey 

This reverts commit 4d552005323034c1d6311796ac1074e9a4b4b57e and adds an
alternative fix to maintain the -L --cached behavior.

4d5520053 caused 'git grep' to no longer find matches in new files in
the working tree where the corresponding index entry had the "intent to
add" bit set, despite the fact that these files are tracked.

The content in the index of a file for which the "intent to add" bit is
set is considered indeterminate and not empty. For most grep queries we
want these to behave the same, however for -L --cached (files without a
match) we don't want to respond positively for "intent to add" files as
their contents are indeterminate. This is in contrast to files with
empty contents in the index (no lines implies no matches for any grep
query expression) which should be reported in the output of a grep -L
--cached invocation.

Add tests to cover this case and a few related cases which previously
lacked coverage.

Helped-by: Nguyễn Thái Ngọc Duy 
Signed-off-by: Charles Bailey 
---

Is "Helped-by" an appropriate attribution in this case?

Personally, I could have gone either way on the -L --cached
functionality but I know that Duy has put a lot more thought into
"intent-to-add" entries so I trust his judgement.

 builtin/grep.c  |  4 ++--
 t/t7810-grep.sh | 38 ++
 2 files changed, 40 insertions(+), 2 deletions(-)

diff --git a/builtin/grep.c b/builtin/grep.c
index 462e607..ae73831 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -386,7 +386,7 @@ static int grep_cache(struct grep_opt *opt, const struct 
pathspec *pathspec, int
 
for (nr = 0; nr < active_nr; nr++) {
const struct cache_entry *ce = active_cache[nr];
-   if (!S_ISREG(ce->ce_mode) || ce_intent_to_add(ce))
+   if (!S_ISREG(ce->ce_mode))
continue;
if (!ce_path_match(ce, pathspec, NULL))
continue;
@@ -396,7 +396,7 @@ static int grep_cache(struct grep_opt *opt, const struct 
pathspec *pathspec, int
 * cache version instead
 */
if (cached || (ce->ce_flags & CE_VALID) || 
ce_skip_worktree(ce)) {
-   if (ce_stage(ce))
+   if (ce_stage(ce) || ce_intent_to_add(ce))
continue;
hit |= grep_sha1(opt, ce->sha1, ce->name, 0, ce->name);
}
diff --git a/t/t7810-grep.sh b/t/t7810-grep.sh
index c4302ed..6c7ccb3 100755
--- a/t/t7810-grep.sh
+++ b/t/t7810-grep.sh
@@ -1364,4 +1364,42 @@ test_expect_success 'grep --color -e A --and -e B -p 
with context' '
test_cmp expected actual
 '
 
+test_expect_success 'grep can find things only in the work tree' '
+   touch work-tree-only &&
+   git add work-tree-only &&
+   echo "find in work tree" >work-tree-only &&
+   git grep --quiet "find in work tree" &&
+   test_must_fail git grep --quiet --cached "find in work tree" &&
+   test_must_fail git grep --quiet "find in work tree" HEAD &&
+   git rm -f work-tree-only
+'
+
+test_expect_success 'grep can find things only in the work tree (i-t-a)' '
+   echo "intend to add this" >intend-to-add &&
+   git add -N intend-to-add &&
+   git grep --quiet "intend to add this" &&
+   test_must_fail git grep --quiet --cached "intend to add this" &&
+   test_must_fail git grep --quiet "intend to add this" HEAD &&
+   git rm -f intend-to-add
+'
+
+test_expect_success 'grep can find things only in the index' '
+   echo "only in the index" >cache-this &&
+   git add cache-this &&
+   rm cache-this &&
+   test_must_fail git grep --quiet "only in the index" &&
+   git grep --quiet --cached "only in the index" &&
+   test_must_fail git grep --quiet "only in the index" HEAD &&
+   git rm --cached cache-this
+'
+
+test_expect_success 'grep does not report i-t-a with -L --cached' '
+   echo "intend to add this" >intend-to-add &&
+   git add -N intend-to-add &&
+   git ls-files | grep -v "^intend-to-add\$" >expected &&
+   git grep -L --cached "nonexistent_string" >actual &&
+   test_cmp expected actual &&
+   git rm -f intend-to-add
+'
+
 test_done
-- 
2.8.2.311.gee88674

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

Re: [PATCH 1/2] archive-tar: write extended headers for file sizes >= 8GB

[Jeff King]

On Tue, Jun 21, 2016 at 10:57:43PM +0200, René Scharfe wrote:

> Am 21.06.2016 um 22:42 schrieb René Scharfe:
> > The value 120 is magic; we need it to pass the tests.  That's
> > because prepare_header() is used for building extended header
> > records as well and we don't create extended headers for extended
> > headers (not sure if that would work anyway), so they simply
> > vanish when they're over the limit as their size field is set to
> > zero.
> 
> So how about something like this to make sure extended headers are
> only written for regular files and not for other extended headers?

This is quite similar to what I wrote originally, but I moved to the
ustar_size() format to better match the mtime code (which needs
something like that, because we pass around args->time).

I think you could drop ustar_size() completely here and just put the
"if" into write_tar_entry().

-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 v2 0/8] object_id part 4

[Junio C Hamano]

"brian m. carlson"  writes:

> This series is part 4 in a series of conversions to replace instances of
> unsigned char [20] with struct object_id.  Most of this series touches
> the merge-recursive code.

I've queued them in 'pu', but please read contrib/examples/README
;-) Tentatively, I used contrib/coccinelle instead, but something
even shorter (e.g. contrib/cocci) might be sufficient.

> Note that in the patches created with the semantic patches, the only manual
> change was the definition of the struct member.  Opinions on whether this is a
> viable technique for further work to ease both the creation and review of
> patches are of course welcomed.

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] Make find_commit_subject() more robust

[Junio C Hamano]

Johannes Schindelin  writes:

> Hi Junio,
>
> On Mon, 20 Jun 2016, Junio C Hamano wrote:
>
>> Johannes Schindelin  writes:
>> 
>> > Just like the pretty printing machinery, we should simply ignore empty
>> > lines at the beginning of the commit messages.
>> >
>> > This discrepancy was noticed when an early version of the rebase--helper
>> > produced commit objects with more than one empty line between the header
>> > and the commit message.
>> >
>> > Signed-off-by: Johannes Schindelin 
>> > ---
>> > Published-As: 
>> > https://github.com/dscho/git/releases/tag/leading-empty-lines-v1
>> >
>> >And another patch from the rebase--helper front. I guess I'll
>> >call it a day with this one.
>> 
>> Makes sense.  This has a trivial textual conflict with cleanup
>> patches in flight, I think, but that is not a big problem.
>
> I will gladly resend rebased to `next`, if you wish.

No, I'd prefer a patch that applies to 'master' for a new feature;
there is no need to deliberately get taken hostage by other topics.

>> It does hint that we might want to find a library function that can
>> replace a hand-rolled while loop we are adding here, though ;-)
>
> Heh. I cannot help you with that ;-)

The reason it hints such a thing is because the line nearby that
does this:

for (eol = p; *eol && *eol != '\n'; eol++)
; /* do nothing */

gets rewritten to

eol = strchrnul(p, '\n');

i.e. "give me the pointer to the first byte that is '\n', or EOS".

Your patch introduces a similar loop with similar (but different)
purpose:

while (*p == '\n')
p++;

which would have been helped if there were a helper with an
opposite function, i.e.

p = strcchrnul(p, '\n');

i.e. "give me the pointer to the first byte that is not '\n', or EOS".

But there is no such thing.  Although p += strcspn(p, "\n") is a
possibility, that somehow feels a bit odd.  And that is why I did
not hint any existing function and said "might want to find".

HOWEVER.

Stepping back a bit, I think what we actually want is

p = skip_blank_lines(p);

that skips any and all blank lines, including an empty line that
consists of all whitespace.

For example

(
# grab the header lines
git cat-file commit HEAD | sed -e '/^$/q'
# throw in random blank lines
echo
echo " "
echo "  "
echo "   "
echo
echo "Title line"
) | git hash-object -w -t commit --stdin

would mint a commit object that has many blank lines in the front,
some have whitespace and are not empty.  If you give it to

git show -s | cat -e
git show -s --oneline | cat -e

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

Re: [PATCH 1/2] archive-tar: write extended headers for file sizes >= 8GB

[Jeff King]

On Tue, Jun 21, 2016 at 10:42:52PM +0200, René Scharfe wrote:

> >> If we could set the limit to a lower value than 8GB for testing then we
> >> could at least check if the extended header is written, e.g. if 
> >> ustar_size()
> >> could be convinced to return 0 every time using a hidden command line
> >> parameter or an environment variable or something better.
> > 
> > Yes, we could do that, though I think it loses most of the value of the
> > test. We can check that if we hit an arbitrary value we generate the pax
> > header, but I think what we _really_ care about is: did we generate an
> > output that somebody else's tar implementation can handle.
> 
> I agree with the last point, but don't see how that diminishes the
> value of such a test.  If we provide file sizes only through extended
> headers (the normal header field being set to 0) and we can extract
> files with correct sizes then tar must have interpreted those header
> as intended, right?

The diminished value is:

  1. This is a situation that doesn't actually happen in real life.

  2. Now we're carrying extra code inside git only for the sake of
 testing (which can have its own bugs, etc).

Still, it may be better than nothing.

> -- >8 --
> Subject: archive-tar: test creation of pax extended size headers
> 
> ---
> The value 120 is magic; we need it to pass the tests.  That's
> because prepare_header() is used for building extended header
> records as well and we don't create extended headers for extended
> headers (not sure if that would work anyway), so they simply
> vanish when they're over the limit as their size field is set to
> zero.

Right, so this is sort of what I meant in (2). Now we have a
tar.ustarsizemax setting shipped in git that is totally broken if you
set it to "1".

I can live with it as a tradeoff, but it is definitely a negative IMHO.

-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 1/2] archive-tar: write extended headers for file sizes >= 8GB

[René Scharfe]

Am 21.06.2016 um 17:59 schrieb Jeff King:
> On Tue, Jun 21, 2016 at 12:54:11AM +0200, René Scharfe wrote:
>
>>> Unfortunately, it's quite an expensive test to run. For one
>>> thing, unless your filesystem supports files with holes, it
>>> takes 64GB of disk space (you might think piping straight to
>>> `hash-object --stdin` would be better, but it's not; that
>>> tries to buffer all 64GB in RAM!). Furthermore, hashing and
>>> compressing the object takes several minutes of CPU time.
>>>
>>> We could ship just the resulting compressed object data as a
>>> loose object, but even that takes 64MB. So sadly, this code
>>> path remains untested in the test suite.
>>
>> If we could set the limit to a lower value than 8GB for testing then we
>> could at least check if the extended header is written, e.g. if ustar_size()
>> could be convinced to return 0 every time using a hidden command line
>> parameter or an environment variable or something better.
>
> Yes, we could do that, though I think it loses most of the value of the
> test. We can check that if we hit an arbitrary value we generate the pax
> header, but I think what we _really_ care about is: did we generate an
> output that somebody else's tar implementation can handle.

I agree with the last point, but don't see how that diminishes the
value of such a test.  If we provide file sizes only through extended
headers (the normal header field being set to 0) and we can extract
files with correct sizes then tar must have interpreted those header
as intended, right?

(Or it just guessed the sizes by searching for the next header magic,
but such a fallback won't be accurate for files ending with NUL
characters due to NUL-padding, so we just have to add such a file.)

René


-- >8 --
Subject: archive-tar: test creation of pax extended size headers

---
The value 120 is magic; we need it to pass the tests.  That's
because prepare_header() is used for building extended header
records as well and we don't create extended headers for extended
headers (not sure if that would work anyway), so they simply
vanish when they're over the limit as their size field is set to
zero.

 archive-tar.c   | 7 ++-
 t/t5000-tar-tree.sh | 7 +++
 2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/archive-tar.c b/archive-tar.c
index f53e61c..fbbc4cc 100644
--- a/archive-tar.c
+++ b/archive-tar.c
@@ -14,6 +14,7 @@ static char block[BLOCKSIZE];
 static unsigned long offset;
 
 static int tar_umask = 002;
+static unsigned long ustar_size_max = 0777UL;
 
 static int write_tar_filter_archive(const struct archiver *ar,
struct archiver_args *args);
@@ -179,7 +180,7 @@ static size_t get_path_prefix(const char *path, size_t 
pathlen, size_t maxlen)
 
 static inline unsigned long ustar_size(uintmax_t size)
 {
-   if (size <= 0777UL)
+   if (size <= ustar_size_max)
return size;
else
return 0;
@@ -412,6 +413,10 @@ static int git_tar_config(const char *var, const char 
*value, void *cb)
}
return 0;
}
+   if (!strcmp(var, "tar.ustarsizemax")) {
+   ustar_size_max = git_config_ulong(var, value);
+   return 0;
+   }
 
return tar_filter_config(var, value, cb);
 }
diff --git a/t/t5000-tar-tree.sh b/t/t5000-tar-tree.sh
index 4b68bba..03bb4c7 100755
--- a/t/t5000-tar-tree.sh
+++ b/t/t5000-tar-tree.sh
@@ -102,6 +102,7 @@ test_expect_success \
  echo long filename >a/four$hundred &&
  mkdir a/bin &&
  test-genrandom "frotz" 50 >a/bin/sh &&
+ printf "\0\0\0" >>a/bin/sh &&
  printf "A\$Format:%s\$O" "$SUBSTFORMAT" >a/substfile1 &&
  printf "A not substituted O" >a/substfile2 &&
  if test_have_prereq SYMLINKS; then
@@ -157,6 +158,12 @@ test_expect_success 'git-archive --prefix=olde-' '
 
 check_tar with_olde-prefix olde-
 
+test_expect_success !TAR_NEEDS_PAX_FALLBACK 'pax extended size headers' '
+   git -c tar.ustarsizemax=120 archive HEAD >extended_size_header.tar
+'
+
+check_tar extended_size_header
+
 test_expect_success 'git archive on large files' '
 test_config core.bigfilethreshold 1 &&
 git archive HEAD >b3.tar &&
-- 
2.9.0

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

Re: [PATCH 1/2] archive-tar: write extended headers for file sizes >= 8GB

[Jeff King]

On Tue, Jun 21, 2016 at 07:44:55PM +, Robin H. Johnson wrote:

> On Thu, Jun 16, 2016 at 12:37:33AM -0400,  Jeff King wrote:
> > We could ship just the resulting compressed object data as a
> > loose object, but even that takes 64MB. So sadly, this code
> > path remains untested in the test suite.
> Additionally compress the object data, and insert it for the purpose of
> testing? It's still an expensive test time-wise, but repeated
> gzip compression on zeros does still help; to that end, here's the
> pieces to add a testcase while only being <9KiB.

Interesting idea. With bzip2 it actually drops to about 600 bytes (I
suspect we could get it even smaller by writing a custom program to
generate it, but it's diminishing returns).

I think there are some other portability issues, though (like does the
receiving tar actually support 64GB sizes).

-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 1/2] archive-tar: write extended headers for file sizes >= 8GB

[René Scharfe]

Am 21.06.2016 um 22:42 schrieb René Scharfe:
> The value 120 is magic; we need it to pass the tests.  That's
> because prepare_header() is used for building extended header
> records as well and we don't create extended headers for extended
> headers (not sure if that would work anyway), so they simply
> vanish when they're over the limit as their size field is set to
> zero.

So how about something like this to make sure extended headers are
only written for regular files and not for other extended headers?
---
 archive-tar.c | 10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/archive-tar.c b/archive-tar.c
index ed562d4..f53e61c 100644
--- a/archive-tar.c
+++ b/archive-tar.c
@@ -199,7 +199,7 @@ static void prepare_header(struct archiver_args *args,
 {
xsnprintf(header->mode, sizeof(header->mode), "%07o", mode & 0);
xsnprintf(header->size, sizeof(header->size), "%011lo",
- S_ISREG(mode) ? ustar_size(size) : 0);
+ S_ISREG(mode) ? size : 0);
xsnprintf(header->mtime, sizeof(header->mtime), "%011lo",
  ustar_mtime(args->time));
 
@@ -240,7 +240,7 @@ static int write_tar_entry(struct archiver_args *args,
struct ustar_header header;
struct strbuf ext_header = STRBUF_INIT;
unsigned int old_mode = mode;
-   unsigned long size;
+   unsigned long size, size_in_header;
void *buffer;
int err = 0;
 
@@ -299,12 +299,14 @@ static int write_tar_entry(struct archiver_args *args,
memcpy(header.linkname, buffer, size);
}
 
-   if (S_ISREG(mode) && ustar_size(size) != size)
+   size_in_header = S_ISREG(mode) ? ustar_size(size) : size;
+   if (size_in_header != size)
strbuf_append_ext_header_uint(_header, "size", size);
+
if (ustar_mtime(args->time) != args->time)
strbuf_append_ext_header_uint(_header, "mtime", args->time);
 
-   prepare_header(args, , mode, size);
+   prepare_header(args, , mode, size_in_header);
 
if (ext_header.len > 0) {
err = write_extended_header(args, sha1, ext_header.buf,
-- 
2.9.0


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

Re: [PATCH 1/2] archive-tar: write extended headers for file sizes >= 8GB

[René Scharfe]

Am 21.06.2016 um 17:59 schrieb Jeff King:
> On Tue, Jun 21, 2016 at 12:54:11AM +0200, René Scharfe wrote:
> 
>>> Unfortunately, it's quite an expensive test to run. For one
>>> thing, unless your filesystem supports files with holes, it
>>> takes 64GB of disk space (you might think piping straight to
>>> `hash-object --stdin` would be better, but it's not; that
>>> tries to buffer all 64GB in RAM!). Furthermore, hashing and
>>> compressing the object takes several minutes of CPU time.
>>>
>>> We could ship just the resulting compressed object data as a
>>> loose object, but even that takes 64MB. So sadly, this code
>>> path remains untested in the test suite.
>>
>> If we could set the limit to a lower value than 8GB for testing then we
>> could at least check if the extended header is written, e.g. if ustar_size()
>> could be convinced to return 0 every time using a hidden command line
>> parameter or an environment variable or something better.
> 
> Yes, we could do that, though I think it loses most of the value of the
> test. We can check that if we hit an arbitrary value we generate the pax
> header, but I think what we _really_ care about is: did we generate an
> output that somebody else's tar implementation can handle.

I agree with the last point, but don't see how that diminishes the
value of such a test.  If we provide file sizes only through extended
headers (the normal header field being set to 0) and we can extract
files with correct sizes then tar must have interpreted those header
as intended, right?

(Or it just guessed the sizes by searching for the next header magic,
but such a fallback won't be accurate for files ending with NUL
characters due to NUL-padding, so we just have to add such a file.)

René


-- >8 --
Subject: archive-tar: test creation of pax extended size headers

---
The value 120 is magic; we need it to pass the tests.  That's
because prepare_header() is used for building extended header
records as well and we don't create extended headers for extended
headers (not sure if that would work anyway), so they simply
vanish when they're over the limit as their size field is set to
zero.

 archive-tar.c   | 7 ++-
 t/t5000-tar-tree.sh | 7 +++
 2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/archive-tar.c b/archive-tar.c
index f53e61c..fbbc4cc 100644
--- a/archive-tar.c
+++ b/archive-tar.c
@@ -14,6 +14,7 @@ static char block[BLOCKSIZE];
 static unsigned long offset;
 
 static int tar_umask = 002;
+static unsigned long ustar_size_max = 0777UL;
 
 static int write_tar_filter_archive(const struct archiver *ar,
struct archiver_args *args);
@@ -179,7 +180,7 @@ static size_t get_path_prefix(const char *path, size_t 
pathlen, size_t maxlen)
 
 static inline unsigned long ustar_size(uintmax_t size)
 {
-   if (size <= 0777UL)
+   if (size <= ustar_size_max)
return size;
else
return 0;
@@ -412,6 +413,10 @@ static int git_tar_config(const char *var, const char 
*value, void *cb)
}
return 0;
}
+   if (!strcmp(var, "tar.ustarsizemax")) {
+   ustar_size_max = git_config_ulong(var, value);
+   return 0;
+   }
 
return tar_filter_config(var, value, cb);
 }
diff --git a/t/t5000-tar-tree.sh b/t/t5000-tar-tree.sh
index 4b68bba..03bb4c7 100755
--- a/t/t5000-tar-tree.sh
+++ b/t/t5000-tar-tree.sh
@@ -102,6 +102,7 @@ test_expect_success \
  echo long filename >a/four$hundred &&
  mkdir a/bin &&
  test-genrandom "frotz" 50 >a/bin/sh &&
+ printf "\0\0\0" >>a/bin/sh &&
  printf "A\$Format:%s\$O" "$SUBSTFORMAT" >a/substfile1 &&
  printf "A not substituted O" >a/substfile2 &&
  if test_have_prereq SYMLINKS; then
@@ -157,6 +158,12 @@ test_expect_success 'git-archive --prefix=olde-' '
 
 check_tar with_olde-prefix olde-
 
+test_expect_success !TAR_NEEDS_PAX_FALLBACK 'pax extended size headers' '
+   git -c tar.ustarsizemax=120 archive HEAD >extended_size_header.tar
+'
+
+check_tar extended_size_header
+
 test_expect_success 'git archive on large files' '
 test_config core.bigfilethreshold 1 &&
 git archive HEAD >b3.tar &&
-- 
2.9.0

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

Re: Problem with --shallow-submodules option

[Stefan Beller]

On Mon, Jun 20, 2016 at 11:32 PM, Istvan Zakar  wrote:
> Hi,
>
> Thanks for the answer.
> So it means that it is a setting on the server side which can be
> activated? (I guess it depends on the version of the server)
> I did some reading in the topic. Are you talking about this setting
> "uploadpack.allowReachableSHA1InWant", or did I misunderstood what I
> read?

No that's exactly what I meant; sorry for not spelling that out.

Thanks,
Stefan

>
> Thanks,
> Istvan
>
> On 20 June 2016 at 19:45, Stefan Beller  wrote:
>> On Mon, Jun 20, 2016 at 6:06 AM, Istvan Zakar  wrote:
>>> Hello,
>>>
>>> I'm working on a relatively big project with many submodules. During
>>> cloning for testing I tried to decrease the amount of data need to be
>>> fetched from the server by using --shallow-submodules option in the clone
>>> command. It seems to check out the tip of the remote repo, and if it's not
>>> the commit registered in the superproject the submodule update fails
>>> (obviously).
>>
>> Yes that is broken as the depth of a submodule is counted from its own HEAD
>> not from the superprojects sha1 as it should.
>>
>> So it does
>>
>> git clone --depth=1  
>>
>> if HEAD != recorded gitlink sha1,
>> git fetch 
>>
>> git checkout 
>>
>>> Can I somehow tell to fetch that exact commit I need for my
>>> superproject?
>>
>> Some servers support fetching by direct sha1, which is what we make use
>> of here, then it sort-of works.
>>
>> If the server doesn't support the capability to fetch an arbitrary sha1,
>> the submodule command fails, with a message such as
>>
>> error: no such remote ref $sha1
>> Fetched in submodule path '', but it did not contain
>> $sha1. Direct fetching of that commit failed.
>>
>> So if it breaks for you now, I would suggest not using that switch, I
>> don't think there is a quick
>> workaround.
>>
>>>
>>> Thanks,
>>>Istvan
>>
>> Thanks,
>> Stefan
>>
>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe git" in
>>> the body of a message to majord...@vger.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] Make find_commit_subject() more robust

[Junio C Hamano]

Johannes Schindelin  writes:

> Just like the pretty printing machinery, we should simply ignore empty
> lines at the beginning of the commit messages.

Thanks.

>
> This discrepancy was noticed when an early version of the rebase--helper
> produced commit objects with more than one empty line between the header
> and the commit message.
>
> Signed-off-by: Johannes Schindelin 
> ---
> Published-As: https://github.com/dscho/git/releases/tag/leading-empty-lines-v2
>
>   git blame seemed to be the most accessible user of
>   find_commit_subject()...
>
>  commit.c |  2 ++
>  t/t8008-blame-formats.sh | 17 +
>  2 files changed, 19 insertions(+)
> Interdiff vs v1:
>
>  diff --git a/t/t8008-blame-formats.sh b/t/t8008-blame-formats.sh
>  index 29f84a6..03bd313 100755
>  --- a/t/t8008-blame-formats.sh
>  +++ b/t/t8008-blame-formats.sh
>  @@ -87,4 +87,21 @@ test_expect_success 'blame --line-porcelain output' '
>   test_cmp expect actual
>   '
>   
>  +test_expect_success '--porcelain detects first non-empty line as subject' '
>  +(
>  +GIT_INDEX_FILE=.git/tmp-index &&
>  +export GIT_INDEX_FILE &&
>  +echo "This is it" >single-file &&
>  +git add single-file &&
>  +tree=$(git write-tree) &&
>  +commit=$(printf "%s\n%s\n%s\n\n\noneline\n\nbody\n" \
>  +"tree $tree" \
>  +"author A  123456789 +" \
>  +"committer C  123456789 +" |
>  +git hash-object -w -t commit --stdin) &&
>  +git blame --porcelain $commit -- single-file >output &&
>  +grep "^summary oneline$" output
>  +)
>  +'
>  +
>   test_done
>
>
> diff --git a/commit.c b/commit.c
> index 3f4f371..7b00989 100644
> --- a/commit.c
> +++ b/commit.c
> @@ -415,6 +415,8 @@ int find_commit_subject(const char *commit_buffer, const 
> char **subject)
>   p++;
>   if (*p) {
>   p += 2;
> + while (*p == '\n')
> + p++;
>   for (eol = p; *eol && *eol != '\n'; eol++)
>   ; /* do nothing */
>   } else
> diff --git a/t/t8008-blame-formats.sh b/t/t8008-blame-formats.sh
> index 29f84a6..03bd313 100755
> --- a/t/t8008-blame-formats.sh
> +++ b/t/t8008-blame-formats.sh
> @@ -87,4 +87,21 @@ test_expect_success 'blame --line-porcelain output' '
>   test_cmp expect actual
>  '
>  
> +test_expect_success '--porcelain detects first non-empty line as subject' '
> + (
> + GIT_INDEX_FILE=.git/tmp-index &&
> + export GIT_INDEX_FILE &&
> + echo "This is it" >single-file &&
> + git add single-file &&
> + tree=$(git write-tree) &&
> + commit=$(printf "%s\n%s\n%s\n\n\noneline\n\nbody\n" \
> + "tree $tree" \
> + "author A  123456789 +" \
> + "committer C  123456789 +" |
> + git hash-object -w -t commit --stdin) &&
> + git blame --porcelain $commit -- single-file >output &&
> + grep "^summary oneline$" output
> + )
> +'
> +
>  test_done
--
To unsubscribe from this list: send the line "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] t7800 readlink not found

[Torsten Bögershausen]

On 06/21/2016 08:39 PM, Junio C Hamano wrote:

Armin Kunaschik  writes:


On Tue, May 31, 2016 at 7:51 AM, Junio C Hamano  wrote:

Torsten Bögershausen  writes:


diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh
index 7ce4cd7..905035c 100755
--- a/t/t7800-difftool.sh
+++ b/t/t7800-difftool.sh
@@ -446,7 +446,7 @@ write_script .git/CHECK_SYMLINKS <<\EOF
   for f in file file2 sub/sub
   do
  echo "$f"
-readlink "$2/$f"
+ls -ld "$2/$f" | sed -e 's/.* -> //'
   done >actual
   EOF


I don't know how portable #ls -ld" really is.

The parts with mode bits, nlinks, uid, gid, size, and date part do
have some variations.  For example, we have been burned on ACL
enabled systems having some funny suffix after the usual mode bits
stuff.

However, as far as this test is concerned, I do not think "how
portable is the output from ls -ld" is an especially relevant
question.  None of the things we expect early in the output (the
fields I enumerated in the previous paragraph) would contain " -> ".
And we know that we do not use a filename that has " -> " (or "->")
as a substring in our tests.

We don't have to use readlink, even on platforms where we do have
readlink.  Building the conditional to be checked at runtime and
providing a shell function read_link that uses "ls -ld | sed" or
"readlink" depending on the runtime check is wasteful.

Just a short, curious question: Is this patch to be accepted/included some time?
I didn't see it in 2.8.4 nor 2.9.0. Maybe it just fell off the table...

Yes, I think this fell off the table as I was waiting for some kind
of agreement or counter-proposal, neither of which came and the
thread was forgotten.

Unless Torsten still has strong objections (or better yet, a better
implementation), I am inclined to queue it as-is.


I just double-checked the man pages for Mac OS and opengroup:
No better implementation from my side -> No objections

--
To unsubscribe from this list: send the line "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] t5614: don't use subshells

[Stefan Beller]

On Tue, Jun 21, 2016 at 12:38 PM, Jeff King  wrote:
> On Tue, Jun 21, 2016 at 12:18:29PM -0700, Junio C Hamano wrote:
>
>> Stefan Beller  writes:
>>
>> >  Unlike the prior patch we would not want this patch to fall through
>> >  to origin/maint fast, but allow cooking?
>>
>> I do not see anything that makes this treated differently from the
>> other fix.  The only difference in behaviour is that "lines" file is
>> now created at the root level of the trash repository's working tree
>> instead of tested clones', and I do not think any test depends on
>> the number of untracked paths in the trash working tree or tries to
>> make sure a file called "lines" is not in there.
>
> I think it is only that the other patch is actually fixing something,
> whereas this is cleanup. So the cost/benefit equation is different. I
> agree neither is high-risk and a test cleanup is generally OK for maint
> (the other is a serious-ish regression IMHO).

I agree on the cost/benefit equation being different. I considered this patch
a "normal risk" thing, which by our standards is not going through to maint
directly, but is cooked at various heat levels before.

>
>> Having said that, I wonder if we want further reduction of the
>> repetition.  Each test except "setup" in this script does an
>> identical thing with very small set of parameters:
>>
>> - make sure super_clone will be removed when done.
>> - clone file://$pwd/. to super_clone but with different set of options.
>> - check the commits in super_clone and super_clone/sub.
>>
>> So, the above would ideally become something like
>>
>> do_test 3 3 --recurse-submodules
>>
>> where the helper would look like
>>
>> do_test () {
>>   cnt_super=$1 cnt_sub=$2 &&
>> shift 2 &&
>> test_when_finished "rm -fr super_clone" &&
>> git clone "$@" "file://$pwd/." super_clone &&
>> git -C super_clone log --oneline >lines &&
>> test_line_count = $cnt_super lines &&
>> git -C super_clone/sub log --oneline >lines &&
>> test_line_count = $cnt_sub lines
>> }
>>
>> Would it rob too much flexibility from future tests to be added to
>> this script if we did it that way?
>
> I think that's an improvement, too. Even if we add further tests, they
> don't have to follow the same format. I would give the function a better
> name than "do_test" though. :P

I thought about implementing this, but I was unsure how much flexibility it
is robbing, as we don't know in which direction we're going to extend the
tests if at all. (Are we testing more options or do we want to inject more
commands like "git submodule update --keep-configured-depth" ?
That kept me away from doing the super short do_test)

Thanks,
Stefan


>
> -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 1/2] archive-tar: write extended headers for file sizes >= 8GB

[Robin H. Johnson]

On Thu, Jun 16, 2016 at 12:37:33AM -0400,  Jeff King wrote:
> We could ship just the resulting compressed object data as a
> loose object, but even that takes 64MB. So sadly, this code
> path remains untested in the test suite.
Additionally compress the object data, and insert it for the purpose of
testing? It's still an expensive test time-wise, but repeated
gzip compression on zeros does still help; to that end, here's the
pieces to add a testcase while only being <9KiB.

t5005-archive-hugefile.sh:
...
mkdir t
zcat t5005-barerepo-64gb-obj.tar.gz.gz | tar xzf - -C t
GIT_DIR=t git archive HEAD | head -c 1 | tar tvf - 2>/dev/null
...

Test repo attached, it's only 8KiB.

Test repo can be recreated with:
truncate -s 64g bigfile
git add bigfile # takes 10 mins
git commit -q -m foo # takes another 10 minutes
tar cf - -C .git . |gzip -9 --no-name |gzip -9f --no-name >barerepo.tar.gz.gz


-- 
Robin Hugh Johnson
Gentoo Linux: Dev, Infra Lead, Foundation Trustee & Treasurer
E-Mail   : robb...@gentoo.org
GnuPG FP : 11ACBA4F 4778E3F6 E4EDF38E B27B944E 34884E85
GnuPG FP : 7D0B3CEB E9B85B1F 825BCECF EE05E6F6 A48F6136
--
To unsubscribe from this list: send the line "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] t5614: don't use subshells

[Jeff King]

On Tue, Jun 21, 2016 at 12:18:29PM -0700, Junio C Hamano wrote:

> Stefan Beller  writes:
> 
> >  Unlike the prior patch we would not want this patch to fall through
> >  to origin/maint fast, but allow cooking?
> 
> I do not see anything that makes this treated differently from the
> other fix.  The only difference in behaviour is that "lines" file is
> now created at the root level of the trash repository's working tree
> instead of tested clones', and I do not think any test depends on
> the number of untracked paths in the trash working tree or tries to
> make sure a file called "lines" is not in there.

I think it is only that the other patch is actually fixing something,
whereas this is cleanup. So the cost/benefit equation is different. I
agree neither is high-risk and a test cleanup is generally OK for maint
(the other is a serious-ish regression IMHO).

> Having said that, I wonder if we want further reduction of the
> repetition.  Each test except "setup" in this script does an
> identical thing with very small set of parameters:
> 
> - make sure super_clone will be removed when done.
> - clone file://$pwd/. to super_clone but with different set of options.
> - check the commits in super_clone and super_clone/sub.
> 
> So, the above would ideally become something like
> 
> do_test 3 3 --recurse-submodules
> 
> where the helper would look like
> 
> do_test () {
>   cnt_super=$1 cnt_sub=$2 &&
> shift 2 &&
> test_when_finished "rm -fr super_clone" &&
> git clone "$@" "file://$pwd/." super_clone &&
> git -C super_clone log --oneline >lines &&
> test_line_count = $cnt_super lines &&
> git -C super_clone/sub log --oneline >lines &&
> test_line_count = $cnt_sub lines
> }
> 
> Would it rob too much flexibility from future tests to be added to
> this script if we did it that way?

I think that's an improvement, too. Even if we add further tests, they
don't have to follow the same format. I would give the function a better
name than "do_test" though. :P

-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 v3 2/9] Disallow diffopt.close_file when using the log_tree machinery

[Junio C Hamano]

Junio C Hamano  writes:

> Junio C Hamano  writes:
>
>> Johannes Schindelin  writes:
>>
>>> We are about to teach the log_tree machinery to reuse the diffopt.file
>>> setting to output to a file stream different from stdout.
>>>
>>> This means that builtin am can no longer ask the diff machinery to
>>> close the file when actually calling the log_tree machinery (which
>>> wants to flush the very same file stream that would then already be
>>> closed).
>>
>> Sorry for being slow, but I am not sure why the first paragraph has
>> to mean the second paragraph.  This existing caller opens a new
>> stream, sets .fp to it, and expects that the log_tree_commit() to
>> close it if told by setting .close_file to true, all of which sounds
>> sensible.
>>
>> If a codepath wants to use the same stream for two or more calls to
>> log_tree by pointing the stream with .fp, it would be of course a
>> problem for the caller to set .close_file to true in its first call,
>> as .fp will be closed and no longer usable for second and subsequent
>> call, and that would be a bug, but for a single-shot call it feels
>> entirely a sensible request to make, no?
>>
>> Obviously you have looked at the codepaths involved a lot longer
>> than I did, and I do not doubt your conclusion, but I cannot quite
>> convince myself with the above explanation.
>>
>> The option parser of "git diff" family sets ->close_file to true
>> when the --output option is given.
>>
>> Wouldn't this patch break "git log --output=foo -3"?
>
> I wonder if the right approach is to stop using .close_file
> everywhere.
>
> With this "do not set .close_file if you use log_tree_commit()",
> "git log --output=/dev/stdout -3" gets broken, but removing that
> check is not sufficient to correct the same command with "-p", as
> letting .close_file to close the output file after finishing a
> single diff would mean that subsequent write to the same file
> descriptor will trigger a failure.

We could say "git log --output=foo -3 [-p]" without any of your
patches is already broken, and it is a valid excuse to take this
change that we are not making things worse with it.

It is just 3/9 is a logical first step to correct that exact
problem, i.e. some codepaths, even though there is a place that
holds the output stream and command line parser does prepare one for
"foo" when --output=foo is given, ignore it and send thigns to the
standard output stream.  You might not have written 3/9 in order to
fix that "git log --output=foo" problem, but a fix for it should
look exactly like your 3/9, I would think.

And it is sad that this step makes that fix impossible.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 2/3] perf: make the tests work in worktrees

[Jeff King]

On Fri, May 13, 2016 at 03:25:58PM +0200, Johannes Schindelin wrote:

> This patch makes perf-lib.sh more robust so that it can run correctly
> even inside a worktree. For example, it assumed that $GIT_DIR/objects is
> the objects directory (which is not the case for worktrees) and it used
> the commondir file verbatim, even if it contained a relative path.
> 
> Furthermore, the setup code expected `git rev-parse --git-dir` to spit
> out a relative path, which is also not true for worktrees. Let's just
> change the code to accept both relative and absolute paths, by avoiding
> the `cd` into the copied working directory.

I was trying to run the perf scripts today and got bit by this patch.
The problem is this line:

> + objects_dir="$(git -C "$source" rev-parse --git-path objects)"

When the perf script is running, the "git" in its PATH is the version of
git being perf-tested, not the most recent one. And --git-path wasn't
introduced until v2.5.0. So if you try to compare results against an
older git, it fails:

  $ cd t/perf
  $ ./run v2.4.0 v2.9.0 -- p-perf-lib-sanity.sh
  [...]

  === Running 1 tests in 
build/67308bd628c6235dbc1bad60c9ad1f2d27d576cc/bin-wrappers ===
  fatal: ambiguous argument 'objects': unknown revision or path not in the 
working tree.
  Use '--' to separate paths from revisions, like this:
  'git  [...] -- [...]'
  cp: unrecognized option '--git-path
  objects'
  Try 'cp --help' for more information.
  error: failed to copy repository '/home/peff/compile/git/t/..' to 
'/var/ram/git-tests/trash directory.p-perf-lib-sanity'

  [...]
  Test   v2.4.0  v2.9.0 

  
--
  .1: test_perf_default_repo works  
0.00(0.00+0.00)
  .2: test_checkout_worktree works  
0.01(0.00+0.00)
  .4: export a weird var
0.00(0.00+0.00)
  .6: important variables available in subshells
0.00(0.00+0.00)
  .7: test-lib-functions correctly loaded in subshells  
0.00(0.00+0.00)

I know that running modern perf tests against older versions isn't
guaranteed to work (the tests might rely on newer options, for example),
but it at least generally worked up until this point.

IMHO the problem is in the design of t/perf, though. It should always
use your modern git for the harness setup, and only use the git-to-test
inside test_expect and test_perf blocks.

-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 v3 2/9] Disallow diffopt.close_file when using the log_tree machinery

[Junio C Hamano]

Junio C Hamano  writes:

> Johannes Schindelin  writes:
>
>> We are about to teach the log_tree machinery to reuse the diffopt.file
>> setting to output to a file stream different from stdout.
>>
>> This means that builtin am can no longer ask the diff machinery to
>> close the file when actually calling the log_tree machinery (which
>> wants to flush the very same file stream that would then already be
>> closed).
>
> Sorry for being slow, but I am not sure why the first paragraph has
> to mean the second paragraph.  This existing caller opens a new
> stream, sets .fp to it, and expects that the log_tree_commit() to
> close it if told by setting .close_file to true, all of which sounds
> sensible.
>
> If a codepath wants to use the same stream for two or more calls to
> log_tree by pointing the stream with .fp, it would be of course a
> problem for the caller to set .close_file to true in its first call,
> as .fp will be closed and no longer usable for second and subsequent
> call, and that would be a bug, but for a single-shot call it feels
> entirely a sensible request to make, no?
>
> Obviously you have looked at the codepaths involved a lot longer
> than I did, and I do not doubt your conclusion, but I cannot quite
> convince myself with the above explanation.
>
> The option parser of "git diff" family sets ->close_file to true
> when the --output option is given.
>
> Wouldn't this patch break "git log --output=foo -3"?

I wonder if the right approach is to stop using .close_file
everywhere.

With this "do not set .close_file if you use log_tree_commit()",
"git log --output=/dev/stdout -3" gets broken, but removing that
check is not sufficient to correct the same command with "-p", as
letting .close_file to close the output file after finishing a
single diff would mean that subsequent write to the same file
descriptor will trigger a failure.
--
To unsubscribe from this list: send the line "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] t5614: don't use subshells

[Junio C Hamano]

Stefan Beller  writes:

>  Unlike the prior patch we would not want this patch to fall through
>  to origin/maint fast, but allow cooking?

I do not see anything that makes this treated differently from the
other fix.  The only difference in behaviour is that "lines" file is
now created at the root level of the trash repository's working tree
instead of tested clones', and I do not think any test depends on
the number of untracked paths in the trash working tree or tries to
make sure a file called "lines" is not in there.

> diff --git a/t/t5614-clone-submodules.sh b/t/t5614-clone-submodules.sh
> index a9aaa01..da2a67f 100755
> --- a/t/t5614-clone-submodules.sh
> +++ b/t/t5614-clone-submodules.sh
> @@ -25,76 +25,46 @@ test_expect_success 'setup' '
>  test_expect_success 'nonshallow clone implies nonshallow submodule' '
>   test_when_finished "rm -rf super_clone" &&
>   git clone --recurse-submodules "file://$pwd/." super_clone &&
> - (
> - cd super_clone &&
> - git log --oneline >lines &&
> - test_line_count = 3 lines
> - ) &&
> - (
> - cd super_clone/sub &&
> - git log --oneline >lines &&
> - test_line_count = 3 lines
> - )
> + git -C super_clone log --oneline >lines &&
> + test_line_count = 3 lines &&
> + git -C super_clone/sub log --oneline >lines &&
> + test_line_count = 3 lines
>  '

Having said that, I wonder if we want further reduction of the
repetition.  Each test except "setup" in this script does an
identical thing with very small set of parameters:

- make sure super_clone will be removed when done.
- clone file://$pwd/. to super_clone but with different set of options.
- check the commits in super_clone and super_clone/sub.

So, the above would ideally become something like

do_test 3 3 --recurse-submodules

where the helper would look like

do_test () {
cnt_super=$1 cnt_sub=$2 &&
shift 2 &&
test_when_finished "rm -fr super_clone" &&
git clone "$@" "file://$pwd/." super_clone &&
git -C super_clone log --oneline >lines &&
test_line_count = $cnt_super lines &&
git -C super_clone/sub log --oneline >lines &&
test_line_count = $cnt_sub lines
}

Would it rob too much flexibility from future tests to be added to
this script if we did it that way?

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

Re: [PATCH v3 1/9] am: stop ignoring errors reported by log_tree_diff()

[Junio C Hamano]

Johannes Schindelin  writes:

> Note: there are more places in the builtin am code that ignore
> errors returned from library functions. Fixing those is outside the
> purview of the current patch series, though.

The caller of parse_mail() and parse_mail_rebase() is not prepared
to see an error code in the returned value from these function,
which are to return a boolean telling the caller to skip or use the
patch file.  At least the caller needs to notice negative return and
made to die/exit(128), instead of silently skipping a corrupt or
unopenable patch, no?  Otherwise this will change the behaviour.

> Cc: Paul Tan 
> Signed-off-by: Johannes Schindelin 
> ---
>  builtin/am.c | 14 +-
>  1 file changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/builtin/am.c b/builtin/am.c
> index 3dfe70b..0e28a62 100644
> --- a/builtin/am.c
> +++ b/builtin/am.c
> @@ -1433,12 +1433,15 @@ static void get_commit_info(struct am_state *state, 
> struct commit *commit)
>  /**
>   * Writes `commit` as a patch to the state directory's "patch" file.
>   */
> -static void write_commit_patch(const struct am_state *state, struct commit 
> *commit)
> +static int write_commit_patch(const struct am_state *state, struct commit 
> *commit)
>  {
>   struct rev_info rev_info;
>   FILE *fp;
>  
> - fp = xfopen(am_path(state, "patch"), "w");
> + fp = fopen(am_path(state, "patch"), "w");
> + if (!fp)
> + return error(_("Could not open %s for writing"),
> + am_path(state, "patch"));
>   init_revisions(_info, NULL);
>   rev_info.diff = 1;
>   rev_info.abbrev = 0;
> @@ -1453,7 +1456,7 @@ static void write_commit_patch(const struct am_state 
> *state, struct commit *comm
>   rev_info.diffopt.close_file = 1;
>   add_pending_object(_info, >object, "");
>   diff_setup_done(_info.diffopt);
> - log_tree_commit(_info, commit);
> + return log_tree_commit(_info, commit);
>  }
>  
>  /**
> @@ -1501,13 +1504,14 @@ static int parse_mail_rebase(struct am_state *state, 
> const char *mail)
>   unsigned char commit_sha1[GIT_SHA1_RAWSZ];
>  
>   if (get_mail_commit_sha1(commit_sha1, mail) < 0)
> - die(_("could not parse %s"), mail);
> + return error(_("could not parse %s"), mail);
>  
>   commit = lookup_commit_or_die(commit_sha1, mail);
>  
>   get_commit_info(state, commit);
>  
> - write_commit_patch(state, commit);
> + if (write_commit_patch(state, commit) < 0)
> + return -1;
>  
>   hashcpy(state->orig_commit, commit_sha1);
>   write_state_text(state, "original-commit", sha1_to_hex(commit_sha1));
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/1] git-p4: correct hasBranchPrefix verbose output

[Junio C Hamano]

Lars Schneider  writes:

>> On 21 Jun 2016, at 15:53, Andrew Oakley  wrote:
>> 
>> The logic here was inverted, you got a message saying the file is
>> ignored for each file that is not ignored.
>> ---
>> git-p4.py | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/git-p4.py b/git-p4.py
>> index b6593cf..b123aa2 100755
>> --- a/git-p4.py
>> +++ b/git-p4.py
>> @@ -2674,7 +2674,7 @@ class P4Sync(Command, P4UserMap):
>> return True
>> hasPrefix = [p for p in self.branchPrefixes
>> if p4PathStartsWith(path, p)]
>> -if hasPrefix and self.verbose:
>> +if not hasPrefix and self.verbose:
>
> Thanks Andrew! Your fix is correct.

Thanks.  This needs sign-off, though.

>
> - Lars
>
>> print('Ignoring file outside of prefix: {0}'.format(path))
>> return hasPrefix
>> 
>> -- 
>> 2.7.3
>> 
>> --
>> To unsubscribe from this list: send the line "unsubscribe git" in
>> the body of a message to majord...@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] perf: accommodate for MacOSX

[Junio C Hamano]

Lars Schneider  writes:

>> On 21 Jun 2016, at 15:53, Johannes Schindelin  
>> wrote:
>> 
> Looks good to me.

Thanks, both, for following thru.
Will apply.

--
To unsubscribe from this list: send the line "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] t7800 readlink not found

[Junio C Hamano]

Armin Kunaschik  writes:

> On Tue, May 31, 2016 at 7:51 AM, Junio C Hamano  wrote:
>> Torsten Bögershausen  writes:
>>
 diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh
 index 7ce4cd7..905035c 100755
 --- a/t/t7800-difftool.sh
 +++ b/t/t7800-difftool.sh
 @@ -446,7 +446,7 @@ write_script .git/CHECK_SYMLINKS <<\EOF
   for f in file file2 sub/sub
   do
  echo "$f"
 -readlink "$2/$f"
 +ls -ld "$2/$f" | sed -e 's/.* -> //'
   done >actual
   EOF

>>> I don't know how portable #ls -ld" really is.
>>
>> The parts with mode bits, nlinks, uid, gid, size, and date part do
>> have some variations.  For example, we have been burned on ACL
>> enabled systems having some funny suffix after the usual mode bits
>> stuff.
>>
>> However, as far as this test is concerned, I do not think "how
>> portable is the output from ls -ld" is an especially relevant
>> question.  None of the things we expect early in the output (the
>> fields I enumerated in the previous paragraph) would contain " -> ".
>> And we know that we do not use a filename that has " -> " (or "->")
>> as a substring in our tests.
>>
>> We don't have to use readlink, even on platforms where we do have
>> readlink.  Building the conditional to be checked at runtime and
>> providing a shell function read_link that uses "ls -ld | sed" or
>> "readlink" depending on the runtime check is wasteful.
>
> Just a short, curious question: Is this patch to be accepted/included some 
> time?
> I didn't see it in 2.8.4 nor 2.9.0. Maybe it just fell off the table...

Yes, I think this fell off the table as I was waiting for some kind
of agreement or counter-proposal, neither of which came and the
thread was forgotten.

Unless Torsten still has strong objections (or better yet, a better
implementation), I am inclined to queue it as-is.

Thanks for pinging the thread.
--
To unsubscribe from this list: send the line "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: How to find commits unique to a branch

[Junio C Hamano]

Nikolaus Rath  writes:

> On Jun 20 2016, Nikolaus Rath  wrote:
>> On Jun 20 2016, Junio C Hamano  wrote:
>>> Nikolaus Rath  writes:
>>>
 What's the best way to find all commits in a branch A that have not been
 cherry-picked from (or to) another branch B?

 I think I could format-patch all commits in every branch into separate
 files, hash the Author and Date of each files, and then compare the two
 lists. But I'm hoping there's a way to instead have git do the
 heavy-lifting?
>>>
>>> "git cherry" perhaps?
>>
>> That seems to work only the "wrong way around". I have a tag
>> fuse_3_0_start, which is the common ancestor to "master" and
>> "fuse_2_9_bugfix". I'd like to find all the commits from fuse_3_0_start
>> to master that have not been cherry-picked into fuse_2_9_bugfix.

Hmm, so the topology roughly would look like:

A'--B'--D' 2fix
   /
  o---A---B---C---D---E---F master
3start

And you want to find commits in 3start..master that do not have
equivalent in 3start..2fix

"git cherry --help" starts like this:

NAME
   git-cherry - Find commits yet to be applied to upstream

SYNOPSIS
   git cherry [-v] [ [ []]]

DESCRIPTION
   Determine whether there are commits in ..
   that are equivalent to those in the range ...

Applying that to our picture, we want to find commits yet to be
applied to 2fix, and do so by comparing the commits between
3start..master and 3start..2fix.

I find that the first sentence of the description is fuzzy
("Determine whether" would imply that you would get "Yes/No" but
what we want is "here are the commits that do not have counterpart
in 2fix"), but we already know  corresponds to 2fix
(i.e. we are finding ones yet to be applied to there, which can be
inferred from the NAME line), so  must be 'master' That means
that  corresponds to 3start, and we will be comparing commits
in two ranges:

master..2fix (i e. .., which is the same thing as 
3start..2fix)
3start..master (i.e. ..)

So perhaps "git cherry -v 2fix master 3start"?
--
To unsubscribe from this list: send the line "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 0/2] friendlier handling of overflows in archive-tar

[Junio C Hamano]

Jeff King  writes:

> Junio, this is the jk/big-and-old-archive-tar topic.
>
> The interdiff is:
> ...
> diff --git a/archive-tar.c b/archive-tar.c
> index c7b85fd..ed562d4 100644
> --- a/archive-tar.c
> +++ b/archive-tar.c
> @@ -179,7 +179,7 @@ static size_t get_path_prefix(const char *path, size_t 
> pathlen, size_t maxlen)
>  
>  static inline unsigned long ustar_size(uintmax_t size)
>  {
> - if (size < 0777UL)
> + if (size <= 0777UL)
>   return size;
>   else
>   return 0;
> @@ -187,7 +187,7 @@ static inline unsigned long ustar_size(uintmax_t size)
>  
>  static inline unsigned long ustar_mtime(time_t mtime)
>  {
> - if (mtime < 0777UL)
> + if (mtime <= 0777UL)
>   return mtime;
>   else
>   return 0;
> @@ -299,7 +299,7 @@ static int write_tar_entry(struct archiver_args *args,
>   memcpy(header.linkname, buffer, size);
>   }
>  
> - if (ustar_size(size) != size)
> + if (S_ISREG(mode) && ustar_size(size) != size)
>   strbuf_append_ext_header_uint(_header, "size", size);
>   if (ustar_mtime(args->time) != args->time)
>   strbuf_append_ext_header_uint(_header, "mtime", args->time);

Thanks.

By the way, I realized the naming mistake and the topic branch is
now named s/old/future/.  Size being big is one condition that needs
this fix, so is timestamp being from far future, not "old".
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 2/9] Disallow diffopt.close_file when using the log_tree machinery

[Junio C Hamano]

Johannes Schindelin  writes:

> We are about to teach the log_tree machinery to reuse the diffopt.file
> setting to output to a file stream different from stdout.
>
> This means that builtin am can no longer ask the diff machinery to
> close the file when actually calling the log_tree machinery (which
> wants to flush the very same file stream that would then already be
> closed).

Sorry for being slow, but I am not sure why the first paragraph has
to mean the second paragraph.  This existing caller opens a new
stream, sets .fp to it, and expects that the log_tree_commit() to
close it if told by setting .close_file to true, all of which sounds
sensible.

If a codepath wants to use the same stream for two or more calls to
log_tree by pointing the stream with .fp, it would be of course a
problem for the caller to set .close_file to true in its first call,
as .fp will be closed and no longer usable for second and subsequent
call, and that would be a bug, but for a single-shot call it feels
entirely a sensible request to make, no?

Obviously you have looked at the codepaths involved a lot longer
than I did, and I do not doubt your conclusion, but I cannot quite
convince myself with the above explanation.

The option parser of "git diff" family sets ->close_file to true
when the --output option is given.

Wouldn't this patch break "git log --output=foo -3"?

> To stave off similar problems in the future, report it as a bug if
> log_tree_commit() is called with a non-zero diffopt.close_file.
>
> Signed-off-by: Johannes Schindelin 
> ---
>  builtin/am.c | 6 --
>  log-tree.c   | 3 +++
>  2 files changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/builtin/am.c b/builtin/am.c
> index 0e28a62..47d78aa 100644
> --- a/builtin/am.c
> +++ b/builtin/am.c
> @@ -1437,6 +1437,7 @@ static int write_commit_patch(const struct am_state 
> *state, struct commit *commi
>  {
>   struct rev_info rev_info;
>   FILE *fp;
> + int res;
>  
>   fp = fopen(am_path(state, "patch"), "w");
>   if (!fp)
> @@ -1453,10 +1454,11 @@ static int write_commit_patch(const struct am_state 
> *state, struct commit *commi
>   DIFF_OPT_SET(_info.diffopt, FULL_INDEX);
>   rev_info.diffopt.use_color = 0;
>   rev_info.diffopt.file = fp;
> - rev_info.diffopt.close_file = 1;
>   add_pending_object(_info, >object, "");
>   diff_setup_done(_info.diffopt);
> - return log_tree_commit(_info, commit);
> + res = log_tree_commit(_info, commit);
> + fclose(fp);
> + return res;
>  }
>  
>  /**
> diff --git a/log-tree.c b/log-tree.c
> index 78a5381..dc0180d 100644
> --- a/log-tree.c
> +++ b/log-tree.c
> @@ -864,6 +864,9 @@ int log_tree_commit(struct rev_info *opt, struct commit 
> *commit)
>   struct log_info log;
>   int shown;
>  
> + if (opt->diffopt.close_file)
> + die("BUG: close_file is incompatible with log_tree_commit()");
> +
>   log.commit = commit;
>   log.parent = NULL;
>   opt->loginfo = 
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/1] git-p4: correct hasBranchPrefix verbose output

[Lars Schneider]

> On 21 Jun 2016, at 15:53, Andrew Oakley  wrote:
> 
> The logic here was inverted, you got a message saying the file is
> ignored for each file that is not ignored.
> ---
> git-p4.py | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/git-p4.py b/git-p4.py
> index b6593cf..b123aa2 100755
> --- a/git-p4.py
> +++ b/git-p4.py
> @@ -2674,7 +2674,7 @@ class P4Sync(Command, P4UserMap):
> return True
> hasPrefix = [p for p in self.branchPrefixes
> if p4PathStartsWith(path, p)]
> -if hasPrefix and self.verbose:
> +if not hasPrefix and self.verbose:

Thanks Andrew! Your fix is correct.

- Lars

> print('Ignoring file outside of prefix: {0}'.format(path))
> return hasPrefix
> 
> -- 
> 2.7.3
> 
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

Re: [PATCH v2] perf: accommodate for MacOSX

[Lars Schneider]

> On 21 Jun 2016, at 15:53, Johannes Schindelin  
> wrote:
> 
> As this developer has no access to MacOSX developer setups anymore,
> Travis becomes the best bet to run performance tests on that OS.
> 
> However, on MacOSX /usr/bin/time is that good old BSD executable that
> no Linux user cares about, as demonstrated by the perf-lib.sh's use
> of GNU-ish extensions. And by the hard-coded path.
> 
> Let's just work around this issue by using gtime on MacOSX, the
> Homebrew-provided GNU implementation onto which pretty much every
> MacOSX power user falls back anyway.
> 
> To help other developers use Travis to run performance tests on
> MacOSX, the .travis.yml file now sports a commented-out line that
> installs GNU time via Homebrew.
> 
> Signed-off-by: Johannes Schindelin 
> ---
> Published-As: https://github.com/dscho/git/releases/tag/perf-macosx-v2
> .travis.yml| 2 ++
> t/perf/perf-lib.sh | 6 +-
> 2 files changed, 7 insertions(+), 1 deletion(-)
> Interdiff vs v1:
> 
> diff --git a/.travis.yml b/.travis.yml
> index 0e569bc..c2b76f9 100644
> --- a/.travis.yml
> +++ b/.travis.yml
> @@ -82,7 +82,9 @@ before_install:
>brew tap homebrew/binary --quiet
>brew_force_set_latest_binary_hash perforce
>brew_force_set_latest_binary_hash perforce-server
> -  brew install git-lfs perforce-server perforce gettext gnu-time
> +  # Uncomment this if you want to run perf tests:
> +  # brew install gnu-time
> +  brew install git-lfs perforce-server perforce gettext
>brew link --force gettext
>;;
>  esac;
> 
> 
> diff --git a/.travis.yml b/.travis.yml
> index c20ec54..c2b76f9 100644
> --- a/.travis.yml
> +++ b/.travis.yml
> @@ -82,6 +82,8 @@ before_install:
>   brew tap homebrew/binary --quiet
>   brew_force_set_latest_binary_hash perforce
>   brew_force_set_latest_binary_hash perforce-server
> +  # Uncomment this if you want to run perf tests:
> +  # brew install gnu-time
>   brew install git-lfs perforce-server perforce gettext
>   brew link --force gettext
>   ;;
> diff --git a/t/perf/perf-lib.sh b/t/perf/perf-lib.sh
> index 18c363e..773f955 100644
> --- a/t/perf/perf-lib.sh
> +++ b/t/perf/perf-lib.sh
> @@ -127,11 +127,15 @@ test_checkout_worktree () {
> # Performance tests should never fail.  If they do, stop immediately
> immediate=t
> 
> +# Perf tests require GNU time
> +case "$(uname -s)" in Darwin) GTIME="${GTIME:-gtime}";; esac
> +GTIME="${GTIME:-/usr/bin/time}"
> +
> test_run_perf_ () {
>   test_cleanup=:
>   test_export_="test_cleanup"
>   export test_cleanup test_export_
> - /usr/bin/time -f "%E %U %S" -o test_time.$i "$SHELL" -c '
> + "$GTIME" -f "%E %U %S" -o test_time.$i "$SHELL" -c '
> . '"$TEST_DIRECTORY"/test-lib-functions.sh'
> test_export () {
>   [ $# != 0 ] || return 0
> -- 
> 2.9.0.118.g0e1a633
> 
> base-commit: ab7797dbe95fff38d9265869ea367020046db118

Looks good to me.

- Lars

--
To unsubscribe from this list: send the line "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: How to find commits unique to a branch

[Nikolaus Rath]

On Jun 20 2016, Nikolaus Rath  wrote:
> On Jun 20 2016, Junio C Hamano  wrote:
>> Nikolaus Rath  writes:
>>
>>> What's the best way to find all commits in a branch A that have not been
>>> cherry-picked from (or to) another branch B?
>>>
>>> I think I could format-patch all commits in every branch into separate
>>> files, hash the Author and Date of each files, and then compare the two
>>> lists. But I'm hoping there's a way to instead have git do the
>>> heavy-lifting?
>>
>> "git cherry" perhaps?
>
> That seems to work only the "wrong way around". I have a tag
> fuse_3_0_start, which is the common ancestor to "master" and
> "fuse_2_9_bugfix". I'd like to find all the commits from fuse_3_0_start
> to master that have not been cherry-picked into fuse_2_9_bugfix.
>
> However:
>
> * "git cherry fuse_3_0_start master release2.9" tells me nothing has
>   been cherry-picked at all (only lines with +)
>
> * "git cherry fuse_3_0_start release2.9 master" also tells me nothing
>   has been cherry picked, but somehow shows a smaller total number of
>   commits.
>
> * "git cherry master release2.9 fuse_3_0_start" gives me the commits
>   from fuse_2_9_bugfix that have not been cherry-picked into master
>   (which seems to be in contradiction to the two earlier commands).
>
>
> Am I missing something obvious?

I meant to add: the repository I'm working with is at
https://github.com/libfuse/libfuse/.

Best,
-Nikolaus

-- 
GPG encrypted emails preferred. Key id: 0xD113FCAC3C4E599F
Fingerprint: ED31 791B 2C5C 1613 AF38 8B8A D113 FCAC 3C4E 599F

 »Time flies like an arrow, fruit flies like a Banana.«
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/1] mingw: work around t2300's assuming non-Windows paths

[Junio C Hamano]

Junio C Hamano  writes:

> I think you would need something similar to "pwd -W", that is, leave
> "git --exec-path" as a way to give shell scripts people have written
> over the years that allows them to say "git-cmd" as long as they do
> PATH="$(git --exec-path):$PATH" upfront.  And for Windows scripts,
> introduce a new option "git --exec-path-windows" that can give
> C:/git-sdk-64/usr/src/git (or even using backslash).

Of course we could go the other way.  We can declare that the output
of "git --exec-path" is the format that is platform-native pathname
to the directory [*1*], introduce a new option that is better
named than "--exec-path-to-include-in-PATH-in-shell-scripts" that
does the "convert C:/ to /c/ on Windows before showing" thing, and
rewrite all the references that does PATH=$(git --exec-path):$PATH
in scripts to use that new option.

The fact remains that on some platforms two variants are needed.  We
can update our test scripts with "convert C:/ to /c/ on Windows" to
work around a test failure like the patch under discussion did, but
that approach would not scale to fix real world scripts that people
already have, which is what I am trying to see if we can address in
these two messages.


[Footnote]

*1* ...instead of "it is suitable for PATH=$(git --exec-path):$PATH
in your shell scripts", which was the definition I used in the
message I am responding to.  "The name '--exec-path' implies it is a
path to the directory, and it is more natural for that to be platform
native" is a valid argument (and that is why I am saying "we could
go the other way" here), but I am not convinced that the conclusion
that the argument leads to is a better one in the practical sense.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/1] mingw: work around t2300's assuming non-Windows paths

[Junio C Hamano]

Johannes Schindelin  writes:

>> Hmm, I am confused.  `git --exec-path` _is_ meant to "spit out" a
>> path that is usable when prepended/appended to $PATH [1], and it
>> does _not_ have to be POSIX-ish path.  It is totally up to the port
>> to adjust it to the platform's convention how the $PATH environment
>> variable is understood.
>
> The port does exactly what it is supposed to do: the output of `git
> --exec-path` can be prepended/appended to %PATH%. The point here is:
> %PATH% is *semicolon*-delimited.

I said $PATH because --exec-path does not care what you do with
%PATH% but it deeply cares that its output is usable in $PATH.

I think you would need something similar to "pwd -W", that is, leave
"git --exec-path" as a way to give shell scripts people have written
over the years that allows them to say "git-cmd" as long as they do
PATH="$(git --exec-path):$PATH" upfront.  And for Windows scripts,
introduce a new option "git --exec-path-windows" that can give
C:/git-sdk-64/usr/src/git (or even using backslash).

I do not mean to say that exec_cmd.c::git_exec_path() function must
return a string /c/git-sdk-64/usr/src/git by the above.  It should
keep returning C:/git-sdk-64/usr/src/git, I think.  Its use in
exec_cmd.c::add_path() to use PATH_SEP to do the equivalent
internally for the platform would want C:/git-sdk-64/usr/src/git
there.  Also I think exec_cmd.c::argv_exec_path should keep using
platform native format.

Perhaps you can do the conversion you are doing with this
"let's change t2300" patch instead in C where the return value of
git_exec_path() is fed to puts() in git.c::handle_options(), or
better yet make a helper function git_exec_path_for_shell() that
calls git_exec_path() and does the conversion on Windows (and passes
the result from git_exec_path() intact on other platforms).

I say all of the above because I see this in hits from "git grep -e
--exec-path":

contrib/subtree/git-subtree.sh:PATH=$PATH:$(git --exec-path)

and I would imagine there are countless other shell scripts that
follow the "Output from 'git --exec-path' can be prepended/appended
to PATH to allow you write 'git-cmd' instead of 'git cmd'".  They
would not work unless your "git --exec-path" gives an output that is
usable by shell in $PATH (and not %PATH%).

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

Re: [PATCH 5/5] format-patch: avoid freopen()

[Junio C Hamano]

Johannes Schindelin  writes:

> That is a very convincing argument. So convincing that I wanted to change
> the patch to guard behind `diff_use_color_default == GIT_COLOR_AUTO`.

I actually was expecting, instead of your:

if (output_directory) {
+   rev.diffopt.use_color = 0;
if (use_stdout)
die(_("standard output, or directory, which one?"));

an update would say

if (output_directory) {
if (rev.diffopt.use_color == GIT_COLOR_AUTO)
rev.diffopt.use_color = 0;
if (use_stdout)
die(_("standard output, or directory, which one?"));

I didn't expect you to check diff_use_color_default exactly for the
reason why you say "But that is the wrong variable".
--
To unsubscribe from this list: send the line "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: [BUG REPORT] git 2.9.0 clone --recursive fails on cloning a submodule

[Duy Nguyen]

On Sun, Jun 19, 2016 at 12:00 PM, Jeff King  wrote:
> Stefan, I think it might be worth revisiting the default set by d22eb04
> to propagate shallowness from the super-project clone. In an ideal
> world, we would be asking each submodule for the actual commit we are
> interested in, and shallowness would not matter. But until
> uploadpack.allowReachableSHA1InWant works everywhere, I suspect this is
> going to be a problem.

Maybe we can pass an option to subsequent clones that say "if
allow-...-sha1-in-want is advertised, do a shallow clone, otherwise
fall back to full clone"?
-- 
Duy
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Problem with git-http-backend.exe as iis cgi

[Junio C Hamano]

Konstantin Khomoutov  writes:

> On Thu, 10 Mar 2016 07:28:50 +
> Florian Manschwetus  wrote:
>
>> I tried to setup git-http-backend with iis, as iis provides proper
>> impersonation for cgi under windows, which leads to have the
>> filesystem access performed with the logon user, therefore the
>> webserver doesn't need generic access to the files. I stumbled across
>> a problem, ending up with post requests hanging forever. After some
>> investigation I managed to get it work by wrapping the http-backend
>> into a bash script, giving a lot of control about the environmental
>> things, I was unable to solve within IIS configuration. The
>> workaround, I use currently, is to use "/bin/head -c
>> ${CONTENT_LENGTH} | ./git-http-backend.exe", which directly shows the
>> issue. Git http-backend should check if CONTENT_LENGTH is set to
>> something reasonable (e.g. >0) and should in this case read only
>> CONTENT_LENGTH bytes from stdin, instead of reading till EOF what I
>> suspect it is doing currently.
> ...
> So yes, if Git currently reads until EOF, it's an error.

This sounded vaguely familiar.  Isn't this responding to a stale
thread?

http://thread.gmane.org/gmane.comp.version-control.git/290114
proposed a patch along the line, and corrections to the patch was
suggested in the review, but it was not followed through, it seems.



--
To unsubscribe from this list: send the line "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: http-backend fatal error message on shallow clone

[Duy Nguyen]

On Tue, Jun 21, 2016 at 2:10 PM, Jeff King  wrote:
> ...
> So this request actually takes _two_ upload-pack instances to serve
> (which is not uncommon when we need multiple rounds of
> get_common_commits(), though I am a little surprised that would be the
> case for a clone). And the first one seems to be missing a closing
> "" flush packet from the client to end it.
>
> If that analysis is correct, this isn't affecting operation in any way;
> it's just giving a useless message from upload-pack (and as you note,
> that could trigger http-backend to exit(1), which may make the webserver
> unhappy).
>
> If we further instrument upload-pack to set GIT_TRACE_PACKET on the
> server side, we can see the two requests:
>
> packet:  upload-pack< want 379518c0c94e3b1a0710129d03d5560814a0ba6f 
> multi_ack_detailed no-done side-band-64k thin-pack include-tag ofs-delta 
> agent=git/2.9.0.37.gb3ad8ab.dirty
> packet:  upload-pack< deepen 1
> packet:  upload-pack< 
> packet:  upload-pack> shallow 379518c0c94e3b1a0710129d03d5560814a0ba6f
> packet:  upload-pack> 
>
> packet:  upload-pack< want 379518c0c94e3b1a0710129d03d5560814a0ba6f 
> multi_ack_detailed no-done side-band-64k thin-pack include-tag ofs-delta 
> agent=git/2.9.0.37.gb3ad8ab.dirty
> packet:  upload-pack< deepen 1
> packet:  upload-pack< 
> packet:  upload-pack> shallow 379518c0c94e3b1a0710129d03d5560814a0ba6f
> packet:  upload-pack> 
> packet:  upload-pack< done
> packet:  upload-pack> NAK
> packet:  upload-pack> 
>
> I think in the first one we would get "deepen 1" from the client in
> receive_needs(), and similarly write out our "shallow" line. But then we
> go into get_common_commits() and the client has hung up, which causes
> the message. Whereas in the second line it gives us a "done", which
> completes the negotiation.
>
> So my not-very-educated thoughts are:
>
>   1. The client should probably be sending an extra flush in the first
>  request. Alternatively, in the stateless-rpc case we should just
>  accept the lack of flush as an acceptable end to the request.

Our pkt-line.c can't deal with eof if I remember correctly (I tried to
use pkt-line for the parallel checkout stuff, where workers can also
exit early...) so sending extra flush may be easier. Old upload-pack
will not have a problem with this extra flush right? I haven't checked
upload-pack.c yet...

>   2. Presumably the shallowness is what causes the double-request, as
>  fetch-pack wants to see the shallow list before proceeding. But
>  since it has no actual commits to negotiate, the negotiation is a
>  noop. So probably this case could actually happen in a single
>  request.

I seem to recall our discussion a few months(?) ago about quickfetch()
where shallow clone would force another fetch initiated from
backfill_tags(). I guess that's why you see two fetches.

>
>  I suspect that other fetches could not, though, so I'm not sure how
>  much effort is worth putting into optimizing.
>
> -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



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

Mail still valid?

[Valid]

--
To unsubscribe from this list: send the line "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/1] git-p4: correct hasBranchPrefix verbose output

[Andrew Oakley]

The logic here was inverted, you got a message saying the file is
ignored for each file that is not ignored.
---
 git-p4.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/git-p4.py b/git-p4.py
index b6593cf..b123aa2 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -2674,7 +2674,7 @@ class P4Sync(Command, P4UserMap):
 return True
 hasPrefix = [p for p in self.branchPrefixes
 if p4PathStartsWith(path, p)]
-if hasPrefix and self.verbose:
+if not hasPrefix and self.verbose:
 print('Ignoring file outside of prefix: {0}'.format(path))
 return hasPrefix
 
-- 
2.7.3

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

Re: [PATCH 1/2] archive-tar: write extended headers for file sizes >= 8GB

[Jeff King]

On Tue, Jun 21, 2016 at 12:54:11AM +0200, René Scharfe wrote:

> > Unfortunately, it's quite an expensive test to run. For one
> > thing, unless your filesystem supports files with holes, it
> > takes 64GB of disk space (you might think piping straight to
> > `hash-object --stdin` would be better, but it's not; that
> > tries to buffer all 64GB in RAM!). Furthermore, hashing and
> > compressing the object takes several minutes of CPU time.
> > 
> > We could ship just the resulting compressed object data as a
> > loose object, but even that takes 64MB. So sadly, this code
> > path remains untested in the test suite.
> 
> If we could set the limit to a lower value than 8GB for testing then we
> could at least check if the extended header is written, e.g. if ustar_size()
> could be convinced to return 0 every time using a hidden command line
> parameter or an environment variable or something better.

Yes, we could do that, though I think it loses most of the value of the
test. We can check that if we hit an arbitrary value we generate the pax
header, but I think what we _really_ care about is: did we generate an
output that somebody else's tar implementation can handle.

And for the smaller-than-64GB case, GNU tar happily handles our existing
output (though I suspect other tars might fail at "only" 8GB).

> > +static inline unsigned long ustar_size(uintmax_t size)
> > +{
> > +   if (size < 0777UL)
> 
> Shouldn't that be less-or-equal?

Yeah, you're right (and for the one in the next patch, too).

> > +   if (ustar_size(size) != size)
> > +   strbuf_append_ext_header_uint(_header, "size", size);
> 
> It needs "S_ISREG(mode) && " as well, no?  In practice it probably doesn't
> matter (until someone stores a 8GB long symlink target), but the size field
> should only be set for regular files.

Thanks for noticing that. I remembered wondering that when I was early
in debugging/diagnosing, but forgot to follow up on it. I agree it's
unlikely in practice, but we should have consistent checks (I think it
would actually make sense to move the ISREG check inside ustar_size, and
then we can apply it consistently here and when generating the header;
my goal with ustar_size() was to avoid having the same logic in multiple
places).

-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 v2 2/2] archive-tar: write extended headers for far-future mtime

[Jeff King]

The ustar format represents timestamps as seconds since the
epoch, but only has room to store 11 octal digits.  To
express anything larger, we need to use an extended header.
This is exactly the same case we fixed for the size field in
the previous commit, and the solution here follows the same
pattern.

This is even mentioned as an issue in f2f0267 (archive-tar:
use xsnprintf for trivial formatting, 2015-09-24), but since
it only affected things far in the future, it wasn't worth
dealing with. But note that my calculations claiming
thousands of years were off there; because our xsnprintf
produces a NUL byte, we only have until the year 2242 to
fix this.

Given that this is just around the corner (geologically
speaking), and because the fix for "size" makes doing this
on top trivial, let's just make it work.

Note that we don't (and never did) handle negative
timestamps (i.e., before 1970). This would probably not be
too hard to support in the same way, but since git does not
support negative timestamps at all, I didn't bother here.

Unlike the last patch for "size", this one is easy to test
efficiently (the magic date is octal 010001):

  $ echo content >file
  $ git add file
  $ GIT_COMMITTER_DATE='@68719476737 +' \
git commit -q -m 'tempori parendum'
  $ git archive HEAD | tar tvf -
  -rw-rw-r-- root/root 8 4147-08-20 03:32 file

With the current tip of master, this dies in xsnprintf (and
prior to f2f0267, it overflowed into the checksum field, and
the resulting tarfile claimed to be from the year 2242).

However, I decided not to include this in the test suite,
because I suspect it will create portability headaches,
including:

  1. The exact format of the system tar's "t" output.

  2. System tars that cannot handle pax headers.

  3. System tars tha cannot handle dates that far in the
 future.

  4. If we replace "tar t" with "tar x", then filesystems
 that cannot handle dates that far in the future.

In other words, we do not really have a reliable tar reader
for these corner cases, so the best we could do is a
byte-for-byte comparison of the output.

Signed-off-by: Jeff King 
---
 archive-tar.c | 13 -
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/archive-tar.c b/archive-tar.c
index a9caf13..ed562d4 100644
--- a/archive-tar.c
+++ b/archive-tar.c
@@ -185,6 +185,14 @@ static inline unsigned long ustar_size(uintmax_t size)
return 0;
 }
 
+static inline unsigned long ustar_mtime(time_t mtime)
+{
+   if (mtime <= 0777UL)
+   return mtime;
+   else
+   return 0;
+}
+
 static void prepare_header(struct archiver_args *args,
   struct ustar_header *header,
   unsigned int mode, unsigned long size)
@@ -192,7 +200,8 @@ static void prepare_header(struct archiver_args *args,
xsnprintf(header->mode, sizeof(header->mode), "%07o", mode & 0);
xsnprintf(header->size, sizeof(header->size), "%011lo",
  S_ISREG(mode) ? ustar_size(size) : 0);
-   xsnprintf(header->mtime, sizeof(header->mtime), "%011lo", (unsigned 
long) args->time);
+   xsnprintf(header->mtime, sizeof(header->mtime), "%011lo",
+ ustar_mtime(args->time));
 
xsnprintf(header->uid, sizeof(header->uid), "%07o", 0);
xsnprintf(header->gid, sizeof(header->gid), "%07o", 0);
@@ -292,6 +301,8 @@ static int write_tar_entry(struct archiver_args *args,
 
if (S_ISREG(mode) && ustar_size(size) != size)
strbuf_append_ext_header_uint(_header, "size", size);
+   if (ustar_mtime(args->time) != args->time)
+   strbuf_append_ext_header_uint(_header, "mtime", args->time);
 
prepare_header(args, , mode, size);
 
-- 
2.9.0.204.g1499a7b
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 1/2] archive-tar: write extended headers for file sizes >= 8GB

[Jeff King]

The ustar format has a fixed-length field for the size of
each file entry which is supposed to contain up to 11 bytes
of octal-formatted data plus a NUL or space terminator.

These means that the largest size we can represent is
0777, or 1 byte short of 8GB. The correct solution
for a larger file, according to POSIX.1-2001, is to add an
extended pax header, similar to how we handle long
filenames. This patch does that, and writes zero for the
size field in the ustar header (the last bit is not
mentioned by POSIX, but it matches how GNU tar behaves with
--format=pax).

This should be a strict improvement over the current
behavior, which is to die in xsnprintf with a "BUG".
However, there's some interesting history here.

Prior to f2f0267 (archive-tar: use xsnprintf for trivial
formatting, 2015-09-24), we silently overflowed the "size"
field. The extra bytes ended up in the "mtime" field of the
header, which was then immediately written itself,
overwriting our extra bytes. What that means depends on how
many bytes we wrote.

If the size was 64GB or greater, then we actually overflowed
digits into the mtime field, meaning our value was was
effectively right-shifted by those lost octal digits. And
this patch is again a strict improvement over that.

But if the size was between 8GB and 64GB, then our 12-byte
field held all of the actual digits, and only our NUL
terminator overflowed. According to POSIX, there should be a
NUL or space at the end of the field. However, GNU tar seems
to be lenient here, and will correctly parse a size up 64GB
(minus one) from the field. So sizes in this range might
have just worked, depending on the implementation reading
the tarfile.

This patch is mostly still an improvement there, as the 8GB
limit is specifically mentioned in POSIX as the correct
limit. But it's possible that it could be a regression
(versus the pre-f2f0267 state) if all of the following are
true:

  1. You have a file between 8GB and 64GB.

  2. Your tar implementation _doesn't_ know about pax
 extended headers.

  3. Your tar implementation _does_ parse 12-byte sizes from
 the ustar header without a delimiter.

It's probably not worth worrying about such an obscure set
of conditions, but I'm documenting it here just in case.

There's no test included here. I did confirm that this works
(using GNU tar) with:

  $ dd if=/dev/zero seek=64G bs=1 count=1 of=huge
  $ git add huge
  $ git commit -q -m foo
  $ git archive HEAD | head -c 1 | tar tvf - 2>/dev/null
  -rw-rw-r-- root/root 68719476737 2016-06-15 21:07 huge

Pre-f2f0267, this would yield a bogus size of 8GB, and
post-f2f0267, git-archive simply dies.

Unfortunately, it's quite an expensive test to run. For one
thing, unless your filesystem supports files with holes, it
takes 64GB of disk space (you might think piping straight to
`hash-object --stdin` would be better, but it's not; that
tries to buffer all 64GB in RAM!). Furthermore, hashing and
compressing the object takes several minutes of CPU time.

We could ship just the resulting compressed object data as a
loose object, but even that takes 64MB. So sadly, this code
path remains untested in the test suite.

Signed-off-by: Jeff King 
---
 archive-tar.c | 28 +++-
 1 file changed, 27 insertions(+), 1 deletion(-)

diff --git a/archive-tar.c b/archive-tar.c
index cb99df2..a9caf13 100644
--- a/archive-tar.c
+++ b/archive-tar.c
@@ -137,6 +137,20 @@ static void strbuf_append_ext_header(struct strbuf *sb, 
const char *keyword,
strbuf_addch(sb, '\n');
 }
 
+/*
+ * Like strbuf_append_ext_header, but for numeric values.
+ */
+static void strbuf_append_ext_header_uint(struct strbuf *sb,
+ const char *keyword,
+ uintmax_t value)
+{
+   char buf[40]; /* big enough for 2^128 in decimal, plus NUL */
+   int len;
+
+   len = xsnprintf(buf, sizeof(buf), "%"PRIuMAX, value);
+   strbuf_append_ext_header(sb, keyword, buf, len);
+}
+
 static unsigned int ustar_header_chksum(const struct ustar_header *header)
 {
const unsigned char *p = (const unsigned char *)header;
@@ -163,12 +177,21 @@ static size_t get_path_prefix(const char *path, size_t 
pathlen, size_t maxlen)
return i;
 }
 
+static inline unsigned long ustar_size(uintmax_t size)
+{
+   if (size <= 0777UL)
+   return size;
+   else
+   return 0;
+}
+
 static void prepare_header(struct archiver_args *args,
   struct ustar_header *header,
   unsigned int mode, unsigned long size)
 {
xsnprintf(header->mode, sizeof(header->mode), "%07o", mode & 0);
-   xsnprintf(header->size, sizeof(header->size), "%011lo", S_ISREG(mode) ? 
size : 0);
+   xsnprintf(header->size, sizeof(header->size), "%011lo",
+ S_ISREG(mode) ? ustar_size(size) : 0);
xsnprintf(header->mtime, 

Re: [PATCH 0/2] friendlier handling of overflows in archive-tar

[Jeff King]

On Thu, Jun 16, 2016 at 12:35:23AM -0400, Jeff King wrote:

> The ustar format has some fixed-length numeric fields, and it's possible
> to generate a git tree that can't be represented (namely file size and
> mtime). Since f2f0267 (archive-tar: use xsnprintf for trivial
> formatting, 2015-09-24), we detect and die() in these cases. But we can
> actually do the friendly (and POSIX-approved) thing, and add extended
> pax headers to represent the correct values.
> 
>   [1/2]: archive-tar: write extended headers for file sizes >= 8GB
>   [2/2]: archive-tar: write extended headers for far-future mtime

And here's a v2 that addresses the smaller comments from René. I punted
on doing something fancy with tests. I'm not opposed to it, but I'm also
not convinced it's all that useful. Either way, I think it can come on
top if we want it.

Junio, this is the jk/big-and-old-archive-tar topic.

The interdiff is:

diff --git a/archive-tar.c b/archive-tar.c
index c7b85fd..ed562d4 100644
--- a/archive-tar.c
+++ b/archive-tar.c
@@ -179,7 +179,7 @@ static size_t get_path_prefix(const char *path, size_t 
pathlen, size_t maxlen)
 
 static inline unsigned long ustar_size(uintmax_t size)
 {
-   if (size < 0777UL)
+   if (size <= 0777UL)
return size;
else
return 0;
@@ -187,7 +187,7 @@ static inline unsigned long ustar_size(uintmax_t size)
 
 static inline unsigned long ustar_mtime(time_t mtime)
 {
-   if (mtime < 0777UL)
+   if (mtime <= 0777UL)
return mtime;
else
return 0;
@@ -299,7 +299,7 @@ static int write_tar_entry(struct archiver_args *args,
memcpy(header.linkname, buffer, size);
}
 
-   if (ustar_size(size) != size)
+   if (S_ISREG(mode) && ustar_size(size) != size)
strbuf_append_ext_header_uint(_header, "size", size);
if (ustar_mtime(args->time) != args->time)
strbuf_append_ext_header_uint(_header, "mtime", args->time);

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

Re: [PATCH 1/2] archive-tar: write extended headers for file sizes >= 8GB

[Jeff King]

On Tue, Jun 21, 2016 at 11:59:20AM -0400, Jeff King wrote:

> > > + if (ustar_size(size) != size)
> > > + strbuf_append_ext_header_uint(_header, "size", size);
> > 
> > It needs "S_ISREG(mode) && " as well, no?  In practice it probably doesn't
> > matter (until someone stores a 8GB long symlink target), but the size field
> > should only be set for regular files.
> 
> Thanks for noticing that. I remembered wondering that when I was early
> in debugging/diagnosing, but forgot to follow up on it. I agree it's
> unlikely in practice, but we should have consistent checks (I think it
> would actually make sense to move the ISREG check inside ustar_size, and
> then we can apply it consistently here and when generating the header;
> my goal with ustar_size() was to avoid having the same logic in multiple
> places).

Actually, scratch that. It makes things awkward because it would be hard
to tell when ustar_size() returned zero because it's a file with a big
size, and thus needs a pax header, versus that it was not a file, and
therefore must _not_ have a pax header.

-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] t7800 readlink not found

[Armin Kunaschik]

On Tue, May 31, 2016 at 7:51 AM, Junio C Hamano  wrote:
> Torsten Bögershausen  writes:
>
>>> diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh
>>> index 7ce4cd7..905035c 100755
>>> --- a/t/t7800-difftool.sh
>>> +++ b/t/t7800-difftool.sh
>>> @@ -446,7 +446,7 @@ write_script .git/CHECK_SYMLINKS <<\EOF
>>>   for f in file file2 sub/sub
>>>   do
>>>  echo "$f"
>>> -readlink "$2/$f"
>>> +ls -ld "$2/$f" | sed -e 's/.* -> //'
>>>   done >actual
>>>   EOF
>>>
>> I don't know how portable #ls -ld" really is.
>
> The parts with mode bits, nlinks, uid, gid, size, and date part do
> have some variations.  For example, we have been burned on ACL
> enabled systems having some funny suffix after the usual mode bits
> stuff.
>
> However, as far as this test is concerned, I do not think "how
> portable is the output from ls -ld" is an especially relevant
> question.  None of the things we expect early in the output (the
> fields I enumerated in the previous paragraph) would contain " -> ".
> And we know that we do not use a filename that has " -> " (or "->")
> as a substring in our tests.
>
> We don't have to use readlink, even on platforms where we do have
> readlink.  Building the conditional to be checked at runtime and
> providing a shell function read_link that uses "ls -ld | sed" or
> "readlink" depending on the runtime check is wasteful.
>

Just a short, curious question: Is this patch to be accepted/included some time?
I didn't see it in 2.8.4 nor 2.9.0. Maybe it just fell off the table...

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

Re: [PATCH v3 0/9] Let log-tree and friends respect diffopt's `file` field

[Johannes Schindelin]

Hi Paul,

On Tue, 21 Jun 2016, Paul Tan wrote:

> On Tue, Jun 21, 2016 at 6:34 PM, Johannes Schindelin
>  wrote:
> > - this uncovered a problem with builtin am, where it asked the diff
> >   machinery to close the file stream, but actually called the log_tree
> >   machinery (which might mean that this patch series inadvertently fixes
> >   a bug where `git am --rebasing` would write the commit message to
> >   stdout instead of the `patch` file when erroring out)
> 
> Please correct me if I'm wrong: looking at log-tree.c, the commit
> message will not be printed when no_commit_id = 1, isn't it?
> This is because we do not hit the code paths that write to stdout since
> show_log() is not called.

Why does builtin/am.c use log_tree_commit(), then? Why not simply run
things through the diff machinery?

> Also, the return value of log_tree_commit() is actually a boolean
> value, not an error status value, isn't it?

It is not really a boolean, no. Sure, at the moment, it happens to return
either 0 or 1. You can figure that out by following the call paths all the
way to do_diff_combined() or line_log_print().

The key words are: at the moment.

We do find more and more places where library functions call die() in case
of errors, and it hurts us. Badly. That is why I, among others, try to
remedy the situation by converting these calls to "return error()"
statements.

The log_tree functions are prepared for that: they return non-negative
values in case of success.

The callers are not really prepared for that, hence my complaints.

> > This last point is a bigger issue, actually. There seem to be quite a
> > few function calls in builtin/am.c whose return values that might
> > indicate errors are flatly ignored. I see two calls to run_diff_index()
> > whose return value then goes poof unchecked,
> 
> Thanks, future-proofing the builtin/am.c code is good, in case
> run_diff_index() is updated to not call exit(128) on error in the
> future.

And run_diff_cache(). And read_ref_at(). And rerere(). And
setup_revisions(). And get_sha1().

> > and several calls to write_state_text() and write_state_bool() with
> > the same issue.
> 
> These functions will die() on error

Indeed. And I do not think that is a good practice.

> > And I did not even try to review the code to that end, all I wanted
> > was to verify that builtin am only has the close_file issue once (it
> > does use it a second time, but that one is okay because it then calls
> > run_diff_index(), i.e. the diff machinery).
> >
> > I am embarrassed to admit that these builtin am problems demonstrate
> > that I, as a mentor of the builtin am project, failed to help make the
> > patches as good as I expected myself to do.
> 
> Sorry to disappoint you :-(

You misunderstood. I am not disappointed in you. *I* did a lousy job. Not
only mentoring, but I also obviously failed to make things fun enough for
you.

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

Re: [PATCH v3 0/9] Let log-tree and friends respect diffopt's `file` field

[Paul Tan]

Hi Johannes,

On Tue, Jun 21, 2016 at 6:34 PM, Johannes Schindelin
 wrote:
> - this uncovered a problem with builtin am, where it asked the diff
>   machinery to close the file stream, but actually called the log_tree
>   machinery (which might mean that this patch series inadvertently fixes
>   a bug where `git am --rebasing` would write the commit message to
>   stdout instead of the `patch` file when erroring out)

Please correct me if I'm wrong: looking at log-tree.c, the commit
message will not be printed when no_commit_id = 1, isn't it? This is
because we do not hit the code paths that write to stdout since
show_log() is not called.

Also, the return value of log_tree_commit() is actually a boolean
value, not an error status value, isn't it?

> This last point is a bigger issue, actually. There seem to be quite a
> few function calls in builtin/am.c whose return values that might
> indicate errors are flatly ignored. I see two calls to run_diff_index()
> whose return value then goes poof unchecked,

Thanks, future-proofing the builtin/am.c code is good, in case
run_diff_index() is updated to not call exit(128) on error in the
future.

> and several calls to
> write_state_text() and write_state_bool() with the same issue.

These functions will die() on error, so checking their error code is
not really useful. I'm not opposed to changing them to use the
write_file_gently() version though, although I don't see a need to
unless builtin/am.c is being libified.

> And I did
> not even try to review the code to that end, all I wanted was to verify
> that builtin am only has the close_file issue once (it does use it a
> second time, but that one is okay because it then calls
> run_diff_index(), i.e. the diff machinery).
>
> I am embarrassed to admit that these builtin am problems demonstrate
> that I, as a mentor of the builtin am project, failed to help make the
> patches as good as I expected myself to do.

Sorry to disappoint you :-(

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

[PATCH v2] perf: accommodate for MacOSX

[Johannes Schindelin]

As this developer has no access to MacOSX developer setups anymore,
Travis becomes the best bet to run performance tests on that OS.

However, on MacOSX /usr/bin/time is that good old BSD executable that
no Linux user cares about, as demonstrated by the perf-lib.sh's use
of GNU-ish extensions. And by the hard-coded path.

Let's just work around this issue by using gtime on MacOSX, the
Homebrew-provided GNU implementation onto which pretty much every
MacOSX power user falls back anyway.

To help other developers use Travis to run performance tests on
MacOSX, the .travis.yml file now sports a commented-out line that
installs GNU time via Homebrew.

Signed-off-by: Johannes Schindelin 
---
Published-As: https://github.com/dscho/git/releases/tag/perf-macosx-v2
 .travis.yml| 2 ++
 t/perf/perf-lib.sh | 6 +-
 2 files changed, 7 insertions(+), 1 deletion(-)
Interdiff vs v1:

 diff --git a/.travis.yml b/.travis.yml
 index 0e569bc..c2b76f9 100644
 --- a/.travis.yml
 +++ b/.travis.yml
 @@ -82,7 +82,9 @@ before_install:
brew tap homebrew/binary --quiet
brew_force_set_latest_binary_hash perforce
brew_force_set_latest_binary_hash perforce-server
 -  brew install git-lfs perforce-server perforce gettext gnu-time
 +  # Uncomment this if you want to run perf tests:
 +  # brew install gnu-time
 +  brew install git-lfs perforce-server perforce gettext
brew link --force gettext
;;
  esac;


diff --git a/.travis.yml b/.travis.yml
index c20ec54..c2b76f9 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -82,6 +82,8 @@ before_install:
   brew tap homebrew/binary --quiet
   brew_force_set_latest_binary_hash perforce
   brew_force_set_latest_binary_hash perforce-server
+  # Uncomment this if you want to run perf tests:
+  # brew install gnu-time
   brew install git-lfs perforce-server perforce gettext
   brew link --force gettext
   ;;
diff --git a/t/perf/perf-lib.sh b/t/perf/perf-lib.sh
index 18c363e..773f955 100644
--- a/t/perf/perf-lib.sh
+++ b/t/perf/perf-lib.sh
@@ -127,11 +127,15 @@ test_checkout_worktree () {
 # Performance tests should never fail.  If they do, stop immediately
 immediate=t
 
+# Perf tests require GNU time
+case "$(uname -s)" in Darwin) GTIME="${GTIME:-gtime}";; esac
+GTIME="${GTIME:-/usr/bin/time}"
+
 test_run_perf_ () {
test_cleanup=:
test_export_="test_cleanup"
export test_cleanup test_export_
-   /usr/bin/time -f "%E %U %S" -o test_time.$i "$SHELL" -c '
+   "$GTIME" -f "%E %U %S" -o test_time.$i "$SHELL" -c '
 . '"$TEST_DIRECTORY"/test-lib-functions.sh'
 test_export () {
[ $# != 0 ] || return 0
-- 
2.9.0.118.g0e1a633

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

BONJOUR (( AVEZ-VOUS REÇU MON PREMIER MESSAGE ))

[Robert Gepor]

SALUT
Je veux vous demander votre avis sur la façon d'investir dans votre
pays ?? nos principaux bailleurs  sommes prêts à financer des projets
rentables et offrir des prêts pour des projets privés, nos Partenaire
( chef financer )  est en Europe, en Asie, Emirats Arabes Unis et
l'Afrique Même si votre propres projets est confrontée à la faillite
et a la  recherche de financement ou si votre société est confrontée à
la faillite, notre bailleur de fonds est prêt à vous satisfaire.Une
fois que nous recevons la confirmation de l'intérêt, nous vous
mettrons en contact direct avec le propriétaire du fond  nos
principaux bailleurs  ( chef financer ) .
Il est très impératif que nous répondons à notre adresse  privé au
( robertgepo...@gmail.com), qui nous permet de recevoir et répondre
vous dès que possible.

Cordialement,

Mr ROBERT GEPOR.
Email:robertgepo...@gmail.com
Paris, France
Tel.   Privé:  0033787844139
--
To unsubscribe from this list: send the line "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] perf: accommodate for MacOSX

[Johannes Schindelin]

Hi Lars,

On Tue, 21 Jun 2016, Lars Schneider wrote:

> > On 21 Jun 2016, at 13:55, Johannes Schindelin  
> > wrote:
> > 
> >> ...
> >> If we don't run any perf tests by default on Travis CI then I wouldn't
> >> take the ".travis.yml" part of the patch just to keep our Travis CI
> >> setup as lean as possible.
> > 
> > Maybe commented-out, so that people like me have a chance to use Travis
> > for MacOSX perf testing?
> > 
> > Could you let me know whether a commented-out
> > 
> > # Uncomment this if you want to run perf tests:
> > # brew install gnu-time
> > 
> > would be acceptable? I will reroll the patch accordingly.
> 
> Commented-out would be fine with me!

Okay! Updated patch coming in a moment.

> >> Running perf tests on Travis CI is probably bogus anyways because we
> >> never know on what hardware our jobs run and what other jobs run in
> >> parallel on that hardware.
> > 
> > While I agree that the absolute timings cannot be trusted, I have to point
> > out that the relative timings on Linux at least are consistent with what I
> > could test locally.
>
> Given that the relative timings are consistent for you. Maybe there is
> value to run the performance tests (e.g. only on the master branch)
> in a separate Travis job. Then we could chart the timings over releases.
> I dunno.

Sure, that would be a fine addition, if there is a way, somehow, to chart
the timings (keep in mind that Travis runs for each and every iteration of
each and every PR, in addition to the branch updates).

I do have enough on my plate, though, so I will have to hope that somebody
else picks this up.

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

Re: [PATCH] perf: accommodate for MacOSX

[Lars Schneider]

> On 21 Jun 2016, at 13:55, Johannes Schindelin  
> wrote:
> 
>> ...
>> I think we definitively should take the "perf-lib.sh" part of the patch
>> as this makes the perf test run on OSX and therefore is a strict
>> improvement.
> 
> Yes, it was meant as the starting point to get more things to run on
> MacOSX.
> 
>> If we don't run any perf tests by default on Travis CI then I wouldn't
>> take the ".travis.yml" part of the patch just to keep our Travis CI
>> setup as lean as possible.
> 
> Maybe commented-out, so that people like me have a chance to use Travis
> for MacOSX perf testing?
> 
>> Running perf tests on Travis CI is probably bogus anyways because we
>> never know on what hardware our jobs run and what other jobs run in
>> parallel on that hardware.
> 
> While I agree that the absolute timings cannot be trusted, I have to point
> out that the relative timings on Linux at least are consistent with what I
> could test locally.
> 
> Could you let me know whether a commented-out
> 
>   # Uncomment this if you want to run perf tests:
>   # brew install gnu-time
> 
> would be acceptable? I will reroll the patch accordingly.

Commented-out would be fine with me!

Independent of your patch:
Given that the relative timings are consistent for you. Maybe there is
value to run the performance tests (e.g. only on the master branch)
in a separate Travis job. Then we could chart the timings over releases.
I dunno.

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

[PATCH v2] Make find_commit_subject() more robust

[Johannes Schindelin]

Just like the pretty printing machinery, we should simply ignore empty
lines at the beginning of the commit messages.

This discrepancy was noticed when an early version of the rebase--helper
produced commit objects with more than one empty line between the header
and the commit message.

Signed-off-by: Johannes Schindelin 
---
Published-As: https://github.com/dscho/git/releases/tag/leading-empty-lines-v2

git blame seemed to be the most accessible user of
find_commit_subject()...

 commit.c |  2 ++
 t/t8008-blame-formats.sh | 17 +
 2 files changed, 19 insertions(+)
Interdiff vs v1:

 diff --git a/t/t8008-blame-formats.sh b/t/t8008-blame-formats.sh
 index 29f84a6..03bd313 100755
 --- a/t/t8008-blame-formats.sh
 +++ b/t/t8008-blame-formats.sh
 @@ -87,4 +87,21 @@ test_expect_success 'blame --line-porcelain output' '
test_cmp expect actual
  '
  
 +test_expect_success '--porcelain detects first non-empty line as subject' '
 +  (
 +  GIT_INDEX_FILE=.git/tmp-index &&
 +  export GIT_INDEX_FILE &&
 +  echo "This is it" >single-file &&
 +  git add single-file &&
 +  tree=$(git write-tree) &&
 +  commit=$(printf "%s\n%s\n%s\n\n\noneline\n\nbody\n" \
 +  "tree $tree" \
 +  "author A  123456789 +" \
 +  "committer C  123456789 +" |
 +  git hash-object -w -t commit --stdin) &&
 +  git blame --porcelain $commit -- single-file >output &&
 +  grep "^summary oneline$" output
 +  )
 +'
 +
  test_done


diff --git a/commit.c b/commit.c
index 3f4f371..7b00989 100644
--- a/commit.c
+++ b/commit.c
@@ -415,6 +415,8 @@ int find_commit_subject(const char *commit_buffer, const 
char **subject)
p++;
if (*p) {
p += 2;
+   while (*p == '\n')
+   p++;
for (eol = p; *eol && *eol != '\n'; eol++)
; /* do nothing */
} else
diff --git a/t/t8008-blame-formats.sh b/t/t8008-blame-formats.sh
index 29f84a6..03bd313 100755
--- a/t/t8008-blame-formats.sh
+++ b/t/t8008-blame-formats.sh
@@ -87,4 +87,21 @@ test_expect_success 'blame --line-porcelain output' '
test_cmp expect actual
 '
 
+test_expect_success '--porcelain detects first non-empty line as subject' '
+   (
+   GIT_INDEX_FILE=.git/tmp-index &&
+   export GIT_INDEX_FILE &&
+   echo "This is it" >single-file &&
+   git add single-file &&
+   tree=$(git write-tree) &&
+   commit=$(printf "%s\n%s\n%s\n\n\noneline\n\nbody\n" \
+   "tree $tree" \
+   "author A  123456789 +" \
+   "committer C  123456789 +" |
+   git hash-object -w -t commit --stdin) &&
+   git blame --porcelain $commit -- single-file >output &&
+   grep "^summary oneline$" output
+   )
+'
+
 test_done
-- 
2.9.0.118.g0e1a633

base-commit: ab7797dbe95fff38d9265869ea367020046db118
--
To unsubscribe from this list: send the line "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 0/3] fix local_tzoffset with far-in-future dates

[Jeff King]

On Mon, Jun 20, 2016 at 11:37:50PM -0700, Norbert Kiesel wrote:

> There are more strange things happening with dates.  One example is
> that `git commit --date=@4102444799` produces a commit with the
> correct author date "Thu Dec 31 15:59:59 2099 -0800" (for my local
> timezone which is Americas/Los_Angeles), while `git commit
> --date=@4102444800` produces a commit with "now" as author date, as
> does any other larger number. `date --date=@4102444800` results in
> "Thu Dec 31 16:00:00 PST 2099". So seems 2100-01-01T00:00:00Z is a
> hard limit for git when using this format.

Yes, I noticed that, too. I suspect it comes from the same source; the
date parser calls tm_to_time_t at some point which will refuse to handle
the date, and we fallback to something else. So certainly there is room
for improvement:

  1. We could handle a wider range of dates in tm_to_time_t(). This is
 essentially mktime(), but notice that mktime() was avoided for good
 reasons long ago, so any proposal to just move to that would need
 to figure out all those reasons and whether they are still valid.

  2. We should perhaps be flagging an error here instead of falling back
 to the current time. I suspect this is happening because --date
 falls back to approxidate() when we fail to parse the date (so you
 can say things like "--date=last.friday". Especially for cases with
 "@", which indicate that no approximate parsing is really required.

 Note that using GIT_AUTHOR_DATE _doesn't_ go through the date
 parser, but expects a raw time_t. So that does work for these
 far-future dates.

I'm not planning on working on either of these in the near term, but I'd
be happy to review patches if somebody else wants to.

-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: http-backend fatal error message on shallow clone

[Jeff King]

On Tue, Jun 21, 2016 at 11:23:03AM +, Eric Wong wrote:

> I noticed "fatal: The remote end hung up unexpectedly" in server
> logs from shallow clones.  Totally reproducible in the test
> cases, too.  The following change shows it:
> [...]
> [Tue Jun 21 11:07:41.391269 2016] [cgi:error] [pid 21589] [client 
> 127.0.0.1:37518] AH01215: fatal: The remote end hung up unexpectedly
> 
> It doesn't show above, but I think http-backend exits
> with a non-zero status, too, which might cause some CGI
> implementations to complain or break.
> 
> Not sure if it's just a corner case that wasn't tested
> or something else, but the clone itself seems successful...

The dying process is actually upload-pack. If we instrument it like
this:

diff --git a/upload-pack.c b/upload-pack.c
index 9e03c27..a1da676 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -820,6 +820,14 @@ static int upload_pack_config(const char *var, const char 
*value, void *unused)
return parse_hide_refs_config(var, value, "uploadpack");
 }
 
+NORETURN
+static void custom_die(const char *err, va_list params)
+{
+   vreportf("fatal: ", err, params);
+   warning("aborting");
+   abort();
+}
+
 int main(int argc, const char **argv)
 {
const char *dir;
@@ -836,6 +844,9 @@ int main(int argc, const char **argv)
OPT_END()
};
 
+   warning("running upload-pack");
+   set_die_routine(custom_die);
+
git_setup_gettext();
 
packet_trace_identity("upload-pack");

we can see two things. One is we can get a backtrace from the core file:

#0  0x7f04aef51458 in __GI_raise (sig=sig@entry=6) at 
../sysdeps/unix/sysv/linux/raise.c:55
#1  0x7f04aef528da in __GI_abort () at abort.c:89
#2  0x00406009 in custom_die (err=0x4ede60 "The remote end hung up 
unexpectedly", 
params=0x7ffe15858758) at upload-pack.c:828
#3  0x0045ec63 in die (err=0x4ede60 "The remote end hung up 
unexpectedly") at usage.c:108
#4  0x0041e016 in get_packet_data (fd=0, src_buf=0x0, src_size=0x0, 
dst=0x7ffe158588b0, 
size=4, options=2) at pkt-line.c:167
#5  0x0041e0ea in packet_read (fd=0, src_buf=0x0, src_len=0x0, 
buffer=0x73cc40  "deepen 1", size=65520, options=2) at 
pkt-line.c:204
#6  0x0041e22b in packet_read_line_generic (fd=0, src=0x0, src_len=0x0, 
dst_len=0x0)
at pkt-line.c:234
#7  0x0041e27c in packet_read_line (fd=0, len_p=0x0) at pkt-line.c:244
#8  0x00404dc9 in get_common_commits () at upload-pack.c:384
#9  0x00405eb4 in upload_pack () at upload-pack.c:798
#10 0x00406229 in main (argc=1, argv=0x7ffe15858c28) at 
upload-pack.c:872

So we expected to read a packet but didn't get one. Not surprising. The
interesting thing is that we are in get_common_commits(), and if we were
to get a flush packet in stateless-rpc mode, we would simply exit(0).
But we get EOF instead, which provokes packet_read_line() to die.

The other interesting thing is that we can see in httpd's error.log that
it is the penultimate upload-pack that dies (I trimmed the log lines for
readability):

AH01215: warning: running upload-pack
AH01215: fatal: The remote end hung up unexpectedly
AH01215: warning: aborting
AH01215: error: git-upload-pack died of signal 6
AH01215: warning: running upload-pack

So this request actually takes _two_ upload-pack instances to serve
(which is not uncommon when we need multiple rounds of
get_common_commits(), though I am a little surprised that would be the
case for a clone). And the first one seems to be missing a closing
"" flush packet from the client to end it.

If that analysis is correct, this isn't affecting operation in any way;
it's just giving a useless message from upload-pack (and as you note,
that could trigger http-backend to exit(1), which may make the webserver
unhappy).

If we further instrument upload-pack to set GIT_TRACE_PACKET on the
server side, we can see the two requests:

packet:  upload-pack< want 379518c0c94e3b1a0710129d03d5560814a0ba6f 
multi_ack_detailed no-done side-band-64k thin-pack include-tag ofs-delta 
agent=git/2.9.0.37.gb3ad8ab.dirty
packet:  upload-pack< deepen 1
packet:  upload-pack< 
packet:  upload-pack> shallow 379518c0c94e3b1a0710129d03d5560814a0ba6f
packet:  upload-pack> 

packet:  upload-pack< want 379518c0c94e3b1a0710129d03d5560814a0ba6f 
multi_ack_detailed no-done side-band-64k thin-pack include-tag ofs-delta 
agent=git/2.9.0.37.gb3ad8ab.dirty
packet:  upload-pack< deepen 1
packet:  upload-pack< 
packet:  upload-pack> shallow 379518c0c94e3b1a0710129d03d5560814a0ba6f
packet:  upload-pack> 
packet:  upload-pack< done
packet:  upload-pack> NAK
packet:  upload-pack> 

I think in the first one we would get "deepen 1" from the client in
receive_needs(), and similarly write out our "shallow" line. But then we
go into get_common_commits() and the client has hung up, which causes
the message. Whereas in the second line it gives us a "done", which
completes 

Re: [PATCH] mingw: let the build succeed with DEVELOPER=1

[Johannes Schindelin]

Hi Junio,

On Mon, 20 Jun 2016, Junio C Hamano wrote:

> Johannes Schindelin  writes:
> 
> > The recently introduced developer flags identified a couple of
> > old-style function declarations in the Windows-specific code where
> > the parameter list was left empty instead of specifying "void"
> > explicitly. Let's just fix them.
> 
> Thanks.  It's about time for them to be cleaned up.
> 
> Will queue.

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

Re: [PATCH] Make find_commit_subject() more robust

[Johannes Schindelin]

Hi Junio,

On Mon, 20 Jun 2016, Junio C Hamano wrote:

> Johannes Schindelin  writes:
> 
> > Just like the pretty printing machinery, we should simply ignore empty
> > lines at the beginning of the commit messages.
> >
> > This discrepancy was noticed when an early version of the rebase--helper
> > produced commit objects with more than one empty line between the header
> > and the commit message.
> >
> > Signed-off-by: Johannes Schindelin 
> > ---
> > Published-As: 
> > https://github.com/dscho/git/releases/tag/leading-empty-lines-v1
> >
> > And another patch from the rebase--helper front. I guess I'll
> > call it a day with this one.
> 
> Makes sense.  This has a trivial textual conflict with cleanup
> patches in flight, I think, but that is not a big problem.

I will gladly resend rebased to `next`, if you wish.

> It does hint that we might want to find a library function that can
> replace a hand-rolled while loop we are adding here, though ;-)

Heh. I cannot help you with that ;-)

> Perhaps cast this new behaviour in stone by adding a test?

Will do.

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

Re: [PATCH 1/1] mingw: work around t2300's assuming non-Windows paths

[Johannes Schindelin]

Hi Junio,

On Mon, 20 Jun 2016, Junio C Hamano wrote:

> Johannes Schindelin  writes:
> 
> > On Windows, we have to juggle two different schemes of representing
> > paths: the native, Windows paths (the only ones known to the main Git
> > executable) on the one hand, and POSIX-ish ones used by the Bash
> > through MSYS2's POSIX emulation layer on the other hand.
> >
> > A Windows path looks like this: C:\git-sdk-64\usr\src\git. In modern
> > Windows, it is almost always legal to use forward slashes as directory
> > separators, which is the reason why the Git executable itself would
> > use the path C:/git-sdk-64/usr/src/git instead. The equivalent
> > POSIX-ish path would be: /c/git-sdk-64/usr/src/git.
> >
> > This patch works around the assumption of t2300-cd-to-toplevel.sh that
> > `git --exec-path` spits out a POSIX-ish path, by converting the output
> > accordingly.
> 
> Hmm, I am confused.  `git --exec-path` _is_ meant to "spit out" a
> path that is usable when prepended/appended to $PATH [1], and it
> does _not_ have to be POSIX-ish path.  It is totally up to the port
> to adjust it to the platform's convention how the $PATH environment
> variable is understood.

The port does exactly what it is supposed to do: the output of `git
--exec-path` can be prepended/appended to %PATH%. The point here is:
%PATH% is *semicolon*-delimited.

All problems start when we do not use scripting native to Windows, but the
Bash. In the Bash, we *assume* that $PATH is *colon*-delimited. All the
time. And that is part of what makes a POSIX emulation layer on Windows so
challenging.

> If $PATH cannot take C:/git-sdk-64/usr/src/git but does understand
> /c/git-sdk-64/usr/src/git, perhaps "git --exec-path" should be
> emitting the latter in the first place?

Again, %PATH% can take C:/... just fine. But the Bash gets the
POSIX-layer-munged $PATH which has POSIX-ish paths so that it can
colon-delimit its PATH and all shell scripts can continue to work.

So the root cause for the problem which requires this work-around is that
our test suite is written in shell script, which is inherently not as
portable as we think it is.

Does that address your puzzlement? If so, would you kindly advise how to
improve the commit message (or the patch)?

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

Re: [PATCH] perf: accommodate for MacOSX

[Johannes Schindelin]

Hi Lars,

On Tue, 21 Jun 2016, Lars Schneider wrote:

> > On 20 Jun 2016, at 21:48, Junio C Hamano  wrote:
> > 
> > Johannes Schindelin  writes:
> > 
> >> On Sun, 19 Jun 2016, Lars Schneider wrote:
> >> 
>  On 18 Jun 2016, at 15:03, Johannes Schindelin
>   wrote:
>  
>  As this developer has no access to MacOSX developer setups anymore,
>  Travis becomes the best bet to run performance tests on that OS.
> >>> 
> >>> We don't run the performance tests on Travis CI right now.
> >>> Maybe we should? With your patch below it should work, right?
> >> ...
> >> 
> >> Yeah, well, I should have been clearer in my commit message: this patch
> >> allows the perf tests to *run*, not to *pass*... ;-)
> > 
> > OK, Lars, do we still want to take this patch?  I am leaning towards
> > taking it, if only to motivate interested others with OS X to look
> > into making the perf tests to actually run.
> 
> I think we definitively should take the "perf-lib.sh" part of the patch
> as this makes the perf test run on OSX and therefore is a strict
> improvement.

Yes, it was meant as the starting point to get more things to run on
MacOSX.

> If we don't run any perf tests by default on Travis CI then I wouldn't
> take the ".travis.yml" part of the patch just to keep our Travis CI
> setup as lean as possible.

Maybe commented-out, so that people like me have a chance to use Travis
for MacOSX perf testing?

> Running perf tests on Travis CI is probably bogus anyways because we
> never know on what hardware our jobs run and what other jobs run in
> parallel on that hardware.

While I agree that the absolute timings cannot be trusted, I have to point
out that the relative timings on Linux at least are consistent with what I
could test locally.

Could you let me know whether a commented-out

# Uncomment this if you want to run perf tests:
# brew install gnu-time

would be acceptable? I will reroll the patch accordingly.

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

[PATCH 08/11] i18n: send-email: mark strings for translation

[Vasco Almeida]

Mark strings often displayed to user for translation.

Signed-off-by: Vasco Almeida 
---
 git-send-email.perl | 53 +++--
 1 file changed, 31 insertions(+), 22 deletions(-)

diff --git a/git-send-email.perl b/git-send-email.perl
index 6958785..2521832 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -28,6 +28,7 @@ use File::Temp qw/ tempdir tempfile /;
 use File::Spec::Functions qw(catfile);
 use Error qw(:try);
 use Git;
+use Git::I18N;
 
 Getopt::Long::Configure qw/ pass_through /;
 
@@ -795,12 +796,12 @@ foreach my $f (@files) {
 }
 
 if (!defined $auto_8bit_encoding && scalar %broken_encoding) {
-   print "The following files are 8bit, but do not declare " .
-   "a Content-Transfer-Encoding.\n";
+   print __("The following files are 8bit, but do not declare " .
+"a Content-Transfer-Encoding.\n");
foreach my $f (sort keys %broken_encoding) {
print "$f\n";
}
-   $auto_8bit_encoding = ask("Which 8bit encoding should I declare 
[UTF-8]? ",
+   $auto_8bit_encoding = ask(__("Which 8bit encoding should I declare 
[UTF-8]? "),
  valid_re => qr/.{4}/, confirm_only => 1,
  default => "UTF-8");
 }
@@ -827,7 +828,7 @@ if (defined $sender) {
 # But it's a no-op to run sanitize_address on an already sanitized address.
 $sender = sanitize_address($sender);
 
-my $to_whom = "To whom should the emails be sent (if anyone)?";
+my $to_whom = __("To whom should the emails be sent (if anyone)?");
 my $prompting = 0;
 if (!@initial_to && !defined $to_cmd) {
my $to = ask("$to_whom ",
@@ -857,7 +858,7 @@ sub expand_one_alias {
 
 if ($thread && !defined $initial_reply_to && $prompting) {
$initial_reply_to = ask(
-   "Message-ID to be used as In-Reply-To for the first email (if 
any)? ",
+   __("Message-ID to be used as In-Reply-To for the first email 
(if any)? "),
default => "",
valid_re => qr/\@.*\./, confirm_only => 1);
 }
@@ -916,7 +917,10 @@ sub validate_address {
my $address = shift;
while (!extract_valid_address($address)) {
print STDERR "error: unable to extract a valid address from: 
$address\n";
-   $_ = ask("What to do with this address? ([q]uit|[d]rop|[e]dit): 
",
+   # TRANSLATORS: Make sure to include [q] [d] [e] in your
+   # translation. The program will only accept English input
+   # at this point.
+   $_ = ask(__("What to do with this address? 
([q]uit|[d]rop|[e]dit): "),
valid_re => qr/^(?:quit|q|drop|d|edit|e)/i,
default => 'q');
if (/^d/i) {
@@ -1291,17 +1295,22 @@ Message-Id: $message_id
if ($needs_confirm eq "inform") {
$confirm_unconfigured = 0; # squelch this message for 
the rest of this run
$ask_default = "y"; # assume yes on EOF since user 
hasn't explicitly asked for confirmation
-   print "The Cc list above has been expanded by 
additional\n";
-   print "addresses found in the patch commit message. 
By default\n";
-   print "send-email prompts before sending whenever 
this occurs.\n";
-   print "This behavior is controlled by the 
sendemail.confirm\n";
-   print "configuration setting.\n";
-   print "\n";
-   print "For additional information, run 'git 
send-email --help'.\n";
-   print "To retain the current behavior, but squelch 
this message,\n";
-   print "run 'git config --global sendemail.confirm 
auto'.\n\n";
+   print __(
+"The Cc list above has been expanded by additional
+addresses found in the patch commit message. By default
+send-email prompts before sending whenever this occurs.
+This behavior is controlled by the sendemail.confirm
+configuration setting.
+
+For additional information, run 'git send-email --help'.
+To retain the current behavior, but squelch this message,
+run 'git config --global sendemail.confirm auto'."),
+"\n\n";
}
-   $_ = ask("Send this email? ([y]es|[n]o|[q]uit|[a]ll): ",
+   # TRANSLATORS: Make sure to include [y] [n] [q] [a] in your
+   # translation. The program will only accept English input
+   # at this point.
+   $_ = ask(__("Send this email? ([y]es|[n]o|[q]uit|[a]ll): "),
 valid_re => qr/^(?:yes|y|no|n|quit|q|all|a)/i,
 default => $ask_default);
die "Send this email reply required" unless defined $_;
@@ -1403,7 +1412,7 @@ Message-Id: $message_id

[PATCH 01/11] i18n: add--interactive: mark strings for translation

[Vasco Almeida]

Mark simple strings (without interpolation) for translation.

Brackets around first parameter of ternary operator is necessary because
otherwise xgettext fails to extract strings marked for translation from
the rest of the file.

Signed-off-by: Vasco Almeida 
---
 git-add--interactive.perl | 68 +--
 1 file changed, 36 insertions(+), 32 deletions(-)

diff --git a/git-add--interactive.perl b/git-add--interactive.perl
index 822f857..fb8e5de 100755
--- a/git-add--interactive.perl
+++ b/git-add--interactive.perl
@@ -4,6 +4,7 @@ use 5.008;
 use strict;
 use warnings;
 use Git;
+use Git::I18N;
 
 binmode(STDOUT, ":raw");
 
@@ -252,7 +253,7 @@ sub list_untracked {
 }
 
 my $status_fmt = '%12s %12s %s';
-my $status_head = sprintf($status_fmt, 'staged', 'unstaged', 'path');
+my $status_head = sprintf($status_fmt, __('staged'), __('unstaged'), 
__('path'));
 
 {
my $initial;
@@ -678,7 +679,7 @@ sub update_cmd {
my @mods = list_modified('file-only');
return if (!@mods);
 
-   my @update = list_and_choose({ PROMPT => 'Update',
+   my @update = list_and_choose({ PROMPT => __('Update'),
   HEADER => $status_head, },
 @mods);
if (@update) {
@@ -690,7 +691,7 @@ sub update_cmd {
 }
 
 sub revert_cmd {
-   my @update = list_and_choose({ PROMPT => 'Revert',
+   my @update = list_and_choose({ PROMPT => __('Revert'),
   HEADER => $status_head, },
 list_modified());
if (@update) {
@@ -724,13 +725,13 @@ sub revert_cmd {
 }
 
 sub add_untracked_cmd {
-   my @add = list_and_choose({ PROMPT => 'Add untracked' },
+   my @add = list_and_choose({ PROMPT => __('Add untracked') },
  list_untracked());
if (@add) {
system(qw(git update-index --add --), @add);
say_n_paths('added', @add);
} else {
-   print "No untracked files.\n";
+   print __("No untracked files.\n");
}
print "\n";
 }
@@ -1159,8 +1160,11 @@ sub edit_hunk_loop {
}
else {
prompt_yesno(
-   'Your edited hunk does not apply. Edit again '
-   . '(saying "no" discards!) [y/n]? '
+   # TRANSLATORS: do not translate [y/n]
+   # The program will only accept that input
+   # at this point.
+   __('Your edited hunk does not apply. Edit again 
'
+  . '(saying "no" discards!) [y/n]? ')
) or return undef;
}
}
@@ -1206,11 +1210,11 @@ sub apply_patch_for_checkout_commit {
run_git_apply 'apply '.$reverse, @_;
return 1;
} elsif (!$applies_index) {
-   print colored $error_color, "The selected hunks do not apply to 
the index!\n";
-   if (prompt_yesno "Apply them to the worktree anyway? ") {
+   print colored $error_color, __("The selected hunks do not apply 
to the index!\n");
+   if (prompt_yesno __("Apply them to the worktree anyway? ")) {
return run_git_apply 'apply '.$reverse, @_;
} else {
-   print colored $error_color, "Nothing was applied.\n";
+   print colored $error_color, __("Nothing was 
applied.\n");
return 0;
}
} else {
@@ -1230,9 +1234,9 @@ sub patch_update_cmd {
 
if (!@mods) {
if (@all_mods) {
-   print STDERR "Only binary files changed.\n";
+   print STDERR __("Only binary files changed.\n");
} else {
-   print STDERR "No changes.\n";
+   print STDERR __("No changes.\n");
}
return 0;
}
@@ -1390,12 +1394,12 @@ sub patch_update_file {
my $response = $1;
my $no = $ix > 10 ? $ix - 10 : 0;
while ($response eq '') {
-   my $extra = "";
$no = display_hunks(\@hunk, $no);
if ($no < $num) {
-   $extra = " ( to see more)";
+   print __("go to which hunk 
( to see more)? ");
+   } else {
+   print __("go to which hunk? ");
}
-   print "go to which hunk$extra? ";
  

[PATCH 07/11] i18n: add--interactive: mark edit_hunk message for translation

[Vasco Almeida]

Mark message of edit_hunk_manually displayed in the editing file when
user chooses 'e' option.  The message had to be unfolded to allow
translation of the $participle verb.

Some messages end up being exactly the same for some uses cases, but
left it for easer change in the future, e.g., wanting to change wording
of one particular use case.

Signed-off-by: Vasco Almeida 
---
 git-add--interactive.perl | 60 ++-
 1 file changed, 49 insertions(+), 11 deletions(-)

diff --git a/git-add--interactive.perl b/git-add--interactive.perl
index af715b3..1652a57 100755
--- a/git-add--interactive.perl
+++ b/git-add--interactive.perl
@@ -1057,22 +1057,60 @@ sub edit_hunk_manually {
my $fh;
open $fh, '>', $hunkfile
or die sprintf __("failed to open hunk edit file for writing: 
%s"), $!;
-   print $fh "# Manual hunk edit mode -- see bottom for a quick guide\n";
+   print $fh __("# Manual hunk edit mode -- see bottom for a quick 
guide\n");
print $fh @$oldtext;
-   my $participle = $patch_mode_flavour{PARTICIPLE};
my $is_reverse = $patch_mode_flavour{IS_REVERSE};
my ($remove_plus, $remove_minus) = $is_reverse ? ('-', '+') : ('+', 
'-');
-   print $fh <

[PATCH 11/11] i18n: difftool: mark warnings for translation

[Vasco Almeida]

Signed-off-by: Vasco Almeida 
---
 git-difftool.perl | 18 ++
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/git-difftool.perl b/git-difftool.perl
index ebd13ba..fe7f003 100755
--- a/git-difftool.perl
+++ b/git-difftool.perl
@@ -22,6 +22,7 @@ use File::Path qw(mkpath rmtree);
 use File::Temp qw(tempdir);
 use Getopt::Long qw(:config pass_through);
 use Git;
+use Git::I18N;
 
 sub usage
 {
@@ -144,7 +145,7 @@ sub setup_dir_diff
my $i = 0;
while ($i < $#rawdiff) {
if ($rawdiff[$i] =~ /^::/) {
-   warn << 'EOF';
+   warn __ <<'EOF';
 Combined diff formats ('-c' and '--cc') are not supported in
 directory diff mode ('-d' and '--dir-diff').
 EOF
@@ -451,11 +452,11 @@ sub dir_diff
}
 
if (exists $wt_modified{$file} and exists $tmp_modified{$file}) 
{
-   my $errmsg = "warning: Both files modified: ";
-   $errmsg .= "'$workdir/$file' and '$b/$file'.\n";
-   $errmsg .= "warning: Working tree file has been 
left.\n";
-   $errmsg .= "warning:\n";
-   warn $errmsg;
+   warn sprintf __(
+"warning: Both files modified:
+'%s/%s' and '%s/%s'.
+warning: Working tree file has been left.
+warning:\n"), $workdir, $file, $b, $file;
$error = 1;
} elsif (exists $tmp_modified{$file}) {
my $mode = stat("$b/$file")->mode;
@@ -467,8 +468,9 @@ sub dir_diff
}
}
if ($error) {
-   warn "warning: Temporary files exist in '$tmpdir'.\n";
-   warn "warning: You may want to cleanup or recover these.\n";
+   warn sprintf __(
+"warning: Temporary files exist in '%s'.
+warning: You may want to cleanup or recover these.\n"), $tmpdir;
exit(1);
} else {
exit_cleanup($tmpdir, $rc);
-- 
2.6.6

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

[PATCH 10/11] i18n: send-email: mark string with interpolation for translation

[Vasco Almeida]

Mark warnings, errors and other messages that are interpolated for
translation.

We must call sprintf() before calling die() and in few other
circumstances in order to interpolation take place.

Signed-off-by: Vasco Almeida 
---
 git-send-email.perl | 71 -
 1 file changed, 38 insertions(+), 33 deletions(-)

diff --git a/git-send-email.perl b/git-send-email.perl
index e7f712e..f445a5e 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -279,10 +279,13 @@ sub signal_handler {
# tmp files from --compose
if (defined $compose_filename) {
if (-e $compose_filename) {
-   print "'$compose_filename' contains an intermediate 
version of the email you were composing.\n";
+   printf __("'%s' contains an intermediate version ".
+ "of the email you were composing.\n"),
+ $compose_filename;
}
if (-e ($compose_filename . ".final")) {
-   print "'$compose_filename.final' contains the composed 
email.\n"
+   printf __("'%s.final' contains the composed email.\n"),
+ $compose_filename;
}
}
 
@@ -431,7 +434,7 @@ $smtp_encryption = '' unless (defined $smtp_encryption);
 my(%suppress_cc);
 if (@suppress_cc) {
foreach my $entry (@suppress_cc) {
-   die "Unknown --suppress-cc field: '$entry'\n"
+   die sprintf __("Unknown --suppress-cc field: '%s'\n"), $entry
unless $entry =~ 
/^(?:all|cccmd|cc|author|self|sob|body|bodycc)$/;
$suppress_cc{$entry} = 1;
}
@@ -460,7 +463,7 @@ my $confirm_unconfigured = !defined $confirm;
 if ($confirm_unconfigured) {
$confirm = scalar %suppress_cc ? 'compose' : 'auto';
 };
-die "Unknown --confirm setting: '$confirm'\n"
+die sprintf(__("Unknown --confirm setting: '%s'\n"), $confirm)
unless $confirm =~ /^(?:auto|cc|compose|always|never)/;
 
 # Debugging, print out the suppressions.
@@ -492,16 +495,16 @@ my %aliases;
 sub parse_sendmail_alias {
local $_ = shift;
if (/"/) {
-   print STDERR "warning: sendmail alias with quotes is not 
supported: $_\n";
+   printf STDERR __("warning: sendmail alias with quotes is not 
supported: %s\n"), $_;
} elsif (/:include:/) {
-   print STDERR "warning: `:include:` not supported: $_\n";
+   printf STDERR __("warning: `:include:` not supported: %s\n"), 
$_;
} elsif (/[\/|]/) {
-   print STDERR "warning: `/file` or `|pipe` redirection not 
supported: $_\n";
+   printf STDERR __("warning: `/file` or `|pipe` redirection not 
supported: %s\n"), $_;
} elsif (/^(\S+?)\s*:\s*(.+)$/) {
my ($alias, $addr) = ($1, $2);
$aliases{$alias} = [ split_addrs($addr) ];
} else {
-   print STDERR "warning: sendmail line is not recognized: $_\n";
+   printf STDERR __("warning: sendmail line is not recognized: 
%s\n"), $_;
}
 }
 
@@ -582,13 +585,12 @@ sub is_format_patch_arg {
if (defined($format_patch)) {
return $format_patch;
}
-   die(<

[PATCH 09/11] i18n: send-email: mark warnings and errors for translation

[Vasco Almeida]

Mark warnings, errors and other messages for translation.

Signed-off-by: Vasco Almeida 
---
 git-send-email.perl | 36 ++--
 1 file changed, 18 insertions(+), 18 deletions(-)

diff --git a/git-send-email.perl b/git-send-email.perl
index 2521832..e7f712e 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -118,20 +118,20 @@ sub format_2822_time {
my $localmin = $localtm[1] + $localtm[2] * 60;
my $gmtmin = $gmttm[1] + $gmttm[2] * 60;
if ($localtm[0] != $gmttm[0]) {
-   die "local zone differs from GMT by a non-minute interval\n";
+   die __("local zone differs from GMT by a non-minute 
interval\n");
}
if ((($gmttm[6] + 1) % 7) == $localtm[6]) {
$localmin += 1440;
} elsif ((($gmttm[6] - 1) % 7) == $localtm[6]) {
$localmin -= 1440;
} elsif ($gmttm[6] != $localtm[6]) {
-   die "local time offset greater than or equal to 24 hours\n";
+   die __("local time offset greater than or equal to 24 hours\n");
}
my $offset = $localmin - $gmtmin;
my $offhour = $offset / 60;
my $offmin = abs($offset % 60);
if (abs($offhour) >= 24) {
-   die ("local time offset greater than or equal to 24 hours\n");
+   die __("local time offset greater than or equal to 24 hours\n");
}
 
return sprintf("%s, %2d %s %d %02d:%02d:%02d %s%02d%02d",
@@ -199,13 +199,13 @@ sub do_edit {
map {
system('sh', '-c', $editor.' "$@"', $editor, $_);
if (($? & 127) || ($? >> 8)) {
-   die("the editor exited uncleanly, aborting 
everything");
+   die(__("the editor exited uncleanly, aborting 
everything"));
}
} @_;
} else {
system('sh', '-c', $editor.' "$@"', $editor, @_);
if (($? & 127) || ($? >> 8)) {
-   die("the editor exited uncleanly, aborting everything");
+   die(__("the editor exited uncleanly, aborting 
everything"));
}
}
 }
@@ -299,7 +299,7 @@ my $help;
 my $rc = GetOptions("h" => \$help,
 "dump-aliases" => \$dump_aliases);
 usage() unless $rc;
-die "--dump-aliases incompatible with other options\n"
+die __("--dump-aliases incompatible with other options\n")
 if !$help and $dump_aliases and @ARGV;
 $rc = GetOptions(
"sender|from=s" => \$sender,
@@ -362,7 +362,7 @@ unless ($rc) {
 usage();
 }
 
-die "Cannot run git format-patch from outside a repository\n"
+die __("Cannot run git format-patch from outside a repository\n")
if $format_patch and not $repo;
 
 # Now, let's fill any that aren't set in with defaults:
@@ -617,7 +617,7 @@ while (defined(my $f = shift @ARGV)) {
 }
 
 if (@rev_list_opts) {
-   die "Cannot run git format-patch from outside a repository\n"
+   die __("Cannot run git format-patch from outside a repository\n")
unless $repo;
push @files, $repo->command('format-patch', '-o', tempdir(CLEANUP => 
1), @rev_list_opts);
 }
@@ -636,7 +636,7 @@ if (@files) {
print $_,"\n" for (@files);
}
 } else {
-   print STDERR "\nNo patch files specified!\n\n";
+   print STDERR __("\nNo patch files specified!\n\n");
usage();
 }
 
@@ -728,7 +728,7 @@ EOT
$sender = $1;
next;
} elsif (/^(?:To|Cc|Bcc):/i) {
-   print "To/Cc/Bcc fields are not interpreted yet, they 
have been ignored\n";
+   print __("To/Cc/Bcc fields are not interpreted yet, 
they have been ignored\n");
next;
}
print $c2 $_;
@@ -737,7 +737,7 @@ EOT
close $c2;
 
if ($summary_empty) {
-   print "Summary email is empty, skipping it\n";
+   print __("Summary email is empty, skipping it\n");
$compose = -1;
}
 } elsif ($annotate) {
@@ -1313,7 +1313,7 @@ Message-Id: $message_id
$_ = ask(__("Send this email? ([y]es|[n]o|[q]uit|[a]ll): "),
 valid_re => qr/^(?:yes|y|no|n|quit|q|all|a)/i,
 default => $ask_default);
-   die "Send this email reply required" unless defined $_;
+   die __("Send this email reply required") unless defined $_;
if (/^n/i) {
return 0;
} elsif (/^q/i) {
@@ -1339,7 +1339,7 @@ Message-Id: $message_id
} else {
 
if (!defined $smtp_server) {
-   die "The required SMTP server is not properly defined."
+   die __("The required SMTP server is not properly 
defined.")
}
 
  

[PATCH 02/11] i18n: add--interactive: mark simple here documents for translation

[Vasco Almeida]

Mark messages in here document without interpolation for translation.

Marking for translation by removing here documents this way, rather than
take advantage of "print __ << EOF" way, makes other instances of help
messages in clean.c match the first two in this file.  Otherwise,
reusing here document would add a trailer newline to the message, making
them not match 100%, hence creating two entries in pot template for
translation rather than a single entry.

Signed-off-by: Vasco Almeida 
---
 git-add--interactive.perl | 24 
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/git-add--interactive.perl b/git-add--interactive.perl
index fb8e5de..e11a33d 100755
--- a/git-add--interactive.perl
+++ b/git-add--interactive.perl
@@ -636,25 +636,25 @@ sub list_and_choose {
 }
 
 sub singleton_prompt_help_cmd {
-   print colored $help_color, <<\EOF ;
-Prompt help:
+   print colored $help_color, __(
+"Prompt help:
 1  - select a numbered item
 foo- select item based on unique prefix
-   - (empty) select nothing
-EOF
+   - (empty) select nothing"),
+"\n";
 }
 
 sub prompt_help_cmd {
-   print colored $help_color, <<\EOF ;
-Prompt help:
+   print colored $help_color, __(
+"Prompt help:
 1  - select a single item
 3-5- select a range of items
 2-3,6-9- select multiple ranges
 foo- select item based on unique prefix
 -...   - unselect specified items
 *  - choose all items
-   - (empty) finish selecting
-EOF
+   - (empty) finish selecting"),
+"\n";
 }
 
 sub status_cmd {
@@ -1573,14 +1573,14 @@ sub quit_cmd {
 }
 
 sub help_cmd {
-   print colored $help_color, <<\EOF ;
-status- show paths with changes
+   print colored $help_color, __(
+"status- show paths with changes
 update- add working tree state to the staged set of changes
 revert- revert staged set of changes back to the HEAD version
 patch - pick hunks and update selectively
 diff - view diff between HEAD and index
-add untracked - add contents of untracked files to the staged set of changes
-EOF
+add untracked - add contents of untracked files to the staged set of changes"),
+"\n";
 }
 
 sub process_args {
-- 
2.6.6

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

[PATCH 05/11] i18n: add--interactive: mark message for translation

[Vasco Almeida]

Mark message assembled in place for translation, unfolding each use case
for each entry in the %patch_modes hash table.

Previously, this script relied on whether $patch_mode was set to run the
command patch_update_cmd() or show status and loop the main loop. Now,
it uses $cmd to indicate we must run patch_update_cmd() and $patch_mode
is used to tell which flavor of the %patch_modes are we on.  This is
introduced in order to be able to mark and unfold the message prompt
knowing in which context we are.

The tracking of context was done previously by point %patch_mode_flavour
hash table to the correct entry of %patch_modes, focusing only on value
of %patch_modes. Now, we are also interested in the key ('staged',
'stash', 'checkout_head', ...).

Signed-off-by: Vasco Almeida 
---
 git-add--interactive.perl | 112 ++
 1 file changed, 104 insertions(+), 8 deletions(-)

diff --git a/git-add--interactive.perl b/git-add--interactive.perl
index ef50ba0..909f396 100755
--- a/git-add--interactive.perl
+++ b/git-add--interactive.perl
@@ -91,6 +91,7 @@ sub colored {
 }
 
 # command line options
+my $cmd;
 my $patch_mode;
 my $patch_mode_revision;
 
@@ -171,7 +172,8 @@ my %patch_modes = (
},
 );
 
-my %patch_mode_flavour = %{$patch_modes{stage}};
+$patch_mode = 'stage';
+my %patch_mode_flavour = %{$patch_modes{$patch_mode}};
 
 sub run_cmd_pipe {
if ($^O eq 'MSWin32') {
@@ -1372,12 +1374,105 @@ sub patch_update_file {
for (@{$hunk[$ix]{DISPLAY}}) {
print;
}
-   print colored $prompt_color, $patch_mode_flavour{VERB},
- ($hunk[$ix]{TYPE} eq 'mode' ? ' mode change' :
-  $hunk[$ix]{TYPE} eq 'deletion' ? ' deletion' :
-  ' this hunk'),
- $patch_mode_flavour{TARGET},
- " [y,n,q,a,d,/$other,?]? ";
+   if ($patch_mode eq 'stage') {
+   if ($hunk[$ix]{TYPE} eq 'mode') {
+ print colored $prompt_color,
+   sprintf __("Stage mode change [y,n,q,a,d,/%s,?]? "),
+  $other;
+   } elsif ($hunk[$ix]{TYPE} eq 'deletion') {
+ print colored $prompt_color,
+   sprintf __("Stage deletion [y,n,q,a,d,/%s,?]? "),
+  $other;
+   } else {
+ print colored $prompt_color,
+   sprintf __("Stage this hunk [y,n,q,a,d,/%s,?]? "),
+  $other;
+   }
+   } elsif ($patch_mode eq 'stash') {
+   if ($hunk[$ix]{TYPE} eq 'mode') {
+ print colored $prompt_color,
+   sprintf __("Stash mode change [y,n,q,a,d,/%s,?]? "),
+  $other;
+   } elsif ($hunk[$ix]{TYPE} eq 'deletion') {
+ print colored $prompt_color,
+   sprintf __("Stash deletion [y,n,q,a,d,/%s,?]? "),
+  $other;
+   } else {
+ print colored $prompt_color,
+   sprintf __("Stash this hunk [y,n,q,a,d,/%s,?]? "),
+  $other;
+   }
+   } elsif ($patch_mode eq 'reset_head') {
+   if ($hunk[$ix]{TYPE} eq 'mode') {
+ print colored $prompt_color,
+   sprintf __("Unstage mode change [y,n,q,a,d,/%s,?]? 
"),
+  $other;
+   } elsif ($hunk[$ix]{TYPE} eq 'deletion') {
+ print colored $prompt_color,
+   sprintf __("Unstage deletion [y,n,q,a,d,/%s,?]? "),
+  $other;
+   } else {
+ print colored $prompt_color,
+   sprintf __("Unstage this hunk [y,n,q,a,d,/%s,?]? "),
+  $other;
+   }
+   } elsif ($patch_mode eq 'reset_nothead') {
+   if ($hunk[$ix]{TYPE} eq 'mode') {
+ print colored $prompt_color,
+   sprintf __("Apply mode change to index 
[y,n,q,a,d,/%s,?]? "),
+  $other;
+   } elsif ($hunk[$ix]{TYPE} eq 'deletion') {
+ print colored $prompt_color,
+   sprintf __("Apply deletion to index 
[y,n,q,a,d,/%s,?]? "),
+  $other;
+   } else {
+ print colored $prompt_color,
+   sprintf __("Apply this hunk to 

[PATCH 04/11] i18n: add--interactive: mark plural strings

[Vasco Almeida]

Mark plural strings for translation.  Unfold each action case in one
entire sentence.

Pass new keyword for xgettext to extract.

Update test to include new subrotine Q__() for plural strings handling.

Signed-off-by: Vasco Almeida 
---
 Makefile  |  3 ++-
 git-add--interactive.perl | 24 
 perl/Git/I18N.pm  |  4 +++-
 t/t0202/test.pl   | 11 ++-
 4 files changed, 31 insertions(+), 11 deletions(-)

diff --git a/Makefile b/Makefile
index de5a030..eedf1fa 100644
--- a/Makefile
+++ b/Makefile
@@ -2061,7 +2061,8 @@ XGETTEXT_FLAGS_C = $(XGETTEXT_FLAGS) --language=C \
--keyword=_ --keyword=N_ --keyword="Q_:1,2"
 XGETTEXT_FLAGS_SH = $(XGETTEXT_FLAGS) --language=Shell \
--keyword=gettextln --keyword=eval_gettextln
-XGETTEXT_FLAGS_PERL = $(XGETTEXT_FLAGS) --keyword=__ --language=Perl
+XGETTEXT_FLAGS_PERL = $(XGETTEXT_FLAGS) --language=Perl \
+   --keyword=__ --keyword="Q__:1,2"
 LOCALIZED_C = $(C_OBJ:o=c) $(LIB_H) $(GENERATED_H)
 LOCALIZED_SH = $(SCRIPT_SH) git-parse-remote.sh
 LOCALIZED_PERL = $(SCRIPT_PERL)
diff --git a/git-add--interactive.perl b/git-add--interactive.perl
index d8d61d4..ef50ba0 100755
--- a/git-add--interactive.perl
+++ b/git-add--interactive.perl
@@ -666,12 +666,18 @@ sub status_cmd {
 sub say_n_paths {
my $did = shift @_;
my $cnt = scalar @_;
-   print "$did ";
-   if (1 < $cnt) {
-   print "$cnt paths\n";
-   }
-   else {
-   print "one path\n";
+   if ($did eq 'added') {
+   printf Q__("added one path\n", "added %d paths\n",
+  $cnt), $cnt;
+   } elsif ($did eq 'updated') {
+   printf Q__("updated one path\n", "updated %d paths\n",
+  $cnt), $cnt;
+   } elsif ($did eq 'reversed') {
+   printf Q__("reversed one path\n", "reversed %d paths\n",
+  $cnt), $cnt;
+   } else {
+   printf Q__("touched one path\n", "touched %d paths\n",
+  $cnt), $cnt;
}
 }
 
@@ -1508,8 +1514,10 @@ sub patch_update_file {
elsif ($other =~ /s/ && $line =~ /^s/) {
my @split = split_hunk($hunk[$ix]{TEXT}, 
$hunk[$ix]{DISPLAY});
if (1 < @split) {
-   print colored $header_color, "Split 
into ",
-   scalar(@split), " hunks.\n";
+   print colored $header_color, sprintf
+   Q__("Split into %d hunk.\n",
+   "Split into %d hunks.\n",
+   scalar(@split)), 
scalar(@split);
}
splice (@hunk, $ix, 1, @split);
$num = scalar @hunk;
diff --git a/perl/Git/I18N.pm b/perl/Git/I18N.pm
index f889fd6..5a75dd5 100644
--- a/perl/Git/I18N.pm
+++ b/perl/Git/I18N.pm
@@ -13,7 +13,7 @@ BEGIN {
}
 }
 
-our @EXPORT = qw(__);
+our @EXPORT = qw(__ Q__);
 our @EXPORT_OK = @EXPORT;
 
 sub __bootstrap_locale_messages {
@@ -44,6 +44,7 @@ BEGIN
eval {
__bootstrap_locale_messages();
*__ = \::Messages::gettext;
+   *Q__ = \::Messages::ngettext;
1;
} or do {
# Tell test.pl that we couldn't load the gettext library.
@@ -51,6 +52,7 @@ BEGIN
 
# Just a fall-through no-op
*__ = sub ($) { $_[0] };
+   *Q__ = sub ($$$) { $_[2] == 1 ? $_[0] : $_[1] };
};
 }
 
diff --git a/t/t0202/test.pl b/t/t0202/test.pl
index 2c10cb4..98aa9a3 100755
--- a/t/t0202/test.pl
+++ b/t/t0202/test.pl
@@ -4,7 +4,7 @@ use lib (split(/:/, $ENV{GITPERLLIB}));
 use strict;
 use warnings;
 use POSIX qw(:locale_h);
-use Test::More tests => 8;
+use Test::More tests => 11;
 use Git::I18N;
 
 my $has_gettext_library = $Git::I18N::__HAS_LIBRARY;
@@ -31,6 +31,7 @@ is_deeply(\@Git::I18N::EXPORT, \@Git::I18N::EXPORT_OK, 
"sanity: Git::I18N export
# more gettext wrapper functions.
my %prototypes = (qw(
__  $
+   Q__ $$$
));
while (my ($sub, $proto) = each %prototypes) {
is(prototype(\&{"Git::I18N::$sub"}), $proto, "sanity: $sub has 
a $proto prototype");
@@ -46,6 +47,14 @@ is_deeply(\@Git::I18N::EXPORT, \@Git::I18N::EXPORT_OK, 
"sanity: Git::I18N export
my ($got, $expect) = (('TEST: A Perl test string') x 2);
 
is(__($got), $expect, "Passing a string through __() in the C locale 
works");
+
+   my ($got_singular, $got_plural, $expect_singular, $expect_plural) =
+   (('TEST: 1 file', 'TEST: n files') x 2);
+
+   is(Q__($got_singular, $got_plural, 1), $expect_singular,
+   "Get 

[PATCH 06/11] i18n: add--interactive: i18n of help_patch_cmd

[Vasco Almeida]

Mark help message of help_patch_cmd for translation.  The message must
be unfolded to be free of variables so we can have hight quality
translations.

Signed-off-by: Vasco Almeida 
---
 git-add--interactive.perl | 65 +++
 1 file changed, 54 insertions(+), 11 deletions(-)

diff --git a/git-add--interactive.perl b/git-add--interactive.perl
index 909f396..af715b3 100755
--- a/git-add--interactive.perl
+++ b/git-add--interactive.perl
@@ -1179,15 +1179,58 @@ sub edit_hunk_loop {
 }
 
 sub help_patch_cmd {
-   my $verb = lc $patch_mode_flavour{VERB};
-   my $target = $patch_mode_flavour{TARGET};
-   print colored $help_color, 

[PATCH 03/11] i18n: add--interactive: mark strings with interpolation for translation

[Vasco Almeida]

Use of sprintf following die or error_msg is necessary for placeholder
substitution take place.

Signed-off-by: Vasco Almeida 
---
 git-add--interactive.perl | 26 ++
 1 file changed, 14 insertions(+), 12 deletions(-)

diff --git a/git-add--interactive.perl b/git-add--interactive.perl
index e11a33d..d8d61d4 100755
--- a/git-add--interactive.perl
+++ b/git-add--interactive.perl
@@ -612,12 +612,12 @@ sub list_and_choose {
else {
$bottom = $top = find_unique($choice, @stuff);
if (!defined $bottom) {
-   error_msg "Huh ($choice)?\n";
+   error_msg sprintf __("Huh (%s)?\n"), 
$choice;
next TOPLOOP;
}
}
if ($opts->{SINGLETON} && $bottom != $top) {
-   error_msg "Huh ($choice)?\n";
+   error_msg sprintf __("Huh (%s)?\n"), $choice;
next TOPLOOP;
}
for ($i = $bottom-1; $i <= $top-1; $i++) {
@@ -1048,7 +1048,7 @@ sub edit_hunk_manually {
my $hunkfile = $repo->repo_path . "/addp-hunk-edit.diff";
my $fh;
open $fh, '>', $hunkfile
-   or die "failed to open hunk edit file for writing: " . $!;
+   or die sprintf __("failed to open hunk edit file for writing: 
%s"), $!;
print $fh "# Manual hunk edit mode -- see bottom for a quick guide\n";
print $fh @$oldtext;
my $participle = $patch_mode_flavour{PARTICIPLE};
@@ -1075,7 +1075,7 @@ EOF
}
 
open $fh, '<', $hunkfile
-   or die "failed to open hunk edit file for reading: " . $!;
+   or die sprintf __("failed to open hunk edit file for reading: 
%s"), $!;
my @newtext = grep { !/^#/ } <$fh>;
close $fh;
unlink $hunkfile;
@@ -1225,7 +1225,7 @@ sub apply_patch_for_checkout_commit {
 
 sub patch_update_cmd {
my @all_mods = list_modified($patch_mode_flavour{FILTER});
-   error_msg "ignoring unmerged: $_->{VALUE}\n"
+   error_msg sprintf __("ignoring unmerged: %s\n"), $_->{VALUE}
for grep { $_->{UNMERGED} } @all_mods;
@all_mods = grep { !$_->{UNMERGED} } @all_mods;
 
@@ -1407,11 +1407,13 @@ sub patch_update_file {
chomp $response;
}
if ($response !~ /^\s*\d+\s*$/) {
-   error_msg "Invalid number: 
'$response'\n";
+   error_msg sprintf __("Invalid number: 
'%s'\n"),
+ $response;
} elsif (0 < $response && $response <= $num) {
$ix = $response - 1;
} else {
-   error_msg "Sorry, only $num hunks 
available.\n";
+   error_msg sprintf __("Sorry, only %s 
hunks available.\n"),
+ $num;
}
next;
}
@@ -1449,7 +1451,7 @@ sub patch_update_file {
if ($@) {
my ($err,$exp) = ($@, $1);
$err =~ s/ at .*git-add--interactive 
line \d+,  line \d+.*$//;
-   error_msg "Malformed search regexp 
$exp: $err\n";
+   error_msg sprintf __("Malformed search 
regexp %s: %s\n"), $exp, $err;
next;
}
my $iy = $ix;
@@ -1612,18 +1614,18 @@ sub process_args {
$patch_mode = $1;
$arg = shift @ARGV or die __("missing --");
} else {
-   die "unknown --patch mode: $1";
+   die sprintf __("unknown --patch mode: %s"), $1;
}
} else {
$patch_mode = 'stage';
$arg = shift @ARGV or die __("missing --");
}
-   die "invalid argument $arg, expecting --"
-   unless $arg eq "--";
+   die sprintf __("invalid argument %s, expecting --"),
+  $arg unless $arg eq "--";
%patch_mode_flavour = %{$patch_modes{$patch_mode}};
}
elsif ($arg ne "--") {
-   die "invalid argument $arg, expecting --";
+   die sprintf __("invalid argument %s, 

http-backend fatal error message on shallow clone

[Eric Wong]

I noticed "fatal: The remote end hung up unexpectedly" in server
logs from shallow clones.  Totally reproducible in the test
cases, too.  The following change shows it:

diff --git a/t/t5561-http-backend.sh b/t/t5561-http-backend.sh
index 90e0d6f..cfa55ce 100755
--- a/t/t5561-http-backend.sh
+++ b/t/t5561-http-backend.sh
@@ -132,5 +132,11 @@ test_expect_success 'server request log matches test 
results' '
test_cmp exp act
 '
 
+test_expect_success 'shallow clone' '
+   config http.uploadpack true &&
+   git clone --depth=1 "$HTTPD_URL/smart/repo.git" shallow &&
+   tail "$HTTPD_ROOT_PATH"/error.log | grep fatal
+'
+
 stop_httpd
 test_done


And the last test ends like this:

expecting success: 
config http.uploadpack true &&
git clone --depth=1 "$HTTPD_URL/smart/repo.git" shallow &&
tail "$HTTPD_ROOT_PATH"/error.log | grep fatal

Cloning into 'shallow'...
[Tue Jun 21 11:07:41.391269 2016] [cgi:error] [pid 21589] [client 
127.0.0.1:37518] AH01215: fatal: The remote end hung up unexpectedly
ok 15 - shallow clone

It doesn't show above, but I think http-backend exits
with a non-zero status, too, which might cause some CGI
implementations to complain or break.

Not sure if it's just a corner case that wasn't tested
or something else, but the clone itself seems successful...
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3 4/9] line-log: respect diffopt's configured output file stream

[Johannes Schindelin]

The diff machinery can optionally output to a file stream other than
stdout, by overriding diffopt.file. In such a case, the rest of the
log tree machinery should also write to that stream.

Currently, there is no user of the line level log that wants to
redirect output to a file. Therefore, one might argue that it is
superfluous to support that now. However, it is better to be
consistent now, rather than to face hard-to-debug problems later.

Signed-off-by: Johannes Schindelin 
---
 line-log.c | 34 +-
 1 file changed, 17 insertions(+), 17 deletions(-)

diff --git a/line-log.c b/line-log.c
index bbe31ed..e62a7f4 100644
--- a/line-log.c
+++ b/line-log.c
@@ -841,7 +841,7 @@ static char *get_nth_line(long line, unsigned long *ends, 
void *data)
 
 static void print_line(const char *prefix, char first,
   long line, unsigned long *ends, void *data,
-  const char *color, const char *reset)
+  const char *color, const char *reset, FILE *file)
 {
char *begin = get_nth_line(line, ends, data);
char *end = get_nth_line(line+1, ends, data);
@@ -852,14 +852,14 @@ static void print_line(const char *prefix, char first,
had_nl = 1;
}
 
-   fputs(prefix, stdout);
-   fputs(color, stdout);
-   putchar(first);
-   fwrite(begin, 1, end-begin, stdout);
-   fputs(reset, stdout);
-   putchar('\n');
+   fputs(prefix, file);
+   fputs(color, file);
+   putc(first, file);
+   fwrite(begin, 1, end-begin, file);
+   fputs(reset, file);
+   putc('\n', file);
if (!had_nl)
-   fputs("\\ No newline at end of file\n", stdout);
+   fputs("\\ No newline at end of file\n", file);
 }
 
 static char *output_prefix(struct diff_options *opt)
@@ -898,12 +898,12 @@ static void dump_diff_hacky_one(struct rev_info *rev, 
struct line_log_data *rang
fill_line_ends(pair->one, _lines, _ends);
fill_line_ends(pair->two, _lines, _ends);
 
-   printf("%s%sdiff --git a/%s b/%s%s\n", prefix, c_meta, pair->one->path, 
pair->two->path, c_reset);
-   printf("%s%s--- %s%s%s\n", prefix, c_meta,
+   fprintf(opt->file, "%s%sdiff --git a/%s b/%s%s\n", prefix, c_meta, 
pair->one->path, pair->two->path, c_reset);
+   fprintf(opt->file, "%s%s--- %s%s%s\n", prefix, c_meta,
   pair->one->sha1_valid ? "a/" : "",
   pair->one->sha1_valid ? pair->one->path : "/dev/null",
   c_reset);
-   printf("%s%s+++ b/%s%s\n", prefix, c_meta, pair->two->path, c_reset);
+   fprintf(opt->file, "%s%s+++ b/%s%s\n", prefix, c_meta, pair->two->path, 
c_reset);
for (i = 0; i < range->ranges.nr; i++) {
long p_start, p_end;
long t_start = range->ranges.ranges[i].start;
@@ -945,7 +945,7 @@ static void dump_diff_hacky_one(struct rev_info *rev, 
struct line_log_data *rang
}
 
/* Now output a diff hunk for this range */
-   printf("%s%s@@ -%ld,%ld +%ld,%ld @@%s\n",
+   fprintf(opt->file, "%s%s@@ -%ld,%ld +%ld,%ld @@%s\n",
   prefix, c_frag,
   p_start+1, p_end-p_start, t_start+1, t_end-t_start,
   c_reset);
@@ -953,18 +953,18 @@ static void dump_diff_hacky_one(struct rev_info *rev, 
struct line_log_data *rang
int k;
for (; t_cur < diff->target.ranges[j].start; t_cur++)
print_line(prefix, ' ', t_cur, t_ends, 
pair->two->data,
-  c_context, c_reset);
+  c_context, c_reset, opt->file);
for (k = diff->parent.ranges[j].start; k < 
diff->parent.ranges[j].end; k++)
print_line(prefix, '-', k, p_ends, 
pair->one->data,
-  c_old, c_reset);
+  c_old, c_reset, opt->file);
for (; t_cur < diff->target.ranges[j].end && t_cur < 
t_end; t_cur++)
print_line(prefix, '+', t_cur, t_ends, 
pair->two->data,
-  c_new, c_reset);
+  c_new, c_reset, opt->file);
j++;
}
for (; t_cur < t_end; t_cur++)
print_line(prefix, ' ', t_cur, t_ends, pair->two->data,
-  c_context, c_reset);
+  c_context, c_reset, opt->file);
}
 
free(p_ends);
@@ -977,7 +977,7 @@ static void dump_diff_hacky_one(struct rev_info *rev, 
struct line_log_data *rang
  */
 static void dump_diff_hacky(struct rev_info *rev, struct line_log_data *range)
 {
-   puts(output_prefix(>diffopt));
+   

[PATCH v3 7/9] format-patch: explicitly switch off color when writing to files

[Johannes Schindelin]

We rely on the auto-detection ("is stdout a terminal?") to determine
whether to use color in the output of format-patch or not. That happens
to work because we freopen() stdout when redirecting the output to files.

However, we are about to fix that work-around, in which case the
auto-detection has no chance to guess whether to use color or not.

But then, we do not need to guess to begin with. We know that we do not
want to use ANSI color sequences in the output files. So let's just be
explicit about our wishes.

It might be argued that we should only do this when the variable
git_use_color_default still has its default value, GIT_COLOR_AUTO.
As of 7787570c (format-patch: ignore ui.color, 2011-09-13) we
do not allow the ui.color setting to affect format-patch's output,
therefore this will always be the case.

Signed-off-by: Johannes Schindelin 
---
 builtin/log.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/builtin/log.c b/builtin/log.c
index 099f4f7..abd889b 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -1569,6 +1569,7 @@ int cmd_format_patch(int argc, const char **argv, const 
char *prefix)
setup_pager();
 
if (output_directory) {
+   rev.diffopt.use_color = 0;
if (use_stdout)
die(_("standard output, or directory, which one?"));
if (mkdir(output_directory, 0777) < 0 && errno != EEXIST)
-- 
2.9.0.118.g0e1a633


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

[PATCH v3 0/9] Let log-tree and friends respect diffopt's `file` field

[Johannes Schindelin]

The idea is to allow callers to redirect log-tree's output to a file
without having to freopen() stdout (which would modify global state,
a big no-no-no for library functions).

I reviewed log-tree.c, graph.c, line-log.c, builtin/shortlog.c and
builtin/log.c line by line to ensure that all calls that assumed stdout
previously now use the `file` field instead, of course. I would
welcome additional eyes to go over the code to confirm that I did not
miss anything.

This patch series ends up removing the freopen() call in the
format-patch command, but that is only a by-product. The ulterior motive
behind this series is to allow the sequencer to write a `patch` file as
part of my endeavor to move large chunks of rebase -i into a builtin.

As mentioned earlier, the `use_color = 0` setting is still kept
unconditional because format-patch explicitly ignores the ui.color
setting.

This iteration has the following changes with regards to the previous
one:

- putchar() is replaced with putc(), not fputc()

- the maybe_flush_or_die() function is now called by log_tree_commit()
  even if outputting to a file stream different from stdout

- this uncovered a problem with builtin am, where it asked the diff
  machinery to close the file stream, but actually called the log_tree
  machinery (which might mean that this patch series inadvertently fixes
  a bug where `git am --rebasing` would write the commit message to
  stdout instead of the `patch` file when erroring out)

This last point is a bigger issue, actually. There seem to be quite a
few function calls in builtin/am.c whose return values that might
indicate errors are flatly ignored. I see two calls to run_diff_index()
whose return value then goes poof unchecked, and several calls to
write_state_text() and write_state_bool() with the same issue. And I did
not even try to review the code to that end, all I wanted was to verify
that builtin am only has the close_file issue once (it does use it a
second time, but that one is okay because it then calls
run_diff_index(), i.e. the diff machinery).

I am embarrassed to admit that these builtin am problems demonstrate
that I, as a mentor of the builtin am project, failed to help make the
patches as good as I expected myself to do.


Johannes Schindelin (9):
  am: stop ignoring errors reported by log_tree_diff()
  Disallow diffopt.close_file when using the log_tree machinery
  log-tree: respect diffopt's configured output file stream
  line-log: respect diffopt's configured output file stream
  graph: respect the diffopt.file setting
  shortlog: support outputting to streams other than stdout
  format-patch: explicitly switch off color when writing to files
  format-patch: avoid freopen()
  format-patch: use stdout directly

 builtin/am.c   | 18 +-
 builtin/log.c  | 71 +++---
 builtin/shortlog.c | 13 ++
 graph.c| 30 +--
 line-log.c | 34 +-
 log-tree.c | 67 +++
 shortlog.h |  1 +
 7 files changed, 125 insertions(+), 109 deletions(-)

Published-As: https://github.com/dscho/git/releases/tag/diffopt.file-v3
Interdiff vs v2:

 diff --git a/builtin/am.c b/builtin/am.c
 index 3dfe70b..47d78aa 100644
 --- a/builtin/am.c
 +++ b/builtin/am.c
 @@ -1433,12 +1433,16 @@ static void get_commit_info(struct am_state *state, 
struct commit *commit)
  /**
   * Writes `commit` as a patch to the state directory's "patch" file.
   */
 -static void write_commit_patch(const struct am_state *state, struct commit 
*commit)
 +static int write_commit_patch(const struct am_state *state, struct commit 
*commit)
  {
struct rev_info rev_info;
FILE *fp;
 +  int res;
  
 -  fp = xfopen(am_path(state, "patch"), "w");
 +  fp = fopen(am_path(state, "patch"), "w");
 +  if (!fp)
 +  return error(_("Could not open %s for writing"),
 +  am_path(state, "patch"));
init_revisions(_info, NULL);
rev_info.diff = 1;
rev_info.abbrev = 0;
 @@ -1450,10 +1454,11 @@ static void write_commit_patch(const struct am_state 
*state, struct commit *comm
DIFF_OPT_SET(_info.diffopt, FULL_INDEX);
rev_info.diffopt.use_color = 0;
rev_info.diffopt.file = fp;
 -  rev_info.diffopt.close_file = 1;
add_pending_object(_info, >object, "");
diff_setup_done(_info.diffopt);
 -  log_tree_commit(_info, commit);
 +  res = log_tree_commit(_info, commit);
 +  fclose(fp);
 +  return res;
  }
  
  /**
 @@ -1501,13 +1506,14 @@ static int parse_mail_rebase(struct am_state *state, 
const char *mail)
unsigned char commit_sha1[GIT_SHA1_RAWSZ];
  
if (get_mail_commit_sha1(commit_sha1, mail) < 0)
 -  die(_("could not parse %s"), mail);
 +  return error(_("could not parse %s"), mail);
  
commit = 

[PATCH v3 2/9] Disallow diffopt.close_file when using the log_tree machinery

[Johannes Schindelin]

We are about to teach the log_tree machinery to reuse the diffopt.file
setting to output to a file stream different from stdout.

This means that builtin am can no longer ask the diff machinery to
close the file when actually calling the log_tree machinery (which
wants to flush the very same file stream that would then already be
closed).

To stave off similar problems in the future, report it as a bug if
log_tree_commit() is called with a non-zero diffopt.close_file.

Signed-off-by: Johannes Schindelin 
---
 builtin/am.c | 6 --
 log-tree.c   | 3 +++
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/builtin/am.c b/builtin/am.c
index 0e28a62..47d78aa 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -1437,6 +1437,7 @@ static int write_commit_patch(const struct am_state 
*state, struct commit *commi
 {
struct rev_info rev_info;
FILE *fp;
+   int res;
 
fp = fopen(am_path(state, "patch"), "w");
if (!fp)
@@ -1453,10 +1454,11 @@ static int write_commit_patch(const struct am_state 
*state, struct commit *commi
DIFF_OPT_SET(_info.diffopt, FULL_INDEX);
rev_info.diffopt.use_color = 0;
rev_info.diffopt.file = fp;
-   rev_info.diffopt.close_file = 1;
add_pending_object(_info, >object, "");
diff_setup_done(_info.diffopt);
-   return log_tree_commit(_info, commit);
+   res = log_tree_commit(_info, commit);
+   fclose(fp);
+   return res;
 }
 
 /**
diff --git a/log-tree.c b/log-tree.c
index 78a5381..dc0180d 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -864,6 +864,9 @@ int log_tree_commit(struct rev_info *opt, struct commit 
*commit)
struct log_info log;
int shown;
 
+   if (opt->diffopt.close_file)
+   die("BUG: close_file is incompatible with log_tree_commit()");
+
log.commit = commit;
log.parent = NULL;
opt->loginfo = 
-- 
2.9.0.118.g0e1a633


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

[PATCH v3 8/9] format-patch: avoid freopen()

[Johannes Schindelin]

We just taught the relevant functions to respect the diffopt.file field,
to allow writing somewhere else than stdout. Let's make use of it.

Technically, we do not need to avoid that call in a builtin: we assume
that builtins (as opposed to library functions) are stand-alone programs
that may do with their (global) state. Yet, we want to be able to reuse
that code in properly lib-ified code, e.g. when converting scripts into
builtins.

Further, while we did not *have* to touch the cmd_show() and cmd_cherry()
code paths (because they do not want to write anywhere but stdout as of
yet), it just makes sense to be consistent, making it easier and safer to
move the code later.

Signed-off-by: Johannes Schindelin 
---
 builtin/log.c | 64 ++-
 1 file changed, 33 insertions(+), 31 deletions(-)

diff --git a/builtin/log.c b/builtin/log.c
index abd889b..8dcf205 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -236,7 +236,7 @@ static void show_early_header(struct rev_info *rev, const 
char *stage, int nr)
if (rev->commit_format != CMIT_FMT_ONELINE)
putchar(rev->diffopt.line_termination);
}
-   printf(_("Final output: %d %s\n"), nr, stage);
+   fprintf(rev->diffopt.file, _("Final output: %d %s\n"), nr, stage);
 }
 
 static struct itimerval early_output_timer;
@@ -445,7 +445,7 @@ static void show_tagger(char *buf, int len, struct rev_info 
*rev)
pp.fmt = rev->commit_format;
pp.date_mode = rev->date_mode;
pp_user_info(, "Tagger", , buf, get_log_output_encoding());
-   printf("%s", out.buf);
+   fprintf(rev->diffopt.file, "%s", out.buf);
strbuf_release();
 }
 
@@ -456,7 +456,7 @@ static int show_blob_object(const unsigned char *sha1, 
struct rev_info *rev, con
char *buf;
unsigned long size;
 
-   fflush(stdout);
+   fflush(rev->diffopt.file);
if (!DIFF_OPT_TOUCHED(>diffopt, ALLOW_TEXTCONV) ||
!DIFF_OPT_TST(>diffopt, ALLOW_TEXTCONV))
return stream_blob_to_fd(1, sha1, NULL, 0);
@@ -496,7 +496,7 @@ static int show_tag_object(const unsigned char *sha1, 
struct rev_info *rev)
}
 
if (offset < size)
-   fwrite(buf + offset, size - offset, 1, stdout);
+   fwrite(buf + offset, size - offset, 1, rev->diffopt.file);
free(buf);
return 0;
 }
@@ -505,7 +505,8 @@ static int show_tree_object(const unsigned char *sha1,
struct strbuf *base,
const char *pathname, unsigned mode, int stage, void *context)
 {
-   printf("%s%s\n", pathname, S_ISDIR(mode) ? "/" : "");
+   FILE *file = context;
+   fprintf(file, "%s%s\n", pathname, S_ISDIR(mode) ? "/" : "");
return 0;
 }
 
@@ -565,7 +566,7 @@ int cmd_show(int argc, const char **argv, const char 
*prefix)
 
if (rev.shown_one)
putchar('\n');
-   printf("%stag %s%s\n",
+   fprintf(rev.diffopt.file, "%stag %s%s\n",
diff_get_color_opt(, 
DIFF_COMMIT),
t->tag,
diff_get_color_opt(, 
DIFF_RESET));
@@ -584,12 +585,12 @@ int cmd_show(int argc, const char **argv, const char 
*prefix)
case OBJ_TREE:
if (rev.shown_one)
putchar('\n');
-   printf("%stree %s%s\n\n",
+   fprintf(rev.diffopt.file, "%stree %s%s\n\n",
diff_get_color_opt(, 
DIFF_COMMIT),
name,
diff_get_color_opt(, 
DIFF_RESET));
read_tree_recursive((struct tree *)o, "", 0, 0, 
_all,
-   show_tree_object, NULL);
+   show_tree_object, rev.diffopt.file);
rev.shown_one = 1;
break;
case OBJ_COMMIT:
@@ -799,7 +800,7 @@ static FILE *realstdout = NULL;
 static const char *output_directory = NULL;
 static int outdir_offset;
 
-static int reopen_stdout(struct commit *commit, const char *subject,
+static int open_next_file(struct commit *commit, const char *subject,
 struct rev_info *rev, int quiet)
 {
struct strbuf filename = STRBUF_INIT;
@@ -823,7 +824,7 @@ static int reopen_stdout(struct commit *commit, const char 
*subject,
if (!quiet)
fprintf(realstdout, "%s\n", filename.buf + outdir_offset);
 
-   if (freopen(filename.buf, "w", stdout) == NULL)
+   if ((rev->diffopt.file = fopen(filename.buf, "w")) == NULL)
return error(_("Cannot open patch file %s"), filename.buf);
 
strbuf_release();
@@ -882,15 +883,15 @@ static void 

[PATCH v3 1/9] am: stop ignoring errors reported by log_tree_diff()

[Johannes Schindelin]

It is never a good idea to ignore errors, so let's handle them.

While at it, handle fopen() errors more gently (i.e. give the caller a
chance to do something about it rather than die()ing).

Note: there are more places in the builtin am code that ignore
errors returned from library functions. Fixing those is outside the
purview of the current patch series, though.

Cc: Paul Tan 
Signed-off-by: Johannes Schindelin 
---
 builtin/am.c | 14 +-
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/builtin/am.c b/builtin/am.c
index 3dfe70b..0e28a62 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -1433,12 +1433,15 @@ static void get_commit_info(struct am_state *state, 
struct commit *commit)
 /**
  * Writes `commit` as a patch to the state directory's "patch" file.
  */
-static void write_commit_patch(const struct am_state *state, struct commit 
*commit)
+static int write_commit_patch(const struct am_state *state, struct commit 
*commit)
 {
struct rev_info rev_info;
FILE *fp;
 
-   fp = xfopen(am_path(state, "patch"), "w");
+   fp = fopen(am_path(state, "patch"), "w");
+   if (!fp)
+   return error(_("Could not open %s for writing"),
+   am_path(state, "patch"));
init_revisions(_info, NULL);
rev_info.diff = 1;
rev_info.abbrev = 0;
@@ -1453,7 +1456,7 @@ static void write_commit_patch(const struct am_state 
*state, struct commit *comm
rev_info.diffopt.close_file = 1;
add_pending_object(_info, >object, "");
diff_setup_done(_info.diffopt);
-   log_tree_commit(_info, commit);
+   return log_tree_commit(_info, commit);
 }
 
 /**
@@ -1501,13 +1504,14 @@ static int parse_mail_rebase(struct am_state *state, 
const char *mail)
unsigned char commit_sha1[GIT_SHA1_RAWSZ];
 
if (get_mail_commit_sha1(commit_sha1, mail) < 0)
-   die(_("could not parse %s"), mail);
+   return error(_("could not parse %s"), mail);
 
commit = lookup_commit_or_die(commit_sha1, mail);
 
get_commit_info(state, commit);
 
-   write_commit_patch(state, commit);
+   if (write_commit_patch(state, commit) < 0)
+   return -1;
 
hashcpy(state->orig_commit, commit_sha1);
write_state_text(state, "original-commit", sha1_to_hex(commit_sha1));
-- 
2.9.0.118.g0e1a633


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

[PATCH v3 9/9] format-patch: use stdout directly

[Johannes Schindelin]

Earlier, we freopen()ed stdout in order to write patches to files.
That forced us to duplicate stdout (naming it "realstdout") because we
*still* wanted to be able to report the file names.

As we do not abuse stdout that way anymore, we no longer need to
duplicate stdout, either.

Signed-off-by: Johannes Schindelin 
---
 builtin/log.c | 6 +-
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/builtin/log.c b/builtin/log.c
index 8dcf205..2bfcc43 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -796,7 +796,6 @@ static int git_format_config(const char *var, const char 
*value, void *cb)
return git_log_config(var, value, cb);
 }
 
-static FILE *realstdout = NULL;
 static const char *output_directory = NULL;
 static int outdir_offset;
 
@@ -822,7 +821,7 @@ static int open_next_file(struct commit *commit, const char 
*subject,
fmt_output_subject(, subject, rev);
 
if (!quiet)
-   fprintf(realstdout, "%s\n", filename.buf + outdir_offset);
+   printf("%s\n", filename.buf + outdir_offset);
 
if ((rev->diffopt.file = fopen(filename.buf, "w")) == NULL)
return error(_("Cannot open patch file %s"), filename.buf);
@@ -1629,9 +1628,6 @@ int cmd_format_patch(int argc, const char **argv, const 
char *prefix)
get_patch_ids(, );
}
 
-   if (!use_stdout)
-   realstdout = xfdopen(xdup(1), "w");
-
if (prepare_revision_walk())
die(_("revision walk setup failed"));
rev.boundary = 1;
-- 
2.9.0.118.g0e1a633
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3 5/9] graph: respect the diffopt.file setting

[Johannes Schindelin]

When the caller overrides diffopt.file (which defaults to stdout),
the diff machinery already redirects its output, and the graph display
should also write to that file.

Signed-off-by: Johannes Schindelin 
---
 graph.c | 30 +-
 1 file changed, 17 insertions(+), 13 deletions(-)

diff --git a/graph.c b/graph.c
index 1350bdd..8ad8ba3 100644
--- a/graph.c
+++ b/graph.c
@@ -17,8 +17,8 @@
 static void graph_padding_line(struct git_graph *graph, struct strbuf *sb);
 
 /*
- * Print a strbuf to stdout.  If the graph is non-NULL, all lines but the
- * first will be prefixed with the graph output.
+ * Print a strbuf.  If the graph is non-NULL, all lines but the first will be
+ * prefixed with the graph output.
  *
  * If the strbuf ends with a newline, the output will end after this
  * newline.  A new graph line will not be printed after the final newline.
@@ -1193,9 +1193,10 @@ void graph_show_commit(struct git_graph *graph)
 
while (!shown_commit_line && !graph_is_commit_finished(graph)) {
shown_commit_line = graph_next_line(graph, );
-   fwrite(msgbuf.buf, sizeof(char), msgbuf.len, stdout);
+   fwrite(msgbuf.buf, sizeof(char), msgbuf.len,
+   graph->revs->diffopt.file);
if (!shown_commit_line)
-   putchar('\n');
+   putc('\n', graph->revs->diffopt.file);
strbuf_setlen(, 0);
}
 
@@ -1210,7 +1211,7 @@ void graph_show_oneline(struct git_graph *graph)
return;
 
graph_next_line(graph, );
-   fwrite(msgbuf.buf, sizeof(char), msgbuf.len, stdout);
+   fwrite(msgbuf.buf, sizeof(char), msgbuf.len, graph->revs->diffopt.file);
strbuf_release();
 }
 
@@ -1222,7 +1223,7 @@ void graph_show_padding(struct git_graph *graph)
return;
 
graph_padding_line(graph, );
-   fwrite(msgbuf.buf, sizeof(char), msgbuf.len, stdout);
+   fwrite(msgbuf.buf, sizeof(char), msgbuf.len, graph->revs->diffopt.file);
strbuf_release();
 }
 
@@ -1239,12 +1240,13 @@ int graph_show_remainder(struct git_graph *graph)
 
for (;;) {
graph_next_line(graph, );
-   fwrite(msgbuf.buf, sizeof(char), msgbuf.len, stdout);
+   fwrite(msgbuf.buf, sizeof(char), msgbuf.len,
+   graph->revs->diffopt.file);
strbuf_setlen(, 0);
shown = 1;
 
if (!graph_is_commit_finished(graph))
-   putchar('\n');
+   putc('\n', graph->revs->diffopt.file);
else
break;
}
@@ -1259,7 +1261,8 @@ static void graph_show_strbuf(struct git_graph *graph, 
struct strbuf const *sb)
char *p;
 
if (!graph) {
-   fwrite(sb->buf, sizeof(char), sb->len, stdout);
+   fwrite(sb->buf, sizeof(char), sb->len,
+   graph->revs->diffopt.file);
return;
}
 
@@ -1277,7 +1280,7 @@ static void graph_show_strbuf(struct git_graph *graph, 
struct strbuf const *sb)
} else {
len = (sb->buf + sb->len) - p;
}
-   fwrite(p, sizeof(char), len, stdout);
+   fwrite(p, sizeof(char), len, graph->revs->diffopt.file);
if (next_p && *next_p != '\0')
graph_show_oneline(graph);
p = next_p;
@@ -1297,7 +1300,8 @@ void graph_show_commit_msg(struct git_graph *graph,
 * CMIT_FMT_USERFORMAT are already missing a terminating
 * newline.  All of the other formats should have it.
 */
-   fwrite(sb->buf, sizeof(char), sb->len, stdout);
+   fwrite(sb->buf, sizeof(char), sb->len,
+   graph->revs->diffopt.file);
return;
}
 
@@ -1318,7 +1322,7 @@ void graph_show_commit_msg(struct git_graph *graph,
 * new line.
 */
if (!newline_terminated)
-   putchar('\n');
+   putc('\n', graph->revs->diffopt.file);
 
graph_show_remainder(graph);
 
@@ -1326,6 +1330,6 @@ void graph_show_commit_msg(struct git_graph *graph,
 * If sb ends with a newline, our output should too.
 */
if (newline_terminated)
-   putchar('\n');
+   putc('\n', graph->revs->diffopt.file);
}
 }
-- 
2.9.0.118.g0e1a633


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

[PATCH v3 6/9] shortlog: support outputting to streams other than stdout

[Johannes Schindelin]

This will be needed to avoid freopen() in `git format-patch`.

Signed-off-by: Johannes Schindelin 
---
 builtin/shortlog.c | 13 -
 shortlog.h |  1 +
 2 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/builtin/shortlog.c b/builtin/shortlog.c
index bfc082e..39d74fe 100644
--- a/builtin/shortlog.c
+++ b/builtin/shortlog.c
@@ -229,6 +229,7 @@ void shortlog_init(struct shortlog *log)
log->wrap = DEFAULT_WRAPLEN;
log->in1 = DEFAULT_INDENT1;
log->in2 = DEFAULT_INDENT2;
+   log->file = stdout;
 }
 
 int cmd_shortlog(int argc, const char **argv, const char *prefix)
@@ -310,22 +311,24 @@ void shortlog_output(struct shortlog *log)
for (i = 0; i < log->list.nr; i++) {
const struct string_list_item *item = >list.items[i];
if (log->summary) {
-   printf("%6d\t%s\n", (int)UTIL_TO_INT(item), 
item->string);
+   fprintf(log->file, "%6d\t%s\n",
+   (int)UTIL_TO_INT(item), item->string);
} else {
struct string_list *onelines = item->util;
-   printf("%s (%d):\n", item->string, onelines->nr);
+   fprintf(log->file, "%s (%d):\n",
+   item->string, onelines->nr);
for (j = onelines->nr - 1; j >= 0; j--) {
const char *msg = onelines->items[j].string;
 
if (log->wrap_lines) {
strbuf_reset();
add_wrapped_shortlog_msg(, msg, log);
-   fwrite(sb.buf, sb.len, 1, stdout);
+   fwrite(sb.buf, sb.len, 1, log->file);
}
else
-   printf("  %s\n", msg);
+   fprintf(log->file, "  %s\n", msg);
}
-   putchar('\n');
+   putc('\n', log->file);
onelines->strdup_strings = 1;
string_list_clear(onelines, 0);
free(onelines);
diff --git a/shortlog.h b/shortlog.h
index de4f86f..5a326c6 100644
--- a/shortlog.h
+++ b/shortlog.h
@@ -17,6 +17,7 @@ struct shortlog {
char *common_repo_prefix;
int email;
struct string_list mailmap;
+   FILE *file;
 };
 
 void shortlog_init(struct shortlog *log);
-- 
2.9.0.118.g0e1a633


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

[PATCH v3 3/9] log-tree: respect diffopt's configured output file stream

[Johannes Schindelin]

The diff options already know how to print the output anywhere else
than stdout. The same is needed for log output in general, e.g.
when writing patches to files in `git format-patch`. Let's allow
users to use log_tree_commit() *without* changing global state via
freopen().

Signed-off-by: Johannes Schindelin 
---
 log-tree.c | 64 +++---
 1 file changed, 32 insertions(+), 32 deletions(-)

diff --git a/log-tree.c b/log-tree.c
index dc0180d..530297d 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -159,12 +159,12 @@ void load_ref_decorations(int flags)
}
 }
 
-static void show_parents(struct commit *commit, int abbrev)
+static void show_parents(struct commit *commit, int abbrev, FILE *file)
 {
struct commit_list *p;
for (p = commit->parents; p ; p = p->next) {
struct commit *parent = p->item;
-   printf(" %s", find_unique_abbrev(parent->object.oid.hash, 
abbrev));
+   fprintf(file, " %s", 
find_unique_abbrev(parent->object.oid.hash, abbrev));
}
 }
 
@@ -172,7 +172,7 @@ static void show_children(struct rev_info *opt, struct 
commit *commit, int abbre
 {
struct commit_list *p = lookup_decoration(>children, 
>object);
for ( ; p; p = p->next) {
-   printf(" %s", find_unique_abbrev(p->item->object.oid.hash, 
abbrev));
+   fprintf(opt->diffopt.file, " %s", 
find_unique_abbrev(p->item->object.oid.hash, abbrev));
}
 }
 
@@ -286,11 +286,11 @@ void show_decorations(struct rev_info *opt, struct commit 
*commit)
struct strbuf sb = STRBUF_INIT;
 
if (opt->show_source && commit->util)
-   printf("\t%s", (char *) commit->util);
+   fprintf(opt->diffopt.file, "\t%s", (char *) commit->util);
if (!opt->show_decorations)
return;
format_decorations(, commit, opt->diffopt.use_color);
-   fputs(sb.buf, stdout);
+   fputs(sb.buf, opt->diffopt.file);
strbuf_release();
 }
 
@@ -364,18 +364,18 @@ void log_write_email_headers(struct rev_info *opt, struct 
commit *commit,
subject = "Subject: ";
}
 
-   printf("From %s Mon Sep 17 00:00:00 2001\n", name);
+   fprintf(opt->diffopt.file, "From %s Mon Sep 17 00:00:00 2001\n", name);
graph_show_oneline(opt->graph);
if (opt->message_id) {
-   printf("Message-Id: <%s>\n", opt->message_id);
+   fprintf(opt->diffopt.file, "Message-Id: <%s>\n", 
opt->message_id);
graph_show_oneline(opt->graph);
}
if (opt->ref_message_ids && opt->ref_message_ids->nr > 0) {
int i, n;
n = opt->ref_message_ids->nr;
-   printf("In-Reply-To: <%s>\n", 
opt->ref_message_ids->items[n-1].string);
+   fprintf(opt->diffopt.file, "In-Reply-To: <%s>\n", 
opt->ref_message_ids->items[n-1].string);
for (i = 0; i < n; i++)
-   printf("%s<%s>\n", (i > 0 ? "\t" : "References: "),
+   fprintf(opt->diffopt.file, "%s<%s>\n", (i > 0 ? "\t" : 
"References: "),
   opt->ref_message_ids->items[i].string);
graph_show_oneline(opt->graph);
}
@@ -432,7 +432,7 @@ static void show_sig_lines(struct rev_info *opt, int 
status, const char *bol)
reset = diff_get_color_opt(>diffopt, DIFF_RESET);
while (*bol) {
eol = strchrnul(bol, '\n');
-   printf("%s%.*s%s%s", color, (int)(eol - bol), bol, reset,
+   fprintf(opt->diffopt.file, "%s%.*s%s%s", color, (int)(eol - 
bol), bol, reset,
   *eol ? "\n" : "");
graph_show_oneline(opt->graph);
bol = (*eol) ? (eol + 1) : eol;
@@ -553,17 +553,17 @@ void show_log(struct rev_info *opt)
 
if (!opt->graph)
put_revision_mark(opt, commit);
-   fputs(find_unique_abbrev(commit->object.oid.hash, 
abbrev_commit), stdout);
+   fputs(find_unique_abbrev(commit->object.oid.hash, 
abbrev_commit), opt->diffopt.file);
if (opt->print_parents)
-   show_parents(commit, abbrev_commit);
+   show_parents(commit, abbrev_commit, opt->diffopt.file);
if (opt->children.name)
show_children(opt, commit, abbrev_commit);
show_decorations(opt, commit);
if (opt->graph && !graph_is_commit_finished(opt->graph)) {
-   putchar('\n');
+   putc('\n', opt->diffopt.file);
graph_show_remainder(opt->graph);
}
-   putchar(opt->diffopt.line_termination);
+   putc(opt->diffopt.line_termination, opt->diffopt.file);
return;
}
 
@@ -589,7 +589,7 @@ void show_log(struct rev_info *opt)
   

Re: [PATCH v2 1/7] log-tree: respect diffopt's configured output file stream

[Johannes Schindelin]

Hi,

On Tue, 21 Jun 2016, Johannes Schindelin wrote:

> On Tue, 21 Jun 2016, Johannes Schindelin wrote:
> 
> > Expect v3 in a moment.
> 
> I am sorry. The regression test suite just sounded red alert. So: do not
> expect v3 in a moment, but later today.

So making maybe_flush_or_die() uncovered a problem with the builtin am: it
called log_tree_commit() after setting diffopt.file *and*
diffopt.close_file. The latter forced diff_flush() to close the file, and
log_tree_commit() happily tried to flush() the same file stream. In my
setup, this triggered a segmentation fault which was really hard to debug
because it happened in a Git subprocess that expected input from stdin
(and therefore my go-to debugging method to fire up gdb was a bit
useless).

In the end, it not only added two new patches to the patch series, but
also opened a big rabbit hole: builtin/am.c could use quite some error
checking, I guess... Now I wish I hadn't seen it...

Well, I just sent v3 of the series.

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

Re: Filters should be parallelized if needed

[Lars Schneider]

> On 21 Jun 2016, at 09:50, LUCAS Thomas (EXT)  
> wrote:
> 
> Setup:
> - 2.9.0.windows.1 x64 on a windows 7 16g ram, ssd
> - No particular installation settings, default.
> 
> Details:
> - Running git from either a cmd or GitExtensions,  I encountered an issue 
> while staging or checkout of lfs files. I am processing 3000 files for a 
> total of 500MB.
> Files are proccessed sequentially which is too slow for this amount of files, 
> while lfs offer in some cases a way to run it from the command line in 
> parallel, I think the git filters should be run in parallel. I think there is 
> no need (or I am forgetting something) to run it sequentially.
> 
> If I were to use a filter processing files in a parallel way, I would have my 
> time to checkout these files be reduced from 1h30min to 1min30s.
> 
> What actually happened instead?
> - This is a no go for our enterprise to use git-lfs because this is too slow.
> 
> I can't give you my repository because it is confidential.
> 
> Anyway, I hope you can do something about it.

Hi Lucas,

this is a known issue and there are a few ideas how to improve the situation:
https://github.com/github/git-lfs/issues/1059
https://github.com/github/git-lfs/issues/931#issuecomment-172939381

I am not sure if anyone is working on the batch/parallel smudge filter 
processing
already (I might have some time for that later this year). AFAIK this is not
a trivial change.

That being said, I think the Git mailing list is the wrong place to discuss 
GitLFS
issues as these two projects are separate. I recommend to raise an issue here:
https://github.com/github/git-lfs/issues

Cheers,
Lars

--
To unsubscribe from this list: send the line "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: How to find commits unique to a branch

[Michael J Gruber]

Nikolaus Rath venit, vidit, dixit 21.06.2016 01:21:
> On Jun 20 2016, Junio C Hamano  wrote:
>> Nikolaus Rath  writes:
>>
>>> What's the best way to find all commits in a branch A that have not been
>>> cherry-picked from (or to) another branch B?
>>>
>>> I think I could format-patch all commits in every branch into separate
>>> files, hash the Author and Date of each files, and then compare the two
>>> lists. But I'm hoping there's a way to instead have git do the
>>> heavy-lifting?
>>
>> "git cherry" perhaps?
> 
> That seems to work only the "wrong way around". I have a tag
> fuse_3_0_start, which is the common ancestor to "master" and
> "fuse_2_9_bugfix". I'd like to find all the commits from fuse_3_0_start
> to master that have not been cherry-picked into fuse_2_9_bugfix.
> 
> However:
> 
> * "git cherry fuse_3_0_start master release2.9" tells me nothing has
>   been cherry-picked at all (only lines with +)
> 
> * "git cherry fuse_3_0_start release2.9 master" also tells me nothing
>   has been cherry picked, but somehow shows a smaller total number of
>   commits.
> 
> * "git cherry master release2.9 fuse_3_0_start" gives me the commits
>   from fuse_2_9_bugfix that have not been cherry-picked into master
>   (which seems to be in contradiction to the two earlier commands).
> 
> 
> Am I missing something obvious?

There is always

git log --left-right --cherry-mark A...B

to give you a good overview of the situation.

"--cherry-pick" instead of "--cherry-mark" will leave out the
"="-commits (equivalent ones), and the description of "git log --cherry"
in the log man page gives you a good idea of how you can combine
"--cherry-pick" with "--left-only" etc. to give you exactly what you want.

For script-usage, you can finally replace "git log" by "git rev-list"
with the same rev selecting options.

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

[PATCH] Improved example "To move the whole tree into a subdirectory..." to not fail when early commits are empty.

[Brett Randall]

Previously this example would fail if the oldest commit(s) in any filtered 
branch is/are empty (no files) because the index would not change, and the mv 
would fail with:

mv: cannot stat /index.new': No such file or directory

This commonly occurs with histories created from git-svn-clone.  The updated 
example checks whether the index file has been created before attempting the 
mv.  The empty commit is retained.

See 
http://stackoverflow.com/questions/7798142/error-combining-git-repositories-into-subdirs
 for an example and explanation.

Signed-off-by: Brett Randall 
---
 Documentation/git-filter-branch.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/git-filter-branch.txt 
b/Documentation/git-filter-branch.txt
index 003731f..271d5b0 100644
--- a/Documentation/git-filter-branch.txt
+++ b/Documentation/git-filter-branch.txt
@@ -385,7 +385,7 @@ git filter-branch --index-filter \
'git ls-files -s | sed "s-\t\"*-/-" |
GIT_INDEX_FILE=$GIT_INDEX_FILE.new \
git update-index --index-info &&
-mv "$GIT_INDEX_FILE.new" "$GIT_INDEX_FILE"' HEAD
+if [ -f "$GIT_INDEX_FILE.new" ]; then mv "$GIT_INDEX_FILE.new" 
"$GIT_INDEX_FILE"; fi' HEAD
 ---
 
 
-- 
2.8.4

--
To unsubscribe from this list: send the line "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] perf: accommodate for MacOSX

[Lars Schneider]

> On 20 Jun 2016, at 21:48, Junio C Hamano  wrote:
> 
> Johannes Schindelin  writes:
> 
>> On Sun, 19 Jun 2016, Lars Schneider wrote:
>> 
 On 18 Jun 2016, at 15:03, Johannes Schindelin
  wrote:
 
 As this developer has no access to MacOSX developer setups anymore,
 Travis becomes the best bet to run performance tests on that OS.
>>> 
>>> We don't run the performance tests on Travis CI right now.
>>> Maybe we should? With your patch below it should work, right?
>> ...
>> 
>> Yeah, well, I should have been clearer in my commit message: this patch
>> allows the perf tests to *run*, not to *pass*... ;-)
> 
> OK, Lars, do we still want to take this patch?  I am leaning towards
> taking it, if only to motivate interested others with OS X to look
> into making the perf tests to actually run.

I think we definitively should take the "perf-lib.sh" part of the patch
as this makes the perf test run on OSX and therefore is a strict
improvement.

If we don't run any perf tests by default on Travis CI then I wouldn't 
take the ".travis.yml" part of the patch just to keep our Travis CI
setup as lean as possible. Running perf tests on Travis CI is probably
bogus anyways because we never know on what hardware our jobs run
and what other jobs run in parallel on that hardware.

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

Filters should be parallelized if needed

[LUCAS Thomas (EXT)]

Setup:
- 2.9.0.windows.1 x64 on a windows 7 16g ram, ssd
- No particular installation settings, default.

Details:
- Running git from either a cmd or GitExtensions,  I encountered an issue while 
staging or checkout of lfs files. I am processing 3000 files for a total of 
500MB.
Files are proccessed sequentially which is too slow for this amount of files, 
while lfs offer in some cases a way to run it from the command line in 
parallel, I think the git filters should be run in parallel. I think there is 
no need (or I am forgetting something) to run it sequentially.

If I were to use a filter processing files in a parallel way, I would have my 
time to checkout these files be reduced from 1h30min to 1min30s.

What actually happened instead?
- This is a no go for our enterprise to use git-lfs because this is too slow.

I can't give you my repository because it is confidential.

Anyway, I hope you can do something about it.
*
This message and any attachments (the "message") are confidential, intended 
solely for the addresse(s), and may contain legally privileged information.
Any unauthorized use or dissemination is prohibited. E-mails are susceptible to 
alteration.   
Neither SOCIETE GENERALE nor any of its subsidiaries or affiliates shall be 
liable for the message if altered, changed or
falsified.
Please visit http://swapdisclosure.sgcib.com for important information with 
respect to derivative products.
  
Ce message et toutes les pieces jointes (ci-apres le "message") sont 
confidentiels et susceptibles de contenir des informations couvertes 
par le secret professionnel. 
Ce message est etabli a l'intention exclusive de ses destinataires. Toute 
utilisation ou diffusion non autorisee est interdite.
Tout message electronique est susceptible d'alteration. 
La SOCIETE GENERALE et ses filiales declinent toute responsabilite au titre de 
ce message s'il a ete altere, deforme ou falsifie.
Veuillez consulter le site http://swapdisclosure.sgcib.com afin de recueillir 
d'importantes informations sur les produits derives.
*

--
To unsubscribe from this list: send the line "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 1/7] log-tree: respect diffopt's configured output file stream

[Johannes Schindelin]

Hi,

On Tue, 21 Jun 2016, Johannes Schindelin wrote:

> Expect v3 in a moment.

I am sorry. The regression test suite just sounded red alert. So: do not
expect v3 in a moment, but later today.

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

Re: [RFC/PATCH] push: deny policy to prevent pushes to unwanted remotes.

[Lars Schneider]

> On 21 Jun 2016, at 08:20, Matthieu Moy  wrote:
> 
> Antoine Queru  writes:
> 
>> However, in the last version, if we want to deny an website,
>> including all schemes, we can blacklist the url without the
>> scheme. For example, "pushBlacklist = github.com". By doing so, this
>> remote is not an url anymore, and it can't be differenced with a local
>> relative path. It's a problem because these two have a different
>> treatement. The choice we made to solve this is to force the user to
>> put the scheme "file://" before any local relative path. What do you
>> think ?
> 
> file:// URL can not be relative (well, you can invent a syntax where
> they are, but that would be weird).
> 
> I think you can just forbid relative path in whitelist/blacklist, hence
> consider that anything that is neither a full URL nor an absolute path
> is a protocol-less URL:
> 
> * http://github.com = github.com with HTTP protocol
> 
> * github.com = github.com with any protocol
> 
> * /path/to/file or file:///path/to/file = local path

I agree. Ignoring relative paths (and mentioning that in the docs)
sounds reasonable to me. I don't think that would be a limitation as
I can't think of a white/blacklist use case for relative URLs.

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