[PATCH] Document the 'svn propset' command.

[Alfred Perlstein]

Add example usage to the git-svn documentation.

Reported-by: Joseph Pecoraro 
Signed-off-by: Alfred Perlstein  
---

Junio, Pranit, + all,

A week ago I was requested to provide documentation for the
'svn propset' command.  I have attached a diff off of the
'maint' branch for this, however it seems to apply cleanly
to 'master' as well.

Thank you for your patience.

This is also available on my github here:
https://github.com/splbio/git/tree/document_propset



 Documentation/git-svn.txt | 14 ++
 1 file changed, 14 insertions(+)

diff --git a/Documentation/git-svn.txt b/Documentation/git-svn.txt
index fb23a98..e104824 100644
--- a/Documentation/git-svn.txt
+++ b/Documentation/git-svn.txt
@@ -459,6 +459,20 @@ Any other arguments are passed directly to 'git log'
Gets the Subversion property given as the first argument, for a
file.  A specific revision can be specified with -r/--revision.
 
+'propset'::
+   Sets the Subversion property given as the first argument, to the
+   value given as the second argument for the file given as the
+   third argument.
++
+Example:
++
+
+git svn propset svn:keywords "FreeBSD=%H" devel/py-tipper/Makefile
+
++
+This will set the property 'svn:keywords' to 'FreeBSD=%H' for the file
+'devel/py-tipper/Makefile'.
+
 'show-externals'::
Shows the Subversion externals.  Use -r/--revision to specify a
specific revision.
-- 
2.7.1

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

Re: I lost my commit signature

[Jeff King]

On Wed, Jun 15, 2016 at 12:27:15PM +0800, ZhenTian wrote:

> I got two more lines from gpg -v during commit with -S:
> ```
> gpg: writing to stdout
> gpg: RSA/SHA1 signature from: "2EF2AD6E Tian Zhen "
> ```
> 
> after I commit, I push it to remote, but someone had pushed before to
> master branch, so I pull on master branch(`git pull --rebase`), then I
> check my commit via `git log --show-signature`, there is no signature
> in it, so I commit it with --ament and -S again, the signature is come
> back.
> 
> I haven't check signature before push, because I have checked four
> commits before, every commit is fine.
> 
> I don't know whether the `git pull` influenced signature or not.

Ah, so the problem is probably that you had a signature _initially_, but
that it did not survive the rebase. Which makes sense, as rebase would
need to re-sign.  It does not by default, but you can tell it to do so
with "-S". Or you can set `commit.gpgsign`, which should sign in both
cases.

-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: I lost my commit signature

[ZhenTian]

Hi Michael and Peff,

I got two more lines from gpg -v during commit with -S:
```
gpg: writing to stdout
gpg: RSA/SHA1 signature from: "2EF2AD6E Tian Zhen "
```

after I commit, I push it to remote, but someone had pushed before to
master branch, so I pull on master branch(`git pull --rebase`), then I
check my commit via `git log --show-signature`, there is no signature
in it, so I commit it with --ament and -S again, the signature is come
back.

I haven't check signature before push, because I have checked four
commits before, every commit is fine.

I don't know whether the `git pull` influenced signature or not.

My signature is just like Schrodinger's cat, when I check it, it lost :)
-Schrödinger


On Tue, Jun 14, 2016 at 6:57 PM, Michael J Gruber
 wrote:
> Jeff King venit, vidit, dixit 14.06.2016 11:41:
>> On Tue, Jun 14, 2016 at 04:39:38PM +0800, ZhenTian wrote:
>>
>>> I want to set gpg -v to pgp.program, but if I set it, it can't call gpg:
>>> ```
>>> error: cannot run gpg -v: No such file or directory
>>> error: could not run gpg.
>>> fatal: failed to write commit object
>>> ```
>>>
>>> I have tried set gpg.program value to `gpg|/tmp/log`, `/usr/bin/gpg
>>> -v`, `gpg -v`, `"/usr/bin/gpg -v"`
>>>
>>> only after I set to `gpg` or `/usr/bin/gpg` without any argument, it will 
>>> work.
>>
>> Ah, right. Most of the time we run such programs as shell commands, but
>> it looks like we do not. So you'd have to do something like:
>>
>>   cat >/tmp/fake-gpg <<-\EOF
>>   #!/bin/sh
>>   gpg -v "$@"
>>   EOF
>>   chmod +x /tmp/fake-gpg
>>   git config gpg.program /tmp/fake-gpg
>>
>> -Peff
>>
>
> The content of "gpg.program" is used as argv[0] when we build up various
> commands to be run; we expect it to heed standard gpg options.
>
> On the other hand:
>
> git -c gpg.program=echo commit -S
>
> 'successfully' creates a commit that has
>
> gpgsig -bsau Michael J Gruber 
>
> as the last header line. gpg.program=true fails (as does cat, unhappy
> with the options), so apparently we do some error checking but not enough.
>
> Michael
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] fetch: document that pruning happens before fetching

[Jeff King]

On Tue, Jun 14, 2016 at 05:36:35PM +0700, Duy Nguyen wrote:

> > or what would happen if the packfile
> > fetch failed (we'd already have deleted the old refs, but wouldn't fetch
> > the new ones).
> 
> Off topic, but this sounds like a bug to me. We could have kept ref
> update more consistent (maybe at some point we could even do atomic
> transaction update for all refs if there's a need for it).

I do think atomic ref transactions are a nice idea, and would probably
not be too hard to implement these days (the main thing is just
refactoring the deep call stack in git-fetch).

It's possible it could be annoying when you have a single broken ref
(and would prefer to just ignore it), but that _should_ be the rare
case.

-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] Refactor recv_sideband()

[Jeff King]

On Tue, Jun 14, 2016 at 02:25:42PM -0700, Junio C Hamano wrote:

> > Also, reorganize the overall control flow, remove some superfluous
> > variables and replace a custom implementation of strpbrk() with a call
> > to the standard C library function.
> 
> I find that calling the loop "a custom implementation" is a bit
> unfair.  The original tried to avoid looking beyond "len", but in
> the updated code because you have buf[len] = '\0' to terminate the
> line, and because you pass LARGE_PACKET_MAX to packet_read() while
> your buf[] allocates one more byte, you can use strpbrk() here
> safely. Which would mean "a custom implementation" was done for a
> reason.

Knowing we have a NUL at the end makes strpbrk() safe to use (as opposed
to walking off the end of the buffer). But what about the opposite case,
when there are embedded NULs in the data?

I think this case is already fairly broken by the use of "%s" specifiers
later in the function, and I doubt it is all that useful. So I am OK if
the answer is "we don't care, and do not even consider it a bug that
should be fixed".

-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: [PATCHv3] gpg-interface: check gpg signature creation status

[Jeff King]

On Tue, Jun 14, 2016 at 06:26:33PM -0400, Jeff King wrote:

> > > > bottom = signature->len;
> > > > -   len = strbuf_read(signature, gpg.out, 1024);
> > > > +   strbuf_read(signature, gpg.out, 1024);
> > > > +   strbuf_read(, gpg.err, 0);
> > > 
> > > H, isn't this asking for a deadlock?  When GPG spews more than
> > > what would fit in a pipe buffer to its standard error (hence gets
> > > blocked), its standard output may not complete, and the we would get
> > > stuck by attempting to read from gpg.out, failing to reach the other
> > > strbuf_read() that would unblock GPG by reading from gpg.err?
> > 
> > Yeah, it definitely is a deadlock. I think we'd need a select loop to
> > read into multiple strbufs at once (we can't use "struct async" because
> > that might happen in another process).
> 
> Something like this on top of Michael's patch (only lightly tested).

I wondered if this is something we might run into in other places, but
just haven't in practice. After grepping existing calls to strbuf_read(),
I think the only other case is the one in verify_signed_buffer(), which
reads all of stderr before trying to read stdout. I suspect that _could_
deadlock but doesn't tend to in practice.

Interestingly, it also writes in full to gpg's stdin before reading
anything. If gpg buffers all of its input before writing, that's fine,
but in theory it does not have to. I have no idea if it's possible for
it to be a problem in practice.

That can be solved in the same select loop, though obviously it's not
just strbuf_read_parallel() at that point, but rather a kind of a
feed_and_capture_command().

-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: What's cooking in git.git (Jun 2016, #04; Tue, 14)

[Mike Hommey]

On Tue, Jun 14, 2016 at 03:08:04PM -0700, Junio C Hamano wrote:
> Here are the topics that have been cooking.  Commits prefixed with
> '-' are only in 'pu' (proposed updates) while commits prefixed with
> '+' are in 'next'.  The ones marked with '.' do not appear in any of
> the integration branches, but I am still holding onto them.
> 
> Git 2.9 has been tagged.  Let's wait for a few days to clean up
> possible fallout and then start a new cycle by rewinding the tip of
> 'next'.  I expect I'd eject a few premature topics out of 'next'
> while doing so.
> 
> You can find the changes described here in the integration branches
> of the repositories listed at
> 
> http://git-blame.blogspot.com/p/git-public-repositories.html
> 
> --
> [Graduated to "master"]
> 
> * jc/t2300-setup (2016-06-01) 1 commit
>   (merged to 'next' on 2016-06-06 at 20f7f83)
>  + t2300: run git-sh-setup in an environment that better mimics the real life
>  (this branch is used by va/i18n-even-more.)
> 
>  A test fix.
> 
> 
> * jk/diff-compact-heuristic (2016-06-10) 1 commit
>  - diff: disable compaction heuristic for now
> 
>  It turns out that the earlier effort to update the heuristics may
>  want to use a bit more time to mature.  Turn it off by default.
> 
> 
> * jk/shell-portability (2016-06-01) 2 commits
>   (merged to 'next' on 2016-06-06 at 5de784e)
>  + t5500 & t7403: lose bash-ism "local"
>  + test-lib: add in-shell "env" replacement
> 
>  test fixes.
> 
> --
> [New Topics]
> 
> * lv/status-say-working-tree-not-directory (2016-06-09) 1 commit
>  - Use "working tree" instead of "working directory" for git status
> 
>  "git status" used to say "working directory" when it meant "working
>  tree".
> 
>  Will merge to 'next'.
> 
> 
> * jk/parseopt-string-list (2016-06-13) 3 commits
>  - blame,shortlog: don't make local option variables static
>  - interpret-trailers: don't duplicate option strings
>  - parse_opt_string_list: stop allocating new strings
>  (this branch is used by jk/string-list-static-init.)
> 
>  The command line argument parsing that uses OPT_STRING_LIST() often
>  made a copy of the argv[] element, which was unnecessary.
> 
>  Will merge to 'next'.
> 
> 
> * jk/repack-keep-unreachable (2016-06-14) 3 commits
>  - repack: extend --keep-unreachable to loose objects
>  - repack: add --keep-unreachable option
>  - repack: document --unpack-unreachable option
> 
>  "git repack" learned the "--keep-unreachable" option, which sends
>  loose unreachable objects to a pack instead of leaving them loose.
>  This helps heuristics based on the number of loose objects
>  (e.g. "gc --auto").
> 
>  Will merge to 'next'.
> 
> 
> * lf/recv-sideband-cleanup (2016-06-13) 1 commit
>  - sideband.c: refactor recv_sideband()
> 
>  Code simplification.  It however loses the atomicity of the output
>  9ac13ec9 (atomic write for sideband remote messages, 2006-10-11)
>  tried to add to an once-much-simpler codebase.
> 
>  Expecting a reroll.
> 
> 
> * nd/test-lib-httpd-show-error-log-in-verbose (2016-06-13) 1 commit
>  - lib-httpd.sh: print error.log on error
> 
>  Debugging aid.
> 
>  Will merge to 'next'.
> 
> 
> * pc/occurred (2016-06-10) 2 commits
>  - config.c: fix misspelt "occurred" in an error message
>  - refs.h: fix misspelt "occurred" in a comment
> 
>  Will merge to 'next'.
> 
> 
> * sb/submodule-clone-retry (2016-06-13) 2 commits
>  - submodule update: continue when a clone fails
>  - submodule--helper: initial clone learns retry logic
>  (this branch uses sb/submodule-recommend-shallowness.)
> 
>  "git submodule update" that drives many "git clone" could
>  eventually hit flaky servers/network conditions on one of the
>  submodules; the command learned to retry the attempt.
> 
> 
> * jc/blame-reverse (2016-06-14) 2 commits
>  - blame: dwim "blame --reverse OLD" as "blame --reverse OLD.."
>  - blame: improve diagnosis for "--reverse NEW"
> 
> 
> * jc/deref-tag (2016-06-14) 1 commit
>  - blame, line-log: do not loop around deref_tag()
> 
>  Code clean-up.
> 
>  Will merge to 'next'.
> 
> 
> * jk/fetch-prune-doc (2016-06-14) 1 commit
>  - fetch: document that pruning happens before fetching
> 
>  Will merge to 'next'.
> 
> 
> * km/fetch-do-not-free-remote-name (2016-06-14) 1 commit
>  - builtin/fetch.c: don't free remote->name after fetch
> 
>  Will merge to 'next'.
> 
> 
> * nb/gnome-keyring-build (2016-06-14) 1 commit
>  - gnome-keyring: Don't hard-code pkg-config executable
> 
>  Build improvements for gnome-keyring (in contrib/)
> 
>  Will merge to 'next'.
> 
> 
> * pb/strbuf-read-file-doc (2016-06-14) 1 commit
>  - strbuf: describe the return value of strbuf_read_file
> 
>  Will merge to 'next'.
> 
> --
> [Stalled]
> 
> * sb/bisect (2016-04-15) 22 commits
>  - SQUASH???
>  - bisect: get back halfway shortcut
>  - bisect: compute best bisection in compute_relevant_weights()
>  - 

Re: [PATCHv3] gpg-interface: check gpg signature creation status

[Jeff King]

On Tue, Jun 14, 2016 at 04:47:35PM -0700, Junio C Hamano wrote:

> Jeff King  writes:
> 
> > I'm still undecided on whether it is a better approach than making
> > sure the stdout we got looks sane. In particular I'd worry that it
> > would make things harder for somebody trying to plug in something
> > gpg-like (e.g., if you wanted to do something exotic like call a
> > program which fetched the signature from a remote device or
> > something).  But it's probably not _that_ hard for such a script
> > to emulate --status-fd.
> 
> I share the same thinking, but at the same time, it already is a
> requirement to give --status-fd output that is close enough on the
> signature verification side, isn't it?

Yeah, though I could see somebody wanting to sit amidst the signing side
but not verification (e.g., if your keys are elsewhere from the machine
running git). Of course such a case could probably ferry --status-fd
from the other side anyway.

I admit I don't know of such a case in practice, though, and
implementing a rudimentary --status-fd to say "SIG OK" or whatever on
the signing side is not _that_ big a deal. So if we think this approach
is a more robust solution in the normal case, let's not hold it up over
what-ifs.

-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: [PATCHv3] gpg-interface: check gpg signature creation status

[Junio C Hamano]

Jeff King  writes:

> I'm still undecided on whether it is a better approach than making
> sure the stdout we got looks sane. In particular I'd worry that it
> would make things harder for somebody trying to plug in something
> gpg-like (e.g., if you wanted to do something exotic like call a
> program which fetched the signature from a remote device or
> something).  But it's probably not _that_ hard for such a script
> to emulate --status-fd.

I share the same thinking, but at the same time, it already is a
requirement to give --status-fd output that is close enough on the
signature verification side, isn't it?

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

Re: [PATCH v4 6/6] send-email: add option --cite to quote the message body

[Samuel GROOT]

On 06/09/2016 01:49 PM, Matthieu Moy wrote:

Samuel GROOT  writes:


If used with `in-reply-to=`, cite the message body of the given
email file. Otherwise, do nothing.


It should at least warn when --in-reply-to= is not given
(either no --in-reply-to or --in-reply-to=). I don't see any
use-case where a user would want --cite on the command-line and not want
--in-reply-to=. OTOH, it seems a plausible user-error, and
the user would appreciate a message saying what's going on.


We weren't sure how to warn the user.

If --in-reply-to is not an mail file, should we check it with a regex to 
make sure it's a message-id?



@@ -56,6 +57,8 @@ git send-email --dump-aliases
 --subject * Email "Subject:"
 --in-reply-to * Email "In-Reply-To:"
 --in-reply-to* Populate header fields appropriately.
+--cite * Quote the message body in the cover if
+ --compose is set, else in the first patch.
 --[no-]xmailer * Add "X-Mailer:" header (default).
 --[no-]annotate* Review each patch that will be sent in an 
