* Re: [PATCH v3 2/5] stash: introduce push verb
From: Thomas Gummerer @ 2017-02-13 22:16 UTC (permalink / raw)
To: Matthieu Moy
Cc: git, Stephan Beyer, Junio C Hamano, Marc Strapetz, Jeff King,
Johannes Schindelin, Øyvind A . Holm, Jakub Narębski
In-Reply-To: <vpqlgtaz09q.fsf@anie.imag.fr>
On 02/13, Matthieu Moy wrote:
> Thomas Gummerer <t.gummerer@gmail.com> writes:
>
> > Introduce a new git stash push verb in addition to git stash save. The
> > push verb is used to transition from the current command line arguments
> > to a more conventional way, in which the message is given as an argument
> > to the -m option.
>
> Sorry if this has been discussed before, but I find 'push' rather
> confusing here. It took me a while to understand that it meant "opposite
> of pop", because in the context of Git, "push" usually means "send to
> remote repository".
There wasn't much of a discussion about it, but it was pretty much the
only thing that came to my mind, and nobody complained or suggested
anything different, so I just went with it. No other verb came to my
mind yet, but if someone has a better suggestion, I'd be happy to
change.
> Unfortunately, I didn't come up with a better name. "create" is already
> taken ...
>
> Another think to have in mind: changing the option name to break
> backward compatibility is something we can't do often, so if there's
> anything else we should change about the UI, we should do it now. I
> don't have anything particular in mind, just thinking aloud.
Now that you mention this, there actually is one inconsistency that I
introduced, which I shouldn't have. git stash push works with
--include-untracked and --all to decide whether or not to include
untracked files, and if also ignored files should be included. I also
added a --include-untracked {untracked,all} argument to git stash
create, which is clearly inconsistent.
There really should only be one way. I'd be fine with either way, but
I think using --include-untracked and --all is the better choice,
because it's easy to understand, and also makes it easier to switch
git stash without a verb over to use push_stash internally.
> --
> Matthieu Moy
> http://www-verimag.imag.fr/~moy/
^ permalink raw reply
* Re: [PATCH] docs/git-submodule: fix unbalanced quote
From: Junio C Hamano @ 2017-02-13 22:06 UTC (permalink / raw)
To: Jeff King; +Cc: git
In-Reply-To: <20170213210549.jns7asrvjp3lb5wc@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> However, let's take a step back. Even when rendered
> correctly, it's hard to read a long command stuck into the
> middle of a paragraph, and the important punctuation is hard
> to notice.
Yes, I like this reasoning behind the solution very much.
Thanks.
> Documentation/git-submodule.txt | 9 ++++++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
> index 918bd1d1b..a8eb1c7ce 100644
> --- a/Documentation/git-submodule.txt
> +++ b/Documentation/git-submodule.txt
> @@ -228,9 +228,12 @@ foreach::
> the processing to terminate. This can be overridden by adding '|| :'
> to the end of the command.
> +
> -As an example, +git submodule foreach \'echo $path {backtick}git
> -rev-parse HEAD{backtick}'+ will show the path and currently checked out
> -commit for each submodule.
> +As an example, the command below will show the path and currently
> +checked out commit for each submodule:
> ++
> +--------------
> +git submodule foreach 'echo $path `git rev-parse HEAD`'
> +--------------
>
> sync::
> Synchronizes submodules' remote URL configuration setting
^ permalink raw reply
* Re: [PATCH v3 4/5] stash: introduce new format create
From: Jeff King @ 2017-02-13 21:57 UTC (permalink / raw)
To: Thomas Gummerer
Cc: git, Stephan Beyer, Junio C Hamano, Marc Strapetz,
Johannes Schindelin, Øyvind A . Holm, Jakub Narębski
In-Reply-To: <20170211145127.GA23081@hank>
On Sat, Feb 11, 2017 at 02:51:27PM +0000, Thomas Gummerer wrote:
> > How do we tell the difference between new-style invocations, and
> > old-style ones that look new-style? IOW, I think:
> >
> > git stash create -m works
> >
> > currently treats "-m works" as the full message, and it would now become
> > just "works".
> >
> > That may be an acceptable loss for the benefit we are getting. The
> > alternative is to make yet another verb for create, as we did with
> > save/push). I have a feeling that hardly anybody uses "create", though,
> > and it's mostly an implementation detail. So given the obscure nature,
> > maybe it's an acceptable level of regression. I dunno.
>
> Right. So I did a quick search on google and github for this, and
> there seems one place where git stash create -m is used [1]. From a
> quick look it does however not seem like the -m in the stash message
> is of any significance there, but rather that the intention was to use
> a flag that doesn't exist.
Yeah, I think your patch is actually fixing that case. But your search
is only part of the story. You found somebody using "-m" explicitly, but
what about somebody blindly calling:
git stash create $*
That's now surprising to somebody who puts "-m" in their message.
> I *think* this regression is acceptable, but I'm happy to introduce
> another verb if people think otherwise.
Despite what I wrote above, I'm still inclined to say that this isn't an
important regression. I'd be surprised if "stash create" is used
independently much at all.
-Peff
^ permalink raw reply
* Re: [PATCH] clean: use warning_errno() when appropriate
From: Junio C Hamano @ 2017-02-13 21:53 UTC (permalink / raw)
To: Jeff King; +Cc: Nguyễn Thái Ngọc Duy, git
In-Reply-To: <20170213211051.vsnnrtcsvuvfcwyk@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> IOW, I think this may be a case where we should be optimizing for
> programmer time (fewer lines of code, and one less thing to worry about
> in the callers) versus squeezing out every instruction.
Fair enough.
Unless we do the save_errno dance in all the helper functions we
commonly use to safely stash away errno as necessary and tell
developers that they can depend on it, the code in the patch that
began this discussion still needs its own saved_errno dance to be
safe, though. I do not have a feeling that we are not there yet,
even after we teach xmalloc() and its family to do so.
^ permalink raw reply
* Re: Incorrect pipe for git log graph when using --name-status option
From: Jeff King @ 2017-02-13 21:51 UTC (permalink / raw)
To: hIpPy; +Cc: Junio C Hamano, git
In-Reply-To: <CAM_JFCzniePuqJTFOy-odLfuKcBjssh0zk3PFBsVcm6Ww6iW5w@mail.gmail.com>
On Mon, Feb 13, 2017 at 01:43:45PM -0800, hIpPy wrote:
> With --name-status: I'm sorry if I nitpick here but I think the
> --name-status items should either be preceeded and followed by
> blank line OR not (as in oneline) but not just preceded (example
> below).
Yeah, I agree the output is a bit ugly.
I don't think there is a way to do what you want currently. The
--oneline code path has some internal magic that suppresses some of the
newlines.
I think it is a good goal that anything that can be expressed via one of
the regular formats (like --oneline) can be expressed via custom
formats. I don't know offhand how hard it would be to make this
particular case work. If somebody is interested in tackling this, they'd
probably need to dig around in pretty.c to see where the ONELINE code
path diverge from the USER_FORMAT one, and then figure out how a user
can specify that they want the ONELINE-ish semantics for their format.
-Peff
^ permalink raw reply
* Re: [PATCH v3 0/5] stash: support pathspec argument
From: Jeff King @ 2017-02-13 21:45 UTC (permalink / raw)
To: Matthieu Moy
Cc: Thomas Gummerer, git, Stephan Beyer, Junio C Hamano,
Marc Strapetz, Johannes Schindelin, Øyvind A . Holm,
Jakub Narębski
In-Reply-To: <vpqo9y5lqos.fsf@anie.imag.fr>
On Mon, Feb 13, 2017 at 10:35:31PM +0100, Matthieu Moy wrote:
> > Is it really that dangerous, though? The likely outcome is Git saying
> > "nope, you don't have any changes to the file named drop". Of course the
> > user may have meant something different, but I feel like "-p" is a good
> > indicator that they are interested in making an actual stash.
>
> Indeed -p is not the best example. In the old thread, I used -q which is
> much more problematic:
>
> git stash -q drop => interpreted as: git stash push -q drop
> git stash drop -q => drop with option -q
Yeah, I'd agree with that. I wouldn't propose to loosen it entirely, but
rather to treat "-p" specially.
> It's not really "dangerous" at least in this case, since we misinterpret
> a destructive command for a less destructive one, but it is rather
> confusing that changing the order between command and options change the
> behavior.
>
> I actually find it a reasonable expectation to allow swapping commands
> and options, some programs other than git allow it.
I think we may have already crossed that bridge with "git -p stash".
Not to mention that the ordering already _is_ relevant (we disallow one
order but not the other). If we really wanted to allow swapping, it
would mean making:
git stash -p drop
the same as:
git stash drop -p
I actually find _that_ more confusing. It would perhaps make more sense
with something like "-q", which is more of a "global" option than a
command-specific one. But I think we'd want to whitelist such global
options (and "-p" would not be on that list).
> > The complexity is that right now, the first-level decision of "which
> > stash sub-command am I running?" doesn't know about any options. So "git
> > stash -m foo" would be rejected in the name of typo prevention, unless
> > that outer decision learns about "-m" as an option.
>
> Ah, OK. But that's not really hard to implement: when going through the
> option list looking for non-option, shift one more time when finding -m.
No, it's not hard conceptually. It just means implementing the
option-parsing policy in two places. That's not too bad now, but if we
started using rev-parse's options helper, then I think you have corner
cases like "git stash -km foo".
My "-p" suggestion suffers from a similar problem if you treat it as
"you can omit the 'push' if you say "-p", rather than "if -p is the
first option, it is a synonym for 'push -p'".
-Peff
^ permalink raw reply
* Re: Incorrect pipe for git log graph when using --name-status option
From: hIpPy @ 2017-02-13 21:43 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, git
In-Reply-To: <20170213212734.howisucqqhusbglc@sigill.intra.peff.net>
On Mon, Feb 13, 2017 at 1:27 PM, Jeff King <peff@peff.net> wrote:
> On Mon, Feb 13, 2017 at 01:18:40PM -0800, Junio C Hamano wrote:
>
>> Jeff King <peff@peff.net> writes:
>>
>> > I think the problem is specifically related to the "terminator" versus
>> > "separator" semantics. Try:
>> >
>> > git log --graph --name-status --pretty=format:%h
>> >
>> > versus:
>> >
>> > git log --graph --name-status --pretty=tformat:%h
>> >
>> > The latter works fine. The problem seems to happen with all diff
>> > formats, so I think the issue is that git is not aggressive enough about
>> > inserting the newline at the right spot when using separator mode.
>>
>> I guess that is one of the reasons why we made --format=%h a synonym
>> to --pretty=tformat:%h and not --pretty=format:%h ;-)
>
> Yeah. I have never found "--pretty=format" to do what I want versus
> tformat. I wish we could just get rid of it, but I think it is likely to
> be used as a plumbing interface.
>
> So I think there probably _is_ a bug in it to be fixed, but my immediate
> response is "don't ever use --pretty=format:".
>
> -Peff
I have 1 issue with tformat in that I feel it reduces
readability a bit when using options. But I can use it if that
is recommended over the other.
Without --name-status: This is OK.
$ git log --graph --date=relative --decorate --abbrev-commit
--format='%h - %aD (%ar)%d%n %s - %an' -2
* bf7ace7daf - Mon, 6 Feb 2017 13:09:32 -0800 (7 days ago) (HEAD ->
rm/option-for-name-status)
| wip: --ns for --name-status - Rishikesh Mandvikar
* 592e5c5bce - Wed, 1 Feb 2017 16:35:30 +0100 (12 days ago)
(origin/master, origin/HEAD, master)
|\ Merge pull request #994 from
jeffhostetler/jeffhostetler/fscache_nfd - Johannes Schindelin
With --name-status: I'm sorry if I nitpick here but I think the
--name-status items should either be preceeded and followed by
blank line OR not (as in oneline) but not just preceded (example
below).
$ git log --graph --date=relative --decorate --abbrev-commit
--format='%h - %aD (%ar)%d%n %s - %an' -2 --name-status
* bf7ace7daf - Mon, 6 Feb 2017 13:09:32 -0800 (7 days ago) (HEAD ->
rm/option-for-name-status)
| wip: --ns for --name-status - Rishikesh Mandvikar
|
| M diff.c
* 592e5c5bce - Wed, 1 Feb 2017 16:35:30 +0100 (12 days ago)
(origin/master, origin/HEAD, master)
|\ Merge pull request #994 from
jeffhostetler/jeffhostetler/fscache_nfd - Johannes Schindelin
So either of below is better that above (I feel):
Blank lines before and after --name-status items:
$ git log --graph --date=relative --decorate --abbrev-commit
--format='%h - %aD (%ar)%d%n %s - %an' -2 --name-status
* bf7ace7daf - Mon, 6 Feb 2017 13:09:32 -0800 (7 days ago) (HEAD ->
rm/option-for-name-status)
| wip: --ns for --name-status - Rishikesh Mandvikar
|
| M diff.c
|
* 592e5c5bce - Wed, 1 Feb 2017 16:35:30 +0100 (12 days ago)
(origin/master, origin/HEAD, master)
|\ Merge pull request #994 from
jeffhostetler/jeffhostetler/fscache_nfd - Johannes Schindelin
No blank lines before and after --name-status items:
$ git log --graph --date=relative --decorate --abbrev-commit
--format='%h - %aD (%ar)%d%n %s - %an' -2 --name-status
* bf7ace7daf - Mon, 6 Feb 2017 13:09:32 -0800 (7 days ago) (HEAD ->
rm/option-for-name-status)
| wip: --ns for --name-status - Rishikesh Mandvikar
| M diff.c
* 592e5c5bce - Wed, 1 Feb 2017 16:35:30 +0100 (12 days ago)
(origin/master, origin/HEAD, master)
|\ Merge pull request #994 from
jeffhostetler/jeffhostetler/fscache_nfd - Johannes Schindelin
What do guys think?
If there is a way to get what I want using tformat?
Thanks,
RM
^ permalink raw reply
* Re: [RFH] Request for Git Merge 2017 impressions for Git Rev News
From: Christian Couder @ 2017-02-13 21:27 UTC (permalink / raw)
To: Jakub Narębski; +Cc: git
In-Reply-To: <8b6e998d-2bea-924b-42e3-936dcd9a2995@gmail.com>
Hi,
On Sat, Feb 11, 2017 at 5:33 PM, Jakub Narębski <jnareb@gmail.com> wrote:
> Hello,
>
> Git Rev News #24 is planned to be released on February 15. It is meant
> to cover what happened during the month of January 2017 (and earely
> February 2017) and the Git Merge 2017 conference that happened on
> February 2nd and 3rd 2017.
Yeah, I would have liked to release Git Rev News #24 on February 15
but I was busy and tired this week end, so let it be on Wednesday
February 22.
> A draft of a new Git Rev News edition is available here:
>
> https://github.com/git/git.github.io/blob/master/rev_news/drafts/edition-24.md
>
> I would like to ask everyone who attended the conference (and the
> GitMerge 2017 Contributors’s Summit day before it), or watched it live
> at http://git-merge.com/watch to write his or her impressions.
>
> You can contribute either by replying to this email, or by editing the
> above page on GitHub and sending a pull request, or by commenting on
> the following GitHub issue about Git Rev News 24:
>
> https://github.com/git/git.github.io/issues/221
>
> If you prefer to post on your own blog (or if you have did it
> already), please send an URL.
Yeah, any material (link, short impression, article, ...) would be very nice.
Thanks Jakub for the links you already added to the draft, by the way!
> P.S. I wonder if there should be not a separate section on
> https://git.github.io/ about recollection from various Git-related
> events, with Git Merge 2017 as the first one. This way we can wait
> for later response, and incorporate videos and slides from events, as
> they begin to be available.
Jakub, if you are willing to create and maintain this section, that
would be great!
> P.P.S. Please distribute this information more widely.
Thanks,
Christian.
^ permalink raw reply
* Re: Incorrect pipe for git log graph when using --name-status option
From: Jeff King @ 2017-02-13 21:27 UTC (permalink / raw)
To: Junio C Hamano; +Cc: hIpPy, git
In-Reply-To: <xmqq37fhdc27.fsf@gitster.mtv.corp.google.com>
On Mon, Feb 13, 2017 at 01:18:40PM -0800, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
>
> > I think the problem is specifically related to the "terminator" versus
> > "separator" semantics. Try:
> >
> > git log --graph --name-status --pretty=format:%h
> >
> > versus:
> >
> > git log --graph --name-status --pretty=tformat:%h
> >
> > The latter works fine. The problem seems to happen with all diff
> > formats, so I think the issue is that git is not aggressive enough about
> > inserting the newline at the right spot when using separator mode.
>
> I guess that is one of the reasons why we made --format=%h a synonym
> to --pretty=tformat:%h and not --pretty=format:%h ;-)
Yeah. I have never found "--pretty=format" to do what I want versus
tformat. I wish we could just get rid of it, but I think it is likely to
be used as a plumbing interface.
So I think there probably _is_ a bug in it to be fixed, but my immediate
response is "don't ever use --pretty=format:".
-Peff
^ permalink raw reply
* Re: Incorrect pipe for git log graph when using --name-status option
From: Junio C Hamano @ 2017-02-13 21:18 UTC (permalink / raw)
To: Jeff King; +Cc: hIpPy, git
In-Reply-To: <20170213211653.x3huwmzprvmm3yxj@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> I think the problem is specifically related to the "terminator" versus
> "separator" semantics. Try:
>
> git log --graph --name-status --pretty=format:%h
>
> versus:
>
> git log --graph --name-status --pretty=tformat:%h
>
> The latter works fine. The problem seems to happen with all diff
> formats, so I think the issue is that git is not aggressive enough about
> inserting the newline at the right spot when using separator mode.
I guess that is one of the reasons why we made --format=%h a synonym
to --pretty=tformat:%h and not --pretty=format:%h ;-)
^ permalink raw reply
* Re: Incorrect pipe for git log graph when using --name-status option
From: Jeff King @ 2017-02-13 21:16 UTC (permalink / raw)
To: hIpPy; +Cc: Junio C Hamano, git
In-Reply-To: <CAM_JFCwN+o54mJ1XJ3rSKnXgPx3wt_i=fd8ZSGpcL-fSegQ=Ow@mail.gmail.com>
On Mon, Feb 13, 2017 at 12:55:07PM -0800, hIpPy wrote:
> I think that requiring to end custom formats with %n with options
> is not good. So I think this is a bug.
Yes, you don't normally need to do that.
I think the problem is specifically related to the "terminator" versus
"separator" semantics. Try:
git log --graph --name-status --pretty=format:%h
versus:
git log --graph --name-status --pretty=tformat:%h
The latter works fine. The problem seems to happen with all diff
formats, so I think the issue is that git is not aggressive enough about
inserting the newline at the right spot when using separator mode.
-Peff
^ permalink raw reply
* Re: [PATCH] clean: use warning_errno() when appropriate
From: Jeff King @ 2017-02-13 21:10 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Nguyễn Thái Ngọc Duy, git
In-Reply-To: <xmqqk28tddwo.fsf@gitster.mtv.corp.google.com>
On Mon, Feb 13, 2017 at 12:38:47PM -0800, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
>
> > I wonder if xmalloc() should be the one doing the saved_errno trick.
> > After all, it has only two outcomes: we successfully allocated the
> > memory, or we called die().
>
> I would be lying if I said I did not considered it when I wrote the
> message you are responding to, but I rejected it because that would
> be optimizing for a wrong case, in that most callers of xmalloc()
> and friends do not do so in the error codepath, and we would be
> penalizing them by doing the saved_errno dance unconditionally.
Yes, that also occurred to me. I'm not sure if two integer swaps is
enough to care about when compared to the cost of a malloc(), though.
IOW, I think this may be a case where we should be optimizing for
programmer time (fewer lines of code, and one less thing to worry about
in the callers) versus squeezing out every instruction.
-Peff
^ permalink raw reply
* Re: [PATCH] mingw: use OpenSSL's SHA-1 routines
From: Johannes Sixt @ 2017-02-13 21:07 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Schindelin, Jeff King, git, Jeff Hostetler
In-Reply-To: <xmqq60kdev2r.fsf@gitster.mtv.corp.google.com>
Am 13.02.2017 um 20:42 schrieb Junio C Hamano:
> I have been operating under the assumption that everybody on Windows
> who builds Git works off of Dscho's Git for Windows tree, and
> patches that are specific to Windows from Dscho's are sent to me via
> the list only after they have been in Git for Windows and proven to
> help Windows users in the wild.
>
> The consequence of these two assumptions is that I would feel safe
> to treat Windows specific changes that do not touch generic part of
> the codebase from Dscho just like updates from any other subsystem
> maintainers (any git-svn thing from Eric, any gitk thing from Paul,
> any p4 thing Luke and Lars are both happy with, etc.).
>
> You seem to be saying that the first of the two assumptions does not
> hold. Should I change my expectations while queuing Windows specific
> patches from Dscho?
Your first assumption is incorrect as far as I am concerned. I build
from your tree plus some topics. During -rc period, I build off of
master; after a release, I build off of next. I merge some of the topics
that you carry in pu when I find them interesting or when I suspect them
to regress on Windows. Then I carry around a few additional patches that
the public has never seen, and these days I also merge Dscho's rebase-i
topic.
-- Hannes
^ permalink raw reply
* [PATCH] docs/git-submodule: fix unbalanced quote
From: Jeff King @ 2017-02-13 21:05 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano
In-Reply-To: <20170213203835.vssj64tcvuq35dny@sigill.intra.peff.net>
The documentation gives an example of the submodule foreach
command that uses both backticks and single-quotes. We stick
the whole thing inside "+" markers to make it monospace, but
the inside punctuation still needs escaping. We handle the
backticks with "{backtick}", and use backslash-escaping for
the single-quotes.
But we missed the escaping on the second quote. Fortunately,
asciidoc renders this unbalanced quote as we want (showing
the quote), but asciidoctor does not. We could fix it by
adding the missing backslash.
However, let's take a step back. Even when rendered
correctly, it's hard to read a long command stuck into the
middle of a paragraph, and the important punctuation is hard
to notice. Let's instead bump it into its own single-line
code block. That makes both the source and the rendered
result more readable, and as a bonus we don't have to worry
about quoting at all.
Signed-off-by: Jeff King <peff@peff.net>
---
Not textually related to the previous fix, but obviously along the same
lines.
Documentation/git-submodule.txt | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index 918bd1d1b..a8eb1c7ce 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -228,9 +228,12 @@ foreach::
the processing to terminate. This can be overridden by adding '|| :'
to the end of the command.
+
-As an example, +git submodule foreach \'echo $path {backtick}git
-rev-parse HEAD{backtick}'+ will show the path and currently checked out
-commit for each submodule.
+As an example, the command below will show the path and currently
+checked out commit for each submodule:
++
+--------------
+git submodule foreach 'echo $path `git rev-parse HEAD`'
+--------------
sync::
Synchronizes submodules' remote URL configuration setting
--
2.12.0.rc1.466.g70234cfd8
^ permalink raw reply related
* Re: [PATCH] rm: reuse strbuf for all remove_dir_recursively() calls, again
From: Stefan Beller @ 2017-02-13 21:03 UTC (permalink / raw)
To: René Scharfe; +Cc: Git List, Junio C Hamano
In-Reply-To: <f2549b5b-b3c8-3935-cdb9-9908914195b6@web.de>
On Sat, Feb 11, 2017 at 11:51 AM, René Scharfe <l.s.r@web.de> wrote:
> Don't throw the memory allocated for remove_dir_recursively() away after
> a single call, use it for the other entries as well instead.
>
> This change was done before in deb8e15a (rm: reuse strbuf for all
> remove_dir_recursively() calls), but was reverted as a side-effect of
> 55856a35 (rm: absorb a submodules git dir before deletion). Reinstate
> the optimization.
>
> Signed-off-by: Rene Scharfe <l.s.r@web.de>
> ---
> Was deb8e15a a rebase victim?
(I do not recall it off the top of my head)
That commit was merged at 03f25e85,
Merge branch 'rs/rm-strbuf-optim', but it looks
like it was reverted as part of 55856a35b2
(rm: absorb a submodules git dir before deletion)
Looking through the discussion at
https://public-inbox.org/git/xmqqmvfich2e.fsf@gitster.mtv.corp.google.com/
there was no apparent signs of confusion, but a reroll was promised, that
I cannot find on the list.
Anyway, the patch below looks good to me.
Thanks,
Stefan
>
> builtin/rm.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/builtin/rm.c b/builtin/rm.c
> index 452170a3ab..fb79dcab18 100644
> --- a/builtin/rm.c
> +++ b/builtin/rm.c
> @@ -360,15 +360,14 @@ int cmd_rm(int argc, const char **argv, const char *prefix)
> */
> if (!index_only) {
> int removed = 0, gitmodules_modified = 0;
> + struct strbuf buf = STRBUF_INIT;
> for (i = 0; i < list.nr; i++) {
> const char *path = list.entry[i].name;
> if (list.entry[i].is_submodule) {
> - struct strbuf buf = STRBUF_INIT;
> -
> + strbuf_reset(&buf);
> strbuf_addstr(&buf, path);
> if (remove_dir_recursively(&buf, 0))
> die(_("could not remove '%s'"), path);
> - strbuf_release(&buf);
>
> removed = 1;
> if (!remove_path_from_gitmodules(path))
> @@ -382,6 +381,7 @@ int cmd_rm(int argc, const char **argv, const char *prefix)
> if (!removed)
> die_errno("git rm: '%s'", path);
> }
> + strbuf_release(&buf);
> if (gitmodules_modified)
> stage_updated_gitmodules();
> }
> --
> 2.11.1
>
^ permalink raw reply
* Re: [RFC PATCH] show decorations at the end of the line
From: Junio C Hamano @ 2017-02-13 21:01 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Git Mailing List
In-Reply-To: <CA+55aFy-vvOBu5Y4KDeteUyK-7U7yTa1HoqHo+hME1=8bq7Xhw@mail.gmail.com>
Linus Torvalds <torvalds@linux-foundation.org> writes:
> And if you actually want decorations, and you're parsing them, you are
> *not* going to script it with "--oneline --decorations", because the
> end result is basically impossible to parse already (because it's
> ambiguous - think about parentheses in the commit message).
OK. So let's wait to hear from others if they like the "obviously"
improved output. Even though I find the decorations indispensable
in my "git log" output, I personally do not have much preference
either way, as my screen is often wide enough ;-)
Thanks. We'd need to update the tests that expects the old style
output, though.
^ permalink raw reply
* Re: [PATCH 04/11] files-backend: replace *git_path*() with files_path()
From: Ramsay Jones @ 2017-02-13 20:58 UTC (permalink / raw)
To: Nguyễn Thái Ngọc Duy, git
Cc: Junio C Hamano, Michael Haggerty, Johannes Schindelin, sbeller,
novalis
In-Reply-To: <20170213152011.12050-5-pclouds@gmail.com>
On 13/02/17 15:20, Nguyễn Thái Ngọc Duy wrote:
> This centralizes all path rewriting of files-backend.c in one place so
> we have easier time removing the path rewriting later. There could be
> some hidden indirect git_path() though, I didn't audit the code to the
> bottom.
>
> Side note: set_worktree_head_symref() is a bad boy and should not be in
> files-backend.c (probably should not exist in the first place). But
> we'll leave it there until we have better multi-worktree support in refs
> before we update it.
>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
> refs/files-backend.c | 193 ++++++++++++++++++++++++++-------------------------
> 1 file changed, 98 insertions(+), 95 deletions(-)
>
> diff --git a/refs/files-backend.c b/refs/files-backend.c
> index 39217a2ca..c69e4fe84 100644
> --- a/refs/files-backend.c
> +++ b/refs/files-backend.c
> @@ -165,12 +165,13 @@ static struct ref_entry *create_dir_entry(struct files_ref_store *ref_store,
> const char *dirname, size_t len,
> int incomplete);
> static void add_entry_to_dir(struct ref_dir *dir, struct ref_entry *entry);
> -static int files_log_ref_write(const char *refname, const unsigned char *old_sha1,
> +static int files_log_ref_write(struct files_ref_store *refs,
> + const char *refname, const unsigned char *old_sha1,
> const unsigned char *new_sha1, const char *msg,
> int flags, struct strbuf *err);
>
> -void files_path(struct files_ref_store *refs, struct strbuf *sb,
> - const char *fmt, ...) __attribute__((format (printf, 3, 4)));
You only added this in the last commit, so maybe mark it static in
the previous patch! Also, just in case you were wondering, the 'Why?'
of the previous email was, "Why do you need this forward declaration?"
(hint: you don't ;-)
> +static void files_path(struct files_ref_store *refs, struct strbuf *sb,
> + const char *fmt, ...) __attribute__((format (printf, 3, 4)));
>
> static struct ref_dir *get_ref_dir(struct ref_entry *entry)
> {
> @@ -933,8 +934,8 @@ struct files_ref_store {
> /* Lock used for the main packed-refs file: */
> static struct lock_file packlock;
>
> -void files_path(struct files_ref_store *refs, struct strbuf *sb,
> - const char *fmt, ...)
> +static void files_path(struct files_ref_store *refs, struct strbuf *sb,
> + const char *fmt, ...)
> {
> struct strbuf tmp = STRBUF_INIT;
> va_list vap;
ATB,
Ramsay Jones
^ permalink raw reply
* Re: Incorrect pipe for git log graph when using --name-status option
From: hIpPy @ 2017-02-13 20:55 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <xmqqa89pevw0.fsf@gitster.mtv.corp.google.com>
On Mon, Feb 13, 2017 at 11:25 AM, Junio C Hamano <gitster@pobox.com> wrote:
> hIpPy <hippy2981@gmail.com> writes:
>
>> The `git log` command with `graph` and pretty format works correctly
>> as expected.
>>
>> $ git log --graph --pretty=format:'%h' -2
>> * 714a14e
>> * 87dce5f
>>
>>
>> However, with `--name-status` option added, there is a pipe
>> incorrectly placed after the commit hash (example below).
>>
>> $ git log --graph --pretty=format:'%h' -2 --name-status
>> * 714a14e|
>> | M README.md
>
> Is it a --name-status, or is it your own custom format, that is
> causing the above issue?
>
> - What happens if you stop using --pretty=format:%h and replace it
> with something like --oneline?
>
> - What happens if you keep using --pretty=format:%h but replace
> --name-status with something else, e.g. --raw or --stat?
>
- What happens if you stop using --pretty=format:%h and replace it
with something like --oneline?
--oneline works correctly as expected (example below).
$ git log --graph --oneline -2 --name-status
* bf7ace7daf (HEAD -> rm/option-for-name-status) wip: --ns for --name-status
| M diff.c
* 592e5c5bce (origin/master, origin/HEAD, master) Merge pull request
#994 from jeffhostetler/jeffhostetler/fscache_nfd
|\
- What happens if you keep using --pretty=format:%h but replace
--name-status with something else, e.g. --raw or --stat?
I see the same issue with --raw and --stat (examples below).
$ git log --graph --pretty=format:'%h' -2 --raw
* bf7ace7daf|
| :100644 100644 84dba60c40... 87dfabf4a9... M diff.c
* 592e5c5bce
|\
$ git log --graph --pretty=format:'%h' -2 --stat
* bf7ace7daf|
| diff.c | 2 +-
| 1 file changed, 1 insertion(+), 1 deletion(-)
* 592e5c5bce
|\
I believe it's not my custom format that is causing the issue.
I'm including this information that may not be relevant. I apologize
in advance for that. I had simplified the custom format in my
previous email. For below custom format I still see the pipe
incorrectly placed.
$ git log --graph --date=relative --decorate --abbrev-commit
--format=format:'%h - %aD (%ar)%d%n %s - %an' -2 --name-status
* bf7ace7daf - Mon, 6 Feb 2017 13:09:32 -0800 (7 days ago) (HEAD ->
rm/option-for-name-status)
| wip: --ns for --name-status - Rishikesh Mandvikar|
| M diff.c
* 592e5c5bce - Wed, 1 Feb 2017 16:35:30 +0100 (12 days ago)
(origin/master, origin/HEAD, master)
|\ Merge pull request #994 from
jeffhostetler/jeffhostetler/fscache_nfd - Johannes Schindelin
If I were to put a '%n' at the end (example below), the pipe is
correctly placed with or without the --name-status option. But in
case of "without the --name-status option", there is an extra blank
line. It seems that my custom format is requiring a %n at the end. I
consider my custom format as a --twoline option and I feel the
behavior should match --oneline when using options.
With --name-status: This works correctly.
$ git log --graph --date=relative --decorate --abbrev-commit
--format=format:'%h - %aD (%ar)%d%n %s - %an%n' -2 --name-status
* bf7ace7daf - Mon, 6 Feb 2017 13:09:32 -0800 (7 days ago) (HEAD ->
rm/option-for-name-status)
| wip: --ns for --name-status - Rishikesh Mandvikar
|
| M diff.c
|
* 592e5c5bce - Wed, 1 Feb 2017 16:35:30 +0100 (12 days ago)
(origin/master, origin/HEAD, master)
|\ Merge pull request #994 from
jeffhostetler/jeffhostetler/fscache_nfd - Johannes Schindelin
Without --name-status: This works but has extra blank line between
commits though.
$ git log --graph --date=relative --decorate --abbrev-commit
--format=format:'%h - %aD (%ar)%d%n %s - %an%n' -2
* bf7ace7daf - Mon, 6 Feb 2017 13:09:32 -0800 (7 days ago) (HEAD ->
rm/option-for-name-status)
| wip: --ns for --name-status - Rishikesh Mandvikar
|
* 592e5c5bce - Wed, 1 Feb 2017 16:35:30 +0100 (12 days ago)
(origin/master, origin/HEAD, master)
|\ Merge pull request #994 from
jeffhostetler/jeffhostetler/fscache_nfd - Johannes Schindelin
I think that requiring to end custom formats with %n with options
is not good. So I think this is a bug.
Thanks,
RM
^ permalink raw reply
* Re: Small regression on Windows
From: Junio C Hamano @ 2017-02-13 20:47 UTC (permalink / raw)
To: Jeff Hostetler
Cc: Johannes Sixt, Johannes Schindelin, Jeff Hostetler,
Git Mailing List
In-Reply-To: <8488f55c-37b1-1ded-53c5-7cd268210391@jeffhostetler.com>
Jeff Hostetler <git@jeffhostetler.com> writes:
> The warning text is being written to stderr and then main thread
> sleeps for the 4 seconds. However, stderr has been redirected to
> the pipe connected to the console_thread that handles the
> coloring/pager. It is in a blocking ReadFile on the pipe and is
> thus stuck until the main thread runs the corrected command and
> closes the pipe. It then sees the EOF and prints everything in
> the pipe buffer.
IOW, somehow your stderr is not flushing automatically?
> I'll look into fixing this now.
Thanks.
^ permalink raw reply
* Re: [PATCH] completion: restore removed line continuating backslash
From: Junio C Hamano @ 2017-02-13 20:45 UTC (permalink / raw)
To: SZEDER Gábor; +Cc: git
In-Reply-To: <20170213192036.10671-1-szeder.dev@gmail.com>
SZEDER Gábor <szeder.dev@gmail.com> writes:
> Recent commit 1cd23e9e0 (completion: don't use __gitdir() for git
> commands, 2017-02-03) rewrapped a couple of long lines, and while
> doing so it inadvertently removed a '\' from the end of a line, thus
> breaking completion for 'git config remote.name.push <TAB>'.
>
> Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
> ---
>
> I wanted this to be a fixup!, but then noticed that this series is
> already in next, hence the proper commit message.
We get "--format=... : command not found"?
Thanks for a set of sharp eyes.
> I see the last What's cooking marks this series as "will merge to
> master". That doesn't mean that it will be in v2.12, does it?
I actually was hoping that these can go in. Others that can (or
should) wait are marked as "Will cook in 'next'".
If you feel uncomfortable and want these to cook longer, please tell
me so.
Thanks.
> contrib/completion/git-completion.bash | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> index 993085af6..f2b294aac 100644
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -2086,7 +2086,7 @@ _git_config ()
> remote.*.push)
> local remote="${prev#remote.}"
> remote="${remote%.push}"
> - __gitcomp_nl "$(__git for-each-ref
> + __gitcomp_nl "$(__git for-each-ref \
> --format='%(refname):%(refname)' refs/heads)"
> return
> ;;
^ permalink raw reply
* Re: [PATCH 03/11] files-backend: add files_path()
From: Ramsay Jones @ 2017-02-13 20:43 UTC (permalink / raw)
To: Nguyễn Thái Ngọc Duy, git
Cc: Junio C Hamano, Michael Haggerty, Johannes Schindelin, sbeller,
novalis
In-Reply-To: <20170213152011.12050-4-pclouds@gmail.com>
On 13/02/17 15:20, Nguyễn Thái Ngọc Duy wrote:
> This will be the replacement for both git_path() and git_path_submodule()
> in this file. The idea is backend takes a git path and use that,
> oblivious of submodule, linked worktrees and such.
>
> This is the middle step towards that. Eventually the "submodule" field
> in 'struct files_ref_store' should be replace by "gitdir". And a
> compound ref_store is created to combine two files backends together,
> one represents the shared refs in $GIT_COMMON_DIR, one per-worktree. At
> that point, files_path() becomes a wrapper of strbuf_vaddf().
>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
> refs/files-backend.c | 20 ++++++++++++++++++++
> 1 file changed, 20 insertions(+)
>
> diff --git a/refs/files-backend.c b/refs/files-backend.c
> index 6582c9b2d..39217a2ca 100644
> --- a/refs/files-backend.c
> +++ b/refs/files-backend.c
> @@ -169,6 +169,9 @@ static int files_log_ref_write(const char *refname, const unsigned char *old_sha
> const unsigned char *new_sha1, const char *msg,
> int flags, struct strbuf *err);
>
> +void files_path(struct files_ref_store *refs, struct strbuf *sb,
> + const char *fmt, ...) __attribute__((format (printf, 3, 4)));
> +
Why?
> static struct ref_dir *get_ref_dir(struct ref_entry *entry)
> {
> struct ref_dir *dir;
> @@ -930,6 +933,23 @@ struct files_ref_store {
> /* Lock used for the main packed-refs file: */
> static struct lock_file packlock;
>
> +void files_path(struct files_ref_store *refs, struct strbuf *sb,
> + const char *fmt, ...)
> +{
> + struct strbuf tmp = STRBUF_INIT;
> + va_list vap;
> +
> + va_start(vap, fmt);
> + strbuf_vaddf(&tmp, fmt, vap);
> + va_end(vap);
> + if (refs->submodule)
> + strbuf_git_path_submodule(sb, refs->submodule,
> + "%s", tmp.buf);
> + else
> + strbuf_git_path(sb, "%s", tmp.buf);
> + strbuf_release(&tmp);
> +}
> +
> /*
> * Increment the reference count of *packed_refs.
> */
>
ATB,
Ramsay Jones
^ permalink raw reply
* Re: Small regression on Windows
From: Jeff Hostetler @ 2017-02-13 20:32 UTC (permalink / raw)
To: Johannes Sixt, Johannes Schindelin, Jeff Hostetler
Cc: Junio C Hamano, Git Mailing List
In-Reply-To: <6bc02b0a-a463-1f0c-3fee-ba27dd2482e4@kdbg.org>
On 2/13/2017 1:00 PM, Johannes Sixt wrote:
> Please type this, preferably outside of any repo to avoid that it is
> changed; note the command typo:
>
> git -c help.autocorrect=40 ceckout
>
> Git waits for 4 seconds, but does not write anything until just before
> it exits. It bisects to
>
> a9b8a09c3c30886c79133da9f48ef9f98c21c3b2 is the first bad commit
> commit a9b8a09c3c30886c79133da9f48ef9f98c21c3b2
> Author: Jeff Hostetler <jeffhost@microsoft.com>
> Date: Thu Dec 22 18:09:23 2016 +0100
>
> mingw: replace isatty() hack
>
> Git for Windows has carried a patch that depended on internals
> of MSVC runtime, but it does not work correctly with recent MSVC
> runtime. A replacement was written originally for compiling
> with VC++. The patch in this message is a backport of that
> replacement, and it also fixes the previous attempt to make
> isatty() tell that /dev/null is *not* an interactive terminal.
>
> Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com>
> Tested-by: Beat Bolli <dev+git@drbeat.li>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
>
> :040000 040000 bc3c75bdfc766fe478e160a9452c31bfef50b505
> f7183c3a0726fef7161d5f9ff8c997260025f1bb M compat
>
> Any ideas? I haven't had time to dig any further.
I'm investigating now.
The warning text is being written to stderr and then main thread sleeps
for the 4 seconds.
However, stderr has been redirected to the pipe connected to the
console_thread that
handles the coloring/pager. It is in a blocking ReadFile on the pipe
and is thus stuck until
the main thread runs the corrected command and closes the pipe. It then
sees the EOF
and prints everything in the pipe buffer.
I'll look into fixing this now.
Jeff
^ permalink raw reply
* Re: [PATCH] clean: use warning_errno() when appropriate
From: Junio C Hamano @ 2017-02-13 20:38 UTC (permalink / raw)
To: Jeff King; +Cc: Nguyễn Thái Ngọc Duy, git
In-Reply-To: <20170213191433.muwhz7zem64p3rxr@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> I wonder if xmalloc() should be the one doing the saved_errno trick.
> After all, it has only two outcomes: we successfully allocated the
> memory, or we called die().
I would be lying if I said I did not considered it when I wrote the
message you are responding to, but I rejected it because that would
be optimizing for a wrong case, in that most callers of xmalloc()
and friends do not do so in the error codepath, and we would be
penalizing them by doing the saved_errno dance unconditionally.
^ permalink raw reply
* Re: [PATCH 02/11] files-backend: convert git_path() to strbuf_git_path()
From: Ramsay Jones @ 2017-02-13 20:38 UTC (permalink / raw)
To: Nguyễn Thái Ngọc Duy, git
Cc: Junio C Hamano, Michael Haggerty, Johannes Schindelin, sbeller,
novalis
In-Reply-To: <20170213152011.12050-3-pclouds@gmail.com>
On 13/02/17 15:20, Nguyễn Thái Ngọc Duy wrote:
> git_path() and friends are going to be killed in files-backend.c in near
> future. And because there's a risk with overwriting buffer in
> git_path(), let's convert them all to strbuf_git_path(). We'll have
> easier time killing/converting strbuf_git_path() then because we won't
> have to worry about memory management again.
>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
> refs/files-backend.c | 114 ++++++++++++++++++++++++++++++++++++++++-----------
> 1 file changed, 90 insertions(+), 24 deletions(-)
>
> diff --git a/refs/files-backend.c b/refs/files-backend.c
> index 75565c3aa..6582c9b2d 100644
> --- a/refs/files-backend.c
> +++ b/refs/files-backend.c
> @@ -2169,6 +2169,8 @@ static int lock_packed_refs(struct files_ref_store *refs, int flags)
> static int timeout_configured = 0;
> static int timeout_value = 1000;
> struct packed_ref_cache *packed_ref_cache;
> + struct strbuf sb = STRBUF_INIT;
> + int ret;
>
> files_assert_main_repository(refs, "lock_packed_refs");
>
> @@ -2177,10 +2179,13 @@ static int lock_packed_refs(struct files_ref_store *refs, int flags)
> timeout_configured = 1;
> }
>
> - if (hold_lock_file_for_update_timeout(
> - &packlock, git_path("packed-refs"),
> - flags, timeout_value) < 0)
> + strbuf_git_path(&sb, "packed-refs");
> + ret = hold_lock_file_for_update_timeout(&packlock, sb.buf,
> + flags, timeout_value);
> + strbuf_release(&sb);
> + if (ret < 0)
> return -1;
> +
> /*
> * Get the current packed-refs while holding the lock. If the
> * packed-refs file has been modified since we last read it,
> @@ -2335,6 +2340,9 @@ static void try_remove_empty_parents(char *name)
> for (q = p; *q; q++)
> ;
> while (1) {
> + struct strbuf sb = STRBUF_INIT;
> + int ret;
> +
> while (q > p && *q != '/')
> q--;
> while (q > p && *(q-1) == '/')
> @@ -2342,7 +2350,10 @@ static void try_remove_empty_parents(char *name)
> if (q == p)
> break;
> *q = '\0';
> - if (rmdir(git_path("%s", name)))
> + strbuf_git_path(&sb, "%s", name);
> + ret = rmdir(sb.buf);
> + strbuf_release(&sb);
> + if (ret)
> break;
> }
> }
> @@ -2431,7 +2442,11 @@ static int repack_without_refs(struct files_ref_store *refs,
> return 0; /* no refname exists in packed refs */
>
> if (lock_packed_refs(refs, 0)) {
> - unable_to_lock_message(git_path("packed-refs"), errno, err);
> + struct strbuf sb = STRBUF_INIT;
> +
> + strbuf_git_path(&sb, "packed-refs");
> + unable_to_lock_message(sb.buf, errno, err);
> + strbuf_release(&sb);
> return -1;
> }
> packed = get_packed_refs(refs);
> @@ -2529,11 +2544,12 @@ static int rename_tmp_log(const char *newrefname)
> {
> int attempts_remaining = 4;
> struct strbuf path = STRBUF_INIT;
> + struct strbuf tmp_renamed_log = STRBUF_INIT;
> int ret = -1;
>
> - retry:
> - strbuf_reset(&path);
> strbuf_git_path(&path, "logs/%s", newrefname);
> + strbuf_git_path(&tmp_renamed_log, TMP_RENAMED_LOG);
> + retry:
> switch (safe_create_leading_directories_const(path.buf)) {
> case SCLD_OK:
> break; /* success */
> @@ -2546,7 +2562,7 @@ static int rename_tmp_log(const char *newrefname)
> goto out;
> }
>
> - if (rename(git_path(TMP_RENAMED_LOG), path.buf)) {
> + if (rename(tmp_renamed_log.buf, path.buf)) {
> if ((errno==EISDIR || errno==ENOTDIR) && --attempts_remaining > 0) {
> /*
> * rename(a, b) when b is an existing
> @@ -2574,6 +2590,7 @@ static int rename_tmp_log(const char *newrefname)
> ret = 0;
> out:
> strbuf_release(&path);
> + strbuf_release(&tmp_renamed_log);
> return ret;
> }
>
> @@ -2614,9 +2631,15 @@ static int files_rename_ref(struct ref_store *ref_store,
> int flag = 0, logmoved = 0;
> struct ref_lock *lock;
> struct stat loginfo;
> - int log = !lstat(git_path("logs/%s", oldrefname), &loginfo);
> + struct strbuf sb_oldref = STRBUF_INIT;
> + struct strbuf sb_newref = STRBUF_INIT;
> + struct strbuf tmp_renamed_log = STRBUF_INIT;
> + int log, ret;
> struct strbuf err = STRBUF_INIT;
>
> + strbuf_git_path(&sb_oldref, "logs/%s", oldrefname);
> + log = !lstat(sb_oldref.buf, &loginfo);
> + strbuf_release(&sb_oldref);
> if (log && S_ISLNK(loginfo.st_mode))
> return error("reflog for %s is a symlink", oldrefname);
>
> @@ -2630,7 +2653,12 @@ static int files_rename_ref(struct ref_store *ref_store,
> if (!rename_ref_available(oldrefname, newrefname))
> return 1;
>
> - if (log && rename(git_path("logs/%s", oldrefname), git_path(TMP_RENAMED_LOG)))
> + strbuf_git_path(&sb_oldref, "logs/%s", oldrefname);
> + strbuf_git_path(&tmp_renamed_log, TMP_RENAMED_LOG);
> + ret = log && rename(sb_oldref.buf, tmp_renamed_log.buf);
It probably won't make too much difference, but the two code
sequences above are not similar in terms of side-effects when
'log' is false. In that case, the two calls to git_path() and
the call to rename() are not made in the original code. In the
new sequence, the two calls to strbuf_git_path() are always made
(but rename() is not).
> + strbuf_release(&sb_oldref);
> + strbuf_release(&tmp_renamed_log);
> + if (ret)
> return error("unable to move logfile logs/%s to "TMP_RENAMED_LOG": %s",
> oldrefname, strerror(errno));
>
> @@ -2709,13 +2737,19 @@ static int files_rename_ref(struct ref_store *ref_store,
> log_all_ref_updates = flag;
>
> rollbacklog:
> - if (logmoved && rename(git_path("logs/%s", newrefname), git_path("logs/%s", oldrefname)))
> + strbuf_git_path(&sb_newref, "logs/%s", newrefname);
> + strbuf_git_path(&sb_oldref, "logs/%s", oldrefname);
> + if (logmoved && rename(sb_newref.buf, sb_oldref.buf))
ditto
> error("unable to restore logfile %s from %s: %s",
> oldrefname, newrefname, strerror(errno));
> + strbuf_git_path(&tmp_renamed_log, TMP_RENAMED_LOG);
> if (!logmoved && log &&
> - rename(git_path(TMP_RENAMED_LOG), git_path("logs/%s", oldrefname)))
> + rename(tmp_renamed_log.buf, sb_oldref.buf))
similar
> error("unable to restore logfile %s from "TMP_RENAMED_LOG": %s",
> oldrefname, strerror(errno));
> + strbuf_release(&sb_newref);
> + strbuf_release(&sb_oldref);
> + strbuf_release(&tmp_renamed_log);
>
> return 1;
> }
ATB,
Ramsay Jones
^ permalink raw reply
* [PATCH] docs/gitremote-helpers: fix unbalanced quotes
From: Jeff King @ 2017-02-13 20:38 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano
Each of these options is missing the closing single-quote on
the option name. This understandably confuses asciidoc,
which ends up rendering a stray quote, like:
option cloning {'true|false}
Signed-off-by: Jeff King <peff@peff.net>
---
Documentation/gitremote-helpers.txt | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/Documentation/gitremote-helpers.txt b/Documentation/gitremote-helpers.txt
index 9e8681f9e..7e59c50b1 100644
--- a/Documentation/gitremote-helpers.txt
+++ b/Documentation/gitremote-helpers.txt
@@ -452,14 +452,14 @@ set by Git if the remote helper has the 'option' capability.
Request the helper to perform a force update. Defaults to
'false'.
-'option cloning {'true'|'false'}::
+'option cloning' {'true'|'false'}::
Notify the helper this is a clone request (i.e. the current
repository is guaranteed empty).
-'option update-shallow {'true'|'false'}::
+'option update-shallow' {'true'|'false'}::
Allow to extend .git/shallow if the new refs require it.
-'option pushcert {'true'|'false'}::
+'option pushcert' {'true'|'false'}::
GPG sign pushes.
SEE ALSO
--
2.12.0.rc1.466.g70234cfd8
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox