* Re: [RFC] stash --continue
From: Samuel Lijin @ 2017-01-18 19:35 UTC (permalink / raw)
To: Marc Branchaud; +Cc: Johannes Schindelin, Stephan Beyer, git
In-Reply-To: <38d592b8-975c-1fd9-4c42-877e34a4ab70@xiplink.com>
>> At least `git stash pop --continue` would be consistent with all other
>> `--continue` options in Git that I can think of...
> Alas, I disagree!
I'm with Johannes here. "git stash" sans subcommand is pretty
explicitly defined as "git stash save", so by similar logic, "git
stash --continue", if anything, would be "git stash save --continue".
I do agree that there's a slight problem with hunting down consistency
in implementations of --continue since there aren't other usages that
involve subcommands (rebase, cp, merge) but I can't think of "git
stash" as a completely specified command, whereas I do see "git stash
pop" and "git stash apply" as completely specified.
On Wed, Jan 18, 2017 at 12:44 PM, Marc Branchaud <marcnarc@xiplink.com> wrote:
> On 2017-01-18 11:34 AM, Johannes Schindelin wrote:
>>
>> Hi Marc,
>>
>> On Wed, 18 Jan 2017, Marc Branchaud wrote:
>>
>>> On 2017-01-16 05:54 AM, Johannes Schindelin wrote:
>>>
>>>> On Mon, 16 Jan 2017, Stephan Beyer wrote:
>>>>
>>>>> a git-newbie-ish co-worker uses git-stash sometimes. Last time he
>>>>> used "git stash pop", he got into a merge conflict. After he
>>>>> resolved the conflict, he did not know what to do to get the
>>>>> repository into the wanted state. In his case, it was only "git add
>>>>> <resolved files>" followed by a "git reset" and a "git stash drop",
>>>>> but there may be more involved cases when your index is not clean
>>>>> before "git stash pop" and you want to have your index as before.
>>>>>
>>>>> This led to the idea to have something like "git stash
>>>>> --continue"[1]
>>>>
>>>>
>>>> More like "git stash pop --continue". Without the "pop" command, it
>>>> does not make too much sense.
>>>
>>>
>>> Why not? git should be able to remember what stash command created the
>>> conflict. Why should I have to? Maybe the fire alarm goes off right
>>> when I
>>> run the stash command, and by the time I get back to it I can't remember
>>> which operation I did. It would be nice to be able to tell git to "just
>>> finish off (or abort) the stash operation, whatever it was".
>>
>>
>> That reeks of a big potential for confusion.
>>
>> Imagine for example a total Git noob who calls `git stash list`, scrolls
>> two pages down, then hits `q` by mistake. How would you explain to that
>> user that `git stash --continue` does not continue showing the list at the
>> third page?
>
>
> Sorry, but I have trouble taking that example seriously. It assumes such a
> level of "noobness" that the user doesn't even understand how standard
> command output paging works, not just with git but with any shell command.
>
>> Even worse: `git stash` (without arguments) defaults to the `save`
>> operation, so any user who does not read the documentation (and who does?)
>> would assume that `git stash --continue` *also* implies `save`.
>
>
> Like the first example, your user is trying to "continue" a command that is
> already complete. It's like try to do "git rebase --continue" when there's
> no rebase operation underway.
>
> Now, maybe there is some way for "git stash save" (implied or explicit) to
> stop partway through the operation. I can't imagine such a situation (out
> of disk space, maybe?), particularly where the user would expect "git stash
> save" to leave things in a half-finished state. To me "git stash save"
> should be essentially all-or-nothing.
>
> However, if there were such a partial-failure scenario, then I think it
> would be perfectly reasonable for "git stash --continue" to finish the save
> operation, assuming that the failure condition has been resolved.
>
>> If that was not enough, there would still be the overall design of Git's
>> user interface. You can call it confusing, inconsistent, with a lot of
>> room for improvement, and you would be correct. But none of Git's commands
>> has a `--continue` option that remembers the latest subcommand and
>> continues that. To introduce that behavior in `git stash` would disimprove
>> the situation.
>
>
> I think it's more the case that none of the current continuable commands
> have subcommands (though I can't think of all the continuable or abortable
> operations offhand, so maybe I'm wrong). I think we're discussing new UI
> ground here.
>
> And since the pattern is already "git foo --continue", it seems more
> consistent to me for it to be "git stash --continue" as well. Especially
> since there can be only one partially-complete stash sub-operation at one
> time (per workdir, at least). So there's no reason to change the pattern
> just for the stash command.
>
> Think of it this way: All the currently continuable/abortable commands put
> the repository in a shaky state, where performing certain other operations
> would be ill advised. Attempting to start a rebase while a merge conflict
> is unresolved, for example. IIRC, git actually tries to stop users from
> shooting their feet in this way.
>
> And so it should be for the stash operation: If applying a stash yields a
> conflict, it has to be resolved or aborted before something like a rebase or
> merge is attempted. It doesn't matter which stash subcommand created the
> shaky situation.
>
> In the long run, I think there's even the possibility of generic "git
> continue" and "git abort" commands, that simply continue or abort the
> current partially-complete operation, whatever it is. (Isn't that the
> ultimate goal of all the "sequencer" work? I admit I have not been
> following that effort.)
>
>> With every new feature, it is not enough to consider its benefits. You
>> always have to take the potential fallout into account, too.
>
>
> Agreed.
>
>> At least `git stash pop --continue` would be consistent with all other
>> `--continue` options in Git that I can think of...
>
>
> Alas, I disagree!
>
> M.
>
^ permalink raw reply
* Re: [PATCH] mingw: follow-up to "replace isatty() hack"
From: Junio C Hamano @ 2017-01-18 19:19 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git, Pranit Bauva, Johannes Sixt, Beat Bolli
In-Reply-To: <867bafbe582df549b10729a5d688458bb6a98d51.1484741665.git.johannes.schindelin@gmx.de>
Johannes Schindelin <johannes.schindelin@gmx.de> writes:
> The version of the "replace isatty() hack" that got applied to the
> `maint` branch did not actually reflect the latest iteration of the
> patch series: v3 was sent out with these changes, as requested by
> the reviewer Johannes Sixt:
Thanks for an update.
Does the above "the version taken was not updated before getting
merged" mistake only apply to 'maint', or is it also true for
'master'?
As a rule we only merge things to 'maint' that have already been
merged to 'master', so I am guessing that the answer is yes, in
which case I'd queue it on js/mingw-isatty and then merge it to
next, master and maint in that order as usual.
> - reworded the comment about "recycling handles"
>
> - moved the reassignment of the `console` variable before the dup2()
> call so that it is valid at all times
>
> - removed the "handle = INVALID_HANDLE_VALUE" assignment, as the local
> variable `handle` is not used afterwards anyway
>
Also if the v3 had been reviewed and acked, it would be nice to have
the acked-by around here (which I can locally do). Hannes?
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
Thanks.
> Published-As: https://github.com/dscho/git/releases/tag/mingw-isatty-fixup-fixup-v1
> Fetch-It-Via: git fetch https://github.com/dscho/git mingw-isatty-fixup-fixup-v1
>
> compat/winansi.c | 11 ++++-------
> 1 file changed, 4 insertions(+), 7 deletions(-)
>
> diff --git a/compat/winansi.c b/compat/winansi.c
> index 3c9ed3cfe0..82b89ab137 100644
> --- a/compat/winansi.c
> +++ b/compat/winansi.c
> @@ -494,19 +494,16 @@ static HANDLE swap_osfhnd(int fd, HANDLE new_handle)
> * It is because of this implicit close() that we created the
> * copy of the original.
> *
> - * Note that the OS can recycle HANDLE (numbers) just like it
> - * recycles fd (numbers), so we must update the cached value
> - * of "console". You can use GetFileType() to see that
> - * handle and _get_osfhandle(fd) may have the same number
> - * value, but they refer to different actual files now.
> + * Note that we need to update the cached console handle to the
> + * duplicated one because the dup2() call will implicitly close
> + * the original one.
> *
> * Note that dup2() when given target := {0,1,2} will also
> * call SetStdHandle(), so we don't need to worry about that.
> */
> - dup2(new_fd, fd);
> if (console == handle)
> console = duplicate;
> - handle = INVALID_HANDLE_VALUE;
> + dup2(new_fd, fd);
>
> /* Close the temp fd. This explicitly closes "new_handle"
> * (because it has been associated with it).
>
> base-commit: 3313b78c145ba9212272b5318c111cde12bfef4a
^ permalink raw reply
* Re: [RFC] stash --continue
From: Stephan Beyer @ 2017-01-18 19:20 UTC (permalink / raw)
To: Marc Branchaud, Johannes Schindelin; +Cc: git
In-Reply-To: <d5456165-bdf2-e9e7-117f-aeab0ff4b417@xiplink.com>
Hi,
On 01/18/2017 04:41 PM, Marc Branchaud wrote:
> On 2017-01-16 05:54 AM, Johannes Schindelin wrote:
>> On Mon, 16 Jan 2017, Stephan Beyer wrote:
>>> a git-newbie-ish co-worker uses git-stash sometimes. Last time he used
>>> "git stash pop", he got into a merge conflict. After he resolved the
>>> conflict, he did not know what to do to get the repository into the
>>> wanted state. In his case, it was only "git add <resolved files>"
>>> followed by a "git reset" and a "git stash drop", but there may be more
>>> involved cases when your index is not clean before "git stash pop" and
>>> you want to have your index as before.
>>>
>>> This led to the idea to have something like "git stash --continue"[1]
>>
>> More like "git stash pop --continue". Without the "pop" command, it does
>> not make too much sense.
>
> Why not? git should be able to remember what stash command created the
> conflict. Why should I have to?
Dscho and Junio gave you a git-perspective argument.
I give you a user-perspective one:
What if you did "git stash pop" and ran into an (unexpected) conflict.
You resolve it, and you probably - for some reason - don't want to drop
the stash now, as "git stash --continue" (assuming "pop") would do. So
I'd regard it as a feature if you could now run "git stash apply
--continue" to just finish the job without dropping.
Best
Stephan
PS: I put this idea in my todo priority queue. If somebody else is
interested: I am not going to work at this idea before February.
PPS: Any opinions about the mentioned "backwards-compatibility" issue
that people are then forced to finish their commits with "--continue"
instead of "git reset" or "git commit"?
^ permalink raw reply
* Re: [PATCH v5 3/3] Retire the scripted difftool
From: Junio C Hamano @ 2017-01-18 19:15 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git, David Aguilar, Dennis Kaarsemaker, Paul Sbarra
In-Reply-To: <alpine.DEB.2.20.1701181332230.3469@virtualbox>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> Hi Junio,
>
> On Tue, 17 Jan 2017, Junio C Hamano wrote:
>
>> Johannes Schindelin <johannes.schindelin@gmx.de> writes:
>>
>> > It served its purpose, but now we have a builtin difftool. Time for the
>> > Perl script to enjoy Florida.
>> >
>> > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
>> > ---
>>
>> The endgame makes a lot of sense. Both in the cover letter and in
>> the previous patch you talk about having both in the released
>> version, so do you want this step to proceed slower than the other
>> two?
>
> I did proceed that slowly. Already three Git for Windows versions have
> been released with both.
>
> But I submitted this iteration with this patch, so my intent is clearly to
> retire the Perl script.
Ok, I was mostly reacting to 2/3 while I am reading it:
The reason: this new, experimental, builtin difftool will be shipped as
part of Git for Windows v2.11.0, to allow for easier large-scale
testing, but of course as an opt-in feature.
as there is no longer an opportunity to participate in this opt-in
testing, unless 3/3 is special cased and delayed.
^ permalink raw reply
* Re: Git: new feature suggestion
From: Joao Pinto @ 2017-01-18 19:04 UTC (permalink / raw)
To: Stefan Beller, Joao Pinto
Cc: git@vger.kernel.org, Linus Torvalds, CARLOS.PALMINHA@synopsys.com
In-Reply-To: <CAGZ79kYXQcUB+rVkboY9fMqu6R3RoHEJ7BTJn_+-RScFDjEduA@mail.gmail.com>
Hi Stefan,
Às 6:50 PM de 1/18/2017, Stefan Beller escreveu:
> On Wed, Jan 18, 2017 at 2:40 AM, Joao Pinto <Joao.Pinto@synopsys.com> wrote:
>> Hello,
>>
>> My name is Joao Pinto, I work at Synopsys and I am a frequent Linux Kernel
>> contributor.
>>
>> Let me start by congratulate you for the fantastic work you have been doing with
>> Git which is an excellent tool.
>>
>> The Linux Kernel as all systems needs to be improved and re-organized to be
>> better prepared for future development and sometimes we need to change
>> folder/files names or even move things around.
>> I have seen a lot of Linux developers avoid this re-organization operations
>> because they would lose the renamed file history, because a new log is created
>> for the new file, even if it is a renamed version of itself.
>
> Well there are a couple of things to help with digging in the logs.
>
> git log:
> --follow
> Continue listing the history of a file beyond renames (works only
> for a single file).
>
> -M[<n>], --find-renames[=<n>]
> If generating diffs, detect and report renames for each commit. For
> following files across renames while traversing history, see
> --follow. If n is specified, it is a threshold on the similarity
> index (i.e. amount of addition/deletions compared to the file’s
> size). For example, -M90% means Git should consider a delete/add
> pair to be a rename if more than 90% of the file hasn’t changed.
> Without a % sign, the number is to be read as a fraction, with a
> decimal point before it. I.e., -M5 becomes 0.5, and is thus the
> same as -M50%. Similarly, -M05 is the same as -M5%. To limit
> detection to exact renames, use -M100%. The default similarity
> index is 50%.
>
> -C[<n>], --find-copies[=<n>]
> Detect copies as well as renames. See also --find-copies-harder. If
> n is specified, it has the same meaning as for -M<n>.
>
>
>
>> I am sending you this e-mail to suggest the creation of a new feature in Git:
>> when renamed, a file or folder should inherit his parent’s log and a “rename: …”
>> would be automatically created or have some kind of pointer to its “old” form to
>> make history analysis easier.
>
> How do you currently analyse history, which detailed feature is missing?
>
> Mind that in the Git data model we deliberately do not record the rename
> at commit time, but rather want to identify the renames at log time.
> This is because
> in the meantime between commit and log viewing someone could have written
> a better rename detection, whereas at commit time we'd be stuck with ancient
> cruft forever. ;)
>
>>
>> I volunteer to help in the new feature if you find it useful. I think it would
>> improve log history analysis and would enable developers to better organize old
>> code.
>
> IMHO complete renames (i.e. git mv path/a/file.c path/b/thing.c) are already
> covered quite well. Partial rename (e.g. moving code from one file into two
> separate files or vice versa) is still a bit hard.
>
> I started such a new feature, see
> https://urldefense.proofpoint.com/v2/url?u=https-3A__public-2Dinbox.org_git_20160903033120.20511-2D1-2Dsbeller-40google.com_&d=DwIFaQ&c=DPL6_X_6JkXFx7AXWqB0tg&r=s2fO0hii0OGNOv9qQy_HRXy-xAJUD1NNoEcc3io_kx0&m=BseICq5hy9UHxmX2XP8oPYLbn-HoEUlEuVUzqPHkX58&s=PybtKK0ELH3Nld_CQSYZnLqCQOWvnU4Fjj5iV_7EKqE&e=
> latest code is at https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_stefanbeller_git_commits_colored-5Fdiff12&d=DwIFaQ&c=DPL6_X_6JkXFx7AXWqB0tg&r=s2fO0hii0OGNOv9qQy_HRXy-xAJUD1NNoEcc3io_kx0&m=BseICq5hy9UHxmX2XP8oPYLbn-HoEUlEuVUzqPHkX58&s=pkTehcEmeHVLHdcNbUiU03meyH10cgUbGqLgOqXcL6w&e= ,
> but the latest two commits are bogus and need rewriting.
>
> I think this feature is not 100% what you are aiming at, but is very close.
>
> Thanks,
> Stefan
>
Great info, helps a lot! I am going to analyse and get back to you ASAP.
Thanks
^ permalink raw reply
* Re: What's cooking in git.git (Jan 2017, #02; Sun, 15)
From: Junio C Hamano @ 2017-01-18 19:12 UTC (permalink / raw)
To: Johannes Sixt; +Cc: Jeff King, Johannes Schindelin, git
In-Reply-To: <1aa4a8b0-4bda-edbc-7be8-1ffd9f74eef7@kdbg.org>
Johannes Sixt <j6t@kdbg.org> writes:
> Am 17.01.2017 um 20:17 schrieb Junio C Hamano:
>> So... can we move this forward?
>
> I have no objects anymore.
>
> -- Hannes
Alright, thanks.
Dscho what's your assessment? My "I do not think" before the quoted
"can we move" above was more about giving a statement for people to
argue against, by saying "no your understanding is wrong", in the
hope that it would highlight why we need more work in this area if
needed and in what way.
Thanks.
^ permalink raw reply
* Re: [RFC] stash --continue
From: Junio C Hamano @ 2017-01-18 19:10 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Marc Branchaud, Stephan Beyer, git
In-Reply-To: <alpine.DEB.2.20.1701181725130.3469@virtualbox>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>> > More like "git stash pop --continue". Without the "pop" command, it
>> > does not make too much sense.
>>
>> Why not? git should be able to remember what stash command created the
>> conflict. Why should I have to? Maybe the fire alarm goes off right when I
>> run the stash command, and by the time I get back to it I can't remember
>> which operation I did. It would be nice to be able to tell git to "just
>> finish off (or abort) the stash operation, whatever it was".
>
> That reeks of a big potential for confusion.
Yup. I agree everything you said in the message I am responding
to. Marc's argument will inevitably lead to: It should be
sufficient to say "git --continue", as Git should remember
everything for me. I do not think we want to go there.
^ permalink raw reply
* RE: [RFC] Add support for downloading blobs on demand
From: Ben Peart @ 2017-01-18 18:27 UTC (permalink / raw)
To: 'Stefan Beller', 'Martin Fick'
Cc: 'Shawn Pearce', 'git', benpeart
In-Reply-To: <CAGZ79kbWHHOj5x=SqSvUPdXtyYZUqDBnPG+erfZHsUkA8Cv-NA@mail.gmail.com>
We actually pursued trying to make submodules work for some time and
even built tooling around trying to work around some of the issues we
ran into (not repo.py but along a similar line) before determining that
we would be better served by having a single repo and solving the scale
issues. I don't want to rehash the arguments for/against a single repo
- suffice it to say, we have opted for a single large repo. :)
Thanks,
Ben
> -----Original Message-----
> From: Stefan Beller [mailto:sbeller@google.com]
> Sent: Tuesday, January 17, 2017 5:24 PM
> To: Martin Fick <mfick@codeaurora.org>
> Cc: Ben Peart <peartben@gmail.com>; Shawn Pearce
> <spearce@spearce.org>; git <git@vger.kernel.org>;
> benpeart@microsoft.com
> Subject: Re: [RFC] Add support for downloading blobs on demand
>
> On Tue, Jan 17, 2017 at 2:05 PM, Martin Fick <mfick@codeaurora.org>
> wrote:
> > On Tuesday, January 17, 2017 04:50:13 PM Ben Peart wrote:
> >> While large files can be a real problem, our biggest issue today is
> >> having a lot (millions!) of source files when any individual
> >> developer only needs a small percentage of them. Git with 3+ million
> >> local files just doesn't perform well.
> >
> > Honestly, this sounds like a problem better dealt with by using git
> > subtree or git submodules, have you considered that?
> >
> > -Martin
> >
>
> I cannot speak for subtrees as I have very little knowledge on them.
> But there you also have the problem that *someone* has to have a whole
> tree? (e.g. the build bot)
>
> submodules however comes with a couple of things attached, both positive
> as well as negative points:
>
> * it offers ACLs along the way. ($user may not be allowed to
> clone all submodules, but only those related to the work)
> * The conceptual understanding of git just got a lot harder.
> ("Yo dawg, I heard you like git, so I put git repos inside
> other git repos"), it is not easy to come up with reasonable
> defaults for all usecases, so the everyday user still has to
> have some understanding of submodules.
> * typical cheap in-tree operations may become very expensive:
> -> moving a file from one location to another (in another
> submodule) adds overhead, no rename detection.
> * We are actively working on submodules, so there is
> some momentum going already.
> * our experiments with Android show that e.g. fetching (even
> if you have all of Android) becomes a lot faster for everyday
> usage as only a few repositories change each day). This
> comparision was against the repo tool, that we currently
> use for Android. I do not know how it would compare against
> single repo Git, as having such a large repository seemed
> complicated.
> * the support for submodules in Git is already there, though
> not polished. The positive side is to have already a good base,
> the negative side is to have support current use cases.
>
> Stefan
^ permalink raw reply
* Re: [PATCH v6 4/6] builtin/tag: add --format argument for tag -v
From: Junio C Hamano @ 2017-01-18 18:52 UTC (permalink / raw)
To: Santiago Torres; +Cc: git, peff, sunshine, walters, Lukas Puehringer
In-Reply-To: <20170118182831.pkhqu2np3bh2puei@LykOS.localdomain>
Santiago Torres <santiago@nyu.edu> writes:
>> Squashing the following into this commit solves this issue with the
>> former approach. The lines it touches are all from 4/6 and I view
>> all of it as general improvement, including type correctness and
>> code formatting.
>
> Thanks!
>
> Should I re-roll this really quick? Or would you rather apply this on
> your tree directly?
Nah, local squashing should be sufficient in this case. The squash
only touches a single patch from the original and it itself is easy
to review (and was reviewed already from what I can tell in this
thread).
^ permalink raw reply
* Re: Git: new feature suggestion
From: Stefan Beller @ 2017-01-18 18:50 UTC (permalink / raw)
To: Joao Pinto
Cc: git@vger.kernel.org, Linus Torvalds, CARLOS.PALMINHA@synopsys.com
In-Reply-To: <4817eb00-6efc-e3c0-53d7-46f2509350d3@synopsys.com>
On Wed, Jan 18, 2017 at 2:40 AM, Joao Pinto <Joao.Pinto@synopsys.com> wrote:
> Hello,
>
> My name is Joao Pinto, I work at Synopsys and I am a frequent Linux Kernel
> contributor.
>
> Let me start by congratulate you for the fantastic work you have been doing with
> Git which is an excellent tool.
>
> The Linux Kernel as all systems needs to be improved and re-organized to be
> better prepared for future development and sometimes we need to change
> folder/files names or even move things around.
> I have seen a lot of Linux developers avoid this re-organization operations
> because they would lose the renamed file history, because a new log is created
> for the new file, even if it is a renamed version of itself.
Well there are a couple of things to help with digging in the logs.
git log:
--follow
Continue listing the history of a file beyond renames (works only
for a single file).
-M[<n>], --find-renames[=<n>]
If generating diffs, detect and report renames for each commit. For
following files across renames while traversing history, see
--follow. If n is specified, it is a threshold on the similarity
index (i.e. amount of addition/deletions compared to the file’s
size). For example, -M90% means Git should consider a delete/add
pair to be a rename if more than 90% of the file hasn’t changed.
Without a % sign, the number is to be read as a fraction, with a
decimal point before it. I.e., -M5 becomes 0.5, and is thus the
same as -M50%. Similarly, -M05 is the same as -M5%. To limit
detection to exact renames, use -M100%. The default similarity
index is 50%.
-C[<n>], --find-copies[=<n>]
Detect copies as well as renames. See also --find-copies-harder. If
n is specified, it has the same meaning as for -M<n>.
> I am sending you this e-mail to suggest the creation of a new feature in Git:
> when renamed, a file or folder should inherit his parent’s log and a “rename: …”
> would be automatically created or have some kind of pointer to its “old” form to
> make history analysis easier.
How do you currently analyse history, which detailed feature is missing?
Mind that in the Git data model we deliberately do not record the rename
at commit time, but rather want to identify the renames at log time.
This is because
in the meantime between commit and log viewing someone could have written
a better rename detection, whereas at commit time we'd be stuck with ancient
cruft forever. ;)
>
> I volunteer to help in the new feature if you find it useful. I think it would
> improve log history analysis and would enable developers to better organize old
> code.
IMHO complete renames (i.e. git mv path/a/file.c path/b/thing.c) are already
covered quite well. Partial rename (e.g. moving code from one file into two
separate files or vice versa) is still a bit hard.
I started such a new feature, see
https://public-inbox.org/git/20160903033120.20511-1-sbeller@google.com/
latest code is at https://github.com/stefanbeller/git/commits/colored_diff12,
but the latest two commits are bogus and need rewriting.
I think this feature is not 100% what you are aiming at, but is very close.
Thanks,
Stefan
^ permalink raw reply
* Re: [RFC] stash --continue
From: Marc Branchaud @ 2017-01-18 18:44 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Stephan Beyer, git
In-Reply-To: <alpine.DEB.2.20.1701181725130.3469@virtualbox>
On 2017-01-18 11:34 AM, Johannes Schindelin wrote:
> Hi Marc,
>
> On Wed, 18 Jan 2017, Marc Branchaud wrote:
>
>> On 2017-01-16 05:54 AM, Johannes Schindelin wrote:
>>
>>> On Mon, 16 Jan 2017, Stephan Beyer wrote:
>>>
>>>> a git-newbie-ish co-worker uses git-stash sometimes. Last time he
>>>> used "git stash pop", he got into a merge conflict. After he
>>>> resolved the conflict, he did not know what to do to get the
>>>> repository into the wanted state. In his case, it was only "git add
>>>> <resolved files>" followed by a "git reset" and a "git stash drop",
>>>> but there may be more involved cases when your index is not clean
>>>> before "git stash pop" and you want to have your index as before.
>>>>
>>>> This led to the idea to have something like "git stash
>>>> --continue"[1]
>>>
>>> More like "git stash pop --continue". Without the "pop" command, it
>>> does not make too much sense.
>>
>> Why not? git should be able to remember what stash command created the
>> conflict. Why should I have to? Maybe the fire alarm goes off right when I
>> run the stash command, and by the time I get back to it I can't remember
>> which operation I did. It would be nice to be able to tell git to "just
>> finish off (or abort) the stash operation, whatever it was".
>
> That reeks of a big potential for confusion.
>
> Imagine for example a total Git noob who calls `git stash list`, scrolls
> two pages down, then hits `q` by mistake. How would you explain to that
> user that `git stash --continue` does not continue showing the list at the
> third page?
Sorry, but I have trouble taking that example seriously. It assumes
such a level of "noobness" that the user doesn't even understand how
standard command output paging works, not just with git but with any
shell command.
> Even worse: `git stash` (without arguments) defaults to the `save`
> operation, so any user who does not read the documentation (and who does?)
> would assume that `git stash --continue` *also* implies `save`.
Like the first example, your user is trying to "continue" a command that
is already complete. It's like try to do "git rebase --continue" when
there's no rebase operation underway.
Now, maybe there is some way for "git stash save" (implied or explicit)
to stop partway through the operation. I can't imagine such a situation
(out of disk space, maybe?), particularly where the user would expect
"git stash save" to leave things in a half-finished state. To me "git
stash save" should be essentially all-or-nothing.
However, if there were such a partial-failure scenario, then I think it
would be perfectly reasonable for "git stash --continue" to finish the
save operation, assuming that the failure condition has been resolved.
> If that was not enough, there would still be the overall design of Git's
> user interface. You can call it confusing, inconsistent, with a lot of
> room for improvement, and you would be correct. But none of Git's commands
> has a `--continue` option that remembers the latest subcommand and
> continues that. To introduce that behavior in `git stash` would disimprove
> the situation.
I think it's more the case that none of the current continuable commands
have subcommands (though I can't think of all the continuable or
abortable operations offhand, so maybe I'm wrong). I think we're
discussing new UI ground here.
And since the pattern is already "git foo --continue", it seems more
consistent to me for it to be "git stash --continue" as well.
Especially since there can be only one partially-complete stash
sub-operation at one time (per workdir, at least). So there's no reason
to change the pattern just for the stash command.
Think of it this way: All the currently continuable/abortable commands
put the repository in a shaky state, where performing certain other
operations would be ill advised. Attempting to start a rebase while a
merge conflict is unresolved, for example. IIRC, git actually tries to
stop users from shooting their feet in this way.
And so it should be for the stash operation: If applying a stash yields
a conflict, it has to be resolved or aborted before something like a
rebase or merge is attempted. It doesn't matter which stash subcommand
created the shaky situation.
In the long run, I think there's even the possibility of generic "git
continue" and "git abort" commands, that simply continue or abort the
current partially-complete operation, whatever it is. (Isn't that the
ultimate goal of all the "sequencer" work? I admit I have not been
following that effort.)
> With every new feature, it is not enough to consider its benefits. You
> always have to take the potential fallout into account, too.
Agreed.
> At least `git stash pop --continue` would be consistent with all other
> `--continue` options in Git that I can think of...
Alas, I disagree!
M.
^ permalink raw reply
* Re: [PATCH v6 4/6] builtin/tag: add --format argument for tag -v
From: Santiago Torres @ 2017-01-18 18:50 UTC (permalink / raw)
To: Junio C Hamano
Cc: meta, git, peff, sunshine, walters, Lukas Puehringer, Eric Wong
In-Reply-To: <xmqq4m0wb43w.fsf@gitster.mtv.corp.google.com>
On Wed, Jan 18, 2017 at 10:44:03AM -0800, Junio C Hamano wrote:
> Santiago Torres <santiago@nyu.edu> writes:
>
Was:
Thanks!
Would you want me to re-roll really quick? or would you rather apply
this on your side?
Thanks,
-Santiago.
>
> Eric, I've noticed that this message
>
> http://public-inbox.org/git/20170118182831.pkhqu2np3bh2puei@LykOS.localdomain/
>
> and all messages from Santiago appear empty when they come via
> public-inbox.org; the reason I suspect we haven't heard much
> complaints is because nobody else around here sends multipart/signed
> disposition inline other than Santiago.
>
Interesting, I thought I wasn't inlining the .asc. I guess I can disable
signing for this ML for the time being.
Thanks for letting me know.
-Santiago.
^ permalink raw reply
* Re: [PATCH v6 4/6] builtin/tag: add --format argument for tag -v
From: Junio C Hamano @ 2017-01-18 18:49 UTC (permalink / raw)
To: Jeff King; +Cc: santiago, git, sunshine, walters, Lukas Puehringer
In-Reply-To: <20170118182843.iyico5jpii6a3z7i@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
>> diff --git a/builtin/tag.c b/builtin/tag.c
>> index f81273a85a..fbb85ba3dc 100644
>> --- a/builtin/tag.c
>> +++ b/builtin/tag.c
>> @@ -66,10 +66,10 @@ static int list_tags(struct ref_filter *filter, struct ref_sorting *sorting, con
>> }
>>
>> typedef int (*each_tag_name_fn)(const char *name, const char *ref,
>> - const unsigned char *sha1, void *cb_data);
>> + const unsigned char *sha1, const void *cb_data);
>
> This would bite us later if one of the iterators really does need to
> pass something mutable. But as this iteration interface is confined to
> builtin/tag.c, I think it's a nice simple fix.
>
> A more general fix would be to pass a non-const pointer to const pointer
> (preferably inside a struct for readability). But I don't see any need
> for that complexity here.
My first trial was to loosen the constness of existing variable,
which was OK, but made me feel dirty by turning what does not need
to be mutable into mutable. The iterator being local made me try
the other way and it turned out that currently there is no need for
mutable callback data ;-)
I agree that this may have to be updated, and if this were more
global thing, we'd better off doing so from the get-go, but for a
calling convention that is limited within a single file, I am more
comfortable saying we'll cross the bridge when we need to.
Thanks.
^ permalink raw reply
* Re: [PATCH v6 4/6] builtin/tag: add --format argument for tag -v
From: Junio C Hamano @ 2017-01-18 18:44 UTC (permalink / raw)
To: Santiago Torres, meta
Cc: git, peff, sunshine, walters, Lukas Puehringer, Eric Wong
In-Reply-To: <20170118182831.pkhqu2np3bh2puei@LykOS.localdomain>
Santiago Torres <santiago@nyu.edu> writes:
<<nothing>>???
Eric, I've noticed that this message
http://public-inbox.org/git/20170118182831.pkhqu2np3bh2puei@LykOS.localdomain/
and all messages from Santiago appear empty when they come via
public-inbox.org; the reason I suspect we haven't heard much
complaints is because nobody else around here sends multipart/signed
disposition inline other than Santiago.
^ permalink raw reply
* Re: [PATCH] gitk: remove translated message from comments
From: Junio C Hamano @ 2017-01-18 18:27 UTC (permalink / raw)
To: Paul Mackerras; +Cc: David Aguilar, git
In-Reply-To: <20170118101515.GA12161@fergus.ozlabs.ibm.com>
Paul Mackerras <paulus@ozlabs.org> writes:
> On Tue, Jan 17, 2017 at 07:52:45PM -0800, David Aguilar wrote:
>> "make update-po" fails because a previously untranslated string
>> has now been translated:
>>
>> Updating po/sv.po
>> po/sv.po:1388: duplicate message definition...
>> po/sv.po:380: ...this is the location of the first definition
>>
>> Remove the duplicate message definition.
>>
>> Reported-by: Junio C Hamano <gitster@pobox.com>
>> Signed-off-by: David Aguilar <davvid@gmail.com>
>
> Thanks, applied.
>
> Junio, please do a pull from my repository to get this fix.
> The new head is 7f03c6e32891.
Thanks.
^ permalink raw reply
* Re: [PATCH v6 4/6] builtin/tag: add --format argument for tag -v
From: Santiago Torres @ 2017-01-18 18:28 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, peff, sunshine, walters, Lukas Puehringer
In-Reply-To: <xmqqh94wb4y0.fsf@gitster.mtv.corp.google.com>
[-- Attachment #1: Type: text/plain, Size: 1515 bytes --]
On Wed, Jan 18, 2017 at 10:25:59AM -0800, Junio C Hamano wrote:
> Junio C Hamano <gitster@pobox.com> writes:
>
> > santiago@nyu.edu writes:
> >
> >> @@ -428,9 +443,12 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
> >> if (filter.merge_commit)
> >> die(_("--merged and --no-merged option are only allowed with -l"));
> >> if (cmdmode == 'd')
> >> - return for_each_tag_name(argv, delete_tag);
> >> - if (cmdmode == 'v')
> >> - return for_each_tag_name(argv, verify_tag);
> >> + return for_each_tag_name(argv, delete_tag, NULL);
> >> + if (cmdmode == 'v') {
> >> + if (format)
> >> + verify_ref_format(format);
> >> + return for_each_tag_name(argv, verify_tag, format);
> >> + }
> >
> > This triggers:
> >
> > builtin/tag.c: In function 'cmd_tag':
> > builtin/tag.c:451:3: error: passing argument 3 of
> > 'for_each_tag_name' discards 'const' qualifier from pointer target type [-Werror]
> > return for_each_tag_name(argv, verify_tag, format);
> >
> > Either for-each-tag-name's new parameter needs to be typed
> > correctly, or the type of the "format" variable needs to be updated.
>
> Squashing the following into this commit solves this issue with the
> former approach. The lines it touches are all from 4/6 and I view
> all of it as general improvement, including type correctness and
> code formatting.
Thanks!
Should I re-roll this really quick? Or would you rather apply this on
your tree directly?
-Santiago.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: problem with insider build for windows and git
From: Junio C Hamano @ 2017-01-18 18:32 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Michael Gooch, git
In-Reply-To: <alpine.DEB.2.20.1701181738490.3469@virtualbox>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> And just to prove me wrong, today I got the first update to `maint` in six
> weeks, with a message "Almost ready for 2.11.1" at its tip, featuring a
> whopping 141 commits (95 of which are not merge commits).
>
> So it seems that v2.11.1 may happen soon, after all.
Sorry for being late. I had a short travel around the year boundary,
got sick and have been slow.
Aside from the "ouch, one topic has merged earlier iteration, that
was merged to 'master', also now merged to 'maint', and we need to
follow up on both" you sent out earlier, are there any other topic
that are already in 'master' that should go to 2.11.x track?
Thanks.
^ permalink raw reply
* Re: [PATCH v6 4/6] builtin/tag: add --format argument for tag -v
From: Jeff King @ 2017-01-18 18:28 UTC (permalink / raw)
To: Junio C Hamano; +Cc: santiago, git, sunshine, walters, Lukas Puehringer
In-Reply-To: <xmqqh94wb4y0.fsf@gitster.mtv.corp.google.com>
On Wed, Jan 18, 2017 at 10:25:59AM -0800, Junio C Hamano wrote:
> > This triggers:
> >
> > builtin/tag.c: In function 'cmd_tag':
> > builtin/tag.c:451:3: error: passing argument 3 of
> > 'for_each_tag_name' discards 'const' qualifier from pointer target type [-Werror]
> > return for_each_tag_name(argv, verify_tag, format);
> >
> > Either for-each-tag-name's new parameter needs to be typed
> > correctly, or the type of the "format" variable needs to be updated.
>
> Squashing the following into this commit solves this issue with the
> former approach. The lines it touches are all from 4/6 and I view
> all of it as general improvement, including type correctness and
> code formatting.
>
> diff --git a/builtin/tag.c b/builtin/tag.c
> index f81273a85a..fbb85ba3dc 100644
> --- a/builtin/tag.c
> +++ b/builtin/tag.c
> @@ -66,10 +66,10 @@ static int list_tags(struct ref_filter *filter, struct ref_sorting *sorting, con
> }
>
> typedef int (*each_tag_name_fn)(const char *name, const char *ref,
> - const unsigned char *sha1, void *cb_data);
> + const unsigned char *sha1, const void *cb_data);
This would bite us later if one of the iterators really does need to
pass something mutable. But as this iteration interface is confined to
builtin/tag.c, I think it's a nice simple fix.
A more general fix would be to pass a non-const pointer to const pointer
(preferably inside a struct for readability). But I don't see any need
for that complexity here.
-Peff
^ permalink raw reply
* Re: [PATCH v6 4/6] builtin/tag: add --format argument for tag -v
From: Junio C Hamano @ 2017-01-18 18:25 UTC (permalink / raw)
To: santiago; +Cc: git, peff, sunshine, walters, Lukas Puehringer
In-Reply-To: <xmqqmvepb4oj.fsf@gitster.mtv.corp.google.com>
Junio C Hamano <gitster@pobox.com> writes:
> santiago@nyu.edu writes:
>
>> @@ -428,9 +443,12 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
>> if (filter.merge_commit)
>> die(_("--merged and --no-merged option are only allowed with -l"));
>> if (cmdmode == 'd')
>> - return for_each_tag_name(argv, delete_tag);
>> - if (cmdmode == 'v')
>> - return for_each_tag_name(argv, verify_tag);
>> + return for_each_tag_name(argv, delete_tag, NULL);
>> + if (cmdmode == 'v') {
>> + if (format)
>> + verify_ref_format(format);
>> + return for_each_tag_name(argv, verify_tag, format);
>> + }
>
> This triggers:
>
> builtin/tag.c: In function 'cmd_tag':
> builtin/tag.c:451:3: error: passing argument 3 of
> 'for_each_tag_name' discards 'const' qualifier from pointer target type [-Werror]
> return for_each_tag_name(argv, verify_tag, format);
>
> Either for-each-tag-name's new parameter needs to be typed
> correctly, or the type of the "format" variable needs to be updated.
Squashing the following into this commit solves this issue with the
former approach. The lines it touches are all from 4/6 and I view
all of it as general improvement, including type correctness and
code formatting.
diff --git a/builtin/tag.c b/builtin/tag.c
index f81273a85a..fbb85ba3dc 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -66,10 +66,10 @@ static int list_tags(struct ref_filter *filter, struct ref_sorting *sorting, con
}
typedef int (*each_tag_name_fn)(const char *name, const char *ref,
- const unsigned char *sha1, void *cb_data);
+ const unsigned char *sha1, const void *cb_data);
static int for_each_tag_name(const char **argv, each_tag_name_fn fn,
- void *cb_data)
+ const void *cb_data)
{
const char **p;
char ref[PATH_MAX];
@@ -95,7 +95,7 @@ static int for_each_tag_name(const char **argv, each_tag_name_fn fn,
}
static int delete_tag(const char *name, const char *ref,
- const unsigned char *sha1, void *cb_data)
+ const unsigned char *sha1, const void *cb_data)
{
if (delete_ref(ref, sha1, 0))
return 1;
@@ -104,10 +104,10 @@ static int delete_tag(const char *name, const char *ref,
}
static int verify_tag(const char *name, const char *ref,
- const unsigned char *sha1, void *cb_data)
+ const unsigned char *sha1, const void *cb_data)
{
int flags;
- char *fmt_pretty = cb_data;
+ const char *fmt_pretty = cb_data;
flags = GPG_VERIFY_VERBOSE;
if (fmt_pretty)
^ permalink raw reply related
* Re: problem with insider build for windows and git
From: Johannes Schindelin @ 2017-01-18 16:41 UTC (permalink / raw)
To: Michael Gooch; +Cc: git
In-Reply-To: <alpine.DEB.2.20.1701131300390.3469@virtualbox>
Hi,
On Sat, 14 Jan 2017, Johannes Schindelin wrote:
> Footnote *2*: http://tinyurl.com/gitCal suggests that Git v2.12.0 will
> be released early February soon after Git Merge (and Git for Windows
> v2.12.0 will follow soon thereafter), and with no patches applied to the
> `maint` branch since v2.11.0, I do actually not expect any v2.11.1 to
> happen before v2.12.0 comes out.
And just to prove me wrong, today I got the first update to `maint` in six
weeks, with a message "Almost ready for 2.11.1" at its tip, featuring a
whopping 141 commits (95 of which are not merge commits).
So it seems that v2.11.1 may happen soon, after all.
Sorry for my misjudgement,
Johannes
^ permalink raw reply
* Re: [RFC] stash --continue
From: Johannes Schindelin @ 2017-01-18 16:34 UTC (permalink / raw)
To: Marc Branchaud; +Cc: Stephan Beyer, git
In-Reply-To: <d5456165-bdf2-e9e7-117f-aeab0ff4b417@xiplink.com>
Hi Marc,
On Wed, 18 Jan 2017, Marc Branchaud wrote:
> On 2017-01-16 05:54 AM, Johannes Schindelin wrote:
>
> > On Mon, 16 Jan 2017, Stephan Beyer wrote:
> >
> > > a git-newbie-ish co-worker uses git-stash sometimes. Last time he
> > > used "git stash pop", he got into a merge conflict. After he
> > > resolved the conflict, he did not know what to do to get the
> > > repository into the wanted state. In his case, it was only "git add
> > > <resolved files>" followed by a "git reset" and a "git stash drop",
> > > but there may be more involved cases when your index is not clean
> > > before "git stash pop" and you want to have your index as before.
> > >
> > > This led to the idea to have something like "git stash
> > > --continue"[1]
> >
> > More like "git stash pop --continue". Without the "pop" command, it
> > does not make too much sense.
>
> Why not? git should be able to remember what stash command created the
> conflict. Why should I have to? Maybe the fire alarm goes off right when I
> run the stash command, and by the time I get back to it I can't remember
> which operation I did. It would be nice to be able to tell git to "just
> finish off (or abort) the stash operation, whatever it was".
That reeks of a big potential for confusion.
Imagine for example a total Git noob who calls `git stash list`, scrolls
two pages down, then hits `q` by mistake. How would you explain to that
user that `git stash --continue` does not continue showing the list at the
third page?
Even worse: `git stash` (without arguments) defaults to the `save`
operation, so any user who does not read the documentation (and who does?)
would assume that `git stash --continue` *also* implies `save`.
If that was not enough, there would still be the overall design of Git's
user interface. You can call it confusing, inconsistent, with a lot of
room for improvement, and you would be correct. But none of Git's commands
has a `--continue` option that remembers the latest subcommand and
continues that. To introduce that behavior in `git stash` would disimprove
the situation.
With every new feature, it is not enough to consider its benefits. You
always have to take the potential fallout into account, too.
At least `git stash pop --continue` would be consistent with all other
`--continue` options in Git that I can think of...
Ciao,
Johannes
^ permalink raw reply
* Re: [PATCH 2/2] Be more careful when determining whether a remote was configured
From: Johannes Schindelin @ 2017-01-18 16:22 UTC (permalink / raw)
To: Jeff King; +Cc: git, Junio C Hamano, Thomas Gummerer, Andrew Arnott
In-Reply-To: <20170118125422.gi3ppqiqhyykk7iy@sigill.intra.peff.net>
Hi Peff,
On Wed, 18 Jan 2017, Jeff King wrote:
> On Wed, Jan 18, 2017 at 01:34:28PM +0100, Johannes Schindelin wrote:
>
> > > > Let's fix this by telling Git that a remote is not configured
> > > > unless any fetch/push URL or refspect is configured explicitly.
> > >
> > > Hmm. Old versions of GitHub for Windows used to set fetch refspecs
> > > in the system gitconfig, for a similar purpose to what you want to
> > > do with remote.origin.prune.
> > >
> > > I notice here that setting a refspec _does_ define a remote. Is
> > > there a reason you drew the line there, and not at, say, whether it
> > > has a URL?
> >
> > I want to err on the side of caution. That's why.
>
> I guess I just don't see why changing the behavior with respect to
> "prune" or "proxy" is any less conservative than changing the one for
> "refspec".
Let's take a step back and consider the problem I try to solve, okay?
The problem is that `git remote rename <old> <new>` refuses to do its job
if it detects a configured `<new>`. And it detects a configured `<new>`
even in cases where there is not *really* any remote configured.
The example use case is to configure `remote.origin.prune = true` in
~/.gitconfig, i.e. changing Git's default for all "origin" remotes in all
of the user's repositories.
Now, the *real* fix would be to detect whether the remote was "configured"
in the current repository, or in ~/.gitconfig. But that may not even be
desirable, as we would want a more general fix for the question: can `git
remote` rename a given remote to a new name, i.e. is that new name already
taken?
And if you try to answer that last question, you will probably pick the
same set of keys for which you assume that remote.<name>.<key> really
configures a remote or not.
Originally, I would even have put the "vcs" into that set, as I could see
a legitimate use case for users to configure "remote.svn.vcs = vcs" in
~/.gitconfig. But the regression test suite specifically tests for that
case, and I trust that there was a good reason, even if Thomas did not
describe that good reason in the commit message nor in any reply to this
patch pair.
So how can things go wrong?
Let's take your example that the user may have specified refspecs (or
prune, or proxy) for a URL via "remote.<url>.fetch", and that `git rename`
incorrectly allows that as a new remote name. You know what? Let's do a
real code review here, not just a patch glance-over. Let's test this and
*know* whether it can be a problem:
git remote rename origin https://github.com/git/git
fatal: 'https://github.com/git/git' is not a valid remote name
A ha! It is *not* possible to hit that case because `git remote rename`
already complains much earlier about the new name not being a valid name.
So let's see what could go wrong with another example you mentioned, that
the proxy may not be used because we changed the logic of
remote_is_configured(). But the only user of the remote proxy settings is
in http_init() and reads:
if (remote && remote->http_proxy)
curl_http_proxy = xstrdup(remote->http_proxy);
if (remote)
var_override(&http_proxy_authmethod, remote->http_proxy_authmethod);
It does not even test whether the remote is configured! So maybe the
caller does? Nope, the only caller of http_init() that passes a remote is
remote-curl's cmd_main() function, and the relevant part reads:
remote = remote_get(argv[1]);
if (argc > 2) {
end_url_with_slash(&url, argv[2]);
} else {
end_url_with_slash(&url, remote->url[0]);
}
http_init(remote, url.buf, 0);
This code also does not care whether the remote "is configured" or not.
So maybe there are any other downsides with callers of
remote_is_configured()?
There is one caller in builtin/fetch.c's add_remote_or_group() which
clearly is covered (we need a URL, unless the vcs is configured).
All other callers are in builtin/remote.c:
- in add(), to test whether we can add a new remote,
- in mv(), to test whether the remote to rename is configured,
- in mv(), to test whether the new name is already taken,
- in rm(), to test whether the remote exists,
- in set_remote_branches(), to test whether the remote to be changed
exists,
- in get_url(), to test whether the remote exists, and
- in set_url(), to test whether the remote exists.
It appears pretty obvious that in all of these cases, the suggested patch
still makes sense and does not introduce any nasty surprise.
> I can think of one alternative approach that might be easier for users
> to understand, and that we already use elsewhere (e.g., with "http.*"
> config): have a set of "default" remote keys (e.g., just "remote.key")
> that git falls back to when the remote.*.key isn't set. Then your use
> case becomes something like:
>
> [remote]
> prune = true
>
> That's not _exactly_ the same, as it applies to all remotes, not just
> "origin" (other remotes can override it, but would need to do so
> explicitly). But I have a suspicion that may actually be what users
> want.
Sure. That'll be another patch series, though. And another contributor.
Ciao,
Johannes
^ permalink raw reply
* Re: [RFC] stash --continue
From: Marc Branchaud @ 2017-01-18 15:41 UTC (permalink / raw)
To: Johannes Schindelin, Stephan Beyer; +Cc: git
In-Reply-To: <alpine.DEB.2.20.1701161153340.3469@virtualbox>
On 2017-01-16 05:54 AM, Johannes Schindelin wrote:
> Hi Stephan,
>
> On Mon, 16 Jan 2017, Stephan Beyer wrote:
>
>> a git-newbie-ish co-worker uses git-stash sometimes. Last time he used
>> "git stash pop", he got into a merge conflict. After he resolved the
>> conflict, he did not know what to do to get the repository into the
>> wanted state. In his case, it was only "git add <resolved files>"
>> followed by a "git reset" and a "git stash drop", but there may be more
>> involved cases when your index is not clean before "git stash pop" and
>> you want to have your index as before.
>>
>> This led to the idea to have something like "git stash --continue"[1]
>
> More like "git stash pop --continue". Without the "pop" command, it does
> not make too much sense.
Why not? git should be able to remember what stash command created the
conflict. Why should I have to? Maybe the fire alarm goes off right
when I run the stash command, and by the time I get back to it I can't
remember which operation I did. It would be nice to be able to tell git
to "just finish off (or abort) the stash operation, whatever it was".
M.
^ permalink raw reply
* Re: git fast-import crashing on big imports
From: Jeff King @ 2017-01-18 14:38 UTC (permalink / raw)
To: Ulrich Spoerlein; +Cc: git, Ed Maste, Junio C Hamano
In-Reply-To: <20170118140117.GK4426@acme.spoerlein.net>
On Wed, Jan 18, 2017 at 03:01:17PM +0100, Ulrich Spoerlein wrote:
> Yo Jeff, your commit 8261e1f139db3f8aa6f9fd7d98c876cbeb0f927c from Aug
> 22nd, that changes delta_base_cache to use hashmap.h is the culprit for
> git fast-import crashing on large imports.
I actually saw your bug report the other day and tried to download the
dump file, but got a 404. Can you double check that it is available?
-Peff
^ permalink raw reply
* Re: git fast-import crashing on big imports
From: Ulrich Spoerlein @ 2017-01-18 14:01 UTC (permalink / raw)
To: git, Jeff King; +Cc: Ed Maste, Junio C Hamano
In-Reply-To: <20170112082138.GJ4426@acme.spoerlein.net>
Yo Jeff, your commit 8261e1f139db3f8aa6f9fd7d98c876cbeb0f927c from Aug
22nd, that changes delta_base_cache to use hashmap.h is the culprit for
git fast-import crashing on large imports.
Please read below, you can find a 55G SVN dump that should show the
problem after a couple of minutes to less than an hour. Please also see
similar issues from 2009 and 2011. This seems to be a rather fragile
part of the code, could you add unit tests that make sure this
regression is not re-introduce again once you fix it? Thanks!
I'm happy to test any patches that you can provide.
Cheers,
Uli
On Do., 2017-01-12 at 09:21:38 +0100, Ulrich Spörlein wrote:
> Hey,
>
> the FreeBSD svn2git conversion is crashing somewhat
> non-deterministically during its long conversion process. From memory,
> this was not as bad is it is with more recent versions of git (but I
> can't be sure, really).
>
> I have a dump file that you can grab at
> http://scan.spoerlein.net/pub/freebsd-base.dump.xz (19G, 55G uncompressed)
> that shows this problem after a couple of minutes of runtime. The caveat is
> that for another member of the team on a different machine the crashes are on
> different revisions.
>
> Googling around I found two previous threads that were discussing
> problems just like this (memory corruption, bad caching, etc)
>
> https://www.spinics.net/lists/git/msg93598.html from 2009
> and
> http://git.661346.n2.nabble.com/long-fast-import-errors-out-quot-failed-to-apply-delta-quot-td6557884.html
> from 2011
>
> % git fast-import --stats < ../freebsd-base.dump
> ...
> progress SVN r49318 branch master = :49869
> progress SVN r49319 branch stable/3 = :49870
> progress SVN r49320 branch master = :49871
> error: failed to apply delta
> error: bad offset for revindex
> error: bad offset for revindex
> error: bad offset for revindex
> error: bad offset for revindex
> error: bad offset for revindex
> fatal: Can't load tree b35ae4e9c2a41677e84a3f14bed09f584c3ff25e
> fast-import: dumping crash report to fast_import_crash_29613
>
>
> fast-import crash report:
> fast-import process: 29613
> parent process : 29612
> at 2017-01-11 19:33:37 +0000
>
> fatal: Can't load tree b35ae4e9c2a41677e84a3f14bed09f584c3ff25e
>
>
> git fsck shows a somewhat incomplete pack file (I guess that's expected if the
> process dies mid-stream?)
>
> % git fsck
> Checking object directories: 100% (256/256), done.
> error: failed to apply delta6/614500)
> error: cannot unpack d1d7ee1f81c6767c5e0f75d14d400d7512a85a0f from ./objects/pack/pack-e28fcea43fc221d2ebe92857b484da58bb888237.pack at offset 122654805
> error: failed to apply delta
> error: failed to read delta base object d1d7ee1f81c6767c5e0f75d14d400d7512a85a0f at offset 122654805 from ./objects/pack/pack-e28fcea43fc221d2ebe92857b484da58bb888237.pack
> error: cannot unpack 8523bde63ef34bef725347994fdaec996d756510 from ./objects/pack/pack-e28fcea43fc221d2ebe92857b484da58bb888237.pack at offset 122671596
> error: failed to apply delta0/614500)
> error: failed to read delta base object d1d7ee1f81c6767c5e0f75d14d400d7512a85a0f at offset 122654805 from ./objects/pack/pack-e28fcea43fc221d2ebe92857b484da58bb888237.pack
> ...
>
>
> Any comments on whether the original problems from 2009 and 2011 were ever
> fixed and committed?
>
> Some more facts:
> - git version 2.11.0
> - I don't recall these sorts of crashes with a git from 2-3 years ago
> - adding more checkpoints does help, but not fix the problem, it merely shifts
> the crashes around to different revisions
> - incremental runs of the conversion *will* complete most of the time, but
> depending on how often checkpoints are used, I've seen it croak on specific
> commits and not being able to progress further :(
>
> Thanks for any pointers or things to try!
> Cheers
> Uli
^ permalink raw reply
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