editor.
 --compose  * Open an editor for introduction.


Just wondering: would it make sense to activate --cite by default when
--in-reply-to=file is used, and to allow --no-cite to disable this?

This is something we can easily do now without breaking backward
compatibility (--in-reply-to=file doesn't exist yet), but would be more
painful to do later.


It's an interesting question.

IMHO we should have `--cite` by default and allow `--no-cite` to disable 
quoting the message body, because it's easier to remove extra unwanted 
lines than copying lines from another file and adding "> " before each line.



@@ -640,6 +644,7 @@ if (@files) {
usage();
 }

+my $message_cited;


Nit: I read "$message_cited" as "Boolean saying whether the message was
cited". $cited_message would be clearer to me (but this is to be taken
with a grain of salt as I'm not a native speaker), since the variable
holds the content of the cited message.


+sub do_insert_cited_message {
+   my $tmp_file = shift;
+   my $original_file = shift;
+
+   open my $c, "<", $original_file
+   or die "Failed to open $original_file: " . $!;
+
+   open my $c2, ">", $tmp_file
+   or die "Failed to open $tmp_file: " . $!;
+
+   # Insertion after the triple-dash
+   while (<$c>) {
+   print $c2 $_;
+   last if (/^---$/);
+   }
+   print $c2 $message_cited;


I would add a newline here to get a blank line between the message cited
and the diffstat.


A newline here makes it easier to distinguish the different sections in 
the email file indeed.



I think non-ascii characters would deserve particular attention here
too. For example, if the patch contain only ascii and the cited part
contains UTF-8, does the generated patch have a proper Content-type:
header?


Non-ascii characters are still an issue, I'll work on that next week.


I can imagine worse, like a patch containing latin1 character and a
cited message with another 8-bit encoding.


+test_expect_success $PREREQ 'correct cited message with --in-reply-to and 
--compose' '
+   grep "> On Sat, 12 Jun 2010 15:53:58 +0200, aut...@example.com wrote:" msgtxt3 
&&


I would prefer to have the full address including the real name here (A
) in this example. Actually, after a quick look at
the code, I don't understand where the name has gone (what's shown here
is extracted from the From: header).


Yep, after a quick look at the code involved, I don't understand either, 
I will investigate this week.

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

Re: [PATCH v4 5/6] send-email: --in-reply-to= populate header fields

[Samuel GROOT]

On 06/09/2016 11:45 AM, Matthieu Moy wrote:

Samuel GROOT  writes:


diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
index edbba3a..21776f0 100644
--- a/Documentation/git-send-email.txt
+++ b/Documentation/git-send-email.txt
@@ -84,13 +84,16 @@ See the CONFIGURATION section for 'sendemail.multiEdit'.
the value of GIT_AUTHOR_IDENT, or GIT_COMMITTER_IDENT if that is not
set, as returned by "git var -l".

---in-reply-to=::
+--in-reply-to=::
Make the first mail (or all the mails with `--no-thread`) appear as a
-   reply to the given Message-Id, which avoids breaking threads to
-   provide a new patch series.
+   reply to the given Message-Id (given directly by argument or via the 
email
+   file), which avoids breaking threads to provide a new patch series.
The second and subsequent emails will be sent as replies according to
the `--[no]-chain-reply-to` setting.
 +
+Furthermore, if the argument is an email file, parse it and populate header
+fields appropriately for the reply.


"populate header fields appropriately" would seem obscure to someone not
having followed this converation. At least s/fields/To: and Cc: fields/.


We weren't sure how precise the documentation had to be, and tried to 
keep it concise.



--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -55,6 +55,7 @@ git send-email --dump-aliases
 --[no-]bcc* Email Bcc:
 --subject * Email "Subject:"
 --in-reply-to * Email "In-Reply-To:"
+--in-reply-to* Populate header fields appropriately.


Likewise. To avoid an overly long line, I'd write just "Populate
To/Cc/In-reply-to".

Probably  should be .


Thanks, will do in v5.


+if ($initial_reply_to && -f $initial_reply_to) {
+   my $error = validate_patch($initial_reply_to);
+   die "fatal: $initial_reply_to: $error\nwarning: no patches were sent\n"
+   if $error;
+
+   open my $fh, "<", $initial_reply_to or die "can't open file 
$initial_reply_to";
+   my $mail = Git::parse_email($fh);
+   close $fh;
+
+   my $initial_sender = $sender || $repoauthor || $repocommitter || '';


This is duplicated from the "if ($compose) { ... my $tpl_sender = ..." a
bit later in the existing file. It would be better to get this "my
$initial_sender = ..." out of your "if" and use $initial_sender directly
later IMHO.

Actually, $initial_sender does not seem to be a good variable name. It's
not really "initial", right?


$sender looks like a better name, I will work on that, thanks!


+   my $prefix_re = "";
+   my $subject_re = $mail->{"subject"}[0];
+   if ($subject_re =~ /^[^Re:]/) {
+   $prefix_re = "Re: ";
+   }
+   $initial_subject = $prefix_re . $subject_re;


Why introduce $prefix_re. You can just

my $subject = $mail->{"subject"}[0];
if (...) {
$subject = "Re: " . $subject;
}

(preferably using sensible as '...' as noted by Junio ;-) ).


I will keep Junio's suggestion :-)


In previous iterations of this series, you had issues with non-ascii
characters in at least To: and Cc: fields (perhaps in the Subject field
too?). Are they solved? I don't see any tests about it ...


Non-ascii characters are still an issue, I will work on that next week.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 5/6] send-email: --in-reply-to= populate header fields

[Samuel GROOT]

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

Samuel GROOT  writes:


+if ($initial_reply_to && -f $initial_reply_to) {
+   my $error = validate_patch($initial_reply_to);


This call is wrong, isn't it?

You are not going to send out the message you are responding to (the
message may not even be a patch), and you do not want to die with an
error message that says "patch contains an overlong line".


We used to handle email files like patch files (with Cc:, To:, From:, 
... fields, a patch is almost an email, with some trailers).


But that `validate_patch` subroutine call is indeed useless here, I will 
remove it.



+   die "fatal: $initial_reply_to: $error\nwarning: no patches were sent\n"
+   if $error;
+
+   open my $fh, "<", $initial_reply_to or die "can't open file 
$initial_reply_to";
+   my $mail = Git::parse_email($fh);
+   close $fh;


my $header = Git::parse_email_header($fh);

perhaps?


Git::parse_email will be renamed into Git::parse_email_header in v5.


+   my $initial_sender = $sender || $repoauthor || $repocommitter || '';
+
+   my $prefix_re = "";
+   my $subject_re = $mail->{"subject"}[0];
+   if ($subject_re =~ /^[^Re:]/) {
+   $prefix_re = "Re: ";
+   }
+   $initial_subject = $prefix_re . $subject_re;


I am not sure what the significance of the fact that the subject
happens to begin with a letter other than 'R', 'e', or ':'.

Did you mean to do something like this instead?

my $subject = $mail->{"subject"}[0];
$subject =~ s/^(re:\s*)+//i; # strip "Re: Re: ..."
$initial_subject = "Re: $subject";

instead?


That's way cleaner, thanks!


By the way, this is a good example why your "unfold" implementation
in 4/6 is unwieldy for the caller.  Imagine a rather long subject
that is folded, i.e.

To: Samuel
Subject: Help! I am having a trouble running git-send-email
correctly.
Message-id: <...>


It's a good point. What you suggested in 4/6 reply will be used in v5.
--
To unsubscribe from this list: send the line "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: [PATCHv3] gpg-interface: check gpg signature creation status

[Jeff King]

On Tue, Jun 14, 2016 at 05:50:19PM -0400, Jeff King wrote:

> On Tue, Jun 14, 2016 at 11:13:54AM -0700, Junio C Hamano wrote:
> 
> > Michael J Gruber  writes:
> > 
> > >   bottom = signature->len;
> > > - len = strbuf_read(signature, gpg.out, 1024);
> > > + strbuf_read(signature, gpg.out, 1024);
> > > + strbuf_read(, gpg.err, 0);
> > 
> > H, isn't this asking for a deadlock?  When GPG spews more than
> > what would fit in a pipe buffer to its standard error (hence gets
> > blocked), its standard output may not complete, and the we would get
> > stuck by attempting to read from gpg.out, failing to reach the other
> > strbuf_read() that would unblock GPG by reading from gpg.err?
> 
> Yeah, it definitely is a deadlock. I think we'd need a select loop to
> read into multiple strbufs at once (we can't use "struct async" because
> that might happen in another process).

Something like this on top of Michael's patch (only lightly tested).

I'm still undecided on whether it is a better approach than making sure
the stdout we got looks sane. In particular I'd worry that it would make
things harder for somebody trying to plug in something gpg-like (e.g.,
if you wanted to do something exotic like call a program which fetched
the signature from a remote device or something). But it's probably not
_that_ hard for such a script to emulate --status-fd.

---
diff --git a/gpg-interface.c b/gpg-interface.c
index 850dc81..576e462 100644
--- a/gpg-interface.c
+++ b/gpg-interface.c
@@ -153,6 +153,7 @@ int sign_buffer(struct strbuf *buffer, struct strbuf 
*signature, const char *sig
const char *args[5];
size_t i, j, bottom;
struct strbuf err = STRBUF_INIT;
+   struct strbuf_reader readers[2];
 
gpg.argv = args;
gpg.in = -1;
@@ -183,8 +184,15 @@ int sign_buffer(struct strbuf *buffer, struct strbuf 
*signature, const char *sig
close(gpg.in);
 
bottom = signature->len;
-   strbuf_read(signature, gpg.out, 1024);
-   strbuf_read(, gpg.err, 0);
+
+   readers[0].buf = signature;
+   readers[0].fd = gpg.out;
+   readers[0].hint = 1024;
+   readers[1].buf = 
+   readers[1].fd = gpg.err;
+   readers[1].hint = 1024;
+   strbuf_read_parallel(readers, 2);
+
close(gpg.out);
close(gpg.err);
 
diff --git a/strbuf.c b/strbuf.c
index 1ba600b..f674b23 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -395,6 +395,58 @@ ssize_t strbuf_read_once(struct strbuf *sb, int fd, size_t 
hint)
return cnt;
 }
 
+void strbuf_read_parallel(struct strbuf_reader *readers, int nr)
+{
+   int i;
+   struct pollfd *pfd;
+
+   ALLOC_ARRAY(pfd, nr);
+   for (i = 0; i < nr; i++)
+   readers[i].error = 0;
+
+   while (1) {
+   int pollsize = 0;
+   int ret;
+
+   for (i = 0; i < nr; i++) {
+   if (readers[i].fd < 0)
+   continue;
+   pfd[pollsize].fd = readers[i].fd;
+   pfd[pollsize].events = POLLIN;
+   readers[i].pfd = [pollsize];
+   pollsize++;
+   }
+
+   if (!pollsize)
+   break;
+
+   ret = poll(pfd, pollsize, -1);
+   if (ret < 0) {
+   if (errno == EINTR)
+   continue;
+   /* should never happen? */
+   die_errno("poll failed");
+   }
+
+   for (i = 0; i < nr; i++) {
+   if (readers[i].fd < 0)
+   continue;
+   if (readers[i].pfd->revents &
+   (POLLIN|POLLHUP|POLLERR|POLLNVAL)) {
+   ret = strbuf_read_once(readers[i].buf,
+  readers[i].fd,
+  readers[i].hint);
+   if (ret < 0)
+   readers[i].error = errno;
+   if (ret <= 0)
+   readers[i].fd = -1;
+   }
+   }
+   }
+
+   free(pfd);
+}
+
 ssize_t strbuf_write(struct strbuf *sb, FILE *f)
 {
return sb->len ? fwrite(sb->buf, 1, sb->len, f) : 0;
diff --git a/strbuf.h b/strbuf.h
index 7987405..b93822e 100644
--- a/strbuf.h
+++ b/strbuf.h
@@ -374,6 +374,25 @@ extern ssize_t strbuf_read(struct strbuf *, int fd, size_t 
hint);
  */
 extern ssize_t strbuf_read_once(struct strbuf *, int fd, size_t hint);
 
+/**
+ * Read from several descriptors in parallel, each into its own strbuf.
+ * This can be used, for example, to capture stdout and stderr from a
+ * sub-process without worrying about deadlocks.
+ */
+struct strbuf_reader {
+   /* Initialized by caller */
+   struct strbuf *buf;
+   int fd;
+   

Re: [PATCH v4 4/6] send-email: create email parser subroutine

[Samuel GROOT]

On 06/14/2016 12:47 AM, Eric Wong wrote:

Samuel GROOT  wrote:

On 06/09/2016 02:21 AM, Eric Wong wrote:

Samuel GROOT  wrote:

Email::Simple library uses qr/\x0a\x0d|\x0d\x0a|\x0a|\x0d/ [1].
Should we handle \n\r at end of line as well?


"\n\r" can never happen with local $/ = "\n"


If the email file contains "\n\r", setting $/ = "\n" will leave "\r" at
the beginning of each line.

We could trim them with:

  s/^\r//;
  s/\r?\n$//;

But is it worth adding `s/^\r//;` to handle that extremely rare case?


I doubt it.  Having a "\r" in the wrong place is likely a bug in
whatever program that generated the email.  It should be exposed
so whoever generated that email has a chance to fix it on their
end rather than being quietly hidden.


s/\r?\n$// is fine then.

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

What's cooking in git.git (Jun 2016, #04; Tue, 14)

[Junio C Hamano]

Here are the topics that have been cooking.  Commits prefixed with
'-' are only in 'pu' (proposed updates) while commits prefixed with
'+' are in 'next'.  The ones marked with '.' do not appear in any of
the integration branches, but I am still holding onto them.

Git 2.9 has been tagged.  Let's wait for a few days to clean up
possible fallout and then start a new cycle by rewinding the tip of
'next'.  I expect I'd eject a few premature topics out of 'next'
while doing so.

You can find the changes described here in the integration branches
of the repositories listed at

http://git-blame.blogspot.com/p/git-public-repositories.html

--
[Graduated to "master"]

* jc/t2300-setup (2016-06-01) 1 commit
  (merged to 'next' on 2016-06-06 at 20f7f83)
 + t2300: run git-sh-setup in an environment that better mimics the real life
 (this branch is used by va/i18n-even-more.)

 A test fix.


* jk/diff-compact-heuristic (2016-06-10) 1 commit
 - diff: disable compaction heuristic for now

 It turns out that the earlier effort to update the heuristics may
 want to use a bit more time to mature.  Turn it off by default.


* jk/shell-portability (2016-06-01) 2 commits
  (merged to 'next' on 2016-06-06 at 5de784e)
 + t5500 & t7403: lose bash-ism "local"
 + test-lib: add in-shell "env" replacement

 test fixes.

--
[New Topics]

* lv/status-say-working-tree-not-directory (2016-06-09) 1 commit
 - Use "working tree" instead of "working directory" for git status

 "git status" used to say "working directory" when it meant "working
 tree".

 Will merge to 'next'.


* jk/parseopt-string-list (2016-06-13) 3 commits
 - blame,shortlog: don't make local option variables static
 - interpret-trailers: don't duplicate option strings
 - parse_opt_string_list: stop allocating new strings
 (this branch is used by jk/string-list-static-init.)

 The command line argument parsing that uses OPT_STRING_LIST() often
 made a copy of the argv[] element, which was unnecessary.

 Will merge to 'next'.


* jk/repack-keep-unreachable (2016-06-14) 3 commits
 - repack: extend --keep-unreachable to loose objects
 - repack: add --keep-unreachable option
 - repack: document --unpack-unreachable option

 "git repack" learned the "--keep-unreachable" option, which sends
 loose unreachable objects to a pack instead of leaving them loose.
 This helps heuristics based on the number of loose objects
 (e.g. "gc --auto").

 Will merge to 'next'.


* lf/recv-sideband-cleanup (2016-06-13) 1 commit
 - sideband.c: refactor recv_sideband()

 Code simplification.  It however loses the atomicity of the output
 9ac13ec9 (atomic write for sideband remote messages, 2006-10-11)
 tried to add to an once-much-simpler codebase.

 Expecting a reroll.


* nd/test-lib-httpd-show-error-log-in-verbose (2016-06-13) 1 commit
 - lib-httpd.sh: print error.log on error

 Debugging aid.

 Will merge to 'next'.


* pc/occurred (2016-06-10) 2 commits
 - config.c: fix misspelt "occurred" in an error message
 - refs.h: fix misspelt "occurred" in a comment

 Will merge to 'next'.


* sb/submodule-clone-retry (2016-06-13) 2 commits
 - submodule update: continue when a clone fails
 - submodule--helper: initial clone learns retry logic
 (this branch uses sb/submodule-recommend-shallowness.)

 "git submodule update" that drives many "git clone" could
 eventually hit flaky servers/network conditions on one of the
 submodules; the command learned to retry the attempt.


* jc/blame-reverse (2016-06-14) 2 commits
 - blame: dwim "blame --reverse OLD" as "blame --reverse OLD.."
 - blame: improve diagnosis for "--reverse NEW"


* jc/deref-tag (2016-06-14) 1 commit
 - blame, line-log: do not loop around deref_tag()

 Code clean-up.

 Will merge to 'next'.


* jk/fetch-prune-doc (2016-06-14) 1 commit
 - fetch: document that pruning happens before fetching

 Will merge to 'next'.


* km/fetch-do-not-free-remote-name (2016-06-14) 1 commit
 - builtin/fetch.c: don't free remote->name after fetch

 Will merge to 'next'.


* nb/gnome-keyring-build (2016-06-14) 1 commit
 - gnome-keyring: Don't hard-code pkg-config executable

 Build improvements for gnome-keyring (in contrib/)

 Will merge to 'next'.


* pb/strbuf-read-file-doc (2016-06-14) 1 commit
 - strbuf: describe the return value of strbuf_read_file

 Will merge to 'next'.

--
[Stalled]

* sb/bisect (2016-04-15) 22 commits
 - SQUASH???
 - bisect: get back halfway shortcut
 - bisect: compute best bisection in compute_relevant_weights()
 - bisect: use a bottom-up traversal to find relevant weights
 - bisect: prepare for different algorithms based on find_all
 - bisect: rename count_distance() to compute_weight()
 - bisect: make total number of commits global
 - bisect: introduce distance_direction()
 - bisect: extract get_distance() function from code duplication
 - bisect: use commit instead of commit list as 

Re: [ANNOUNCE] Sharness v1.0.0

[Junio C Hamano]

Jeff King  writes:

> Yeah, I don't think it has kept up with our work. My statement above was
> "I don't think we'd _ever_ want to consider sharness the upstream, even
> if it were up to date", but obviously there would be a lot of work to
> even get it there.

I actually was a lot more optimistic when they started, and if it
were up to date, I would have at least tried to see what the damage
to our t/ part will be if we swapped t/test-lib{,-functions}.sh with
it, which was the original reason why I took a look until I found
out that it did not have some things.

--
To unsubscribe from this list: send the line "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: [PATCHv3] gpg-interface: check gpg signature creation status

[Jeff King]

On Tue, Jun 14, 2016 at 11:13:54AM -0700, Junio C Hamano wrote:

> Michael J Gruber  writes:
> 
> > bottom = signature->len;
> > -   len = strbuf_read(signature, gpg.out, 1024);
> > +   strbuf_read(signature, gpg.out, 1024);
> > +   strbuf_read(, gpg.err, 0);
> 
> H, isn't this asking for a deadlock?  When GPG spews more than
> what would fit in a pipe buffer to its standard error (hence gets
> blocked), its standard output may not complete, and the we would get
> stuck by attempting to read from gpg.out, failing to reach the other
> strbuf_read() that would unblock GPG by reading from gpg.err?

Yeah, it definitely is a deadlock. I think we'd need a select loop to
read into multiple strbufs at once (we can't use "struct async" because
that might happen in another process).

-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: [ANNOUNCE] Sharness v1.0.0

[Jeff King]

On Tue, Jun 14, 2016 at 02:43:19PM -0700, Junio C Hamano wrote:

> Jeff King  writes:
> 
> > I don't think the Git project would ever want to say "sharness is the
> > upstream, and we are now just a user of it". But I wonder if we could
> > break down test-lib.sh to keep the Git-specific parts separate, which
> > would make it easier for sharness to pull the other bits as a whole.
> 
> I took a quick look around, and it seems that this is an outdated
> fork made without getting much of the improvement from its upstream
> since it forked.  It does not seem to have lazy prerequisite, for
> example.

Yeah, I don't think it has kept up with our work. My statement above was
"I don't think we'd _ever_ want to consider sharness the upstream, even
if it were up to date", but obviously there would be a lot of work to
even get it there.

-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: compactionHeuristic=true is not used by interactive staging

[Junio C Hamano]

Jeff King  writes:

> Nobody noticed so far because originally the compaction heuristic was on
> by default, and so just worked everywhere. But we backed off on that at
> the last minute after finding a few cases where the diff looks worse.

Yup, and that is why this is called "experimental" in the release
notes ;-)
--
To unsubscribe from this list: send the line "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: [ANNOUNCE] Sharness v1.0.0

[Junio C Hamano]

Jeff King  writes:

> I don't think the Git project would ever want to say "sharness is the
> upstream, and we are now just a user of it". But I wonder if we could
> break down test-lib.sh to keep the Git-specific parts separate, which
> would make it easier for sharness to pull the other bits as a whole.

I took a quick look around, and it seems that this is an outdated
fork made without getting much of the improvement from its upstream
since it forked.  It does not seem to have lazy prerequisite, for
example.

--
To unsubscribe from this list: send the line "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: compactionHeuristic=true is not used by interactive staging

[Jeff King]

On Tue, Jun 14, 2016 at 04:22:54PM +0200, Alex Prengère wrote:

> Hello,
> I just did a fresh clone of git from Github and installed the version
> 2.9.0 on Fedora 22.
> 
> I tried the new compactionHeuristic = true, which is awesome.
> The only thing that struck me was that this option was not used when
> doing an interactive staging, meaning `git diff` and `git add -p` will
> format patches differently. Perhaps this is intended and there is a
> way to force interactive staging to use specific diff options, but I
> did not find it in the doc.

That's because it's handled in the "UI config", and plumbing commands
are not affected (and "add -p" is built on plumbing commands). The same
is true of diff.algorithm, for instance.

To make this work, add--interactive would have to manually enable
particular options that it thinks it can handle (and in fact this is
done with diff.algorithm already). So we'd need a patch similar to
2cc0f53 (add--interactive: respect diff.algorithm, 2013-06-12).

Nobody noticed so far because originally the compaction heuristic was on
by default, and so just worked everywhere. But we backed off on that at
the last minute after finding a few cases where the diff looks worse.

-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: 'untracked working tree files would be overwritten by merge' on ignored files?

[Andreas Krey]

On Tue, 14 Jun 2016 10:06:16 +, Junio C Hamano wrote:
...
> 
> IIRC, untracked files are kept during merge and across checking out
> another branch.  Files that are deliberately marked as ignored by
> listing them to .gitignore mechanism are considered expendable, and
> they will be removed as necessary.

Apparently not. Waitaminute - I can check out the other branch,
and then the ignored file is replaced by the versioned file there,
as you say.

But when I try to merge the other branch, merge complains as in $subject.

That is, 'git merge other' does not work, but 'git checkout out;
git checkout -; git merge other' works because then the ignored file is
removed in the first checkout and no longer in the way for the merge:

$ git status --ignored
On branch side
Ignored files:
  (use "git add -f ..." to include in what will be committed)

a.txt

nothing to commit, working directory clean
$ git merge master
error: The following untracked working tree files would be overwritten by 
merge:
a.txt
Please move or remove them before you can merge.
Aborting
$ git checkout master
Switched to branch 'master'
$ git checkout -
Switched to branch 'side'
$ git merge master
Merge made by the 'recursive' strategy.
 a.txt | 1 +
 1 file changed, 1 insertion(+)
 create mode 100644 a.txt
$

Andreas

-- 
"Totally trivial. Famous last words."
From: Linus Torvalds 
Date: Fri, 22 Jan 2010 07:29:21 -0800
--
To unsubscribe from this list: send the line "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: [ANNOUNCE] Sharness v1.0.0

[Jeff King]

On Tue, Jun 14, 2016 at 09:34:17PM +0200, Christian Couder wrote:

> Version 1.0.0 of Sharness [1] -- the test harness library derived from
> Git's test lib -- is released.

Cool. Git's test harness is something I really miss having in other
projects. I'm glad this project exists. :)

> This release contains many upstream fixes and improvements from Git
> and a lot of specific user contributed features [2].

It looks like it takes some manual work to massage upstream improvements
into Sharness.

I don't think the Git project would ever want to say "sharness is the
upstream, and we are now just a user of it". But I wonder if we could
break down test-lib.sh to keep the Git-specific parts separate, which
would make it easier for sharness to pull the other bits as a whole.

I dunno. I guess it would depend on how invasive the patches were, but I
would not be opposed to such a thing. I suspect the separation might
actually make our test setup more clear.

-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] Refactor recv_sideband()

[Junio C Hamano]

Lukas Fleischer  writes:

Lukas Fleischer  writes:

> Improve the readability of recv_sideband() significantly by replacing

s/significantly //; "making it readable" is already a subjective
goodness criterion, and you do not have to make it sound even more
subjective.  Let the updated result convince the reader that it is
vastly more readable.

> Also, reorganize the overall control flow, remove some superfluous
> variables and replace a custom implementation of strpbrk() with a call
> to the standard C library function.

I find that calling the loop "a custom implementation" is a bit
unfair.  The original tried to avoid looking beyond "len", but in
the updated code because you have buf[len] = '\0' to terminate the
line, and because you pass LARGE_PACKET_MAX to packet_read() while
your buf[] allocates one more byte, you can use strpbrk() here
safely. Which would mean "a custom implementation" was done for a
reason.

But that is a minor point.

I'll omit the preimage lines from the following.

>  int recv_sideband(const char *me, int in_stream, int out)
>  {
> + const char *term, *suffix;
> + char buf[LARGE_PACKET_MAX + 1];
> + struct strbuf outbuf = STRBUF_INIT;
> + const char *b, *brk;
>  
> + strbuf_addf(, "%s", PREFIX);

I highly suspect that you are better off without this line being
here.

> ...
>   while (1) {
>   int band, len;
> + 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[0] & 0xff;
> + buf[len] = '\0';
>   len--;
>   switch (band) {
>   case 3:
> + fprintf(stderr, "%s%s\n", PREFIX, buf + 1);
>   return SIDEBAND_REMOTE_ERROR;

Two "return"s we see above will leak outbuf.buf that holds PREFIX.

>   case 2:
> + b = buf + 1;
>  
> + /*
> +  * 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;
>  
> + if (linelen > 0) {
> + strbuf_addf(, "%.*s%s%c",
> + linelen, b, suffix, *brk);
>   } else {
> + strbuf_addf(, "%c", *brk);
>   }
> + xwrite(STDERR_FILENO, outbuf.buf, outbuf.len);
> + strbuf_reset();
> + strbuf_addf(, "%s", PREFIX);

Instead of doing "we assume outbuf already has PREFIX when we add
contents from buf[]", the code structure would be better if you:

 * make outbuf.buf contain PREFIX at the beginning of this innermost
   loop; lose the reset/addf from here.

 * move strbuf_reset() at the end of the next if (*b) block
   to just before "continue;"

perhaps?

> + b = brk + 1;
> + }
>  
> + if (*b) {
> + xwrite(STDERR_FILENO, outbuf.buf, outbuf.len);
> + /* Incomplete line, skip the next prefix. */
> + strbuf_reset();
> + }
>   continue;
>   case 1:
> + write_or_die(out, buf + 1, len);
>   continue;
>   default:
>   fprintf(stderr, "%s: protocol error: bad band #%d\n",
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] do not loop around deref_tag()

[Jeff King]

On Tue, Jun 14, 2016 at 01:28:39PM -0700, Junio C Hamano wrote:

> These callers appear to expect that deref_tag() is to peel one layer
> of a tag, but the function does not work that way; it has its own
> loop to unwrap tags until an object that is not a tag appears.

Looks obviously correct.

-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: [ANNOUNCE] Sharness v1.0.0

[Christian Couder]

On Tue, Jun 14, 2016 at 9:48 PM, Stefan Beller  wrote:
> On Tue, Jun 14, 2016 at 12:34 PM, Christian Couder
>  wrote:
>>
>> Sharness was first announced on this list in July 2012 [4]. It was
>> created from Git's test lib in April 2011 by Mathias Lafeldt who
>> improved and maintained it until a few days ago.
>>
>> Thanks a lot Mathias!
>
> Thanks for picking up the work, Christian!
>
> Is there any word out there from Mathias on making you the designated
> new maintainer? (I cannot tell if this is a genuine maintainer change, or
> a [hostile] fork by reading this email, and I don't know much of the context,

Mathias gave me commiter rights on the repo
(https://github.com/mlafeldt/sharness at that time) one year ago.

A few days ago, as I wanted since last December to release v1.0.0, but
was uneasy to do so without at least a LGTM from him, I contacted him
by private email and he decided to transfer the repo to
https://github.com/chriscool/sharness, to give me maintainership and
to step down completely. So yeah this is a genuine maintainer change
and not an hostile fork at all.
(I cc'ed Mathias again so he can confirm if he wants.)

> as I did not know git Git test suite was a stand alone thing)

I am not sure it qualifies as a stand alone thing there are some
differences and for now some features are going from Git's test-lib.sh
into Sharness but not the other way around.
--
To unsubscribe from this list: send the line "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] Refactor recv_sideband()

[Lukas Fleischer]

On Tue, 14 Jun 2016 at 23:00:38, Lukas Fleischer wrote:
> Improve the readability of recv_sideband() significantly by replacing
> fragile buffer manipulations with string buffers and more sophisticated
> format strings. Note that each line is printed using a single write()
> syscall to avoid garbled output when multiple processes write to stderr
> in parallel, see 9ac13ec (atomic write for sideband remote messages,
> 2006-10-11) for details.
> 
> Also, reorganize the overall control flow, remove some superfluous
> variables and replace a custom implementation of strpbrk() with a call
> to the standard C library function.
> 
> Signed-off-by: Lukas Fleischer 
> ---
> Now uses a single write() invocation per line. Thanks to Nicolas and
> Junio for bringing 9ac13ec to my attention.
> 
>  sideband.c | 97 
> +-
>  1 file changed, 33 insertions(+), 64 deletions(-)
> 
> diff --git a/sideband.c b/sideband.c
> index fde8adc..8340a1b 100644
> --- a/sideband.c
> +++ b/sideband.c
> @@ -13,103 +13,72 @@
> [...]
> +   struct strbuf outbuf = STRBUF_INIT;
> [...]
> +   strbuf_reset();
> +   strbuf_addf(, "%s", PREFIX);
> [...]
> +   /* Incomplete line, skip the next prefix. */
> +   strbuf_reset();

Missing strbuf_release() at the end of the function, I guess. I will
wait for further comments and send v3 tomorrow.

Lukas
--
To unsubscribe from this list: send the line "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] Refactor recv_sideband()

[Lukas Fleischer]

Improve the readability of recv_sideband() significantly by replacing
fragile buffer manipulations with string buffers and more sophisticated
format strings. Note that each line is printed using a single write()
syscall to avoid garbled output when multiple processes write to stderr
in parallel, see 9ac13ec (atomic write for sideband remote messages,
2006-10-11) for details.

Also, reorganize the overall control flow, remove some superfluous
variables and replace a custom implementation of strpbrk() with a call
to the standard C library function.

Signed-off-by: Lukas Fleischer 
---
Now uses a single write() invocation per line. Thanks to Nicolas and
Junio for bringing 9ac13ec to my attention.

 sideband.c | 97 +-
 1 file changed, 33 insertions(+), 64 deletions(-)

diff --git a/sideband.c b/sideband.c
index fde8adc..8340a1b 100644
--- a/sideband.c
+++ b/sideband.c
@@ -13,103 +13,72 @@
  * 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];
+   struct strbuf outbuf = STRBUF_INIT;
+   const char *b, *brk;
 
-   memcpy(buf, PREFIX, pf);
+   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 string must be
-* present too.
-*/
-   if (brk > (skip_pf ? 0 : (pf+1 + 1))) {
-  

[PATCH] do not loop around deref_tag()

[Junio C Hamano]

These callers appear to expect that deref_tag() is to peel one layer
of a tag, but the function does not work that way; it has its own
loop to unwrap tags until an object that is not a tag appears.

Signed-off-by: Junio C Hamano 
---
 builtin/blame.c | 6 ++
 line-log.c  | 3 +--
 2 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/builtin/blame.c b/builtin/blame.c
index 21f42b0..7417edf 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -2425,8 +2425,7 @@ static struct commit *find_single_final(struct rev_info 
*revs,
struct object *obj = revs->pending.objects[i].item;
if (obj->flags & UNINTERESTING)
continue;
-   while (obj->type == OBJ_TAG)
-   obj = deref_tag(obj, NULL, 0);
+   obj = deref_tag(obj, NULL, 0);
if (obj->type != OBJ_COMMIT)
die("Non commit %s?", revs->pending.objects[i].name);
if (found)
@@ -2461,8 +2460,7 @@ static char *prepare_initial(struct scoreboard *sb)
struct object *obj = revs->pending.objects[i].item;
if (!(obj->flags & UNINTERESTING))
continue;
-   while (obj->type == OBJ_TAG)
-   obj = deref_tag(obj, NULL, 0);
+   obj = deref_tag(obj, NULL, 0);
if (obj->type != OBJ_COMMIT)
die("Non commit %s?", revs->pending.objects[i].name);
if (sb->final)
diff --git a/line-log.c b/line-log.c
index bbe31ed..1fbbe4f 100644
--- a/line-log.c
+++ b/line-log.c
@@ -480,8 +480,7 @@ static struct commit *check_single_commit(struct rev_info 
*revs)
struct object *obj = revs->pending.objects[i].item;
if (obj->flags & UNINTERESTING)
continue;
-   while (obj->type == OBJ_TAG)
-   obj = deref_tag(obj, NULL, 0);
+   obj = deref_tag(obj, NULL, 0);
if (obj->type != OBJ_COMMIT)
die("Non commit %s?", revs->pending.objects[i].name);
if (commit)
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Refactor recv_sideband()

[Nicolas Pitre]

On Tue, 14 Jun 2016, Lukas Fleischer wrote:

> On Tue, 14 Jun 2016 at 19:55:06, Nicolas Pitre wrote:
> > It is not buffered as it writes to stderr. And some C libs do separate 
> > calls to write() for every string format specifier. So "%s%s%c" may end 
> > up calling write() 3 times depending on the implementation.  The example 
> > I gave in commit ed1902ef5c is real and I even observed it with strace 
> > back then.
> 
> Ah, right, it is not buffered. Forgot that we are talking about stderr.
> 
> I still do not see how that could become a real problem, though. Apart 
> from the (purely theoretical?) performance issue, the only way 
> multiple write() calls could become a problem is when a single 
> fprintf() invocation is interrupted between two write() invocations 
> and then, there is a write() call from another thread, right?

Right. More another process in this case.

> I do not know whether we print to stderr from different threads but 
> even then, each fprintf() invocation should be atomic if I do not 
> misinterpret the POSIX specification. Could you please clarify? Are 
> there separate processes involved?

Yes, the intermixed writes come from another process such as 
git-index-pack via git-fetch-pack.


Nicolas
--
To unsubscribe from this list: send the line "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: [ANNOUNCE] Sharness v1.0.0

[Stefan Beller]

On Tue, Jun 14, 2016 at 12:34 PM, Christian Couder
 wrote:
> Hi,
>
> Version 1.0.0 of Sharness [1] -- the test harness library derived from
> Git's test lib -- is released.
>
> This release contains many upstream fixes and improvements from Git
> and a lot of specific user contributed features [2].
>
> It's the first release since v0.3.0 in April 2013 [3].
>
> Sharness was first announced on this list in July 2012 [4]. It was
> created from Git's test lib in April 2011 by Mathias Lafeldt who
> improved and maintained it until a few days ago.
>
> Thanks a lot Mathias!

Thanks for picking up the work, Christian!

Is there any word out there from Mathias on making you the designated
new maintainer? (I cannot tell if this is a genuine maintainer change, or
a [hostile] fork by reading this email, and I don't know much of the context,
as I did not know git Git test suite was a stand alone thing)

Thanks,
Stefan

>
> Thanks also to all the other contributors (by alphabetical order)
> since the beginning:
>
> Alexander Sulfrian
> Dennis Kaarsemaker
> Jeff King
> John Keeping
> Konstantin Koroviev
> Mark A. Grondona
> Matthieu Moy
> Maxim Bublis
> Richard Hansen
> Roman Neuhauser
> Simon Chiang
>
> Thanks also to all the Git contributors who improved test-lib.sh as
> many of the features and fixes come from upstream!
>
> Enjoy,
> Christian.
>
> [1] https://github.com/chriscool/sharness
> [2] https://github.com/chriscool/sharness/blob/master/CHANGELOG.md
> [3] http://thread.gmane.org/gmane.comp.version-control.git/219944/
> [4] http://thread.gmane.org/gmane.comp.version-control.git/201591/
> --
> To unsubscribe from this list: send the line "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

[ANNOUNCE] Sharness v1.0.0

[Christian Couder]

Hi,

Version 1.0.0 of Sharness [1] -- the test harness library derived from
Git's test lib -- is released.

This release contains many upstream fixes and improvements from Git
and a lot of specific user contributed features [2].

It's the first release since v0.3.0 in April 2013 [3].

Sharness was first announced on this list in July 2012 [4]. It was
created from Git's test lib in April 2011 by Mathias Lafeldt who
improved and maintained it until a few days ago.

Thanks a lot Mathias!

Thanks also to all the other contributors (by alphabetical order)
since the beginning:

Alexander Sulfrian
Dennis Kaarsemaker
Jeff King
John Keeping
Konstantin Koroviev
Mark A. Grondona
Matthieu Moy
Maxim Bublis
Richard Hansen
Roman Neuhauser
Simon Chiang

Thanks also to all the Git contributors who improved test-lib.sh as
many of the features and fixes come from upstream!

Enjoy,
Christian.

[1] https://github.com/chriscool/sharness
[2] https://github.com/chriscool/sharness/blob/master/CHANGELOG.md
[3] http://thread.gmane.org/gmane.comp.version-control.git/219944/
[4] http://thread.gmane.org/gmane.comp.version-control.git/201591/
--
To unsubscribe from this list: send the line "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] Refactor recv_sideband()

[Junio C Hamano]

Lukas Fleischer  writes:

> One possible solution is using strbuf and constructing the message as we
> did before. However, that still relies on fprintf() only calling write()
> once per format specifier. While that is probably true for all existing
> implementations, I don't think it is guaranteed by some standard.
> Shouldn't we always use the stderr stream when printing error messages
> instead, especially when we care about thread safety?

Or you can always write(2) to fd=2 and that is safe, too.
--
To unsubscribe from this list: send the line "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] Refactor recv_sideband()

[Lukas Fleischer]

On Tue, 14 Jun 2016 at 20:11:12, Junio C Hamano wrote:
> Nicolas Pitre  writes:
> 
> > It is not buffered as it writes to stderr. And some C libs do separate 
> > calls to write() for every string format specifier. So "%s%s%c" may end 
> > up calling write() 3 times depending on the implementation.  The example 
> > I gave in commit ed1902ef5c is real and I even observed it with strace 
> > back then.
> 
> I think you meant 9ac13ec9 (atomic write for sideband remote
> messages, 2006-10-11).
> 
> IIRC, back then we did't use to make as much use of strbuf API as we
> do today; if we were doing that commit today, we would be doing
> strbuf, I would suspect.

Thanks for looking up the right commit. I think I might see what is
going on; however, the situation is a bit different to the situation we
had at the time of writing 9ac13ec9 (atomic write for sideband remote
messages, 2006-10-11). Before that, we called write() manually a couple
of times. Now, we use fprintf() which makes stronger guarantees about
thread safety. However, if I understand correctly, there is still one
issue: It seems like in some places, we also directly write() to file
descriptor 2. So the following might happen: An fprintf() implementation
that calls write() once per format specifier is used. The fprintf()
function is called once from one thread, then interrupted between two
write() syscalls. In the meantime, output is written directly to file
descriptor 2 using write().

One possible solution is using strbuf and constructing the message as we
did before. However, that still relies on fprintf() only calling write()
once per format specifier. While that is probably true for all existing
implementations, I don't think it is guaranteed by some standard.
Shouldn't we always use the stderr stream when printing error messages
instead, especially when we care about thread safety?

Regards,
Lukas
--
To unsubscribe from this list: send the line "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/1] Don't free remote->name after fetch

[Junio C Hamano]

Keith McGuigan  writes:

> Yeah that was the only place I found where it was doing the strdup
> already (and in that situation, it has to).  All the other places just
> grabbed remote->name.
>
> Yes, sorry, I can sign off on this.  Do you want me to resend with the
> header in place, or is this confirmation good enough?

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

Re: [PATCH v2 1/1] Don't free remote->name after fetch

[Keith McGuigan]

Yeah that was the only place I found where it was doing the strdup
already (and in that situation, it has to).  All the other places just
grabbed remote->name.

Yes, sorry, I can sign off on this.  Do you want me to resend with the
header in place, or is this confirmation good enough?

--
- Keith

On Tue, Jun 14, 2016 at 2:49 PM, Junio C Hamano  wrote:
> kmcgui...@twopensource.com writes:
>
>> From: Keith McGuigan 
>>
>> Make fetch's string_list of remote names owns all of its string items
>> (strdup'ing when necessary) so that it can deallocate them safely when
>> clearing.
>>
>> ---
>
> OK.
>
> When I pointed out the call to string_list_append() in
> get_remote_group() as one example, I did not check if there are
> others that need similar adjustment.  I just skimmed through the
> file again and it seems there is none, so this change looks good.
>
> I assume you meant to sign this off, too?
>
>>  builtin/fetch.c | 6 ++
>>  1 file changed, 2 insertions(+), 4 deletions(-)
>>
>> diff --git a/builtin/fetch.c b/builtin/fetch.c
>> index 630ae6a1bb78..1b4e924bd222 100644
>> --- a/builtin/fetch.c
>> +++ b/builtin/fetch.c
>> @@ -1071,7 +1071,7 @@ static int get_remote_group(const char *key, const 
>> char *value, void *priv)
>>   size_t wordlen = strcspn(value, " \t\n");
>>
>>   if (wordlen >= 1)
>> - string_list_append(g->list,
>> + string_list_append_nodup(g->list,
>>  xstrndup(value, wordlen));
>>   value += wordlen + (value[wordlen] != '\0');
>>   }
>> @@ -1261,7 +1261,7 @@ done:
>>  int cmd_fetch(int argc, const char **argv, const char *prefix)
>>  {
>>   int i;
>> - struct string_list list = STRING_LIST_INIT_NODUP;
>> + struct string_list list = STRING_LIST_INIT_DUP;
>>   struct remote *remote;
>>   int result = 0;
>>   struct argv_array argv_gc_auto = ARGV_ARRAY_INIT;
>> @@ -1347,8 +1347,6 @@ int cmd_fetch(int argc, const char **argv, const char 
>> *prefix)
>>   argv_array_clear();
>>   }
>>
>> - /* All names were strdup()ed or strndup()ed */
>> - list.strdup_strings = 1;
>>   string_list_clear(, 0);
>>
>>   close_all_packs();
--
To unsubscribe from this list: send the line "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/1] Don't free remote->name after fetch

[Junio C Hamano]

kmcgui...@twopensource.com writes:

> From: Keith McGuigan 
>
> Make fetch's string_list of remote names owns all of its string items
> (strdup'ing when necessary) so that it can deallocate them safely when
> clearing.
>
> ---

OK.

When I pointed out the call to string_list_append() in
get_remote_group() as one example, I did not check if there are
others that need similar adjustment.  I just skimmed through the
file again and it seems there is none, so this change looks good.

I assume you meant to sign this off, too?

>  builtin/fetch.c | 6 ++
>  1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/builtin/fetch.c b/builtin/fetch.c
> index 630ae6a1bb78..1b4e924bd222 100644
> --- a/builtin/fetch.c
> +++ b/builtin/fetch.c
> @@ -1071,7 +1071,7 @@ static int get_remote_group(const char *key, const char 
> *value, void *priv)
>   size_t wordlen = strcspn(value, " \t\n");
>  
>   if (wordlen >= 1)
> - string_list_append(g->list,
> + string_list_append_nodup(g->list,
>  xstrndup(value, wordlen));
>   value += wordlen + (value[wordlen] != '\0');
>   }
> @@ -1261,7 +1261,7 @@ done:
>  int cmd_fetch(int argc, const char **argv, const char *prefix)
>  {
>   int i;
> - struct string_list list = STRING_LIST_INIT_NODUP;
> + struct string_list list = STRING_LIST_INIT_DUP;
>   struct remote *remote;
>   int result = 0;
>   struct argv_array argv_gc_auto = ARGV_ARRAY_INIT;
> @@ -1347,8 +1347,6 @@ int cmd_fetch(int argc, const char **argv, const char 
> *prefix)
>   argv_array_clear();
>   }
>  
> - /* All names were strdup()ed or strndup()ed */
> - list.strdup_strings = 1;
>   string_list_clear(, 0);
>  
>   close_all_packs();
--
To unsubscribe from this list: send the line "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] blame: dwim "blame --reverse OLD" as "blame --reverse OLD.."

[Junio C Hamano]

Instead of always requiring both ends of a range, we could DWIM
"OLD", which could be a misspelt "OLD..", to be a range that ends at
the current commit.

Signed-off-by: Junio C Hamano 
---

 * I am not convinced that this is a good change, though.  It is
   true that there is no other sensible interpretation of the user's
   intent when --reverse is given a single positive commit without
   any other revision, but at the same time, this feels a bit too
   much special casing that could hurt casual users when they are
   still forming their mental world model by learning from examples.

 Documentation/blame-options.txt |  5 +++--
 builtin/blame.c | 38 ++
 2 files changed, 41 insertions(+), 2 deletions(-)

diff --git a/Documentation/blame-options.txt b/Documentation/blame-options.txt
index 02cb684..6c6c78f 100644
--- a/Documentation/blame-options.txt
+++ b/Documentation/blame-options.txt
@@ -28,12 +28,13 @@ include::line-range-format.txt[]
 -S ::
Use revisions from revs-file instead of calling linkgit:git-rev-list[1].
 
---reverse::
+--reverse ..::
Walk history forward instead of backward. Instead of showing
the revision in which a line appeared, this shows the last
revision in which a line has existed. This requires a range of
revision like START..END where the path to blame exists in
-   START.
+   START.  `git blame --reverse START` is taken as `git blame
+   --reverse START..HEAD` for convenience.
 
 -p::
 --porcelain::
diff --git a/builtin/blame.c b/builtin/blame.c
index a027b8a..574b47d 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -2447,6 +2447,41 @@ static char *prepare_final(struct scoreboard *sb)
return xstrdup_or_null(name);
 }
 
+static const char *dwim_reverse_initial(struct scoreboard *sb)
+{
+   /*
+* DWIM "git blame --reverse ONE -- PATH" as
+* "git blame --reverse ONE..HEAD -- PATH" but only do so
+* when it makes sense.
+*/
+   struct object *obj;
+   struct commit *head_commit;
+   unsigned char head_sha1[20];
+
+   if (sb->revs->pending.nr != 1)
+   return NULL;
+
+   /* Is that sole rev a committish? */
+   obj = sb->revs->pending.objects[0].item;
+   obj = deref_tag(obj, NULL, 0);
+   if (obj->type != OBJ_COMMIT)
+   return NULL;
+
+   /* Do we have HEAD? */
+   if (!resolve_ref_unsafe("HEAD", RESOLVE_REF_READING, head_sha1, NULL))
+   return NULL;
+   head_commit = lookup_commit_reference_gently(head_sha1, 1);
+   if (!head_commit)
+   return NULL;
+
+   /* Turn "ONE" into "ONE..HEAD" then */
+   obj->flags |= UNINTERESTING;
+   add_pending_object(sb->revs, _commit->object, "HEAD");
+
+   sb->final = (struct commit *)obj;
+   return sb->revs->pending.objects[0].name;
+}
+
 static char *prepare_initial(struct scoreboard *sb)
 {
int i;
@@ -2472,6 +2507,9 @@ static char *prepare_initial(struct scoreboard *sb)
sb->final = (struct commit *) obj;
final_commit_name = revs->pending.objects[i].name;
}
+
+   if (!final_commit_name)
+   final_commit_name = dwim_reverse_initial(sb);
if (!final_commit_name)
die("No commit to dig up from?");
return xstrdup(final_commit_name);
--
To unsubscribe from this list: send the line "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/1] Don't free remote->name after fetch

[kmcguigan]

From: Keith McGuigan 

Make fetch's string_list of remote names owns all of its string items
(strdup'ing when necessary) so that it can deallocate them safely when
clearing.

---
 builtin/fetch.c | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/builtin/fetch.c b/builtin/fetch.c
index 630ae6a1bb78..1b4e924bd222 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -1071,7 +1071,7 @@ static int get_remote_group(const char *key, const char 
*value, void *priv)
size_t wordlen = strcspn(value, " \t\n");
 
if (wordlen >= 1)
-   string_list_append(g->list,
+   string_list_append_nodup(g->list,
   xstrndup(value, wordlen));
value += wordlen + (value[wordlen] != '\0');
}
@@ -1261,7 +1261,7 @@ done:
 int cmd_fetch(int argc, const char **argv, const char *prefix)
 {
int i;
-   struct string_list list = STRING_LIST_INIT_NODUP;
+   struct string_list list = STRING_LIST_INIT_DUP;
struct remote *remote;
int result = 0;
struct argv_array argv_gc_auto = ARGV_ARRAY_INIT;
@@ -1347,8 +1347,6 @@ int cmd_fetch(int argc, const char **argv, const char 
*prefix)
argv_array_clear();
}
 
-   /* All names were strdup()ed or strndup()ed */
-   list.strdup_strings = 1;
string_list_clear(, 0);
 
close_all_packs();
-- 
2.8.0.rc3.94.g2dc3105.dirty

--
To unsubscribe from this list: send the line "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: [PATCHv3] gpg-interface: check gpg signature creation status

[Junio C Hamano]

Michael J Gruber  writes:

>   bottom = signature->len;
> - len = strbuf_read(signature, gpg.out, 1024);
> + strbuf_read(signature, gpg.out, 1024);
> + strbuf_read(, gpg.err, 0);

H, isn't this asking for a deadlock?  When GPG spews more than
what would fit in a pipe buffer to its standard error (hence gets
blocked), its standard output may not complete, and the we would get
stuck by attempting to read from gpg.out, failing to reach the other
strbuf_read() that would unblock GPG by reading from gpg.err?
--
To unsubscribe from this list: send the line "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] Refactor recv_sideband()

[Junio C Hamano]

Nicolas Pitre  writes:

> It is not buffered as it writes to stderr. And some C libs do separate 
> calls to write() for every string format specifier. So "%s%s%c" may end 
> up calling write() 3 times depending on the implementation.  The example 
> I gave in commit ed1902ef5c is real and I even observed it with strace 
> back then.

I think you meant 9ac13ec9 (atomic write for sideband remote
messages, 2006-10-11).

IIRC, back then we did't use to make as much use of strbuf API as we
do today; if we were doing that commit today, we would be doing
strbuf, I would suspect.
--
To unsubscribe from this list: send the line "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] Refactor recv_sideband()

[Nicolas Pitre]

On Tue, 14 Jun 2016, Lukas Fleischer wrote:

> Hi Nicolas,
> 
> On Tue, 14 Jun 2016 at 19:09:15, Nicolas Pitre wrote:
> > I just looked again at all the contraptions _I_ wrote (not Junio's) for 
> > a reason why I went to such extremes in making this code co complicated.
> > 
> > One aspect that is now lost with your patch is the atomic nature of the 
> > write.  See commit ed1902ef5c for the explanation.  You could probably 
> > use sprintf() into a temporary buffer and write it in one go to avoid 
> > segmented writes from the C library. It's probably not worth having that 
> > complex code just to avoid a string copy.
> 
> The old code calls fprintf() once per line and so does the new code. The
> only difference is that in the old code, the single parts were
> concatenated manually while the new code tells fprintf() to do the
> concatenation itself while printing. Also note that fprintf() is
> buffered -- so even if the new code would call it more often, it would
> not really matter.

It is not buffered as it writes to stderr. And some C libs do separate 
calls to write() for every string format specifier. So "%s%s%c" may end 
up calling write() 3 times depending on the implementation.  The example 
I gave in commit ed1902ef5c is real and I even observed it with strace 
back then.


Nicolas
--
To unsubscribe from this list: send the line "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] Don't free remote->name after fetch

[Junio C Hamano]

Keith McGuigan  writes:

> As an alternative, I could xstrdup each instance where remote->name is 
> appended,
> which would make the string_list a homogenous dup'd list, which we
> could then free.

Yeah, I think that is the right way to fix it, even though I agree
with you that a small leak you introduced is probably better than
unwanted freeing we currently have.

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

Re: [PATCH v7 02/40] builtin/apply: make apply_patch() return -1 instead of die()ing

[Junio C Hamano]

Christian Couder  writes:

>> Before this patch, the program said "fatal: $message" and exited
>> with status = 128.  All these changes in this step modifies the
>> external behaviour and make it say "error: $message" and exit with
>> status = 1 (at least the caller in apply_all_patches() does so).
>>
>> Will that be an issue for the calling scripts?
>
> Hopefully the scripts don't check the specific error code and message.

The error codes are designed to be a vehicle that carries
information in a robust way, so it is perfectly understandable if
the callers used it to tell hopelessly broken input that caused
"die" and an otherwise sensible patch that does not happen to
apply.  Whoever is changing the external behaviour is responsible
for making sure the existing callers are not broken.

Hope is not a sound software engineering practice.
--
To unsubscribe from this list: send the line "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] blame: improve diagnosis for "--reverse NEW"

[Junio C Hamano]

"git blame --reverse OLD..NEW -- PATH" tells us to start from the
contents in PATH at OLD and observe how each line is changed while
the history developers up to NEW, and report for each line the
latest commit up to which the line survives in the original form.

If you say "git blame --reverse NEW -- PATH" by mistake, we complain
about the missing OLD, but we phrased it as "No commit to dig down
to?"  In this case, however, we are digging up from OLD, so say so.

Signed-off-by: Junio C Hamano 
---

 * There was another instance of "dig down to" in the same function.

 Documentation/git-blame.txt | 2 +-
 builtin/blame.c | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-blame.txt b/Documentation/git-blame.txt
index ba54175..16323eb 100644
--- a/Documentation/git-blame.txt
+++ b/Documentation/git-blame.txt
@@ -10,7 +10,7 @@ SYNOPSIS
 [verse]
 'git blame' [-c] [-b] [-l] [--root] [-t] [-f] [-n] [-s] [-e] [-p] [-w] 
[--incremental]
[-L ] [-S ] [-M] [-C] [-C] [-C] [--since=]
-   [--progress] [--abbrev=] [ | --contents  | --reverse 
]
+   [--progress] [--abbrev=] [ | --contents  | --reverse 
..]
[--] 
 
 DESCRIPTION
diff --git a/builtin/blame.c b/builtin/blame.c
index 21f42b0..a027b8a 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -2466,14 +2466,14 @@ static char *prepare_initial(struct scoreboard *sb)
if (obj->type != OBJ_COMMIT)
die("Non commit %s?", revs->pending.objects[i].name);
if (sb->final)
-   die("More than one commit to dig down to %s and %s?",
+   die("More than one commit to dig up from, %s and %s?",
revs->pending.objects[i].name,
final_commit_name);
sb->final = (struct commit *) obj;
final_commit_name = revs->pending.objects[i].name;
}
if (!final_commit_name)
-   die("No commit to dig down to?");
+   die("No commit to dig up from?");
return xstrdup(final_commit_name);
 }
 
-- 
2.9.0-290-g8794d48



--
To unsubscribe from this list: send the line "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] blame: improve diagnosis for "--reverse NEW"

[Junio C Hamano]

"git blame --reverse OLD..NEW -- PATH" tells us to start from the
contents in PATH at OLD and observe how each line is changed while
the history developers up to NEW, and report for each line the
latest commit up to which the line survives in the original form.

If you say "git blame --reverse NEW -- PATH" by mistake, we complain
about the missing OLD, but we phrased it as "No commit to dig down
to?"  In this case, however, we are digging up from OLD, so say so.

Signed-off-by: Junio C Hamano 
---
 Documentation/git-blame.txt | 2 +-
 builtin/blame.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-blame.txt b/Documentation/git-blame.txt
index ba54175..16323eb 100644
--- a/Documentation/git-blame.txt
+++ b/Documentation/git-blame.txt
@@ -10,7 +10,7 @@ SYNOPSIS
 [verse]
 'git blame' [-c] [-b] [-l] [--root] [-t] [-f] [-n] [-s] [-e] [-p] [-w] 
[--incremental]
[-L ] [-S ] [-M] [-C] [-C] [-C] [--since=]
-   [--progress] [--abbrev=] [ | --contents  | --reverse 
]
+   [--progress] [--abbrev=] [ | --contents  | --reverse 
..]
[--] 
 
 DESCRIPTION
diff --git a/builtin/blame.c b/builtin/blame.c
index 21f42b0..281f372 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -2473,7 +2473,7 @@ static char *prepare_initial(struct scoreboard *sb)
final_commit_name = revs->pending.objects[i].name;
}
if (!final_commit_name)
-   die("No commit to dig down to?");
+   die("No commit to dig up from?");
return xstrdup(final_commit_name);
 }
 
-- 
2.9.0-290-g8794d48

--
To unsubscribe from this list: send the line "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: problems installing GIT on my MAC OS X 10.11.5

[Konstantin Khomoutov]

On Tue, 14 Jun 2016 17:49:50 +0100
Maria Jose Fernandez  wrote:

> I am very sorry but I am not understanding what you are saying. 
> I will try with homebrew and see if it works that way.

Torstean means:

1. Using the Finder, locate the Terminal application and run it.
   An open with command prompt (the "$" character) will appear.

2. Type there, literally:

 git --version

   and hit the Enter key.

   See whether it errors out at you -- something like

 bash: git: command not found

   or the version of the installed Git program will be printed,
   like in

 git version 2.9.0

   (yours might be different but that doesn't matter).

Whatever the outcome is, select that text and paste it into the message
you're composing to send here.

[...]
> > Do you think that you open a terminal and type
> > which git
> > git --version
> > and post the output here ?
--
To unsubscribe from this list: send the line "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] Refactor recv_sideband()

[Nicolas Pitre]

On Mon, 13 Jun 2016, Nicolas Pitre wrote:

> On Mon, 13 Jun 2016, Lukas Fleischer wrote:
> 
> > Improve the readability of recv_sideband() significantly by replacing
> > fragile buffer manipulations with more sophisticated format strings.
> > Also, reorganize the overall control flow, remove some superfluous
> > variables and replace a custom implementation of strpbrk() with a call
> > to the standard C library function.
> > 
> > Signed-off-by: Lukas Fleischer 
> 
> The previous code was a total abomination, even if I happen to know who 
> wrote it.
> 
> Acked-by: Nicolas Pitre 

I just looked again at all the contraptions _I_ wrote (not Junio's) for 
a reason why I went to such extremes in making this code co complicated.

One aspect that is now lost with your patch is the atomic nature of the 
write.  See commit ed1902ef5c for the explanation.  You could probably 
use sprintf() into a temporary buffer and write it in one go to avoid 
segmented writes from the C library. It's probably not worth having that 
complex code just to avoid a string copy.

> > I had a really hard time reading and understanding this function when I
> > came up with my last patch. What I ended up with is almost a complete
> > rewrite of recv_sideband() and I find the end result to be much more
> > readable than what we have now. Given that this is quite invasive, it
> > would be good to have some more eyes and opinions...

I'd also suggest you look at "git log --author=Pitre sideband.c" output 
where I documented other examples of messed up displays that led to 
the current code so you could confirm your patch covers them.


Nicolas
--
To unsubscribe from this list: send the line "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: 'untracked working tree files would be overwritten by merge' on ignored files?

[Junio C Hamano]

Andreas Krey  writes:

> when I have an ignored file in my workspace, is git
> then also assumed not to remove it in the course
> of a merge?

IIRC, untracked files are kept during merge and across checking out
another branch.  Files that are deliberately marked as ignored by
listing them to .gitignore mechanism are considered expendable, and
they will be removed as necessary.

This is because we do not have the fourth-class, "we do not want to
add them to the history as tracked contents, but we do not want to
lose them", aka "precious"; we only have three, i.e. "paths in the
index", "path that are untracked" and "path that are ignored".
--
To unsubscribe from this list: send the line "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: problems installing GIT on my MAC OS X 10.11.5

[Maria Jose Fernandez]

I am very sorry but I am not understanding what you are saying. 
I will try with homebrew and see if it works that way.

> On 14 Jun 2016, at 17:48, Torsten Bögershausen  wrote:
> 
> On 14.06.16 18:45, Maria Jose Fernandez wrote:
>> From http://git-scm.com/download/mac I clicked to download manually.
>> Then it goes to 
>> https://sourceforge.net/projects/git-osx-installer/files/git-2.8.1-intel-universal-mavericks.dmg/download?use_mirror=autoselect
>> I found the git - 2.8.1-intel-universal-mavericks.dmg 
>> 
>>  downloaded on my desktop. I open that and go through the installation 
>> process and then it says is installed but not found anywhere on my computer. 
>> 
>> 
> It says ?
> 
> Do you think that you open a terminal and type
> which git
> git --version
> and post the output here ?
> 
> 

--
To unsubscribe from this list: send the line "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: problems installing GIT on my MAC OS X 10.11.5

[Torsten Bögershausen]

On 14.06.16 18:45, Maria Jose Fernandez wrote:
> From http://git-scm.com/download/mac I clicked to download manually.
> Then it goes to 
> https://sourceforge.net/projects/git-osx-installer/files/git-2.8.1-intel-universal-mavericks.dmg/download?use_mirror=autoselect
> I found the git - 2.8.1-intel-universal-mavericks.dmg 
> 
>  downloaded on my desktop. I open that and go through the installation 
> process and then it says is installed but not found anywhere on my computer. 
>
>
It says ?

Do you think that you open a terminal and type
which git
git --version
and post the output here ?


--
To unsubscribe from this list: send the line "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] Refactor recv_sideband()

[Nicolas Pitre]

On Tue, 14 Jun 2016, Johannes Schindelin wrote:

> Hi Nico,
> 
> On Tue, 14 Jun 2016, Nicolas Pitre wrote:
> 
> > On Tue, 14 Jun 2016, Johannes Schindelin wrote:
> > 
> > > On Mon, 13 Jun 2016, Nicolas Pitre wrote:
> > > 
> > > > On Mon, 13 Jun 2016, Lukas Fleischer wrote:
> > > > 
> > > > > Improve the readability of recv_sideband() significantly by
> > > > > replacing fragile buffer manipulations with more sophisticated
> > > > > format strings.  Also, reorganize the overall control flow, remove
> > > > > some superfluous variables and replace a custom implementation of
> > > > > strpbrk() with a call to the standard C library function.
> > > > > 
> > > > > Signed-off-by: Lukas Fleischer 
> > > > 
> > > > The previous code was a total abomination, even if I happen to know
> > > > who wrote it.
> > > 
> > > Let's give Junio a break, okay? He does a kick-ass job at maintaining
> > > Git.  What we see here is simply good software development, nothing
> > > more, nothing less: an initial, working code being improved. No need
> > > to make the original author feel bad... :-)
> > 
> > In case my sarcasm wasn't clear, _I_ am the author of the alluded
> > abomination.
> 
> Sorry, I did not catch that. I just looked at
> 583b7ea31b7c16f872b178d541591ab816d16f85 and felt that we could be nicer
> to Junio...

Oh, the initial code from Junio was sane enough. I made a mess of it 
afterwards.


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

'untracked working tree files would be overwritten by merge' on ignored files?

[Andreas Krey]

Hi all,

when I have an ignored file in my workspace, is git
then also assumed not to remove it in the course
of a merge?

Shouldn't it then say that the file is ignored,
as it does not show up in the untracked section
of git status?

Regards, Andreas

PS: Test script (will remove anything named 'tst'):
rm -rf tst
mkdir tst
cd tst || exit 1
git init
echo '*.txt' >.gitignore
git add .
git commit -m initial
git checkout -b side
git checkout -
date >a.txt
git add -f a.txt
git commit -m 'new file'
git checkout side
git commit -m 'nix' --allow-empty
touch a.txt
git status --ignored# Shows a.txt as ignored
git merge master# Will complain
git version

-- 
"Totally trivial. Famous last words."
From: Linus Torvalds 
Date: Fri, 22 Jan 2010 07:29:21 -0800
--
To unsubscribe from this list: send the line "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: problems installing GIT on my MAC OS X 10.11.5

[Torsten Bögershausen]

On 14.06.16 18:06, Konstantin Khomoutov wrote:
> On Tue, 14 Jun 2016 16:56:15 +0100
> Maria Jose Fernandez  wrote:
> 
>> I am doing a data science course and need to download GIT but for
>> some reason I can’t installed it. I called Apple but they couldn’t
>> help and suggested me to contact you guys.
> 
> So you proceeded to http://git-scm.com/download/mac fetched the
> installer, then found the downloaded file in the Finder, clicked it --
> and then what happened?
> --

Not sure, we need to verify first, what had happened.

Typically, if you install Xcode, there is a program
wrapper called git, saying that you need to install it
as part of Xcode.

You can do this.
You can go to http://git-scm.com/download/mac.
You can install git via homebrew, via fink or mac ports.

So it could be nice to know what exactly happened, please.

--
To unsubscribe from this list: send the line "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: problems installing GIT on my MAC OS X 10.11.5

[Konstantin Khomoutov]

On Tue, 14 Jun 2016 16:56:15 +0100
Maria Jose Fernandez  wrote:

> I am doing a data science course and need to download GIT but for
> some reason I can’t installed it. I called Apple but they couldn’t
> help and suggested me to contact you guys.

So you proceeded to http://git-scm.com/download/mac fetched the
installer, then found the downloaded file in the Finder, clicked it --
and then what happened?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

problems installing GIT on my MAC OS X 10.11.5

[Maria Jose Fernandez]

Hello, 
I am doing a data science course and need to download GIT but for some reason I 
can’t installed it. I called Apple but they couldn’t help and suggested me to 
contact you guys. 

I hope you can help me.
Thank you very much,

Maria Jose Freeman--
To unsubscribe from this list: send the line "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] Refactor recv_sideband()

[Johannes Schindelin]

Hi Nico,

On Tue, 14 Jun 2016, Nicolas Pitre wrote:

> On Tue, 14 Jun 2016, Johannes Schindelin wrote:
> 
> > On Mon, 13 Jun 2016, Nicolas Pitre wrote:
> > 
> > > On Mon, 13 Jun 2016, Lukas Fleischer wrote:
> > > 
> > > > Improve the readability of recv_sideband() significantly by
> > > > replacing fragile buffer manipulations with more sophisticated
> > > > format strings.  Also, reorganize the overall control flow, remove
> > > > some superfluous variables and replace a custom implementation of
> > > > strpbrk() with a call to the standard C library function.
> > > > 
> > > > Signed-off-by: Lukas Fleischer 
> > > 
> > > The previous code was a total abomination, even if I happen to know
> > > who wrote it.
> > 
> > Let's give Junio a break, okay? He does a kick-ass job at maintaining
> > Git.  What we see here is simply good software development, nothing
> > more, nothing less: an initial, working code being improved. No need
> > to make the original author feel bad... :-)
> 
> In case my sarcasm wasn't clear, _I_ am the author of the alluded
> abomination.

Sorry, I did not catch that. I just looked at
583b7ea31b7c16f872b178d541591ab816d16f85 and felt that we could be nicer
to Junio...

Ciao,
Johannes
--
To unsubscribe from this list: send the line "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] Refactor recv_sideband()

[Nicolas Pitre]

On Tue, 14 Jun 2016, Johannes Schindelin wrote:

> Hi,
> 
> On Mon, 13 Jun 2016, Nicolas Pitre wrote:
> 
> > On Mon, 13 Jun 2016, Lukas Fleischer wrote:
> > 
> > > Improve the readability of recv_sideband() significantly by replacing
> > > fragile buffer manipulations with more sophisticated format strings.
> > > Also, reorganize the overall control flow, remove some superfluous
> > > variables and replace a custom implementation of strpbrk() with a call
> > > to the standard C library function.
> > > 
> > > Signed-off-by: Lukas Fleischer 
> > 
> > The previous code was a total abomination, even if I happen to know who 
> > wrote it.
> 
> Let's give Junio a break, okay? He does a kick-ass job at maintaining Git.
> What we see here is simply good software development, nothing more,
> nothing less: an initial, working code being improved. No need to make the
> original author feel bad... :-)

In case my sarcasm wasn't clear, _I_ am the author of the alluded 
abomination.


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

[PATCHv3] gpg-interface: check gpg signature creation status

[Michael J Gruber]

When we create a signature, it may happen that gpg returns with
"success" but not with an actual detached signature on stdout.

Check for the correct signature creation status to catch these cases
better. Really, --status-fd parsing is the only way to check gpg status
reliably. We do the same for verify already.

Signed-off-by: Michael J Gruber 
---
That must be the real real thing now...

 gpg-interface.c | 22 +++---
 t/t7004-tag.sh  | 10 +-
 2 files changed, 24 insertions(+), 8 deletions(-)

diff --git a/gpg-interface.c b/gpg-interface.c
index c4b1e8c..850dc81 100644
--- a/gpg-interface.c
+++ b/gpg-interface.c
@@ -150,17 +150,19 @@ const char *get_signing_key(void)
 int sign_buffer(struct strbuf *buffer, struct strbuf *signature, const char 
*signing_key)
 {
struct child_process gpg = CHILD_PROCESS_INIT;
-   const char *args[4];
-   ssize_t len;
+   const char *args[5];
size_t i, j, bottom;
+   struct strbuf err = STRBUF_INIT;
 
gpg.argv = args;
gpg.in = -1;
gpg.out = -1;
+   gpg.err = -1;
args[0] = gpg_program;
-   args[1] = "-bsau";
-   args[2] = signing_key;
-   args[3] = NULL;
+   args[1] = "--status-fd=2";
+   args[2] = "-bsau";
+   args[3] = signing_key;
+   args[4] = NULL;
 
if (start_command())
return error(_("could not run gpg."));
@@ -174,19 +176,25 @@ int sign_buffer(struct strbuf *buffer, struct strbuf 
*signature, const char *sig
if (write_in_full(gpg.in, buffer->buf, buffer->len) != buffer->len) {
close(gpg.in);
close(gpg.out);
+   close(gpg.err);
finish_command();
return error(_("gpg did not accept the data"));
}
close(gpg.in);
 
bottom = signature->len;
-   len = strbuf_read(signature, gpg.out, 1024);
+   strbuf_read(signature, gpg.out, 1024);
+   strbuf_read(, gpg.err, 0);
close(gpg.out);
+   close(gpg.err);
 
sigchain_pop(SIGPIPE);
 
-   if (finish_command() || !len || len < 0)
+   if (finish_command() || !strstr(err.buf, "\n[GNUPG:] SIG_CREATED 
")) {
+   strbuf_release();
return error(_("gpg failed to sign the data"));
+   }
+   strbuf_release();
 
/* Strip CR from the line endings, in case we are on Windows. */
for (i = j = bottom; i < signature->len; i++)
diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index f9b7d79..467e968 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -1202,10 +1202,18 @@ test_expect_success GPG,RFC1991 \
 # try to sign with bad user.signingkey
 git config user.signingkey BobTheMouse
 test_expect_success GPG \
-   'git tag -s fails if gpg is misconfigured' \
+   'git tag -s fails if gpg is misconfigured (bad key)' \
'test_must_fail git tag -s -m tail tag-gpg-failure'
 git config --unset user.signingkey
 
+# try to produce invalid signature
+git config gpg.program echo
+test_expect_success GPG \
+   'git tag -s fails if gpg is misconfigured (bad signature format)' \
+   'test_must_fail git tag -s -m tail tag-gpg-failure'
+git config --unset gpg.program
+
+
 # try to verify without gpg:
 
 rm -rf gpghome
-- 
2.9.0.382.g87fd384

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

[PATCH 1/2] Make send_sideband() return void

[Lukas Fleischer]

The send_sideband() function uses write_or_die() for writing data which
immediately terminates the process on errors. If no such error occurred,
send_sideband() always returned the value that was passed as fourth
parameter prior to this commit. This value is already known to the
caller in any case, so let's turn send_sideband() into a void function
instead.

Signed-off-by: Lukas Fleischer 
---
 sideband.c| 4 +---
 sideband.h| 2 +-
 upload-pack.c | 6 --
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/sideband.c b/sideband.c
index 0a078c3..9504429 100644
--- a/sideband.c
+++ b/sideband.c
@@ -90,9 +90,8 @@ int recv_sideband(const char *me, int in_stream, int out)
  * fd is connected to the remote side; send the sideband data
  * over multiplexed packet stream.
  */
-ssize_t send_sideband(int fd, int band, const char *data, ssize_t sz, int 
packet_max)
+void send_sideband(int fd, int band, const char *data, ssize_t sz, int 
packet_max)
 {
-   ssize_t ssz = sz;
const char *p = data;
 
while (sz) {
@@ -114,5 +113,4 @@ ssize_t send_sideband(int fd, int band, const char *data, 
ssize_t sz, int packet
p += n;
sz -= n;
}
-   return ssz;
 }
diff --git a/sideband.h b/sideband.h
index e46bed0..7a8146f 100644
--- a/sideband.h
+++ b/sideband.h
@@ -5,6 +5,6 @@
 #define SIDEBAND_REMOTE_ERROR -1
 
 int recv_sideband(const char *me, int in_stream, int out);
-ssize_t send_sideband(int fd, int band, const char *data, ssize_t sz, int 
packet_max);
+void send_sideband(int fd, int band, const char *data, ssize_t sz, int 
packet_max);
 
 #endif
diff --git a/upload-pack.c b/upload-pack.c
index 56d101f..cab71b1 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -67,8 +67,10 @@ static void reset_timeout(void)
 
 static ssize_t send_client_data(int fd, const char *data, ssize_t sz)
 {
-   if (use_sideband)
-   return send_sideband(1, fd, data, sz, use_sideband);
+   if (use_sideband) {
+   send_sideband(1, fd, data, sz, use_sideband);
+   return sz;
+   }
if (fd == 3)
/* emergency quit */
fd = 2;
-- 
2.8.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

[PATCH 2/2] Make send_client_data() return void

[Lukas Fleischer]

The send_client_data() function uses write_or_die() for writing data
which immediately terminates the process on errors. If no such error
occurred, send_client_data() always returned the value that was passed
as third parameter prior to this commit. This value is already known to
the caller in any case, so let's turn send_client_data() into a void
function instead.

Signed-off-by: Lukas Fleischer 
---
 upload-pack.c | 15 +--
 1 file changed, 5 insertions(+), 10 deletions(-)

diff --git a/upload-pack.c b/upload-pack.c
index cab71b1..432d585 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -65,11 +65,11 @@ static void reset_timeout(void)
alarm(timeout);
 }
 
-static ssize_t send_client_data(int fd, const char *data, ssize_t sz)
+static void send_client_data(int fd, const char *data, ssize_t sz)
 {
if (use_sideband) {
send_sideband(1, fd, data, sz, use_sideband);
-   return sz;
+   return;
}
if (fd == 3)
/* emergency quit */
@@ -77,10 +77,9 @@ static ssize_t send_client_data(int fd, const char *data, 
ssize_t sz)
if (fd == 2) {
/* XXX: are we happy to lose stuff here? */
xwrite(fd, data, sz);
-   return sz;
+   return;
}
write_or_die(fd, data, sz);
-   return sz;
 }
 
 static int write_one_shallow(const struct commit_graft *graft, void *cb_data)
@@ -245,9 +244,7 @@ static void create_pack_file(void)
}
else
buffered = -1;
-   sz = send_client_data(1, data, sz);
-   if (sz < 0)
-   goto fail;
+   send_client_data(1, data, sz);
}
 
/*
@@ -274,9 +271,7 @@ static void create_pack_file(void)
/* flush the data */
if (0 <= buffered) {
data[0] = buffered;
-   sz = send_client_data(1, data, 1);
-   if (sz < 0)
-   goto fail;
+   send_client_data(1, data, 1);
fprintf(stderr, "flushed.\n");
}
if (use_sideband)
-- 
2.8.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

compactionHeuristic=true is not used by interactive staging

[Alex Prengère]

Hello,
I just did a fresh clone of git from Github and installed the version
2.9.0 on Fedora 22.

I tried the new compactionHeuristic = true, which is awesome.
The only thing that struck me was that this option was not used when
doing an interactive staging, meaning `git diff` and `git add -p` will
format patches differently. Perhaps this is intended and there is a
way to force interactive staging to use specific diff options, but I
did not find it in the doc.

Thanks,
Alex
--
To unsubscribe from this list: send the line "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] Refactor recv_sideband()

[Johannes Schindelin]

Hi,

On Mon, 13 Jun 2016, Nicolas Pitre wrote:

> On Mon, 13 Jun 2016, Lukas Fleischer wrote:
> 
> > Improve the readability of recv_sideband() significantly by replacing
> > fragile buffer manipulations with more sophisticated format strings.
> > Also, reorganize the overall control flow, remove some superfluous
> > variables and replace a custom implementation of strpbrk() with a call
> > to the standard C library function.
> > 
> > Signed-off-by: Lukas Fleischer 
> 
> The previous code was a total abomination, even if I happen to know who 
> wrote it.

Let's give Junio a break, okay? He does a kick-ass job at maintaining Git.
What we see here is simply good software development, nothing more,
nothing less: an initial, working code being improved. No need to make the
original author feel bad... :-)

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

[PATCHv2] gpg-interface: check gpg signature for correct header

[Michael J Gruber]

When we create a signature, it may happen that gpg returns with
"success" but not with an actual detached signature on stdout.

Check for the correct header to catch these cases better. We use the
same parse_signature function for that that we use otherwise, so that
gpg specifics are localised there.

Signed-off-by: Michael J Gruber 
---
So, this is the real thing.

Between you and me: parse_signature in fact is more lenient, but hey - it's
exactly as lenient as we are otherwise, bar running gpg --verify.

 gpg-interface.c |  2 +-
 t/t7004-tag.sh  | 10 +-
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/gpg-interface.c b/gpg-interface.c
index c4b1e8c..784953c 100644
--- a/gpg-interface.c
+++ b/gpg-interface.c
@@ -185,7 +185,7 @@ int sign_buffer(struct strbuf *buffer, struct strbuf 
*signature, const char *sig
 
sigchain_pop(SIGPIPE);
 
-   if (finish_command() || !len || len < 0)
+   if (finish_command() || !len || len < 0 || 
parse_signature(signature->buf, signature->len) == signature->len)
return error(_("gpg failed to sign the data"));
 
/* Strip CR from the line endings, in case we are on Windows. */
diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index f9b7d79..467e968 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -1202,10 +1202,18 @@ test_expect_success GPG,RFC1991 \
 # try to sign with bad user.signingkey
 git config user.signingkey BobTheMouse
 test_expect_success GPG \
-   'git tag -s fails if gpg is misconfigured' \
+   'git tag -s fails if gpg is misconfigured (bad key)' \
'test_must_fail git tag -s -m tail tag-gpg-failure'
 git config --unset user.signingkey
 
+# try to produce invalid signature
+git config gpg.program echo
+test_expect_success GPG \
+   'git tag -s fails if gpg is misconfigured (bad signature format)' \
+   'test_must_fail git tag -s -m tail tag-gpg-failure'
+git config --unset gpg.program
+
+
 # try to verify without gpg:
 
 rm -rf gpghome
-- 
2.9.0.382.g87fd384

--
To unsubscribe from this list: send the line "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] gpg-interface: check gpg signature for correct header

[Michael J Gruber]

Michael J Gruber venit, vidit, dixit 14.06.2016 13:34:
> Jeff King venit, vidit, dixit 14.06.2016 13:20:
>> On Tue, Jun 14, 2016 at 01:11:19PM +0200, Michael J Gruber wrote:
>>
>>> When we create a signature, it may happen that gpg returns with
>>> "success" but not with an actual detached signature on stdout.
>>>
>>> Check for the correct header to catch these cases better.
>>
>> Seems like a reasonable idea.
>>
>> I do worry that checking for PGP_SIGNATURE is a little fragile, though.
>> We currently let you sign with gpgsm, for example, and I think this
>> would break it (the verification side is not great because we don't
>> recognize gpgsm headers, but this feels like a step backwards).
>>
>> That wouldn't be too hard to work around with a "is this a signature"
>> function that checks both types.
>>
>>> diff --git a/gpg-interface.c b/gpg-interface.c
>>> index c4b1e8c..664796f 100644
>>> --- a/gpg-interface.c
>>> +++ b/gpg-interface.c
>>> @@ -185,7 +185,7 @@ int sign_buffer(struct strbuf *buffer, struct strbuf 
>>> *signature, const char *sig
>>>  
>>> sigchain_pop(SIGPIPE);
>>>  
>>> -   if (finish_command() || !len || len < 0)
>>> +   if (finish_command() || !len || len < 0 || strncmp(signature->buf, 
>>> PGP_SIGNATURE, strlen(PGP_SIGNATURE)))
>>> return error(_("gpg failed to sign the data"));
>>
>> I think your strncmp is better spelled:
>>
>>   starts_with(signature->buf, PGP_SIGNATURE);
>>
>> The check for "!len" is redundant now. I think you could drop "len < 0"
>> as well (and in fact, drop the "len" variable entirely), as in the error
>> case we'd simply have an empty signature->len.
>>
>> Your patch effectively swaps out "did we get any data" for "did we get
>> the data we expect", which is what those "len" checks were doing.
>>
>> -Peff
>>
> 
> My patch actually makes several tests fail, sorry. (I did check before
> that I can still create signatures...) Maybe my offset in buf is wrong.
> 
> starts_with, yes.
> 
> Can't check any further now, sorry. But we do check for the
> PGP_SIGNATURE in our signed objects anyways. So I feel that we can either
> 
> - tighthen the check for valid gpg signatures
> 
> or
> 
> - make our signature interface completely pluggable.
> 
> We can't have it both ways, but at least things are localised in
> gpg-interface.c now.
> 
> The proposed patch is just some consistency check that does not rely on
> gpg.program itself(!), or else we could simply call verify.
> 
> Michael

So, with

!starts_with(signature->buf+bottom, PGP_SIGNATURE)

everything is fine except our tests for RFC1991 signatures.
Sigh...

I'll resend a patch that uses parse_signature so that all gpg specifics
are localised there.

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] gnome-keyring: Don't hard-code pkg-config executable

[Heiko Becker]

Helpful if your pkg-config executable has a prefix based on the
architecture, for example.

Signed-off-by: Heiko Becker 
---
 contrib/credential/gnome-keyring/Makefile | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/contrib/credential/gnome-keyring/Makefile 
b/contrib/credential/gnome-keyring/Makefile
index c3c7c98..22c19df 100644
--- a/contrib/credential/gnome-keyring/Makefile
+++ b/contrib/credential/gnome-keyring/Makefile
@@ -4,12 +4,13 @@ all:: $(MAIN)
 CC = gcc
 RM = rm -f
 CFLAGS = -g -O2 -Wall
+PKG_CONFIG = pkg-config
 
 -include ../../../config.mak.autogen
 -include ../../../config.mak
 
-INCS:=$(shell pkg-config --cflags gnome-keyring-1 glib-2.0)
-LIBS:=$(shell pkg-config --libs gnome-keyring-1 glib-2.0)
+INCS:=$(shell $(PKG_CONFIG) --cflags gnome-keyring-1 glib-2.0)
+LIBS:=$(shell $(PKG_CONFIG) --libs gnome-keyring-1 glib-2.0)
 
 SRCS:=$(MAIN).c
 OBJS:=$(SRCS:.c=.o)
-- 
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] gpg-interface: check gpg signature for correct header

[Michael J Gruber]

Jeff King venit, vidit, dixit 14.06.2016 13:20:
> On Tue, Jun 14, 2016 at 01:11:19PM +0200, Michael J Gruber wrote:
> 
>> When we create a signature, it may happen that gpg returns with
>> "success" but not with an actual detached signature on stdout.
>>
>> Check for the correct header to catch these cases better.
> 
> Seems like a reasonable idea.
> 
> I do worry that checking for PGP_SIGNATURE is a little fragile, though.
> We currently let you sign with gpgsm, for example, and I think this
> would break it (the verification side is not great because we don't
> recognize gpgsm headers, but this feels like a step backwards).
> 
> That wouldn't be too hard to work around with a "is this a signature"
> function that checks both types.
> 
>> diff --git a/gpg-interface.c b/gpg-interface.c
>> index c4b1e8c..664796f 100644
>> --- a/gpg-interface.c
>> +++ b/gpg-interface.c
>> @@ -185,7 +185,7 @@ int sign_buffer(struct strbuf *buffer, struct strbuf 
>> *signature, const char *sig
>>  
>>  sigchain_pop(SIGPIPE);
>>  
>> -if (finish_command() || !len || len < 0)
>> +if (finish_command() || !len || len < 0 || strncmp(signature->buf, 
>> PGP_SIGNATURE, strlen(PGP_SIGNATURE)))
>>  return error(_("gpg failed to sign the data"));
> 
> I think your strncmp is better spelled:
> 
>   starts_with(signature->buf, PGP_SIGNATURE);
> 
> The check for "!len" is redundant now. I think you could drop "len < 0"
> as well (and in fact, drop the "len" variable entirely), as in the error
> case we'd simply have an empty signature->len.
> 
> Your patch effectively swaps out "did we get any data" for "did we get
> the data we expect", which is what those "len" checks were doing.
> 
> -Peff
> 

My patch actually makes several tests fail, sorry. (I did check before
that I can still create signatures...) Maybe my offset in buf is wrong.

starts_with, yes.

Can't check any further now, sorry. But we do check for the
PGP_SIGNATURE in our signed objects anyways. So I feel that we can either

- tighthen the check for valid gpg signatures

or

- make our signature interface completely pluggable.

We can't have it both ways, but at least things are localised in
gpg-interface.c now.

The proposed patch is just some consistency check that does not rely on
gpg.program itself(!), or else we could simply call verify.

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

Re: [PATCH] gpg-interface: check gpg signature for correct header

[Jeff King]

On Tue, Jun 14, 2016 at 01:11:19PM +0200, Michael J Gruber wrote:

> When we create a signature, it may happen that gpg returns with
> "success" but not with an actual detached signature on stdout.
> 
> Check for the correct header to catch these cases better.

Seems like a reasonable idea.

I do worry that checking for PGP_SIGNATURE is a little fragile, though.
We currently let you sign with gpgsm, for example, and I think this
would break it (the verification side is not great because we don't
recognize gpgsm headers, but this feels like a step backwards).

That wouldn't be too hard to work around with a "is this a signature"
function that checks both types.

> diff --git a/gpg-interface.c b/gpg-interface.c
> index c4b1e8c..664796f 100644
> --- a/gpg-interface.c
> +++ b/gpg-interface.c
> @@ -185,7 +185,7 @@ int sign_buffer(struct strbuf *buffer, struct strbuf 
> *signature, const char *sig
>  
>   sigchain_pop(SIGPIPE);
>  
> - if (finish_command() || !len || len < 0)
> + if (finish_command() || !len || len < 0 || strncmp(signature->buf, 
> PGP_SIGNATURE, strlen(PGP_SIGNATURE)))
>   return error(_("gpg failed to sign the data"));

I think your strncmp is better spelled:

  starts_with(signature->buf, PGP_SIGNATURE);

The check for "!len" is redundant now. I think you could drop "len < 0"
as well (and in fact, drop the "len" variable entirely), as in the error
case we'd simply have an empty signature->len.

Your patch effectively swaps out "did we get any data" for "did we get
the data we expect", which is what those "len" checks were doing.

-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

[ANNOUNCE] Git for Windows 2.9.0

[Johannes Schindelin]

Dear Git users,

It is my pleasure to announce that Git for Windows 2.9.0 is available from:

https://git-for-windows.github.io/

Changes since Git for Windows v2.8.4 (June 7th 2016)

Bug Fixes

  ??? When running git gc --aggressive or git repack -ald in the presence
of multiple pack files, the command still had open handles to the
pack files it wanted to remove. This has been fixed.

Filename | SHA-256
 | ---
Git-2.9.0-64-bit.exe | 
4e736ae188f4b75c2c24282d8a9543726f6cb7e0b2f1e7ad8e37e3b61cfa8d1d
Git-2.9.0-32-bit.exe | 
c511db6eb0a23ae53fbf753f688a1a180a371e082c3b202bf8b64f3bccf9bc95
PortableGit-2.9.0-64-bit.7z.exe | 
566bf55d7a3ba18660e76034d4af3e0cdd985cbb6f73eb881f287aa23a0f6bbc
PortableGit-2.9.0-32-bit.7z.exe | 
ed7c648f58decbd70f27e124704e88ac1c9934e78bcc60516aaeddace2275581
Git-2.9.0-64-bit.tar.bz2 | 
85662a8b7d69fa604eb68e337eab3991ef77aa724794b9f27705190b206a4d7d
Git-2.9.0-32-bit.tar.bz2 | 
33c4e9f75c883c003baf350d16d3ebaad9f93f3789e34c6251e23e9fa98cf659

Ciao,
Johannes

[PATCH] gpg-interface: check gpg signature for correct header

[Michael J Gruber]

When we create a signature, it may happen that gpg returns with
"success" but not with an actual detached signature on stdout.

Check for the correct header to catch these cases better.

Signed-off-by: Michael J Gruber 
---
This catches at least my echo example.

We could do a full blown gpg signature check, of course.

 gpg-interface.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gpg-interface.c b/gpg-interface.c
index c4b1e8c..664796f 100644
--- a/gpg-interface.c
+++ b/gpg-interface.c
@@ -185,7 +185,7 @@ int sign_buffer(struct strbuf *buffer, struct strbuf 
*signature, const char *sig
 
sigchain_pop(SIGPIPE);
 
-   if (finish_command() || !len || len < 0)
+   if (finish_command() || !len || len < 0 || strncmp(signature->buf, 
PGP_SIGNATURE, strlen(PGP_SIGNATURE)))
return error(_("gpg failed to sign the data"));
 
/* Strip CR from the line endings, in case we are on Windows. */
-- 
2.9.0.382.g87fd384

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

Re: [PATCH v7 01/40] apply: move 'struct apply_state' to apply.h

[Christian Couder]

On Tue, Jun 14, 2016 at 12:49 AM, Junio C Hamano  wrote:
> Christian Couder  writes:
>
>> To libify `git apply` functionality we must make 'struct apply_state'
>> usable outside "builtin/apply.c".
>>
>> Let's do that by creating a new "apply.h" and moving
>> 'struct apply_state' there.
>>
>> Signed-off-by: Christian Couder 
>> ---
>>  apply.h | 100 
>> 
>>  builtin/apply.c |  98 +-
>>  2 files changed, 101 insertions(+), 97 deletions(-)
>>  create mode 100644 apply.h
>>
>> diff --git a/apply.h b/apply.h
>> new file mode 100644
>> index 000..9a98eae
>> --- /dev/null
>> +++ b/apply.h
>> @@ -0,0 +1,100 @@
>> +#ifndef APPLY_H
>> +#define APPLY_H
>> +
>> +enum ws_error_action {
>> + nowarn_ws_error,
>> + warn_on_ws_error,
>> + die_on_ws_error,
>> + correct_ws_error
>> +};
>> +
>> +enum ws_ignore {
>> + ignore_ws_none,
>> + ignore_ws_change
>> +};
>> +
>> +/*
>> + * We need to keep track of how symlinks in the preimage are
>> + * manipulated by the patches.  A patch to add a/b/c where a/b
>> + * is a symlink should not be allowed to affect the directory
>> + * the symlink points at, but if the same patch removes a/b,
>> + * it is perfectly fine, as the patch removes a/b to make room
>> + * to create a directory a/b so that a/b/c can be created.
>> + *
>> + * See also "struct string_list symlink_changes" in "struct
>> + * apply_state".
>> + */
>> +#define SYMLINK_GOES_AWAY 01
>> +#define SYMLINK_IN_RESULT 02
>
> Everything below is agreeable, but all the names that are made
> public above by this change do not sound specific enough to "apply".
> I wonder if they should get "apply" somewhere in their names to
> avoid confusion coming from the namespace contamination...

Yeah, I will add "apply" in the names.

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

Re: [PATCH v7 02/40] builtin/apply: make apply_patch() return -1 instead of die()ing

[Christian Couder]

On Tue, Jun 14, 2016 at 12:55 AM, Junio C Hamano  wrote:
> Christian Couder  writes:
>
>> +/*
>> + * Try to apply a patch.
>> + *
>> + * Returns:
>> + *  -1 if an error happened
>> + *   0 if the patch applied
>> + *   1 if the patch did not apply
>> + */
>>  static int apply_patch(struct apply_state *state,
>>  int fd,
>>  const char *filename,
>> @@ -4413,6 +4421,7 @@ static int apply_patch(struct apply_state *state,
>>   struct strbuf buf = STRBUF_INIT; /* owns the patch text */
>>   struct patch *list = NULL, **listp = 
>>   int skipped_patch = 0;
>> + int res = 0;
>>
>>   state->patch_input_file = filename;
>>   read_patch_file(, fd);
>> @@ -4445,8 +4454,10 @@ static int apply_patch(struct apply_state *state,
>>   offset += nr;
>>   }
>>
>> - if (!list && !skipped_patch)
>> - die(_("unrecognized input"));
>> + if (!list && !skipped_patch) {
>> + res = error(_("unrecognized input"));
>> + goto end;
>> + }
>
> Before this patch, the program said "fatal: $message" and exited
> with status = 128.  All these changes in this step modifies the
> external behaviour and make it say "error: $message" and exit with
> status = 1 (at least the caller in apply_all_patches() does so).
>
> Will that be an issue for the calling scripts?

Hopefully the scripts don't check the specific error code and message.

I will add something about this in the commit message.

Do you think something else that should be done about this?
--
To unsubscribe from this list: send the line "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] fetch: document that pruning happens before fetching

[Duy Nguyen]

On Tue, Jun 14, 2016 at 5:22 PM, Jeff King  wrote:
> I think the documentation should be updated either way. This is not
> about the ordering in the status table, but rather about the order of
> the real operations. The user may care about that ordering if they want
> to know what races are possible

Good point.

> or what would happen if the packfile
> fetch failed (we'd already have deleted the old refs, but wouldn't fetch
> the new ones).

Off topic, but this sounds like a bug to me. We could have kept ref
update more consistent (maybe at some point we could even do atomic
transaction update for all refs if there's a need for it).
-- 
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: I lost my commit signature

[Jeff King]

On Tue, Jun 14, 2016 at 04:39:38PM +0800, ZhenTian wrote:

> I want to set gpg -v to pgp.program, but if I set it, it can't call gpg:
> ```
> error: cannot run gpg -v: No such file or directory
> error: could not run gpg.
> fatal: failed to write commit object
> ```
> 
> I have tried set gpg.program value to `gpg|/tmp/log`, `/usr/bin/gpg
> -v`, `gpg -v`, `"/usr/bin/gpg -v"`
> 
> only after I set to `gpg` or `/usr/bin/gpg` without any argument, it will 
> work.

Ah, right. Most of the time we run such programs as shell commands, but
it looks like we do not. So you'd have to do something like:

cat >/tmp/fake-gpg <<-\EOF
#!/bin/sh
gpg -v "$@"
EOF
chmod +x /tmp/fake-gpg
git config gpg.program /tmp/fake-gpg

-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] fetch: document that pruning happens before fetching

[Duy Nguyen]

On Tue, Jun 14, 2016 at 6:58 AM, Jeff King  wrote:
> This was changed in 10a6cc8 (fetch --prune: Run prune before
> fetching, 2014-01-02), but it seems that nobody in that
> discussion realized we were advertising the "after"
> explicitly.

Ah... ok. Good to know it's moved up top on purpose because I almost
tried to move it down :) It's irritating that current output looks
like this




remote: 




It probably looks better if we can move the  part after
"remote: ..." lines (iow still _after_ fetch, but _before_ ref
updates), e.g.

remote: 







If we do so, there's no need to update document. But I don't know,
maybe it's not worth doing.

> Signed-off-by: Jeff King 
> ---
> I include myself in that "nobody" of course. :)
>
>  Documentation/fetch-options.txt | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/Documentation/fetch-options.txt b/Documentation/fetch-options.txt
> index 036edfb..b05a834 100644
> --- a/Documentation/fetch-options.txt
> +++ b/Documentation/fetch-options.txt
> @@ -52,7 +52,7 @@ ifndef::git-pull[]
>
>  -p::
>  --prune::
> -   After fetching, remove any remote-tracking references that no
> +   Before fetching, remove any remote-tracking references that no
> longer exist on the remote.  Tags are not subject to pruning
> if they are fetched only because of the default tag
> auto-following or due to a --tags option.  However, if tags
> --
> 2.9.0.150.g8bd4cf6
> --
> To unsubscribe from this list: send the line "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

Re: [PATCH 00/27] nd/shallow-deepen updates

[Duy Nguyen]

On Tue, Jun 14, 2016 at 12:10 AM, Junio C Hamano  wrote:
> Eric Sunshine  writes:
>
>> I agree with Junio that moving the sigchain_pop() into the error
>> handling code-path, if possible, would be a nice improvement.
>
> Yeah, "if possible" is really what I was not sure about---is it safe
> to do the _push() thing before start_command(), which presumably
> would affect both the main and the forked processes?

Good point. But we do exec() very shortly in the forked process, which
should reset signal table back to default. So probably not a bad
thing.
-- 
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: I lost my commit signature

[ZhenTian]

Hi Peff,

I want to set gpg -v to pgp.program, but if I set it, it can't call gpg:
```
error: cannot run gpg -v: No such file or directory
error: could not run gpg.
fatal: failed to write commit object
```

I have tried set gpg.program value to `gpg|/tmp/log`, `/usr/bin/gpg
-v`, `gpg -v`, `"/usr/bin/gpg -v"`

only after I set to `gpg` or `/usr/bin/gpg` without any argument, it will work.
Sincerely,
田震


On Tue, Jun 14, 2016 at 4:18 PM, Jeff King  wrote:
> On Tue, Jun 14, 2016 at 04:09:52PM +0800, ZhenTian wrote:
>
>> I have tested sign my work in another project, it works fine, I have
>> committed five times, all commits are signed.
>>
>> I can't find encoded signature block in the output of "git cat-file
>> commit HEAD", only these:
>> ```
>> tree 17a572e349ce2fda47470951b5011b9c2f6533b7
>> parent 2c35701725d34325520acb9b45daf42f64adc536
>> author TianZhen  1465887785 +0800
>> committer TianZhen  1465887791 +0800
>>
>> feat: mobile support free freight hint, closed #1417
>> ```
>>
>> Some of my commits are signed, for example I have committed four times
>> today, only first commit is signed. Is it possible some issue with
>> gpg-agent? I can't find it via `ps -Af | grep gpg`.
>
> Possibly. If you set gpg.program "gpg -v", does it help? You could also
> try setting it to "gpg | /tmp/log" to see what gpg is passing back to
> git.
>
> -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

[TIG PATCH] test: make diff/log work with git 2.9

[Michael J Gruber]

git 2.9.0 switches the default for diff.renames to true.

Set this to false in config so that the test suite runs unmodified for
old and new git.

Signed-off-by: Michael J Gruber 
---
 test/tools/libgit.sh | 1 +
 1 file changed, 1 insertion(+)

diff --git a/test/tools/libgit.sh b/test/tools/libgit.sh
index 6a8aa54..0bce451 100644
--- a/test/tools/libgit.sh
+++ b/test/tools/libgit.sh
@@ -33,6 +33,7 @@ git_config()
 {
git config --local user.name "Committer"
git config --local user.email "c.ommit...@example.net"
+   git config --local diff.renames false
 }
 
 git_init()
-- 
2.9.0.382.g87fd384

--
To unsubscribe from this list: send the line "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: I lost my commit signature

[Jeff King]

On Tue, Jun 14, 2016 at 04:09:52PM +0800, ZhenTian wrote:

> I have tested sign my work in another project, it works fine, I have
> committed five times, all commits are signed.
> 
> I can't find encoded signature block in the output of "git cat-file
> commit HEAD", only these:
> ```
> tree 17a572e349ce2fda47470951b5011b9c2f6533b7
> parent 2c35701725d34325520acb9b45daf42f64adc536
> author TianZhen  1465887785 +0800
> committer TianZhen  1465887791 +0800
> 
> feat: mobile support free freight hint, closed #1417
> ```
> 
> Some of my commits are signed, for example I have committed four times
> today, only first commit is signed. Is it possible some issue with
> gpg-agent? I can't find it via `ps -Af | grep gpg`.

Possibly. If you set gpg.program "gpg -v", does it help? You could also
try setting it to "gpg | /tmp/log" to see what gpg is passing back to
git.

-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: I lost my commit signature

[ZhenTian]

Hi Peff,

I commit via this command: gcs -nm "feat: mobile support free freight
hint, closed #1417"

gcs is an alias in zsh, which is: git commit -S

I have tested sign my work in another project, it works fine, I have
committed five times, all commits are signed.

I can't find encoded signature block in the output of "git cat-file
commit HEAD", only these:
```
tree 17a572e349ce2fda47470951b5011b9c2f6533b7
parent 2c35701725d34325520acb9b45daf42f64adc536
author TianZhen  1465887785 +0800
committer TianZhen  1465887791 +0800

feat: mobile support free freight hint, closed #1417
```

Some of my commits are signed, for example I have committed four times
today, only first commit is signed. Is it possible some issue with
gpg-agent? I can't find it via `ps -Af | grep gpg`.

-Dawncold
Sincerely,
田震


On Tue, Jun 14, 2016 at 3:58 PM, Jeff King  wrote:
> On Tue, Jun 14, 2016 at 03:50:43PM +0800, ZhenTian wrote:
>
>> I commit with -S argument, and I got some output like this:
>>
>> You need a passphrase to unlock the secret key for
>> user: "Tian Zhen "
>> 4096-bit RSA key, ID 2EF2AD6E, created 2016-05-21
>>
>> [master d107770] feat: mobile support free freight hint, closed #1417
>>  8 files changed, 58 insertions(+), 29 deletions(-)
>>  rewrite static/css/mobile.min.css (64%)
>>
>> but when I check git log with --show-signature, I can't find my sign.
>>
>> my git is 2.4.8, and OS is Ubuntu 14.04.4
>
> Here's a reproduction which should work (and does for me):
>
>   $ git init
>   $ echo content >file
>   $ git add file
>   $ git commit -m foo -S
>   You need a passphrase to unlock the secret key for
>   user: "Jeff King "
>   4096-bit RSA key, ID F9430ED9, created 2016-02-03 (main key ID D7B337A8)
>
>   [master (root-commit) 6b0b230] foo
>1 file changed, 1 insertion(+)
>create mode 100644 file
>
>   $ git log --show-signature
>   commit 6b0b230c79f8912bf8b21afc0d12d2cbf54cc74d (HEAD -> master)
>   gpg: Signature made Tue 14 Jun 2016 03:55:11 AM EDT using RSA key ID 
> F9430ED9
>   gpg: Good signature from "Jeff King "
>   gpg: aka "Jeff King "
>   Author: Jeff King 
>   Date:   Tue Jun 14 03:55:11 2016 -0400
>
>   foo
>
> Does something similar work for you? If so, then we need to figure out
> what happened in your original case. Can you show the exact commands you
> ran, and what they did output?
>
> If the simple case above doesn't work, then we need to figure out
> whether the commit doesn't get a signature, or whether "log
> --show-signature" is not working on your system. For the former, I'd try
> "git cat-file commit HEAD", which should show the encoded signature
> block. If it's there, then presumably something is not working in
> calling gpg.
>
> -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: I lost my commit signature

[Jeff King]

On Tue, Jun 14, 2016 at 03:50:43PM +0800, ZhenTian wrote:

> I commit with -S argument, and I got some output like this:
> 
> You need a passphrase to unlock the secret key for
> user: "Tian Zhen "
> 4096-bit RSA key, ID 2EF2AD6E, created 2016-05-21
> 
> [master d107770] feat: mobile support free freight hint, closed #1417
>  8 files changed, 58 insertions(+), 29 deletions(-)
>  rewrite static/css/mobile.min.css (64%)
>
> but when I check git log with --show-signature, I can't find my sign.
> 
> my git is 2.4.8, and OS is Ubuntu 14.04.4

Here's a reproduction which should work (and does for me):

  $ git init
  $ echo content >file
  $ git add file
  $ git commit -m foo -S
  You need a passphrase to unlock the secret key for
  user: "Jeff King "
  4096-bit RSA key, ID F9430ED9, created 2016-02-03 (main key ID D7B337A8)

  [master (root-commit) 6b0b230] foo
   1 file changed, 1 insertion(+)
   create mode 100644 file

  $ git log --show-signature
  commit 6b0b230c79f8912bf8b21afc0d12d2cbf54cc74d (HEAD -> master)
  gpg: Signature made Tue 14 Jun 2016 03:55:11 AM EDT using RSA key ID F9430ED9
  gpg: Good signature from "Jeff King "
  gpg: aka "Jeff King "
  Author: Jeff King 
  Date:   Tue Jun 14 03:55:11 2016 -0400

  foo

Does something similar work for you? If so, then we need to figure out
what happened in your original case. Can you show the exact commands you
ran, and what they did output?

If the simple case above doesn't work, then we need to figure out
whether the commit doesn't get a signature, or whether "log
--show-signature" is not working on your system. For the former, I'd try
"git cat-file commit HEAD", which should show the encoded signature
block. If it's there, then presumably something is not working in
calling gpg.

-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/1] Don't free remote->name after fetch

[Jeff King]

On Mon, Jun 13, 2016 at 08:14:43PM -0400, Keith McGuigan wrote:

> Right.  The string_list ends up getting (potentially) populated with a
> mix of dup'd and borrowed values.  I figured it was safer to leak here
> (especially as we're on the way out anyway), than free memory that
> shouldn't be freed.
> 
> Actually, what motivates this (and I apologize that I didn't say this
> earlier) is that we added (in our repo) a bit of stats collection code
> that executes after the string_list_clear(), and calls remote_get()
> which goes all sideways when some of its memory has been freed.

Yeah, I think nobody noticed because we don't have any actual code that
runs after this string_list_clear(), but that doesn't make it not-buggy.

> As an alternative, I could xstrdup each instance where remote->name is
> appended, which would make the string_list a homogenous dup'd list,
> which we could then free.  If you prefer that I'll do a re-roll in
> that style (it just seemed to me at the time like it would be doing
> some useless allocations).  I don't much mind either way.

That sounds much better. Fixing any case like this is really a two-part
thing:

  1. Making the memory ownership policy of the string_list consistent
 (either all allocated, or all not). And if you have _some_ items
 which must be newly allocated (i.e., there is no other place to own
 them), then the only consistent solution is to allocate all of
 them.

  2. Matching the strdup_strings field to that policy.

 The main() function should not have to play tricks with setting
 list->strdup_strings after the fact. It should set it up front,
 and the functions it calls should use string_list_append as
 appropriate.

-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

I lost my commit signature

[ZhenTian]

Hi git developers,

I commit with -S argument, and I got some output like this:

You need a passphrase to unlock the secret key for
user: "Tian Zhen "
4096-bit RSA key, ID 2EF2AD6E, created 2016-05-21

[master d107770] feat: mobile support free freight hint, closed #1417
 8 files changed, 58 insertions(+), 29 deletions(-)
 rewrite static/css/mobile.min.css (64%)


but when I check git log with --show-signature, I can't find my sign.

my git is 2.4.8, and OS is Ubuntu 14.04.4

Sincerely,
dawncold
--
To unsubscribe from this list: send the line "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] fetch: document that pruning happens before fetching

[Jacob Keller]

On Mon, Jun 13, 2016 at 11:17 PM, Jeff King  wrote:
> On Mon, Jun 13, 2016 at 11:14:36PM -0700, Jacob Keller wrote:
>
>> On Mon, Jun 13, 2016 at 4:58 PM, Jeff King  wrote:
>> > This was changed in 10a6cc8 (fetch --prune: Run prune before
>> > fetching, 2014-01-02), but it seems that nobody in that
>> > discussion realized we were advertising the "after"
>> > explicitly.
>> >
>> > Signed-off-by: Jeff King 
>> > ---
>> > I include myself in that "nobody" of course. :)
>> >
>> >  Documentation/fetch-options.txt | 2 +-
>> >  1 file changed, 1 insertion(+), 1 deletion(-)
>> >
>> > diff --git a/Documentation/fetch-options.txt 
>> > b/Documentation/fetch-options.txt
>> > index 036edfb..b05a834 100644
>> > --- a/Documentation/fetch-options.txt
>> > +++ b/Documentation/fetch-options.txt
>> > @@ -52,7 +52,7 @@ ifndef::git-pull[]
>> >
>> >  -p::
>> >  --prune::
>> > -   After fetching, remove any remote-tracking references that no
>> > +   Before fetching, remove any remote-tracking references that no
>> > longer exist on the remote.  Tags are not subject to pruning
>> > if they are fetched only because of the default tag
>> > auto-following or due to a --tags option.  However, if tags
>>
>> What's the difference in behavior due to pruning before instead of
>> after? Curious. It seems like pruning after would make more sense?
>
> See 10a6cc8. :)
>
> Basically, you have to prune first to make way for new incoming refs
> when there is a D/F conflict.
>
> The downside is that there is a moment where objects may be unreferenced
> (e.g., if upstream moved "foo" to "bar", we delete "foo" and _then_
> create "bar"). And due to the way refs are stored, we do not keep even a
> reflog for the deleted ref in the interim.
>
> -Peff

Ah that makes sense, thanks.

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

Re: [PATCH] fetch: document that pruning happens before fetching

[Jeff King]

On Mon, Jun 13, 2016 at 11:14:36PM -0700, Jacob Keller wrote:

> On Mon, Jun 13, 2016 at 4:58 PM, Jeff King  wrote:
> > This was changed in 10a6cc8 (fetch --prune: Run prune before
> > fetching, 2014-01-02), but it seems that nobody in that
> > discussion realized we were advertising the "after"
> > explicitly.
> >
> > Signed-off-by: Jeff King 
> > ---
> > I include myself in that "nobody" of course. :)
> >
> >  Documentation/fetch-options.txt | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/Documentation/fetch-options.txt 
> > b/Documentation/fetch-options.txt
> > index 036edfb..b05a834 100644
> > --- a/Documentation/fetch-options.txt
> > +++ b/Documentation/fetch-options.txt
> > @@ -52,7 +52,7 @@ ifndef::git-pull[]
> >
> >  -p::
> >  --prune::
> > -   After fetching, remove any remote-tracking references that no
> > +   Before fetching, remove any remote-tracking references that no
> > longer exist on the remote.  Tags are not subject to pruning
> > if they are fetched only because of the default tag
> > auto-following or due to a --tags option.  However, if tags
> 
> What's the difference in behavior due to pruning before instead of
> after? Curious. It seems like pruning after would make more sense?

See 10a6cc8. :)

Basically, you have to prune first to make way for new incoming refs
when there is a D/F conflict.

The downside is that there is a moment where objects may be unreferenced
(e.g., if upstream moved "foo" to "bar", we delete "foo" and _then_
create "bar"). And due to the way refs are stored, we do not keep even a
reflog for the deleted ref in the interim.

-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] strbuf: describe the return value of strbuf_read_file

[Pranit Bauva]

Mentored-by: Lars Schneider 
Mentored-by: Christian Couder 
Signed-off-by: Pranit Bauva 
---
 strbuf.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/strbuf.h b/strbuf.h
index 7987405..83c5c98 100644
--- a/strbuf.h
+++ b/strbuf.h
@@ -377,6 +377,8 @@ extern ssize_t strbuf_read_once(struct strbuf *, int fd, 
size_t hint);
 /**
  * Read the contents of a file, specified by its path. The third argument
  * can be used to give a hint about the file size, to avoid reallocs.
+ * Return the number of bytes read or a negative value if some error
+ * occurred while opening or reading the file.
  */
 extern ssize_t strbuf_read_file(struct strbuf *sb, const char *path, size_t 
hint);
 
-- 
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] fetch: document that pruning happens before fetching

[Jacob Keller]

On Mon, Jun 13, 2016 at 4:58 PM, Jeff King  wrote:
> This was changed in 10a6cc8 (fetch --prune: Run prune before
> fetching, 2014-01-02), but it seems that nobody in that
> discussion realized we were advertising the "after"
> explicitly.
>
> Signed-off-by: Jeff King 
> ---
> I include myself in that "nobody" of course. :)
>
>  Documentation/fetch-options.txt | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/Documentation/fetch-options.txt b/Documentation/fetch-options.txt
> index 036edfb..b05a834 100644
> --- a/Documentation/fetch-options.txt
> +++ b/Documentation/fetch-options.txt
> @@ -52,7 +52,7 @@ ifndef::git-pull[]
>
>  -p::
>  --prune::
> -   After fetching, remove any remote-tracking references that no
> +   Before fetching, remove any remote-tracking references that no
> longer exist on the remote.  Tags are not subject to pruning
> if they are fetched only because of the default tag
> auto-following or due to a --tags option.  However, if tags

What's the difference in behavior due to pruning before instead of
after? Curious. It seems like pruning after would make more 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