Re: [PATCH/RFC v3 00/14] Introduce new commands switch-branch and restore-files
On Fri, Nov 30, 2018 at 1:16 AM Dan Fabulich wrote: > > Other thoughts on a global UI rethink: > > One of the most common complaints I hear about git is the conceptual > difficulty required in undoing changes. https://ohshitgit.com/ > > > Git is hard: screwing up is easy, and figuring out how to fix your mistakes > > is fucking impossible. Git documentation has this chicken and egg problem > > where you can't search for how to get yourself out of a mess, unless you > > *already know the name of the thing you need to know about* in order to fix > > your problem. > > A significant fraction of the top-voted questions on StackOverflow are about > undoing changes. https://stackoverflow.com/questions/tagged/git > > What if there were a 'git undo' command that could unify the answers to all > of these questions? > > git undo stage > git undo rm > git undo edit (checkout files from the stage) > > git undo commit (prompt the user whether to revert or reset) > git undo reset > git undo checkout > > git undo merge > git undo pull > git undo push (prompt the user whether to force push back to the past or > whether to revert pushed commits) > git undo rebase > > git undo undo > > git undo clean > git undo delete-branch > git undo delete-stash > > Some of these would be trivial effort, but a lot of them would require > fundamental changes in the way git operates. (You can't undo a clean right > now because the files are just destroyed.) We're getting there. The biggest problem I have is how this "git undo" should work, not the changes behind to support it. For example, if I pulled then did some rebase, what would "git undo pull" do? -- Duy
Re: [PATCH/RFC v3 00/14] Introduce new commands switch-branch and restore-files
Duy Nguyen writes: > core.uiVersion is a big no no to me. I don't want to go to someone's > terminal, type something and have a total surprise because they set > different ui version. If you want a total UI redesign, go with a new > prefix, like "ng" (for new git) or something instead of "git". Yup, very good point to keep in mind.
Re: [PATCH/RFC v3 00/14] Introduce new commands switch-branch and restore-files
Duy Nguyen writes: >> >> OK. Is "auto-vivify the named branch based on a remote-tracking" >> also rejected, as it is a confusing behaviour that is a too subtle >> and implicit, just like the detaching head is, and require --guess >> or sticking to 'git checkout'? I think it should. > > This touches the "remote" concept which I think is another confusing > thing for new people (your "master" is not the same as the server's > "master", aka origin/master) and perhaps this dwim thing helps. > Frankly I don't do dwim much so I don't know if it's that often used. I actually think a user who sees a DWIM without understanding what the user wants to do would perceive magic that sometimes works and sometimes does not, and some other times it does a random thing that the user does not even understand what is going on. And such a random magic that sometimes works, even if the "sometimes" is "most of the time", say 85% of the time, would not help user form the right mental model. "git checkout master~2" that DWIMs to "git checkout --deatch master~2", but does totally different thing when "git checkout master" is given, leaving the user confused "what is so different between these two?". Until the user realizes 'master' can serve both as a branch name and a name for a commit object, while master~2 can only be a name for a commit object and is not a branch name, the behaviour of the command will stay to be mysterious and DWIMmage would not help user form the right mental model. I earlier said that I agree with your decision to leave the implied form out of switch-branch for exactly that reason. The behaviour falls into the same category as "git checkout frotz" that DWIMS to "git checkout -b frotz -t remotes/origin/frotz", which also is mysterious until the user understands your 'master' is unique and is different from 'master' to everybody else, I would think.
Re: [PATCH/RFC v3 00/14] Introduce new commands switch-branch and restore-files
On Fri, Nov 30, 2018 at 3:16 AM Junio C Hamano wrote: > > Nguyễn Thái Ngọc Duy writes: > > > 'git switch-branch' > > > > - implicit detaching is rejected. If you need to detach, you need to > > give --detach. Or stick to 'git checkout'. > > OK. Is "auto-vivify the named branch based on a remote-tracking" > also rejected, as it is a confusing behaviour that is a too subtle > and implicit, just like the detaching head is, and require --guess > or sticking to 'git checkout'? I think it should. This touches the "remote" concept which I think is another confusing thing for new people (your "master" is not the same as the server's "master", aka origin/master) and perhaps this dwim thing helps. Frankly I don't do dwim much so I don't know if it's that often used. > > - -b/-B is renamed to -c/-C with long option names > > I did not expect that these two are the only options that would be > out of place with the command name split, but presumably you looked > at all options for both of the two new commands to see if they made > sense in the new context? Yeah (at least the description in struct option[] array) -- Duy
Re: [PATCH/RFC v3 00/14] Introduce new commands switch-branch and restore-files
On Fri, Nov 30, 2018 at 12:05 AM Ævar Arnfjörð Bjarmason wrote: > Assuming greenfield development (which we definitely don't have), I > don't like the "restore-files" name, but the alternative that makes > sense is "checkout". Then this "--from" argument could become "git > checkout-tree -- ", and we'd have: > > git switch > git checkout > git checkout-tree -- > > Or maybe that sucks, anyway what I was going to suggest is *if* others > think that made sense as a "if we designed git today" endgame whether we > could have an opt-in setting where you set e.g. core.uiVersion=3 (in > anticipation of Git 3.0) and you'd get that behavior. There could be > some other setting where core.uiVersion would use the old behavior (or > with another setting, only warn) if we weren't connected to a terminal. core.uiVersion is a big no no to me. I don't want to go to someone's terminal, type something and have a total surprise because they set different ui version. If you want a total UI redesign, go with a new prefix, like "ng" (for new git) or something instead of "git". -- Duy
[PATCH] format-patch: do not let its diff-options affect --range-diff (was Re: [PATCH 2/2] format-patch: allow for independent diff & range-diff options)
Junio C Hamano writes:
> In any case, I tend to agree with the conclusion in the downthread
> by Dscho that we should just clearly mark that invocations of the
> "format-patch --range-diff" command with additional diff options is
> an experimental feature that may not do anything sensible in the
> upcoming release, and declare that the UI to pass diff options to
> affect only the range-diff part may later be invented. IOW, I am
> coming a bit stronger than Dscho's suggestion in that we should not
> even pretend that we aimed to make the options used for range-diff
> customizable when driven from format-patch in the upcoming release,
> or aimed to make --range-diff option compatible with other diff
> options given to the format-patch command.
>
> I had to delay -rc2 to see these last minute tweaks come to some
> reasonable place to stop at, and I do not think we want to delay the
> final any longer or destablizing it further by piling last minute
> undercooked changes on top.
So how about doing this on top of 'master' instead? As this leaks
*no* information wrt how range-diff machinery should behave from the
format-patch side by not passing any diffopt, as long as the new
code I added to show_range_diff() comes up with a reasonable default
diffopts (for which I really would appreciate extra sets of eyes to
make sure), this change by definition cannot be wrong (famous last
words).
-- >8 --
Subject: format-patch: do not let its diff-options affect --range-diff
Stop leaking how the primary output of format-patch is customized to
the range-diff machinery and instead let the latter use its own
"reasonable default", in order to correct the breakage introduced by
a5170794 ("Merge branch 'ab/range-diff-no-patch'", 2018-11-18) on
the 'master' front. "git format-patch --range-diff..." without any
weird diff option started to include the "range-diff --stat" output,
which is rather useless right now, that made the whole thing
unusable and this is probably the least disruptive way to whip the
codebase into a shippable shape.
We may want to later make the range-diff driven by format-patch more
configurable, but that would have to wait until we have a good
design.
Signed-off-by: Junio C Hamano
---
Documentation/git-format-patch.txt | 5 +
builtin/log.c | 2 +-
log-tree.c | 2 +-
range-diff.c | 6 +-
range-diff.h | 5 +
5 files changed, 17 insertions(+), 3 deletions(-)
diff --git a/Documentation/git-format-patch.txt
b/Documentation/git-format-patch.txt
index aba4c5febe..27304428a1 100644
--- a/Documentation/git-format-patch.txt
+++ b/Documentation/git-format-patch.txt
@@ -250,6 +250,11 @@ feeding the result to `git send-email`.
feature/v2`), or a revision range if the two versions of the series are
disjoint (for example `git format-patch --cover-letter
--range-diff=feature/v1~3..feature/v1 -3 feature/v2`).
++
+Note that diff options passed to the command affect how the primary
+product of `format-patch` is generated, and they are not passed to
+the underlying `range-diff` machinery used to generate the cover-letter
+material (this may change in the future).
--creation-factor=::
Used with `--range-diff`, tweak the heuristic which matches up commits
diff --git a/builtin/log.c b/builtin/log.c
index 0fe6f9ba1e..5ac18e2848 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -1096,7 +1096,7 @@ static void make_cover_letter(struct rev_info *rev, int
use_stdout,
if (rev->rdiff1) {
fprintf_ln(rev->diffopt.file, "%s", rev->rdiff_title);
show_range_diff(rev->rdiff1, rev->rdiff2,
- rev->creation_factor, 1, >diffopt);
+ rev->creation_factor, 1, NULL);
}
}
diff --git a/log-tree.c b/log-tree.c
index 7a83e99250..b243779a0b 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -762,7 +762,7 @@ void show_log(struct rev_info *opt)
next_commentary_block(opt, NULL);
fprintf_ln(opt->diffopt.file, "%s", opt->rdiff_title);
show_range_diff(opt->rdiff1, opt->rdiff2,
- opt->creation_factor, 1, >diffopt);
+ opt->creation_factor, 1, NULL);
memcpy(_queued_diff, , sizeof(diff_queued_diff));
}
diff --git a/range-diff.c b/range-diff.c
index 767af8c5bb..8e52a85c19 100644
--- a/range-diff.c
+++ b/range-diff.c
@@ -460,7 +460,11 @@ int show_range_diff(const char *range1, const char *range2,
struct diff_options opts;
struct strbuf indent = STRBUF_INIT;
- memcpy(, diffopt, sizeof(opts));
+ if (diffopt)
+ memcpy(, diffopt, sizeof(opts));
+ else
+ repo_diff_setup(the_repository, );
+
if (!opts.output_format)
opts.output_format =
Re: [PATCH 3/5] pack-objects: add --sparse option
Derrick Stolee writes: > You're right that having this hidden as an opt-in config variable > makes it hard to discover as a typical user. > > I would argue that we should actually make the config setting true by > default, and recommend that servers opt-out. Here are my reasons: > > 1. The vast majority of users are clients. > > 2. Client users are not likely to know about and tweak these settings. > > 3. Server users are more likely to keep an eye on the different knobs > they can tweak. > > 4. Servers should use the reachability bitmaps, which don't use this > logic anyway. > > While _eventually_ we should make this opt-out, we shouldn't do that > until it has cooked a while. I actually do not agree. If the knob gives enough benefit, the users will learn about it viva voce, and in a few more releases we'll hear "enough users complain that they have to turn it on, let's make it the default". If that does not happen, the knob does not deserve to be turned on in the first place. The same applies to many shiny new toys people are discussing recently on this list (e.g. precious vs expendable and non-overlay checkout are the ones that immediately come to my mind). Having said that, I won't be commenting on this shiny new toy before the final. I want to see more people help tying the loose ends and give it final varnish to the upcoming release, as it seems to have become rockier and larger release than we originally anticipated.
Re: [PATCH 2/2] format-patch: allow for independent diff & range-diff options
Ævar Arnfjörð Bjarmason writes: >> What prevents you from using `sq_dequote_to_argv()`? > > I mean not just nasty in terms of implementation, yeah we could do it, > but also a nasty UX for things like --word-diff-regex. I.e. instead of: > > --range-diff-word-diff-regex='[0-9"]' > > You need: > > --range-diff-opts="--word-diff-regex='[0-9\"]'" > > Now admittedly that in itself isn't very painful *in this case*, but in > terms of precedent I really dislike that option, i.e. git having some > mode where I need to work to escape input to pass to another command. In addition, sq_dequote are meant to be used on quoted string we internally produce; I do not think we want to promise that it is safe to use on a random string that comes from end users. In any case, I tend to agree with the conclusion in the downthread by Dscho that we should just clearly mark that invocations of the "format-patch --range-diff" command with additional diff options is an experimental feature that may not do anything sensible in the upcoming release, and declare that the UI to pass diff options to affect only the range-diff part may later be invented. IOW, I am coming a bit stronger than Dscho's suggestion in that we should not even pretend that we aimed to make the options used for range-diff customizable when driven from format-patch in the upcoming release, or aimed to make --range-diff option compatible with other diff options given to the format-patch command. I had to delay -rc2 to see these last minute tweaks come to some reasonable place to stop at, and I do not think we want to delay the final any longer or destablizing it further by piling last minute undercooked changes on top.
Re: [PATCH/RFC v3 00/14] Introduce new commands switch-branch and restore-files
Nguyễn Thái Ngọc Duy writes: > 'git switch-branch' > > - implicit detaching is rejected. If you need to detach, you need to > give --detach. Or stick to 'git checkout'. OK. Is "auto-vivify the named branch based on a remote-tracking" also rejected, as it is a confusing behaviour that is a too subtle and implicit, just like the detaching head is, and require --guess or sticking to 'git checkout'? I think it should. > - -b/-B is renamed to -c/-C with long option names I did not expect that these two are the only options that would be out of place with the command name split, but presumably you looked at all options for both of the two new commands to see if they made sense in the new context? > 'git restore-files' > > - takes a ref from --from argument, not as a free ref. As a result, > '--' is no longer needed. All non-option arguments are pathspec OK. That does make things easier to teach, as there is no need for disambiguation. > - pathspec is mandatory, you can't do "git restore-files" without any > pathspec. > > - I just remember -p which is allowed to take no pathspec :( I'll fix > it later. Or leave it out of restore-files as a more advanced feature (just like detaching with HEAD^0 is left out of switch-branch) that the user can stick to 'git checkout' to use. > - Two more fancy features (the "git checkout --index" being the > default mode and the backup log for accidental overwrites) are of > course still missing. But they are coming. I am unsure about the wisdom of calling it "--index", though. The "--index" option is "the command can work only on the index, or only on the working tree files, or on both the index and the working tree files, and this option tells it to work in the 'both the index and the working tree files' mode", but when "restore-files" touches paths, it always modifies both the index and the working tree, so the "--index" option does not capture the differences well in this context [*1*]. As I saw this was described as "not using the usual 'overlay' semantics [*2*]", perhaps --overlay/--no-overlay option that defaults to --no-overlay is easier to explain. side note 1. I think the original mention of "--index" came in the context of contrasting "git reset" with "git checkout". "git reset (--hard|--mixed) -- " (that does not move HEAD), which does not but perhaps should exist, is very much like "git checkout -- ", and if "reset" were written after the "--index/--cached" convention was established, "reset --hard" would have called "reset --index" while "reset --mixed" would have been "reset --cached" (i.e. only work on the index and not on the working tree). And "reset --index " would have worked by removing paths in that are not in the HEAD and updating paths in that are in the HEAD, i.e. identical to the non overlay behaviour proposed for the "git checkout" command. So calling the non overlay mode "--index" makes sense in the context of discussing "git reset", and not in the context of "git checkout". side note 2. "git checkout " grabs entries from the that patch and adds them to the index and checks them out to the working tree. If the original index has entries that match but do not appear in , they are left in the result. That is "overlaying what was taken from the on top of what is in the index". Having said all that, I will not be looking at the series until 2.20 final. And I hope more people do the same to concentrate on helping us prevent the last minute glitch slipping in the final release. Thanks.
Re: [PATCH/RFC v2 0/7] Introduce new commands switch-branch and checkout-files
Duy Nguyen writes: > On Wed, Nov 28, 2018 at 9:01 PM Duy Nguyen wrote: >> should we do >> something about detached HEAD in this switch-branch command (or >> whatever its name will be)? >> >> This is usually a confusing concept to new users > > And it just occurred to me that perhaps we should call this "unnamed > branch" (at least at high UI level) instead of detached HEAD. It is > technically not as accurate, but much better to understand. As I said elsewhere in nearby thread, I agree that "unnamed branch" is a reasonable way to explain what the state the user is in. It is not incorrect per-se that HEAD is detached from anything in refs/ in such a state, but that is an implementation detail of how the worktree gets on the unnamed branch (which lasts until the worktree next gets on a named branch, at which point the unnamed branch disappears).
Re: [PATCH 1/1] rebase --stat: fix when rebasing to an unrelated history
Johannes Schindelin writes: > But I guess that I should not be so lazy and really use two different > messages here: > > Changes from to > > and if there is no merge base, > > Changes in Ah, that's excellent. Thanks.
Re: [PATCH] pack-protocol.txt: accept error packets in any context
Masaya Suzuki writes: > Yes, I did. And it also didn't end up in a build error. Do I have a > different build option...? Passig DEVELOPER=Yes to make turns a bit more warnings on (in this case, I think it was "unused-variable") and also uses -Werror to turn warnings into errors.
Security Alert. git@vger.kernel.org has password callgsm01. Password must be changed.
Hello! I have very bad news for you. 09/08/2018 - on this day I hacked your OS and got full access to your account git@vger.kernel.org On this day your account git@vger.kernel.org has password: callgsm01 So, you can change the password, yes.. But my malware intercepts it every time. How I made it: In the software of the router, through which you went online, was a vulnerability. I just hacked this router and placed my malicious code on it. When you went online, my trojan was installed on the OS of your device. After that, I made a full dump of your disk (I have all your address book, history of viewing sites, all files, phone numbers and addresses of all your contacts). A month ago, I wanted to lock your device and ask for a not big amount of btc to unlock. But I looked at the sites that you regularly visit, and I was shocked by what I saw!!! I'm talk you about sites for adults. I want to say - you are a BIG pervert. Your fantasy is shifted far away from the normal course! And I got an idea I made a screenshot of the adult sites where you have fun (do you understand what it is about, huh?). After that, I made a screenshot of your joys (using the camera of your device) and glued them together. Turned out amazing! You are so spectacular! I'm know that you would not like to show these screenshots to your friends, relatives or colleagues. I think $796 is a very, very small amount for my silence. Besides, I have been spying on you for so long, having spent a lot of time! Pay ONLY in Bitcoins! My BTC wallet: 1HkKgPbcMyfhrdPsbufTFczzVnhyT5snB3 You do not know how to use bitcoins? Enter a query in any search engine: "how to replenish btc wallet". It's extremely easy For this payment I give you two days (48 hours). As soon as this letter is opened, the timer will work. After payment, my virus and dirty screenshots with your enjoys will be self-destruct automatically. If I do not receive from you the specified amount, then your device will be locked, and all your contacts will receive a screenshots with your "enjoys". I hope you understand your situation. - Do not try to find and destroy my virus! (All your data, files and screenshots is already uploaded to a remote server) - Do not try to contact me (you yourself will see that this is impossible, the sender address is automatically generated) - Various security services will not help you; formatting a disk or destroying a device will not help, since your data is already on a remote server. P.S. You are not my single victim. so, I guarantee you that I will not disturb you again after payment! This is the word of honor hacker I also ask you to regularly update your antiviruses in the future. This way you will no longer fall into a similar situation. Do not hold evil! I just do my job. Good luck.
Re: [PATCH/RFC v3 00/14] Introduce new commands switch-branch and restore-files
Other thoughts on a global UI rethink: One of the most common complaints I hear about git is the conceptual difficulty required in undoing changes. https://ohshitgit.com/ > Git is hard: screwing up is easy, and figuring out how to fix your mistakes > is fucking impossible. Git documentation has this chicken and egg problem > where you can't search for how to get yourself out of a mess, unless you > *already know the name of the thing you need to know about* in order to fix > your problem. A significant fraction of the top-voted questions on StackOverflow are about undoing changes. https://stackoverflow.com/questions/tagged/git What if there were a 'git undo' command that could unify the answers to all of these questions? git undo stage git undo rm git undo edit (checkout files from the stage) git undo commit (prompt the user whether to revert or reset) git undo reset git undo checkout git undo merge git undo pull git undo push (prompt the user whether to force push back to the past or whether to revert pushed commits) git undo rebase git undo undo git undo clean git undo delete-branch git undo delete-stash Some of these would be trivial effort, but a lot of them would require fundamental changes in the way git operates. (You can't undo a clean right now because the files are just destroyed.) -Dan > On Nov 29, 2018, at 3:05 PM, Ævar Arnfjörð Bjarmason wrote: > > > On Thu, Nov 29 2018, Nguyễn Thái Ngọc Duy wrote: > >> v3 sees switch-branch go back to switch-branch (in v2 it was >> checkout-branch). checkout-files is also renamed restore-files (v1 was >> restore-paths). Hopefully we won't see another rename. >> >> I'll try to summarize the differences between the new commands and >> 'git checkout' down here, but you're welcome to just head to 07/14 and >> read the new man pages. >> >> 'git switch-branch' >> >> - does not "do nothing", you have to either switch branch, create a >> new branch, or detach. "git switch-branch" with no arguments is >> rejected. >> >> - implicit detaching is rejected. If you need to detach, you need to >> give --detach. Or stick to 'git checkout'. >> >> - -b/-B is renamed to -c/-C with long option names >> >> - of course does not accept pathspec >> >> 'git restore-files' >> >> - takes a ref from --from argument, not as a free ref. As a result, >> '--' is no longer needed. All non-option arguments are pathspec >> >> - pathspec is mandatory, you can't do "git restore-files" without any >> pathspec. >> >> - I just remember -p which is allowed to take no pathspec :( I'll fix >> it later. >> >> - Two more fancy features (the "git checkout --index" being the >> default mode and the backup log for accidental overwrites) are of >> course still missing. But they are coming. >> >> I did not go replace "detached HEAD" with "unnamed branch" (or "no >> branch") everywhere because I think a unique term is still good to >> refer to this concept. Or maybe "no branch" is good enough. I dunno. > > I finally tracked down > https://redfin.engineering/two-commits-that-wrecked-the-user-experience-of-git-f0075b77eab1 > which I'd remembered reading and couldn't find again in these > discussions. Re-reading it while one may not 100% agree with the > author's opinion, it's an interesting rabbit hole. > > I also didn't know about EasyGit, or that Elijah Newren had written > it. I haven't seen him chime in on this series, and would be interested > to see what he thinks about it. > > Re the naming question in > https://public-inbox.org/git/87o9abzv46@evledraar.gmail.com/ & > seeing that eg-switch exists, I wonder if just s/switch-branch/switch/ > makes more sense. > > Assuming greenfield development (which we definitely don't have), I > don't like the "restore-files" name, but the alternative that makes > sense is "checkout". Then this "--from" argument could become "git > checkout-tree -- ", and we'd have: > >git switch >git checkout >git checkout-tree -- > > Or maybe that sucks, anyway what I was going to suggest is *if* others > think that made sense as a "if we designed git today" endgame whether we > could have an opt-in setting where you set e.g. core.uiVersion=3 (in > anticipation of Git 3.0) and you'd get that behavior. There could be > some other setting where core.uiVersion would use the old behavior (or > with another setting, only warn) if we weren't connected to a terminal. > > I.e. I'm thinking of this as step #2 in a #3 step series. Where this is > the fully backwards compatible UI improvement, but someone who'd > e.g. use EasyGit or didn't have backwards compatibility concerns could > enable step #3 and opt-in to a mode where we'd fixed a bunch of UI warts > in a backwards-incompatible way. > > What would that mode look like? I'd to work on piling that on top of > this :)
Re: [PATCH/RFC v3 00/14] Introduce new commands switch-branch and restore-files
Assuming the great day has come to think about this, one thing I'd love to do
is to unify the name of the index/stage/cache in command-line parameters and
the documentation.
The index/stage/cache should have one canonical name, and the documentation
should support that consistently. My taste is to call it the "stage," but
references to --index, --keep-index, --no-index, etc. are all over the code, as
well as legacy references to "--cached".
* You can 'git rm --cached myfile' but you can't 'git rm --staged myfile' or
'git rm --index myfile'.
* You can 'git diff --no-index' or you can 'git diff --cached' with 'git diff
--staged' as a synonym, deprioritized in the documentation ("--staged is a
synonym of --cached"). But you can't 'git diff --index' or 'git diff
--no-stage' or 'git diff --no-cache'.
* You can 'git stage' but 'git help stage' is only one line long, declaring
that it's a synonym for 'git add'. 'add' appears in the 'git help' common
commands, but not 'git stage'. There's no built-in 'git unstage', and certainly
no appetite for 'git index' or 'git cache'.
* Not to mention all of the plumbing commands: checkout-index, diff-index,
index-pack, merge-index, show-index, and update-index.
My understanding based on historical threads is that changes like this would be
unwelcome, even just to add synonyms and standardize the documentation around
"stage" as the term (leaving the plumbing commands alone), but if
documentation+synonym patches are welcome here, I'd be very enthusiastic.
-Dan
> On Nov 29, 2018, at 3:05 PM, Ævar Arnfjörð Bjarmason wrote:
>
>
> On Thu, Nov 29 2018, Nguyễn Thái Ngọc Duy wrote:
>
>> v3 sees switch-branch go back to switch-branch (in v2 it was
>> checkout-branch). checkout-files is also renamed restore-files (v1 was
>> restore-paths). Hopefully we won't see another rename.
>>
>> I'll try to summarize the differences between the new commands and
>> 'git checkout' down here, but you're welcome to just head to 07/14 and
>> read the new man pages.
>>
>> 'git switch-branch'
>>
>> - does not "do nothing", you have to either switch branch, create a
>> new branch, or detach. "git switch-branch" with no arguments is
>> rejected.
>>
>> - implicit detaching is rejected. If you need to detach, you need to
>> give --detach. Or stick to 'git checkout'.
>>
>> - -b/-B is renamed to -c/-C with long option names
>>
>> - of course does not accept pathspec
>>
>> 'git restore-files'
>>
>> - takes a ref from --from argument, not as a free ref. As a result,
>> '--' is no longer needed. All non-option arguments are pathspec
>>
>> - pathspec is mandatory, you can't do "git restore-files" without any
>> pathspec.
>>
>> - I just remember -p which is allowed to take no pathspec :( I'll fix
>> it later.
>>
>> - Two more fancy features (the "git checkout --index" being the
>> default mode and the backup log for accidental overwrites) are of
>> course still missing. But they are coming.
>>
>> I did not go replace "detached HEAD" with "unnamed branch" (or "no
>> branch") everywhere because I think a unique term is still good to
>> refer to this concept. Or maybe "no branch" is good enough. I dunno.
>
> I finally tracked down
> https://redfin.engineering/two-commits-that-wrecked-the-user-experience-of-git-f0075b77eab1
> which I'd remembered reading and couldn't find again in these
> discussions. Re-reading it while one may not 100% agree with the
> author's opinion, it's an interesting rabbit hole.
>
> I also didn't know about EasyGit, or that Elijah Newren had written
> it. I haven't seen him chime in on this series, and would be interested
> to see what he thinks about it.
>
> Re the naming question in
> https://public-inbox.org/git/87o9abzv46@evledraar.gmail.com/ &
> seeing that eg-switch exists, I wonder if just s/switch-branch/switch/
> makes more sense.
>
> Assuming greenfield development (which we definitely don't have), I
> don't like the "restore-files" name, but the alternative that makes
> sense is "checkout". Then this "--from" argument could become "git
> checkout-tree -- ", and we'd have:
>
>git switch
>git checkout
>git checkout-tree --
>
> Or maybe that sucks, anyway what I was going to suggest is *if* others
> think that made sense as a "if we designed git today" endgame whether we
> could have an opt-in setting where you set e.g. core.uiVersion=3 (in
> anticipation of Git 3.0) and you'd get that behavior. There could be
> some other setting where core.uiVersion would use the old behavior (or
> with another setting, only warn) if we weren't connected to a terminal.
>
> I.e. I'm thinking of this as step #2 in a #3 step series. Where this is
> the fully backwards compatible UI improvement, but someone who'd
> e.g. use EasyGit or didn't have backwards compatibility concerns could
> enable step #3 and opt-in to a mode where we'd fixed a bunch of UI warts
> in a backwards-incompatible way.
>
> What would that mode look like?
Re: [PATCH/RFC v3 00/14] Introduce new commands switch-branch and restore-files
On Thu, Nov 29 2018, Ævar Arnfjörð Bjarmason wrote: > On Thu, Nov 29 2018, Nguyễn Thái Ngọc Duy wrote: > >> v3 sees switch-branch go back to switch-branch (in v2 it was >> checkout-branch). checkout-files is also renamed restore-files (v1 was >> restore-paths). Hopefully we won't see another rename. >> >> I'll try to summarize the differences between the new commands and >> 'git checkout' down here, but you're welcome to just head to 07/14 and >> read the new man pages. >> >> 'git switch-branch' >> >> - does not "do nothing", you have to either switch branch, create a >> new branch, or detach. "git switch-branch" with no arguments is >> rejected. >> >> - implicit detaching is rejected. If you need to detach, you need to >> give --detach. Or stick to 'git checkout'. >> >> - -b/-B is renamed to -c/-C with long option names >> >> - of course does not accept pathspec >> >> 'git restore-files' >> >> - takes a ref from --from argument, not as a free ref. As a result, >> '--' is no longer needed. All non-option arguments are pathspec >> >> - pathspec is mandatory, you can't do "git restore-files" without any >> pathspec. >> >> - I just remember -p which is allowed to take no pathspec :( I'll fix >> it later. >> >> - Two more fancy features (the "git checkout --index" being the >> default mode and the backup log for accidental overwrites) are of >> course still missing. But they are coming. >> >> I did not go replace "detached HEAD" with "unnamed branch" (or "no >> branch") everywhere because I think a unique term is still good to >> refer to this concept. Or maybe "no branch" is good enough. I dunno. > > I finally tracked down > https://redfin.engineering/two-commits-that-wrecked-the-user-experience-of-git-f0075b77eab1 > which I'd remembered reading and couldn't find again in these > discussions. Re-reading it while one may not 100% agree with the > author's opinion, it's an interesting rabbit hole. > > I also didn't know about EasyGit, or that Elijah Newren had written > it. I haven't seen him chime in on this series, and would be interested > to see what he thinks about it. > > Re the naming question in > https://public-inbox.org/git/87o9abzv46@evledraar.gmail.com/ & > seeing that eg-switch exists, I wonder if just s/switch-branch/switch/ > makes more sense. > > Assuming greenfield development (which we definitely don't have), I > don't like the "restore-files" name, but the alternative that makes > sense is "checkout". Then this "--from" argument could become "git > checkout-tree -- ", and we'd have: > > git switch > git checkout > git checkout-tree -- > > Or maybe that sucks, anyway what I was going to suggest is *if* others > think that made sense as a "if we designed git today" endgame whether we > could have an opt-in setting where you set e.g. core.uiVersion=3 (in > anticipation of Git 3.0) and you'd get that behavior. There could be > some other setting where core.uiVersion would use the old behavior (or > with another setting, only warn) if we weren't connected to a terminal. > > I.e. I'm thinking of this as step #2 in a #3 step series. Where this is > the fully backwards compatible UI improvement, but someone who'd > e.g. use EasyGit or didn't have backwards compatibility concerns could > enable step #3 and opt-in to a mode where we'd fixed a bunch of UI warts > in a backwards-incompatible way. > > What would that mode look like? I'd to work on piling that on top of > this :) (Digging some more) There's also more interesting prior art at https://gitless.com/ (CC'd authors) and 2x research papers linked at the bottom of that page which were briefly discussed on-list before: https://public-inbox.org/git/20160930191413.002049b94b3908b15881b...@domain007.com/ The "gitless" UI has a "gl checkout" which just takes paths as I was musing about above (and that Redfin post also suggests).
Re: [PATCH/RFC v3 00/14] Introduce new commands switch-branch and restore-files
On Thu, Nov 29 2018, Nguyễn Thái Ngọc Duy wrote: > v3 sees switch-branch go back to switch-branch (in v2 it was > checkout-branch). checkout-files is also renamed restore-files (v1 was > restore-paths). Hopefully we won't see another rename. > > I'll try to summarize the differences between the new commands and > 'git checkout' down here, but you're welcome to just head to 07/14 and > read the new man pages. > > 'git switch-branch' > > - does not "do nothing", you have to either switch branch, create a > new branch, or detach. "git switch-branch" with no arguments is > rejected. > > - implicit detaching is rejected. If you need to detach, you need to > give --detach. Or stick to 'git checkout'. > > - -b/-B is renamed to -c/-C with long option names > > - of course does not accept pathspec > > 'git restore-files' > > - takes a ref from --from argument, not as a free ref. As a result, > '--' is no longer needed. All non-option arguments are pathspec > > - pathspec is mandatory, you can't do "git restore-files" without any > pathspec. > > - I just remember -p which is allowed to take no pathspec :( I'll fix > it later. > > - Two more fancy features (the "git checkout --index" being the > default mode and the backup log for accidental overwrites) are of > course still missing. But they are coming. > > I did not go replace "detached HEAD" with "unnamed branch" (or "no > branch") everywhere because I think a unique term is still good to > refer to this concept. Or maybe "no branch" is good enough. I dunno. I finally tracked down https://redfin.engineering/two-commits-that-wrecked-the-user-experience-of-git-f0075b77eab1 which I'd remembered reading and couldn't find again in these discussions. Re-reading it while one may not 100% agree with the author's opinion, it's an interesting rabbit hole. I also didn't know about EasyGit, or that Elijah Newren had written it. I haven't seen him chime in on this series, and would be interested to see what he thinks about it. Re the naming question in https://public-inbox.org/git/87o9abzv46@evledraar.gmail.com/ & seeing that eg-switch exists, I wonder if just s/switch-branch/switch/ makes more sense. Assuming greenfield development (which we definitely don't have), I don't like the "restore-files" name, but the alternative that makes sense is "checkout". Then this "--from" argument could become "git checkout-tree -- ", and we'd have: git switch git checkout git checkout-tree -- Or maybe that sucks, anyway what I was going to suggest is *if* others think that made sense as a "if we designed git today" endgame whether we could have an opt-in setting where you set e.g. core.uiVersion=3 (in anticipation of Git 3.0) and you'd get that behavior. There could be some other setting where core.uiVersion would use the old behavior (or with another setting, only warn) if we weren't connected to a terminal. I.e. I'm thinking of this as step #2 in a #3 step series. Where this is the fully backwards compatible UI improvement, but someone who'd e.g. use EasyGit or didn't have backwards compatibility concerns could enable step #3 and opt-in to a mode where we'd fixed a bunch of UI warts in a backwards-incompatible way. What would that mode look like? I'd to work on piling that on top of this :)
[PATCH v3 14/14] doc: promote "git switch-branch" and "git restore-files"
The two new commands "git switch-branch" and "git restore-files" are added to avoid the confusion of one-command-do-all "git checkout" for new users. They are also helpful to avoid ambiguation context. For these reasons, promote them everywhere possible. This includes documentation, suggestions/advice from other commands... "git checkout" is also removed from "git help" (i.e. it's no longer considered a commonly used command) Signed-off-by: Nguyễn Thái Ngọc Duy --- Documentation/config/advice.txt| 10 +++-- Documentation/config/checkout.txt | 5 ++- Documentation/git-branch.txt | 8 ++-- Documentation/git-check-ref-format.txt | 2 +- Documentation/git-format-patch.txt | 2 +- Documentation/git-merge-base.txt | 2 +- Documentation/git-rebase.txt | 2 +- Documentation/git-remote.txt | 2 +- Documentation/git-rerere.txt | 10 ++--- Documentation/git-reset.txt| 18 - Documentation/git-revert.txt | 2 +- Documentation/git-stash.txt| 6 +-- Documentation/gitattributes.txt| 2 +- Documentation/gitcli.txt | 4 +- Documentation/gitcore-tutorial.txt | 18 - Documentation/giteveryday.txt | 24 ++-- Documentation/githooks.txt | 5 ++- Documentation/gittutorial-2.txt| 2 +- Documentation/gittutorial.txt | 4 +- Documentation/revisions.txt| 2 +- Documentation/user-manual.txt | 54 +- advice.c | 11 -- command-list.txt | 2 +- sha1-name.c| 2 +- wt-status.c| 2 +- 25 files changed, 104 insertions(+), 97 deletions(-) diff --git a/Documentation/config/advice.txt b/Documentation/config/advice.txt index 57fcd4c862..bffc503385 100644 --- a/Documentation/config/advice.txt +++ b/Documentation/config/advice.txt @@ -35,7 +35,8 @@ advice.*:: state in the output of linkgit:git-status[1], in the template shown when writing commit messages in linkgit:git-commit[1], and in the help message shown - by linkgit:git-checkout[1] when switching branch. + by linkgit:git-switch-branch[1] or + linkgit:git-checkout[1] when switching branch. statusUoption:: Advise to consider using the `-u` option to linkgit:git-status[1] when the command takes more than 2 seconds to enumerate untracked @@ -55,9 +56,10 @@ advice.*:: your information is guessed from the system username and domain name. detachedHead:: - Advice shown when you used linkgit:git-checkout[1] to - move to the detach HEAD state, to instruct how to create - a local branch after the fact. + Advice shown when you used + linkgit:git-switch-branch[1] or linkgit:git-checkout[1] + to move to the detach HEAD state, to instruct how to + create a local branch after the fact. checkoutAmbiguousRemoteBranchName:: Advice shown when the argument to linkgit:git-checkout[1] ambiguously resolves to a diff --git a/Documentation/config/checkout.txt b/Documentation/config/checkout.txt index c4118fa196..81b0d47ced 100644 --- a/Documentation/config/checkout.txt +++ b/Documentation/config/checkout.txt @@ -8,8 +8,9 @@ checkout.defaultRemote:: disambiguation. The typical use-case is to set this to `origin`. + -Currently this is used by linkgit:git-checkout[1] when 'git checkout -' will checkout the '' branch on another remote, +Currently this is used by linkgit:git-switch-branch[1] and +linkgit:git-checkout[1] when 'git checkout ' +will checkout the '' branch on another remote, and by linkgit:git-worktree[1] when 'git worktree add' refers to a remote branch. This setting might be used for other checkout-like commands or functionality in the future. diff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt index bf5316ffa9..1564df47d2 100644 --- a/Documentation/git-branch.txt +++ b/Documentation/git-branch.txt @@ -48,7 +48,7 @@ The command's second form creates a new branch head named which points to the current `HEAD`, or if given. Note that this will create the new branch, but it will not switch the -working tree to it; use "git checkout " to switch to the +working tree to it; use "git switch-branch " to switch to the new branch. When a local branch is started off a remote-tracking branch, Git sets up the @@ -194,7 +194,7 @@ This option is only applicable in non-verbose mode. + This behavior is the default when the start point is a remote-tracking branch. Set the branch.autoSetupMerge configuration variable to `false` if you -want `git checkout` and `git branch` to always behave as if `--no-track`
[PATCH v3 05/14] checkout: move 'confict_style' and 'dwim_..' to checkout_opts
These local variables are referenced by struct option[]. This struct
will soon be broken down, moved away and we can't rely on local
variables anymore. Move these two to struct checkout_opts in
preparation for that.
Signed-off-by: Nguyễn Thái Ngọc Duy
---
builtin/checkout.c | 16 +---
1 file changed, 9 insertions(+), 7 deletions(-)
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 1b19328d0a..2423fdbf94 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -44,6 +44,8 @@ struct checkout_opts {
int ignore_skipworktree;
int ignore_other_worktrees;
int show_progress;
+ int dwim_new_local_branch;
+
/*
* If new checkout options are added, skip_merge_working_tree
* should be updated accordingly.
@@ -55,6 +57,7 @@ struct checkout_opts {
int new_branch_log;
enum branch_track track;
struct diff_options diff_options;
+ char *conflict_style;
int branch_exists;
const char *prefix;
@@ -1239,8 +1242,6 @@ int cmd_checkout(int argc, const char **argv, const char
*prefix)
struct checkout_opts real_opts;
struct checkout_opts *opts = _opts;
struct branch_info new_branch_info;
- char *conflict_style = NULL;
- int dwim_new_local_branch = 1;
int dwim_remotes_matched = 0;
struct option options[] = {
OPT__QUIET(>quiet, N_("suppress progress reporting")),
@@ -1265,12 +1266,12 @@ int cmd_checkout(int argc, const char **argv, const
char *prefix)
OPT_BOOL_F(0, "overwrite-ignore", >overwrite_ignore,
N_("update ignored files (default)"),
PARSE_OPT_NOCOMPLETE),
- OPT_STRING(0, "conflict", _style, N_("style"),
+ OPT_STRING(0, "conflict", >conflict_style, N_("style"),
N_("conflict style (merge or diff3)")),
OPT_BOOL('p', "patch", >patch_mode, N_("select hunks
interactively")),
OPT_BOOL(0, "ignore-skip-worktree-bits",
>ignore_skipworktree,
N_("do not limit pathspecs to sparse entries only")),
- OPT_HIDDEN_BOOL(0, "guess", _new_local_branch,
+ OPT_HIDDEN_BOOL(0, "guess", >dwim_new_local_branch,
N_("second guess 'git checkout
'")),
OPT_BOOL(0, "ignore-other-worktrees",
>ignore_other_worktrees,
N_("do not check if another worktree is holding the
given ref")),
@@ -1286,6 +1287,7 @@ int cmd_checkout(int argc, const char **argv, const char
*prefix)
opts->overwrite_ignore = 1;
opts->prefix = prefix;
opts->show_progress = -1;
+ opts->dwim_new_local_branch = 1;
git_config(git_checkout_config, opts);
@@ -1301,9 +1303,9 @@ int cmd_checkout(int argc, const char **argv, const char
*prefix)
opts->show_progress = isatty(2);
}
- if (conflict_style) {
+ if (opts->conflict_style) {
opts->merge = 1; /* implied */
- git_xmerge_config("merge.conflictstyle", conflict_style, NULL);
+ git_xmerge_config("merge.conflictstyle", opts->conflict_style,
NULL);
}
if ((!!opts->new_branch + !!opts->new_branch_force +
!!opts->new_orphan_branch) > 1)
@@ -1350,7 +1352,7 @@ int cmd_checkout(int argc, const char **argv, const char
*prefix)
struct object_id rev;
int dwim_ok =
!opts->patch_mode &&
- dwim_new_local_branch &&
+ opts->dwim_new_local_branch &&
opts->track == BRANCH_TRACK_UNSPECIFIED &&
!opts->new_branch;
int n = parse_branchname_arg(argc, argv, dwim_ok,
--
2.20.0.rc1.380.g3eb999425c.dirty
[PATCH v3 11/14] switch-branch: only allow explicit detached HEAD
"git checkout " will checkout the commit in question and
detach HEAD from the current branch. It is naturally a right thing to
do once you get git references. But detached HEAD is a scary concept
to new users because we show a lot of warnings and stuff, and it could
be hard to get out of (until you know better).
To keep switch-branch a bit more friendly to new users, we only allow
entering detached HEAD mode when --detach is given. "git
switch-branch" must take a branch (unless you create a new branch,
then of course switch-branch can take any commit-ish)
Signed-off-by: Nguyễn Thái Ngọc Duy
---
builtin/checkout.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/builtin/checkout.c b/builtin/checkout.c
index c7ae068d2c..fbfebba2d9 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -49,6 +49,7 @@ struct checkout_opts {
int merge;
int force;
int force_detach;
+ int implicit_detach;
int writeout_stage;
int overwrite_ignore;
int ignore_skipworktree;
@@ -1241,6 +1242,13 @@ static int checkout_branch(struct checkout_opts *opts,
!opts->force_detach)
die(_("nothing to do"));
+ if (!opts->implicit_detach &&
+ !opts->new_branch &&
+ !opts->new_branch_force &&
+ new_branch_info->name &&
+ !new_branch_info->path)
+ die(_("a branch is expected, got %s"), new_branch_info->name);
+
if (new_branch_info->path && !opts->force_detach && !opts->new_branch &&
!opts->ignore_other_worktrees) {
int flag;
@@ -1485,6 +1493,7 @@ int cmd_checkout(int argc, const char **argv, const char
*prefix)
opts.dwim_new_local_branch = 1;
opts.switch_branch_doing_nothing_not_ok = 0;
opts.accept_pathspec = 1;
+ opts.implicit_detach = 1;
options = parse_options_dup(checkout_options);
options = add_common_options(, options);
@@ -1513,6 +1522,7 @@ int cmd_switch_branch(int argc, const char **argv, const
char *prefix)
opts.dwim_new_local_branch = 1;
opts.accept_pathspec = 0;
opts.switch_branch_doing_nothing_not_ok = 1;
+ opts.implicit_detach = 0;
options = parse_options_dup(switch_options);
options = add_common_options(, options);
--
2.20.0.rc1.380.g3eb999425c.dirty
[PATCH v3 04/14] checkout: make "opts" in cmd_checkout() a pointer
"opts" will soon be moved out of cmd_checkout(). To keep changes in
that patch smaller, convert "opts" to a pointer and keep the real
thing behind "real_opts".
Signed-off-by: Nguyễn Thái Ngọc Duy
---
builtin/checkout.c | 109 +++--
1 file changed, 55 insertions(+), 54 deletions(-)
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 1887c996c6..1b19328d0a 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -1236,76 +1236,77 @@ static int checkout_branch(struct checkout_opts *opts,
int cmd_checkout(int argc, const char **argv, const char *prefix)
{
- struct checkout_opts opts;
+ struct checkout_opts real_opts;
+ struct checkout_opts *opts = _opts;
struct branch_info new_branch_info;
char *conflict_style = NULL;
int dwim_new_local_branch = 1;
int dwim_remotes_matched = 0;
struct option options[] = {
- OPT__QUIET(, N_("suppress progress reporting")),
- OPT_STRING('b', NULL, _branch, N_("branch"),
+ OPT__QUIET(>quiet, N_("suppress progress reporting")),
+ OPT_STRING('b', NULL, >new_branch, N_("branch"),
N_("create and checkout a new branch")),
- OPT_STRING('B', NULL, _branch_force, N_("branch"),
+ OPT_STRING('B', NULL, >new_branch_force, N_("branch"),
N_("create/reset and checkout a branch")),
- OPT_BOOL('l', NULL, _branch_log, N_("create reflog for
new branch")),
- OPT_BOOL(0, "detach", _detach, N_("detach HEAD at
named commit")),
- OPT_SET_INT('t', "track", , N_("set upstream info
for new branch"),
+ OPT_BOOL('l', NULL, >new_branch_log, N_("create reflog
for new branch")),
+ OPT_BOOL(0, "detach", >force_detach, N_("detach HEAD at
named commit")),
+ OPT_SET_INT('t', "track", >track, N_("set upstream info
for new branch"),
BRANCH_TRACK_EXPLICIT),
- OPT_STRING(0, "orphan", _orphan_branch,
N_("new-branch"), N_("new unparented branch")),
- OPT_SET_INT_F('2', "ours", _stage,
+ OPT_STRING(0, "orphan", >new_orphan_branch,
N_("new-branch"), N_("new unparented branch")),
+ OPT_SET_INT_F('2', "ours", >writeout_stage,
N_("checkout our version for unmerged files"),
2, PARSE_OPT_NONEG),
- OPT_SET_INT_F('3', "theirs", _stage,
+ OPT_SET_INT_F('3', "theirs", >writeout_stage,
N_("checkout their version for unmerged files"),
3, PARSE_OPT_NONEG),
- OPT__FORCE(, N_("force checkout (throw away local
modifications)"),
+ OPT__FORCE(>force, N_("force checkout (throw away local
modifications)"),
PARSE_OPT_NOCOMPLETE),
- OPT_BOOL('m', "merge", , N_("perform a 3-way merge
with the new branch")),
- OPT_BOOL_F(0, "overwrite-ignore", _ignore,
+ OPT_BOOL('m', "merge", >merge, N_("perform a 3-way merge
with the new branch")),
+ OPT_BOOL_F(0, "overwrite-ignore", >overwrite_ignore,
N_("update ignored files (default)"),
PARSE_OPT_NOCOMPLETE),
OPT_STRING(0, "conflict", _style, N_("style"),
N_("conflict style (merge or diff3)")),
- OPT_BOOL('p', "patch", _mode, N_("select hunks
interactively")),
- OPT_BOOL(0, "ignore-skip-worktree-bits",
_skipworktree,
+ OPT_BOOL('p', "patch", >patch_mode, N_("select hunks
interactively")),
+ OPT_BOOL(0, "ignore-skip-worktree-bits",
>ignore_skipworktree,
N_("do not limit pathspecs to sparse entries only")),
OPT_HIDDEN_BOOL(0, "guess", _new_local_branch,
N_("second guess 'git checkout
'")),
- OPT_BOOL(0, "ignore-other-worktrees",
_other_worktrees,
+ OPT_BOOL(0, "ignore-other-worktrees",
>ignore_other_worktrees,
N_("do not check if another worktree is holding the
given ref")),
{ OPTION_CALLBACK, 0, "recurse-submodules", NULL,
"checkout", "control recursive updating of
submodules",
PARSE_OPT_OPTARG,
option_parse_recurse_submodules_worktree_updater },
- OPT_BOOL(0, "progress", _progress, N_("force progress
reporting")),
+ OPT_BOOL(0, "progress", >show_progress, N_("force
progress reporting")),
OPT_END(),
};
- memset(, 0, sizeof(opts));
+ memset(opts, 0, sizeof(*opts));
memset(_branch_info, 0, sizeof(new_branch_info));
- opts.overwrite_ignore = 1;
- opts.prefix = prefix;
-
[PATCH v3 02/14] git-checkout.txt: split detached head section out
This is to be reused by the coming git-switch-branch.txt man page which also deals with detached HEAD. Signed-off-by: Nguyễn Thái Ngọc Duy --- Documentation/detach-head.txt | 132 Documentation/git-checkout.txt | 133 + 2 files changed, 133 insertions(+), 132 deletions(-) create mode 100644 Documentation/detach-head.txt diff --git a/Documentation/detach-head.txt b/Documentation/detach-head.txt new file mode 100644 index 00..bb6f5d7843 --- /dev/null +++ b/Documentation/detach-head.txt @@ -0,0 +1,132 @@ +HEAD normally refers to a named branch (e.g. 'master'). Meanwhile, each +branch refers to a specific commit. Let's look at a repo with three +commits, one of them tagged, and with branch 'master' checked out: + + + HEAD (refers to branch 'master') +| +v +a---b---c branch 'master' (refers to commit 'c') +^ +| + tag 'v2.0' (refers to commit 'b') + + +When a commit is created in this state, the branch is updated to refer to +the new commit. Specifically, 'git commit' creates a new commit 'd', whose +parent is commit 'c', and then updates branch 'master' to refer to new +commit 'd'. HEAD still refers to branch 'master' and so indirectly now refers +to commit 'd': + + +$ edit; git add; git commit + + HEAD (refers to branch 'master') +| +v +a---b---c---d branch 'master' (refers to commit 'd') +^ +| + tag 'v2.0' (refers to commit 'b') + + +It is sometimes useful to be able to checkout a commit that is not at +the tip of any named branch, or even to create a new commit that is not +referenced by a named branch. Let's look at what happens when we +checkout commit 'b' (here we show two ways this may be done): + + +$ git checkout v2.0 # or +$ git checkout master^^ + + HEAD (refers to commit 'b') +| +v +a---b---c---d branch 'master' (refers to commit 'd') +^ +| + tag 'v2.0' (refers to commit 'b') + + +Notice that regardless of which checkout command we use, HEAD now refers +directly to commit 'b'. This is known as being in detached HEAD state. +It means simply that HEAD refers to a specific commit, as opposed to +referring to a named branch. Let's see what happens when we create a commit: + + +$ edit; git add; git commit + + HEAD (refers to commit 'e') + | + v + e + / +a---b---c---d branch 'master' (refers to commit 'd') +^ +| + tag 'v2.0' (refers to commit 'b') + + +There is now a new commit 'e', but it is referenced only by HEAD. We can +of course add yet another commit in this state: + + +$ edit; git add; git commit + + HEAD (refers to commit 'f') + | + v + e---f + / +a---b---c---d branch 'master' (refers to commit 'd') +^ +| + tag 'v2.0' (refers to commit 'b') + + +In fact, we can perform all the normal Git operations. But, let's look +at what happens when we then checkout master: + + +$ git checkout master + + HEAD (refers to branch 'master') + e---f | + / v +a---b---c---d branch 'master' (refers to commit 'd') +^ +| + tag 'v2.0' (refers to commit 'b') + + +It is important to realize that at this point nothing refers to commit +'f'. Eventually commit 'f' (and by extension commit 'e') will be deleted +by the routine Git garbage collection process, unless we create a reference +before that happens. If we have not yet moved away from commit 'f', +any of these will create a reference to it: + + +$ git checkout -b foo <1> +$ git branch foo<2> +$ git tag foo <3> + + +<1> creates a new branch 'foo', which refers to commit 'f', and then +updates HEAD to refer to branch 'foo'. In other words, we'll no longer +be in detached HEAD state after this command. + +<2> similarly creates a new branch 'foo', which refers to commit 'f', +but leaves HEAD detached. + +<3> creates a new tag 'foo', which refers to commit 'f', +leaving HEAD detached. + +If we have moved away from commit 'f', then we must first recover its object +name (typically by using git reflog), and then we can create a reference to +it. For example, to see the last two commits to which HEAD referred, we +can use either of these commands: + + +$ git reflog -2 HEAD # or +$ git log -g -2 HEAD + diff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt index 65bd1bc50d..25887a6087 100644 --- a/Documentation/git-checkout.txt +++ b/Documentation/git-checkout.txt @@ -306,138 +306,7 @@ leave out at most one of `A` and `B`, in which case it defaults to `HEAD`. DETACHED HEAD - -HEAD normally refers to a named branch (e.g. 'master'). Meanwhile, each -branch refers to a specific commit. Let's look at a repo with
[PATCH v3 12/14] restore-files: take tree-ish from --from option instead
This is another departure from 'git checkout' syntax, which uses -- to
separate ref and pathspec. The observation is restore-files (or "git
checkout ,, ") is most often used to restore some files from
the index. If this is correct, we can simplify it by taking a way the
ref, so that we can write
git restore-files some-file
without worrying about some-file being a ref and whether we need to do
git restore-files -- some-file
for safety. If the source of the restore comes from a tree, it will be
in the form of an option with value, e.g.
git restore-files --from=this-tree some-file
This is of course longer to type than using "--". But hopefully it
will not be used as often, and it is clearly easier to understand.
dwim_new_local_branch is no longer set (or unset) in cmd_restore_files()
because it's irrelevant because we don't really care about dwim-ing.
With accept_ref being unset, dwim can't happen.
Signed-off-by: Nguyễn Thái Ngọc Duy
---
builtin/checkout.c | 41 ++---
1 file changed, 34 insertions(+), 7 deletions(-)
diff --git a/builtin/checkout.c b/builtin/checkout.c
index fbfebba2d9..7ff9951818 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -39,7 +39,7 @@ static const char * const switch_branch_usage[] = {
};
static const char * const restore_files_usage[] = {
- N_("git restore-files [] [] -- ..."),
+ N_("git restore-files [] [--from=] ..."),
NULL,
};
@@ -56,6 +56,7 @@ struct checkout_opts {
int ignore_other_worktrees;
int show_progress;
int dwim_new_local_branch;
+ int accept_ref;
int accept_pathspec;
int switch_branch_doing_nothing_not_ok;
@@ -75,6 +76,7 @@ struct checkout_opts {
int branch_exists;
const char *prefix;
struct pathspec pathspec;
+ const char *from_treeish;
struct tree *source_tree;
};
@@ -1337,6 +1339,7 @@ static int checkout_main(int argc, const char **argv,
const char *prefix,
{
struct branch_info new_branch_info;
int dwim_remotes_matched = 0;
+ int parseopt_flags = 0;
memset(_branch_info, 0, sizeof(new_branch_info));
opts->overwrite_ignore = 1;
@@ -1347,8 +1350,13 @@ static int checkout_main(int argc, const char **argv,
const char *prefix,
opts->track = BRANCH_TRACK_UNSPECIFIED;
- argc = parse_options(argc, argv, prefix, options, usagestr,
-PARSE_OPT_KEEP_DASHDASH);
+ if (!opts->accept_pathspec && !opts->accept_ref)
+ BUG("make up your mind, you need to take _something_");
+ if (opts->accept_pathspec && opts->accept_ref)
+ parseopt_flags = PARSE_OPT_KEEP_DASHDASH;
+
+ argc = parse_options(argc, argv, prefix, options,
+usagestr, parseopt_flags);
if (opts->show_progress < 0) {
if (opts->quiet)
@@ -1402,7 +1410,7 @@ static int checkout_main(int argc, const char **argv,
const char *prefix,
* including "last branch" syntax and DWIM-ery for names of
* remote branches, erroring out for invalid or ambiguous cases.
*/
- if (argc) {
+ if (argc && opts->accept_ref && opts->accept_pathspec) {
struct object_id rev;
int dwim_ok =
!opts->patch_mode &&
@@ -1414,6 +1422,18 @@ static int checkout_main(int argc, const char **argv,
const char *prefix,
_remotes_matched);
argv += n;
argc -= n;
+ } else if (!opts->accept_ref && opts->from_treeish) {
+ struct object_id rev;
+
+ if (get_oid_mb(opts->from_treeish, ))
+ die(_("could not resolve %s"), opts->from_treeish);
+
+ setup_new_branch_info_and_source_tree(_branch_info,
+ opts, ,
+ opts->from_treeish);
+
+ if (!opts->source_tree)
+ die(_("reference is not a tree: %s"),
opts->from_treeish);
}
if (argc) {
@@ -1492,6 +1512,7 @@ int cmd_checkout(int argc, const char **argv, const char
*prefix)
memset(, 0, sizeof(opts));
opts.dwim_new_local_branch = 1;
opts.switch_branch_doing_nothing_not_ok = 0;
+ opts.accept_ref = 1;
opts.accept_pathspec = 1;
opts.implicit_detach = 1;
@@ -1520,6 +1541,7 @@ int cmd_switch_branch(int argc, const char **argv, const
char *prefix)
memset(, 0, sizeof(opts));
opts.dwim_new_local_branch = 1;
+ opts.accept_ref = 1;
opts.accept_pathspec = 0;
opts.switch_branch_doing_nothing_not_ok = 1;
opts.implicit_detach = 0;
@@ -1537,14 +1559,19 @@ int cmd_switch_branch(int argc, const char **argv,
const char *prefix)
int cmd_restore_files(int argc, const char **argv, const char *prefix)
[PATCH v3 08/14] switch-branch: better names for -b and -B
The shortcut of these options do not make much sense when used with
switch-branch. And their descriptions are also tied to checkout
out. Move -b/-B to cmd_checkout() and new -c/-C with the same
functionality in cmd_switch_branch()
Signed-off-by: Nguyễn Thái Ngọc Duy
---
builtin/checkout.c | 30 +++---
1 file changed, 19 insertions(+), 11 deletions(-)
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 7dc0f4d3f3..ceb635de36 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -1268,14 +1268,10 @@ static struct option *add_common_options(struct
checkout_opts *opts,
return newopts;
}
-static struct option *add_switch_branch_options(struct checkout_opts *opts,
- struct option *prevopts)
+static struct option *add_common_switch_branch_options(
+ struct checkout_opts *opts, struct option *prevopts)
{
struct option options[] = {
- OPT_STRING('b', NULL, >new_branch, N_("branch"),
- N_("create and checkout a new branch")),
- OPT_STRING('B', NULL, >new_branch_force, N_("branch"),
- N_("create/reset and checkout a branch")),
OPT_BOOL('l', NULL, >new_branch_log, N_("create reflog
for new branch")),
OPT_BOOL(0, "detach", >force_detach, N_("detach HEAD at
named commit")),
OPT_SET_INT('t', "track", >track, N_("set upstream info
for new branch"),
@@ -1461,15 +1457,21 @@ static int checkout_main(int argc, const char **argv,
const char *prefix,
int cmd_checkout(int argc, const char **argv, const char *prefix)
{
struct checkout_opts opts;
- struct option *options = NULL;
+ struct option *options;
+ struct option checkout_options[] = {
+ OPT_STRING('b', NULL, _branch, N_("branch"),
+ N_("create and checkout a new branch")),
+ OPT_STRING('B', NULL, _branch_force, N_("branch"),
+ N_("create/reset and checkout a branch")),
+ };
int ret;
memset(, 0, sizeof(opts));
opts.dwim_new_local_branch = 1;
- options = parse_options_dup(options);
+ options = parse_options_dup(checkout_options);
options = add_common_options(, options);
- options = add_switch_branch_options(, options);
+ options = add_common_switch_branch_options(, options);
options = add_checkout_path_options(, options);
ret = checkout_main(argc, argv, prefix, ,
@@ -1482,14 +1484,20 @@ int cmd_switch_branch(int argc, const char **argv,
const char *prefix)
{
struct checkout_opts opts;
struct option *options = NULL;
+ struct option switch_options[] = {
+ OPT_STRING('c', "create", _branch, N_("branch"),
+ N_("create and switch to a new branch")),
+ OPT_STRING('C', "force-create", _branch_force,
N_("branch"),
+ N_("create/reset and switch to a new branch")),
+ };
int ret;
memset(, 0, sizeof(opts));
opts.dwim_new_local_branch = 1;
- options = parse_options_dup(options);
+ options = parse_options_dup(switch_options);
options = add_common_options(, options);
- options = add_switch_branch_options(, options);
+ options = add_common_switch_branch_options(, options);
ret = checkout_main(argc, argv, prefix, ,
options, switch_branch_usage);
--
2.20.0.rc1.380.g3eb999425c.dirty
[PATCH v3 06/14] checkout: split options[] array in three pieces
This is a preparation step for introducing new commands that do parts
of what checkout does. There will be two new commands, one is about
switching branches, detaching HEAD... one about checking out
paths. These share the a subset of command line options. The rest of
command line options are separate.
Signed-off-by: Nguyễn Thái Ngọc Duy
---
builtin/checkout.c | 77 +-
parse-options-cb.c | 16 ++
parse-options.h| 3 +-
3 files changed, 73 insertions(+), 23 deletions(-)
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 2423fdbf94..764e1a83a1 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -1237,14 +1237,31 @@ static int checkout_branch(struct checkout_opts *opts,
return switch_branches(opts, new_branch_info);
}
-int cmd_checkout(int argc, const char **argv, const char *prefix)
+static struct option *add_common_options(struct checkout_opts *opts,
+struct option *prevopts)
{
- struct checkout_opts real_opts;
- struct checkout_opts *opts = _opts;
- struct branch_info new_branch_info;
- int dwim_remotes_matched = 0;
struct option options[] = {
OPT__QUIET(>quiet, N_("suppress progress reporting")),
+ { OPTION_CALLBACK, 0, "recurse-submodules", NULL,
+ "checkout", "control recursive updating of
submodules",
+ PARSE_OPT_OPTARG,
option_parse_recurse_submodules_worktree_updater },
+ OPT_BOOL(0, "progress", >show_progress, N_("force
progress reporting")),
+ OPT__FORCE(>force, N_("force checkout (throw away local
modifications)"),
+ PARSE_OPT_NOCOMPLETE),
+ OPT_BOOL('m', "merge", >merge, N_("perform a 3-way merge
with the new branch")),
+ OPT_STRING(0, "conflict", >conflict_style, N_("style"),
+ N_("conflict style (merge or diff3)")),
+ OPT_END()
+ };
+ struct option *newopts = parse_options_concat(prevopts, options);
+ free(prevopts);
+ return newopts;
+}
+
+static struct option *add_switch_branch_options(struct checkout_opts *opts,
+ struct option *prevopts)
+{
+ struct option options[] = {
OPT_STRING('b', NULL, >new_branch, N_("branch"),
N_("create and checkout a new branch")),
OPT_STRING('B', NULL, >new_branch_force, N_("branch"),
@@ -1254,33 +1271,44 @@ int cmd_checkout(int argc, const char **argv, const
char *prefix)
OPT_SET_INT('t', "track", >track, N_("set upstream info
for new branch"),
BRANCH_TRACK_EXPLICIT),
OPT_STRING(0, "orphan", >new_orphan_branch,
N_("new-branch"), N_("new unparented branch")),
+ OPT_HIDDEN_BOOL(0, "guess", >dwim_new_local_branch,
+ N_("second guess 'git checkout
'")),
+ OPT_BOOL(0, "ignore-other-worktrees",
>ignore_other_worktrees,
+N_("do not check if another worktree is holding the
given ref")),
+ OPT_END()
+ };
+ struct option *newopts = parse_options_concat(prevopts, options);
+ free(prevopts);
+ return newopts;
+}
+
+static struct option *add_checkout_path_options(struct checkout_opts *opts,
+ struct option *prevopts)
+{
+ struct option options[] = {
OPT_SET_INT_F('2', "ours", >writeout_stage,
N_("checkout our version for unmerged files"),
2, PARSE_OPT_NONEG),
OPT_SET_INT_F('3', "theirs", >writeout_stage,
N_("checkout their version for unmerged files"),
3, PARSE_OPT_NONEG),
- OPT__FORCE(>force, N_("force checkout (throw away local
modifications)"),
- PARSE_OPT_NOCOMPLETE),
- OPT_BOOL('m', "merge", >merge, N_("perform a 3-way merge
with the new branch")),
- OPT_BOOL_F(0, "overwrite-ignore", >overwrite_ignore,
- N_("update ignored files (default)"),
- PARSE_OPT_NOCOMPLETE),
- OPT_STRING(0, "conflict", >conflict_style, N_("style"),
- N_("conflict style (merge or diff3)")),
OPT_BOOL('p', "patch", >patch_mode, N_("select hunks
interactively")),
OPT_BOOL(0, "ignore-skip-worktree-bits",
>ignore_skipworktree,
N_("do not limit pathspecs to sparse entries only")),
- OPT_HIDDEN_BOOL(0, "guess", >dwim_new_local_branch,
- N_("second guess 'git checkout
'")),
- OPT_BOOL(0, "ignore-other-worktrees",
>ignore_other_worktrees,
-
[PATCH v3 13/14] restore-files: make pathspec mandatory
"git restore-files" without arguments does not make much sense when
it's about restoring files (what files now?). We could default to
either
git restore-files .
or
git restore-files :/
Neither is intuitive. Make the user always give pathspec, force the
user to think the scope of restore they want.
Signed-off-by: Nguyễn Thái Ngọc Duy
---
builtin/checkout.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 7ff9951818..961a90b1c0 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -59,6 +59,7 @@ struct checkout_opts {
int accept_ref;
int accept_pathspec;
int switch_branch_doing_nothing_not_ok;
+ int empty_pathspec_ok;
/*
* If new checkout options are added, skip_merge_working_tree
@@ -1436,6 +1437,9 @@ static int checkout_main(int argc, const char **argv,
const char *prefix,
die(_("reference is not a tree: %s"),
opts->from_treeish);
}
+ if (opts->accept_pathspec && !opts->empty_pathspec_ok && !argc)
+ die(_("pathspec is required"));
+
if (argc) {
parse_pathspec(>pathspec, 0,
opts->patch_mode ? PATHSPEC_PREFIX_ORIGIN : 0,
@@ -1515,6 +1519,7 @@ int cmd_checkout(int argc, const char **argv, const char
*prefix)
opts.accept_ref = 1;
opts.accept_pathspec = 1;
opts.implicit_detach = 1;
+ opts.empty_pathspec_ok = 1;
options = parse_options_dup(checkout_options);
options = add_common_options(, options);
@@ -1570,6 +1575,7 @@ int cmd_restore_files(int argc, const char **argv, const
char *prefix)
memset(, 0, sizeof(opts));
opts.accept_ref = 0;
opts.accept_pathspec = 1;
+ opts.empty_pathspec_ok = 0;
options = parse_options_dup(restore_options);
options = add_common_options(, options);
--
2.20.0.rc1.380.g3eb999425c.dirty
[PATCH v3 09/14] switch-branch: stop accepting pathspec
This command is about switching branch (or creating a new one) and
should not accept pathspec. This helps simplify ambiguation
handling. The other two ("git checkout" and "git restore-files") of
course do accept pathspec as before.
Signed-off-by: Nguyễn Thái Ngọc Duy
---
builtin/checkout.c | 14 --
1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/builtin/checkout.c b/builtin/checkout.c
index ceb635de36..880030e929 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -55,6 +55,7 @@ struct checkout_opts {
int ignore_other_worktrees;
int show_progress;
int dwim_new_local_branch;
+ int accept_pathspec;
/*
* If new checkout options are added, skip_merge_working_tree
@@ -1089,10 +1090,16 @@ static int parse_branchname_arg(int argc, const char
**argv,
if (!argc)
return 0;
+ if (!opts->accept_pathspec) {
+ if (argc > 1)
+ die(_("only one reference expected"));
+ has_dash_dash = 1; /* helps disambiguate */
+ }
+
arg = argv[0];
dash_dash_pos = -1;
for (i = 0; i < argc; i++) {
- if (!strcmp(argv[i], "--")) {
+ if (opts->accept_pathspec && !strcmp(argv[i], "--")) {
dash_dash_pos = i;
break;
}
@@ -1167,7 +1174,7 @@ static int parse_branchname_arg(int argc, const char
**argv,
*/
if (argc)
verify_non_filename(opts->prefix, arg);
- } else {
+ } else if (opts->accept_pathspec) {
argcount++;
argv++;
argc--;
@@ -1468,6 +1475,7 @@ int cmd_checkout(int argc, const char **argv, const char
*prefix)
memset(, 0, sizeof(opts));
opts.dwim_new_local_branch = 1;
+ opts.accept_pathspec = 1;
options = parse_options_dup(checkout_options);
options = add_common_options(, options);
@@ -1494,6 +1502,7 @@ int cmd_switch_branch(int argc, const char **argv, const
char *prefix)
memset(, 0, sizeof(opts));
opts.dwim_new_local_branch = 1;
+ opts.accept_pathspec = 0;
options = parse_options_dup(switch_options);
options = add_common_options(, options);
@@ -1513,6 +1522,7 @@ int cmd_restore_files(int argc, const char **argv, const
char *prefix)
memset(, 0, sizeof(opts));
opts.dwim_new_local_branch = 1;
+ opts.accept_pathspec = 1;
options = parse_options_dup(options);
options = add_common_options(, options);
--
2.20.0.rc1.380.g3eb999425c.dirty
[PATCH v3 03/14] checkout: factor out some code in parse_branchname_arg()
This is in preparation for the new command restore-files, which also
needs to parse opts->source_tree but does not need all the
disambiguation logic.
Signed-off-by: Nguyễn Thái Ngọc Duy
---
builtin/checkout.c | 51 --
1 file changed, 31 insertions(+), 20 deletions(-)
diff --git a/builtin/checkout.c b/builtin/checkout.c
index acdafc6e4c..1887c996c6 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -990,6 +990,34 @@ static int git_checkout_config(const char *var, const char
*value, void *cb)
return git_xmerge_config(var, value, NULL);
}
+static void setup_new_branch_info_and_source_tree(
+ struct branch_info *new_branch_info,
+ struct checkout_opts *opts,
+ struct object_id *rev,
+ const char *arg)
+{
+ struct tree **source_tree = >source_tree;
+ struct object_id branch_rev;
+
+ new_branch_info->name = arg;
+ setup_branch_path(new_branch_info);
+
+ if (!check_refname_format(new_branch_info->path, 0) &&
+ !read_ref(new_branch_info->path, _rev))
+ oidcpy(rev, _rev);
+ else
+ new_branch_info->path = NULL; /* not an existing branch */
+
+ new_branch_info->commit =
lookup_commit_reference_gently(the_repository, rev, 1);
+ if (!new_branch_info->commit) {
+ /* not a commit */
+ *source_tree = parse_tree_indirect(rev);
+ } else {
+ parse_commit_or_die(new_branch_info->commit);
+ *source_tree = get_commit_tree(new_branch_info->commit);
+ }
+}
+
static int parse_branchname_arg(int argc, const char **argv,
int dwim_new_local_branch_ok,
struct branch_info *new_branch_info,
@@ -997,10 +1025,8 @@ static int parse_branchname_arg(int argc, const char
**argv,
struct object_id *rev,
int *dwim_remotes_matched)
{
- struct tree **source_tree = >source_tree;
const char **new_branch = >new_branch;
int argcount = 0;
- struct object_id branch_rev;
const char *arg;
int dash_dash_pos;
int has_dash_dash = 0;
@@ -1114,26 +1140,11 @@ static int parse_branchname_arg(int argc, const char
**argv,
argv++;
argc--;
- new_branch_info->name = arg;
- setup_branch_path(new_branch_info);
-
- if (!check_refname_format(new_branch_info->path, 0) &&
- !read_ref(new_branch_info->path, _rev))
- oidcpy(rev, _rev);
- else
- new_branch_info->path = NULL; /* not an existing branch */
+ setup_new_branch_info_and_source_tree(new_branch_info, opts, rev, arg);
- new_branch_info->commit =
lookup_commit_reference_gently(the_repository, rev, 1);
- if (!new_branch_info->commit) {
- /* not a commit */
- *source_tree = parse_tree_indirect(rev);
- } else {
- parse_commit_or_die(new_branch_info->commit);
- *source_tree = get_commit_tree(new_branch_info->commit);
- }
-
- if (!*source_tree) /* case (1): want a tree */
+ if (!opts->source_tree) /* case (1): want a tree */
die(_("reference is not a tree: %s"), arg);
+
if (!has_dash_dash) { /* case (3).(d) -> (1) */
/*
* Do not complain the most common case
--
2.20.0.rc1.380.g3eb999425c.dirty
[PATCH v3 07/14] checkout: split into switch-branch and restore-files
"git checkout" doing too many things is a source of confusion for many users (and it even bites old timers sometimes). To rememdy that, the command is now split in two: switch-branch and checkout-files. The good old "git checkout" command is still here and will be until all (or most of users) are sick of it. See the new man pages for the final design of these commands. The actual implementation though is still pretty much the same as "git checkout". Following patches will adjust their behavior to match the man pages. Signed-off-by: Nguyễn Thái Ngọc Duy --- .gitignore | 2 + Documentation/git-checkout.txt | 5 + Documentation/git-restore-files.txt | 167 Documentation/git-switch-branch.txt | 289 Makefile| 2 + builtin.h | 2 + builtin/checkout.c | 84 ++-- command-list.txt| 2 + git.c | 2 + 9 files changed, 543 insertions(+), 12 deletions(-) create mode 100644 Documentation/git-restore-files.txt create mode 100644 Documentation/git-switch-branch.txt diff --git a/.gitignore b/.gitignore index 0d77ea5894..c63dcb1427 100644 --- a/.gitignore +++ b/.gitignore @@ -143,6 +143,7 @@ /git-request-pull /git-rerere /git-reset +/git-restore-files /git-rev-list /git-rev-parse /git-revert @@ -167,6 +168,7 @@ /git-submodule /git-submodule--helper /git-svn +/git-switch-branch /git-symbolic-ref /git-tag /git-unpack-file diff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt index 25887a6087..25ec7f508f 100644 --- a/Documentation/git-checkout.txt +++ b/Documentation/git-checkout.txt @@ -406,6 +406,11 @@ $ edit frotz $ git add frotz +SEE ALSO + +linkgit:git-switch-branch[1] +linkgit:git-restore-files[1] + GIT --- Part of the linkgit:git[1] suite diff --git a/Documentation/git-restore-files.txt b/Documentation/git-restore-files.txt new file mode 100644 index 00..03c1250ad0 --- /dev/null +++ b/Documentation/git-restore-files.txt @@ -0,0 +1,167 @@ +git-restore-files(1) + + +NAME + +git-restore-files - Restore working tree files + +SYNOPSIS + +[verse] +'git restore-files'
[PATCH v3 10/14] switch-branch: reject "do nothing" case
"git checkout" can be executed without any arguments. What it does is
not exactly great: it switches from HEAD to HEAD and showing worktree
modification as a side effect.
Make switch-branch reject this case. You have to either
- really switch a branch
- (explicitly) detach from the current branch
- create a new branch
Signed-off-by: Nguyễn Thái Ngọc Duy
---
builtin/checkout.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 880030e929..c7ae068d2c 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -56,6 +56,7 @@ struct checkout_opts {
int show_progress;
int dwim_new_local_branch;
int accept_pathspec;
+ int switch_branch_doing_nothing_not_ok;
/*
* If new checkout options are added, skip_merge_working_tree
@@ -1233,6 +1234,13 @@ static int checkout_branch(struct checkout_opts *opts,
die(_("Cannot switch branch to a non-commit '%s'"),
new_branch_info->name);
+ if (opts->switch_branch_doing_nothing_not_ok &&
+ !new_branch_info->name &&
+ !opts->new_branch &&
+ !opts->new_branch_force &&
+ !opts->force_detach)
+ die(_("nothing to do"));
+
if (new_branch_info->path && !opts->force_detach && !opts->new_branch &&
!opts->ignore_other_worktrees) {
int flag;
@@ -1475,6 +1483,7 @@ int cmd_checkout(int argc, const char **argv, const char
*prefix)
memset(, 0, sizeof(opts));
opts.dwim_new_local_branch = 1;
+ opts.switch_branch_doing_nothing_not_ok = 0;
opts.accept_pathspec = 1;
options = parse_options_dup(checkout_options);
@@ -1503,6 +1512,7 @@ int cmd_switch_branch(int argc, const char **argv, const
char *prefix)
memset(, 0, sizeof(opts));
opts.dwim_new_local_branch = 1;
opts.accept_pathspec = 0;
+ opts.switch_branch_doing_nothing_not_ok = 1;
options = parse_options_dup(switch_options);
options = add_common_options(, options);
--
2.20.0.rc1.380.g3eb999425c.dirty
[PATCH v3 01/14] git-checkout.txt: fix one syntax line
can be omitted in this syntax, and it's actually documented a few paragraphs down: You could omit , in which case the command degenerates to "check out the current branch", which is a glorified no-op with rather expensive side-effects to show only the tracking information, if exists, for the current branch. Signed-off-by: Nguyễn Thái Ngọc Duy --- Documentation/git-checkout.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt index 801de2f764..65bd1bc50d 100644 --- a/Documentation/git-checkout.txt +++ b/Documentation/git-checkout.txt @@ -23,7 +23,7 @@ or the specified tree. If no paths are given, 'git checkout' will also update `HEAD` to set the specified branch as the current branch. -'git checkout' :: +'git checkout' []:: To prepare for working on , switch to it by updating the index and the files in the working tree, and by pointing HEAD at the branch. Local modifications to the files in the -- 2.20.0.rc1.380.g3eb999425c.dirty
[PATCH/RFC v3 00/14] Introduce new commands switch-branch and restore-files
v3 sees switch-branch go back to switch-branch (in v2 it was checkout-branch). checkout-files is also renamed restore-files (v1 was restore-paths). Hopefully we won't see another rename. I'll try to summarize the differences between the new commands and 'git checkout' down here, but you're welcome to just head to 07/14 and read the new man pages. 'git switch-branch' - does not "do nothing", you have to either switch branch, create a new branch, or detach. "git switch-branch" with no arguments is rejected. - implicit detaching is rejected. If you need to detach, you need to give --detach. Or stick to 'git checkout'. - -b/-B is renamed to -c/-C with long option names - of course does not accept pathspec 'git restore-files' - takes a ref from --from argument, not as a free ref. As a result, '--' is no longer needed. All non-option arguments are pathspec - pathspec is mandatory, you can't do "git restore-files" without any pathspec. - I just remember -p which is allowed to take no pathspec :( I'll fix it later. - Two more fancy features (the "git checkout --index" being the default mode and the backup log for accidental overwrites) are of course still missing. But they are coming. I did not go replace "detached HEAD" with "unnamed branch" (or "no branch") everywhere because I think a unique term is still good to refer to this concept. Or maybe "no branch" is good enough. I dunno. Nguyễn Thái Ngọc Duy (14): git-checkout.txt: fix one syntax line git-checkout.txt: split detached head section out checkout: factor out some code in parse_branchname_arg() checkout: make "opts" in cmd_checkout() a pointer checkout: move 'confict_style' and 'dwim_..' to checkout_opts checkout: split options[] array in three pieces checkout: split into switch-branch and restore-files switch-branch: better names for -b and -B switch-branch: stop accepting pathspec switch-branch: reject "do nothing" case switch-branch: only allow explicit detached HEAD restore-files: take tree-ish from --from option instead restore-files: make pathspec mandatory doc: promote "git switch-branch" and "git restore-files" .gitignore | 2 + Documentation/config/advice.txt| 10 +- Documentation/config/checkout.txt | 5 +- Documentation/detach-head.txt | 132 + Documentation/git-branch.txt | 8 +- Documentation/git-check-ref-format.txt | 2 +- Documentation/git-checkout.txt | 140 + Documentation/git-format-patch.txt | 2 +- Documentation/git-merge-base.txt | 2 +- Documentation/git-rebase.txt | 2 +- Documentation/git-remote.txt | 2 +- Documentation/git-rerere.txt | 10 +- Documentation/git-reset.txt| 18 +- Documentation/git-restore-files.txt| 167 +++ Documentation/git-revert.txt | 2 +- Documentation/git-stash.txt| 6 +- Documentation/git-switch-branch.txt| 289 +++ Documentation/gitattributes.txt| 2 +- Documentation/gitcli.txt | 4 +- Documentation/gitcore-tutorial.txt | 18 +- Documentation/giteveryday.txt | 24 +- Documentation/githooks.txt | 5 +- Documentation/gittutorial-2.txt| 2 +- Documentation/gittutorial.txt | 4 +- Documentation/revisions.txt| 2 +- Documentation/user-manual.txt | 54 ++-- Makefile | 2 + advice.c | 11 +- builtin.h | 2 + builtin/checkout.c | 380 ++--- command-list.txt | 4 +- git.c | 2 + parse-options-cb.c | 16 ++ parse-options.h| 3 +- sha1-name.c| 2 +- wt-status.c| 2 +- 36 files changed, 1006 insertions(+), 332 deletions(-) create mode 100644 Documentation/detach-head.txt create mode 100644 Documentation/git-restore-files.txt create mode 100644 Documentation/git-switch-branch.txt -- 2.20.0.rc1.380.g3eb999425c.dirty
Re: [PATCH v2 6/7] checkout: split into switch-branch and checkout-files
> > Which brings us back to your "git checkout-files " use case > above. It should be treat the same way in my opinion, so we either do > > git checkout-files --from=tree-ish :/ > > or > > git checkout-files --from=tree-ish . > > But "git checkout-files --from=tree-ish" alone is rejected. Agreed. Those arguments are better. The gist of my comment was the treatment of newly created local files rather than the form of the arguments, but it sounds like you've got that under control, too. > > Suggestion: > > If git checkout-files overwrites or deletes any locally-modified files > > from the workspace or index, those files could be auto-stashed. That > > would make it easy to restore them in the event of a mistyped command. > > Auto-stashing could be suppressed with a command-line argument (with > > alternate behaviors being fail-if-modified or always-overwrite). > > Stashing I think is not the right tool for this. When you stash, you > plan to retrieve it back later but here you should rarely ever need to > unstash until the accident. For recovery from accidents like this, I > have another thing in queue to achieve the same (I'm calling it > "backup log" now). So we will have the same functionality, just with > different means. Yes, this makes sense too. You wouldn't want to pollute the stash list with autogenerated things the user probably doesn't want. > This one is tricky because we should deal with submodule autoupdate > consistently across all porcelain commands (or at least common ones), > not just checkout-files. This is also a good point. I'd like it if submodules just behaved like a single giant repository for most commands, but you're right that this is something that should be done intentionally for all the commands at one rather than just for a single command. I also like your new names "switch-branch" and "restore-files".
[PATCH 1/1] rebase: fix GIT_REFLOG_ACTION regression
From: Johannes Schindelin
The scripted version (partially) heeded the `GIT_REFLOG_ACTION` and when
we converted to a built-in, this regressed.
Fix that, and add a regression test, both with `GIT_REFLOG_ACTION` set
and unset.
Note: the reflog message for "rebase finished" did *not* heed
GIT_REFLOG_ACTION, and as we are very late in the v2.20.0-rcN phase, we
leave that bug for later (as it seems that that bug has been with us
from the very beginning).
Reported by Ian Jackson.
Signed-off-by: Johannes Schindelin
---
builtin/rebase.c | 29 ++---
t/t3406-rebase-message.sh | 26 ++
2 files changed, 52 insertions(+), 3 deletions(-)
diff --git a/builtin/rebase.c b/builtin/rebase.c
index 5b3e5baec8..ba0c3c954b 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -776,6 +776,23 @@ static void NORETURN
error_on_missing_default_upstream(void)
exit(1);
}
+static void set_reflog_action(struct rebase_options *options)
+{
+ const char *env;
+ struct strbuf buf = STRBUF_INIT;
+
+ if (!is_interactive(options))
+ return;
+
+ env = getenv(GIT_REFLOG_ACTION_ENVIRONMENT);
+ if (env && strcmp("rebase", env))
+ return; /* only override it if it is "rebase" */
+
+ strbuf_addf(, "rebase -i (%s)", options->action);
+ setenv(GIT_REFLOG_ACTION_ENVIRONMENT, buf.buf, 1);
+ strbuf_release();
+}
+
int cmd_rebase(int argc, const char **argv, const char *prefix)
{
struct rebase_options options = {
@@ -978,6 +995,7 @@ int cmd_rebase(int argc, const char **argv, const char
*prefix)
if (action != NO_ACTION && !in_progress)
die(_("No rebase in progress?"));
+ setenv(GIT_REFLOG_ACTION_ENVIRONMENT, "rebase", 0);
if (action == ACTION_EDIT_TODO && !is_interactive())
die(_("The --edit-todo action can only be used during "
@@ -990,6 +1008,7 @@ int cmd_rebase(int argc, const char **argv, const char
*prefix)
int fd;
options.action = "continue";
+ set_reflog_action();
/* Sanity check */
if (get_oid("HEAD", ))
@@ -1018,6 +1037,7 @@ int cmd_rebase(int argc, const char **argv, const char
*prefix)
struct string_list merge_rr = STRING_LIST_INIT_DUP;
options.action = "skip";
+ set_reflog_action();
rerere_clear(_rr);
string_list_clear(_rr, 1);
@@ -1033,6 +1053,7 @@ int cmd_rebase(int argc, const char **argv, const char
*prefix)
case ACTION_ABORT: {
struct string_list merge_rr = STRING_LIST_INIT_DUP;
options.action = "abort";
+ set_reflog_action();
rerere_clear(_rr);
string_list_clear(_rr, 1);
@@ -1440,11 +1461,12 @@ int cmd_rebase(int argc, const char **argv, const char
*prefix)
}
strbuf_reset();
- strbuf_addf(, "rebase: checkout %s",
+ strbuf_addf(, "%s: checkout %s",
+
getenv(GIT_REFLOG_ACTION_ENVIRONMENT),
options.switch_to);
if (reset_head(, "checkout",
options.head_name, 0,
- NULL, NULL) < 0) {
+ NULL, buf.buf) < 0) {
ret = !!error(_("could not switch to "
"%s"),
options.switch_to);
@@ -1508,7 +1530,8 @@ int cmd_rebase(int argc, const char **argv, const char
*prefix)
printf(_("First, rewinding head to replay your work on top of "
"it...\n"));
- strbuf_addf(, "rebase: checkout %s", options.onto_name);
+ strbuf_addf(, "%s: checkout %s",
+ getenv(GIT_REFLOG_ACTION_ENVIRONMENT), options.onto_name);
if (reset_head(>object.oid, "checkout", NULL,
RESET_HEAD_DETACH, NULL, msg.buf))
die(_("Could not detach HEAD"));
diff --git a/t/t3406-rebase-message.sh b/t/t3406-rebase-message.sh
index 38bd876cab..db8505eb86 100755
--- a/t/t3406-rebase-message.sh
+++ b/t/t3406-rebase-message.sh
@@ -91,4 +91,30 @@ test_expect_success 'error out early upon -C or
--whitespace=' '
test_i18ngrep "Invalid whitespace option" err
'
+test_expect_success 'GIT_REFLOG_ACTION' '
+ git checkout start &&
+ test_commit reflog-onto &&
+ git checkout -b reflog-topic start &&
+ test_commit reflog-to-rebase &&
+
+ git rebase reflog-onto &&
+ git log -g --format=%gs -3 >actual &&
+ cat >expect <<-\EOF &&
+ rebase finished:
[PATCH 0/1] Fix built-in rebase regression noticed by Debian's dgit
It has been reported on the Debian bug tracker [https://bugs.debian.org/914695] that the built-in rebase regresses on the scripted version, and later details emerged that this has something to do with the reflog messages: they were different with the built-in rebase than with the scripted one. This here patch fixes that. Johannes Schindelin (1): rebase: fix GIT_REFLOG_ACTION regression builtin/rebase.c | 29 ++--- t/t3406-rebase-message.sh | 26 ++ 2 files changed, 52 insertions(+), 3 deletions(-) base-commit: 7068cbc4abac53d9c3675dfba81c1e97d25e8eeb Published-As: https://github.com/gitgitgadget/git/releases/tags/pr-91%2Fdscho%2Ffix-reflog-action-v1 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-91/dscho/fix-reflog-action-v1 Pull-Request: https://github.com/gitgitgadget/git/pull/91 -- gitgitgadget
Re: [PATCH 2/2] format-patch: allow for independent diff & range-diff options
Hi Ævar, On Thu, 29 Nov 2018, Ævar Arnfjörð Bjarmason wrote: > On Thu, Nov 29 2018, Johannes Schindelin wrote: > > > On Thu, 29 Nov 2018, Ævar Arnfjörð Bjarmason wrote: > > > >> On Thu, Nov 29 2018, Johannes Schindelin wrote: > >> > >> > On Thu, 29 Nov 2018, Ævar Arnfjörð Bjarmason wrote: > >> > > >> >> On Thu, Nov 29 2018, Johannes Schindelin wrote: > >> >> > >> >> > On Wed, 28 Nov 2018, Ævar Arnfjörð Bjarmason wrote: > >> >> > > >> >> >> Change the semantics of the "--range-diff" option so that the regular > >> >> >> diff options can be provided separately for the range-diff and the > >> >> >> patch. This allows for supplying e.g. --range-diff-U0 and -U1 to > >> >> >> "format-patch" to provide different context for the range-diff and > >> >> >> the > >> >> >> patch. This wasn't possible before. > >> >> > > >> >> > I really, really dislike the `--range-diff-`. We have > >> >> > precedent for passing optional arguments that are passed to some other > >> >> > command, so a much more logical and consistent convention would be to > >> >> > use > >> >> > `--range-diff[=..]`, allowing all of the diff options > >> >> > that > >> >> > you might want to pass to the outer diff in one go rather than having > >> >> > a > >> >> > lengthy string of `--range-diff-this` and `--range-diff-that` options. > >> >> > >> >> Where do we pass those sorts of arguments? > >> >> > >> >> Reasons I did it this way: > >> >> > >> >> a) Passing it as one option will require the user to double-quote those > >> >> options that take quoted arguments (e.g. --word-diff-regex), which I > >> >> thought sucked more than the prefix. On the implementation side we > >> >> couldn't leave the parsing of the command-line to the shell anymore. > >> >> > >> >> b) I think people will want to tweak this very rarely, much more rarely > >> >> than e.g. -U10 in format-patch itself, so having something long-ish > >> >> doesn't sound bad. > >> > > >> > Hmm. I still don't like it. It sets a precedent, and we simply do not do > >> > it that way in other circumstances (most obvious would be the -X merge > >> > options). The more divergent user interfaces for the same sort of thing > >> > are, the more brain cycles you force users to spend on navigating said > >> > interfaces. > >> > >> Yeah it sucks, I just think it sucks less than the alternative :) > >> I.e. I'm not picky about --range-diff-* prefix the name, but I think > >> doing our own shell parsing would be nasty. > > > > What prevents you from using `sq_dequote_to_argv()`? > > I mean not just nasty in terms of implementation, yeah we could do it, > but also a nasty UX for things like --word-diff-regex. I.e. instead of: > > --range-diff-word-diff-regex='[0-9"]' > > You need: > > --range-diff-opts="--word-diff-regex='[0-9\"]'" Really? I think that would not work. It would pass the single quotes as part of the regex to the diff machinery. Or maybe not. But the extra quotes do not strike me as necessary, as there is no shell script involved (thank deity!) after `git range-diff` parsed the options. > Now admittedly that in itself isn't very painful *in this case*, but in > terms of precedent I really dislike that option, i.e. git having some > mode where I need to work to escape input to pass to another command. > > Not saying that this --range-diff-* thing is what we should go for, but > surely we can find some way to do deal with this that doesn't involve > the user needing to escape stuff like this. > > It also has other downstream effects in the UI, e.g. it's presumably > easy to teach the bash completion that a --foo=XYZ option is also called > --some-prefix--foo=XYZ and to enable completion for that, less so for > making it smart enough to complete "--some-prefix-opts="--foo=". These are all good points, and need proper discussion. Sadly, all that time needed for a proper discussion is not left before v2.20.0 is supposed to come out. Quite honestly, I think what we will have to do is to describe in the documentation of `format-patch`'s `--range-diff` option that the exact user interface how to pass diff options down to `range-diff` is in flux and not final. That way, we can give your design the proper treatment, and work together on making a user interface we all can be happy with. Ciao, Dscho
Re: [PATCH v2 6/7] checkout: split into switch-branch and checkout-files
On Thu, Nov 29, 2018 at 7:14 PM Stefan Beller wrote: > > > > Idea: > > > If git checkout-files modifies the submodules file, it could also > > > auto-update the submodules. (For example, with something like "git > > > submodule update --init --recursive --progress"). > > > > This one is tricky because we should deal with submodule autoupdate > > consistently across all porcelain commands (or at least common ones), > > not just checkout-files. I'd prefer to delay this until later. Once we > > figure out what to do with other commands, then we can still change > > defaults for checkout-files. > > checkout/reset are respecting the submodule.recurse setting for this > already, and as your patches only change the UX frontend > > git -c submodule.recurse checkout-files > > would also touch submodules. Given that deep down in > the submodules it's all about files again, we could think > checkout-files is still a good name. > > I think Stefan X. is asking for making submodule.recurse > to default to true, which is indeed unrelated to this. Yes and I'm concerned that checkout-files now recurses into submodules this by default but grep for example does not. That just adds more confusion. -- Duy
Re: [PATCH v2 6/7] checkout: split into switch-branch and checkout-files
> > Idea: > > If git checkout-files modifies the submodules file, it could also > > auto-update the submodules. (For example, with something like "git > > submodule update --init --recursive --progress"). > > This one is tricky because we should deal with submodule autoupdate > consistently across all porcelain commands (or at least common ones), > not just checkout-files. I'd prefer to delay this until later. Once we > figure out what to do with other commands, then we can still change > defaults for checkout-files. checkout/reset are respecting the submodule.recurse setting for this already, and as your patches only change the UX frontend git -c submodule.recurse checkout-files would also touch submodules. Given that deep down in the submodules it's all about files again, we could think checkout-files is still a good name. I think Stefan X. is asking for making submodule.recurse to default to true, which is indeed unrelated to this. Stefan
Security Alert. git@vger.kernel.org has password ani420. Password must be changed.
Hello! I have very bad news for you. 09/08/2018 - on this day I hacked your OS and got full access to your account git@vger.kernel.org On this day your account git@vger.kernel.org has password: ani420 So, you can change the password, yes.. But my malware intercepts it every time. How I made it: In the software of the router, through which you went online, was a vulnerability. I just hacked this router and placed my malicious code on it. When you went online, my trojan was installed on the OS of your device. After that, I made a full dump of your disk (I have all your address book, history of viewing sites, all files, phone numbers and addresses of all your contacts). A month ago, I wanted to lock your device and ask for a not big amount of btc to unlock. But I looked at the sites that you regularly visit, and I was shocked by what I saw!!! I'm talk you about sites for adults. I want to say - you are a BIG pervert. Your fantasy is shifted far away from the normal course! And I got an idea I made a screenshot of the adult sites where you have fun (do you understand what it is about, huh?). After that, I made a screenshot of your joys (using the camera of your device) and glued them together. Turned out amazing! You are so spectacular! I'm know that you would not like to show these screenshots to your friends, relatives or colleagues. I think $756 is a very, very small amount for my silence. Besides, I have been spying on you for so long, having spent a lot of time! Pay ONLY in Bitcoins! My BTC wallet: 1HkKgPbcMyfhrdPsbufTFczzVnhyT5snB3 You do not know how to use bitcoins? Enter a query in any search engine: "how to replenish btc wallet". It's extremely easy For this payment I give you two days (48 hours). As soon as this letter is opened, the timer will work. After payment, my virus and dirty screenshots with your enjoys will be self-destruct automatically. If I do not receive from you the specified amount, then your device will be locked, and all your contacts will receive a screenshots with your "enjoys". I hope you understand your situation. - Do not try to find and destroy my virus! (All your data, files and screenshots is already uploaded to a remote server) - Do not try to contact me (you yourself will see that this is impossible, the sender address is automatically generated) - Various security services will not help you; formatting a disk or destroying a device will not help, since your data is already on a remote server. P.S. You are not my single victim. so, I guarantee you that I will not disturb you again after payment! This is the word of honor hacker I also ask you to regularly update your antiviruses in the future. This way you will no longer fall into a similar situation. Do not hold evil! I just do my job. Good luck.
Re: [PATCH] rebase: mark the C reimplementation as an experimental opt-in feature (was Re: [ANNOUNCE] Git v2.20.0-rc1)
Johannes Schindelin writes ("Re: [PATCH] rebase: mark the C reimplementation as
an experimental opt-in feature (was Re: [ANNOUNCE] Git v2.20.0-rc1)"):
> I'll have to take a (lengthy) dinner break now, but this is what I have so
> far: a regression test that verifies the breakage (see the
> `fix-reflog-action` branch at https://github.com/dscho/git). I'll continue
> after dinner and am confident that this bug will be fixed within the next
> four hours.
That seems super speedy to me!
When you have a fix I will leave it up to the Debian git maintainers
to decide whether they want to cherry pick your fix into their
package, or await an updated upstream branch with rc, or what.
> [Ian:]
> > While you're looking at this, I observe that the fact that the `rebase
> > finished' message also does not honour GIT_REFLOG_ACTION appears to be
> > a pre-existing bug.
>
> I noticed that, too, but at this point I am only fixing regressions. We
> can try to fix this long-standing bug in the v2.20 cycle.
Right.
Thanks,
Ian.
--
Ian JacksonThese opinions are my own.
If I emailed you from an address @fyvzl.net or @evade.org.uk, that is
a private address which bypasses my fierce spamfilter.
Re: [PATCH] rebase: mark the C reimplementation as an experimental opt-in feature (was Re: [ANNOUNCE] Git v2.20.0-rc1)
Hi Ian,
On Thu, 29 Nov 2018, Ian Jackson wrote:
> Johannes Schindelin writes ("Re: [PATCH] rebase: mark the C reimplementation
> as an experimental opt-in feature (was Re: [ANNOUNCE] Git v2.20.0-rc1)"):
> > > In a successful run with older git I get a reflog like this:
> > >
> > >4833d74 HEAD@{0}: rebase finished: returning to
> > > refs/heads/with-preexisting
> > >4833d74 HEAD@{1}: debrebase new-upstream 2.1-1: rebase: Add another
> > > new upstream file
> > >cabd5ec HEAD@{2}: debrebase new-upstream 2.1-1: rebase: Edit the .c
> > > file
> > >0b362ce HEAD@{3}: debrebase new-upstream 2.1-1: rebase: Add a new
> > > upstream file
> > >29653e5 HEAD@{4}: debrebase new-upstream 2.1-1: rebase: checkout
> > > 29653e5a17bee4ac23a68bba3e12bc1f52858ac3
> > >85e0c46 HEAD@{5}: debrebase: launder for new upstream
> ...
> > > This breaks the test because my test suite is checking that I set
> > > GIT_REFLOG_ACTION appropriately.
> > >
> > > If you want I can provide a minimal test case but this should suffice
> > > to see the bug I hope...
> >
> > This should be plenty for me to get going. Thank you!
>
> Happy hunting.
I'll have to take a (lengthy) dinner break now, but this is what I have so
far: a regression test that verifies the breakage (see the
`fix-reflog-action` branch at https://github.com/dscho/git). I'll continue
after dinner and am confident that this bug will be fixed within the next
four hours.
> While you're looking at this, I observe that the fact that the `rebase
> finished' message also does not honour GIT_REFLOG_ACTION appears to be
> a pre-existing bug.
I noticed that, too, but at this point I am only fixing regressions. We
can try to fix this long-standing bug in the v2.20 cycle.
Ciao,
Johannes
> (In general one often can't rely on GIT_REFLOG_ACTION still being set
> because the rebase might have been interrupted and restarted, which I
> think is why my test case looks for it in the initial `checkout'
> message.)
>
> Regards,
> Ian.
>
> --
> Ian JacksonThese opinions are my own.
>
> If I emailed you from an address @fyvzl.net or @evade.org.uk, that is
> a private address which bypasses my fierce spamfilter.
>
Re: [PATCH] pack-protocol.txt: accept error packets in any context
On Thu, Nov 29, 2018 at 3:42 AM Junio C Hamano wrote: > > Masaya Suzuki writes: > > > In the Git pack protocol definition, an error packet may appear only in > > a certain context. However, servers can face a runtime error (e.g. I/O > > error) at an arbitrary timing. This patch changes the protocol to allow > > an error packet to be sent instead of any packet. > > > > Following this protocol spec change, the error packet handling code is > > moved to pkt-line.c. > > > > Signed-off-by: Masaya Suzuki > > --- > > Have you run tests with this applied? I noticed 5703.9 now has > stale expectations, but there may be others. Yes, I did. And it also didn't end up in a build error. Do I have a different build option...?
Re: [PATCH 2/2] format-patch: allow for independent diff & range-diff options
On Thu, Nov 29 2018, Johannes Schindelin wrote:
> Hi Ævar,
>
> On Thu, 29 Nov 2018, Ævar Arnfjörð Bjarmason wrote:
>
>> On Thu, Nov 29 2018, Johannes Schindelin wrote:
>>
>> > On Thu, 29 Nov 2018, Ævar Arnfjörð Bjarmason wrote:
>> >
>> >> On Thu, Nov 29 2018, Johannes Schindelin wrote:
>> >>
>> >> > On Wed, 28 Nov 2018, Ævar Arnfjörð Bjarmason wrote:
>> >> >
>> >> >> Change the semantics of the "--range-diff" option so that the regular
>> >> >> diff options can be provided separately for the range-diff and the
>> >> >> patch. This allows for supplying e.g. --range-diff-U0 and -U1 to
>> >> >> "format-patch" to provide different context for the range-diff and the
>> >> >> patch. This wasn't possible before.
>> >> >
>> >> > I really, really dislike the `--range-diff-`. We have
>> >> > precedent for passing optional arguments that are passed to some other
>> >> > command, so a much more logical and consistent convention would be to
>> >> > use
>> >> > `--range-diff[=..]`, allowing all of the diff options that
>> >> > you might want to pass to the outer diff in one go rather than having a
>> >> > lengthy string of `--range-diff-this` and `--range-diff-that` options.
>> >>
>> >> Where do we pass those sorts of arguments?
>> >>
>> >> Reasons I did it this way:
>> >>
>> >> a) Passing it as one option will require the user to double-quote those
>> >> options that take quoted arguments (e.g. --word-diff-regex), which I
>> >> thought sucked more than the prefix. On the implementation side we
>> >> couldn't leave the parsing of the command-line to the shell anymore.
>> >>
>> >> b) I think people will want to tweak this very rarely, much more rarely
>> >> than e.g. -U10 in format-patch itself, so having something long-ish
>> >> doesn't sound bad.
>> >
>> > Hmm. I still don't like it. It sets a precedent, and we simply do not do
>> > it that way in other circumstances (most obvious would be the -X merge
>> > options). The more divergent user interfaces for the same sort of thing
>> > are, the more brain cycles you force users to spend on navigating said
>> > interfaces.
>>
>> Yeah it sucks, I just think it sucks less than the alternative :)
>> I.e. I'm not picky about --range-diff-* prefix the name, but I think
>> doing our own shell parsing would be nasty.
>
> What prevents you from using `sq_dequote_to_argv()`?
I mean not just nasty in terms of implementation, yeah we could do it,
but also a nasty UX for things like --word-diff-regex. I.e. instead of:
--range-diff-word-diff-regex='[0-9"]'
You need:
--range-diff-opts="--word-diff-regex='[0-9\"]'"
Now admittedly that in itself isn't very painful *in this case*, but in
terms of precedent I really dislike that option, i.e. git having some
mode where I need to work to escape input to pass to another command.
Not saying that this --range-diff-* thing is what we should go for, but
surely we can find some way to do deal with this that doesn't involve
the user needing to escape stuff like this.
It also has other downstream effects in the UI, e.g. it's presumably
easy to teach the bash completion that a --foo=XYZ option is also called
--some-prefix--foo=XYZ and to enable completion for that, less so for
making it smart enough to complete "--some-prefix-opts="--foo=".
>> >> > I only had time to skim the patch, and I have to wonder why you pass
>> >> > around full-blown `rev_info` structs for range diff (and with that
>> >> > really
>> >> > awful name `rd_rev`) rather than just the `diff_options` that you
>> >> > *actually* care about?
>> >>
>> >> Because setup_revisions() which does all the command-line parsing needs
>> >> a rev_info, so even if we only need the diffopt in the end we need to
>> >> initiate the whole thing.
>> >>
>> >> Suggestions for a better varibale name most welcome.
>> >
>> > `range_diff_revs`
>> >
>> > And you do not need to pass around the whole thing. You can easily pass
>> > `_diff_revs.diffopt`.
>> >
>> > Don't pass around what you do not need to pass around.
>>
>> Ah, you mean internally in log.c, yes that makes sense. I thought you
>> meant just pass diffopt to setup_revisions() (which needs the containing
>> struct). Willdo.
>
> Thanks,
> Dscho
>
>>
>> > Ciao,
>> > Dscho
>> >
>> >>
>> >> > Ciao,
>> >> > Dscho
>> >> >
>> >> >>
>> >> >> Ever since the "--range-diff" option was added in
>> >> >> 31e2617a5f ("format-patch: add --range-diff option to embed diff in
>> >> >> cover letter", 2018-07-22) the "rev->diffopt" we pass down to the diff
>> >> >> machinery has been the one we get from "format-patch"'s own
>> >> >> setup_revisions().
>> >> >>
>> >> >> This sort of thing is unique among the log-like commands in
>> >> >> builtin/log.c, no command than format-patch will embed the output of
>> >> >> another log-like command. Since the "rev->diffopt" is reused we need
>> >> >> to munge it before we pass it to show_range_diff(). See
>> >> >> 43dafc4172 ("format-patch: don't include --stat with --range-diff
>> >>
Re: [PATCH] rebase: mark the C reimplementation as an experimental opt-in feature (was Re: [ANNOUNCE] Git v2.20.0-rc1)
Johannes Schindelin writes ("Re: [PATCH] rebase: mark the C reimplementation as
an experimental opt-in feature (was Re: [ANNOUNCE] Git v2.20.0-rc1)"):
> > In a successful run with older git I get a reflog like this:
> >
> >4833d74 HEAD@{0}: rebase finished: returning to
> > refs/heads/with-preexisting
> >4833d74 HEAD@{1}: debrebase new-upstream 2.1-1: rebase: Add another new
> > upstream file
> >cabd5ec HEAD@{2}: debrebase new-upstream 2.1-1: rebase: Edit the .c file
> >0b362ce HEAD@{3}: debrebase new-upstream 2.1-1: rebase: Add a new
> > upstream file
> >29653e5 HEAD@{4}: debrebase new-upstream 2.1-1: rebase: checkout
> > 29653e5a17bee4ac23a68bba3e12bc1f52858ac3
> >85e0c46 HEAD@{5}: debrebase: launder for new upstream
...
> > This breaks the test because my test suite is checking that I set
> > GIT_REFLOG_ACTION appropriately.
> >
> > If you want I can provide a minimal test case but this should suffice
> > to see the bug I hope...
>
> This should be plenty for me to get going. Thank you!
Happy hunting.
While you're looking at this, I observe that the fact that the `rebase
finished' message also does not honour GIT_REFLOG_ACTION appears to be
a pre-existing bug.
(In general one often can't rely on GIT_REFLOG_ACTION still being set
because the rebase might have been interrupted and restarted, which I
think is why my test case looks for it in the initial `checkout'
message.)
Regards,
Ian.
--
Ian JacksonThese opinions are my own.
If I emailed you from an address @fyvzl.net or @evade.org.uk, that is
a private address which bypasses my fierce spamfilter.
Re: [PATCH v2 6/7] checkout: split into switch-branch and checkout-files
On Thu, Nov 29, 2018 at 12:22 AM Stefan Xenos wrote:
> Some behaviors I'd expect to see from these commands (I haven't yet
> checked to see if you've already done this):
>
> git checkout-files
> should reset all the files in the repository regardless of the current
> directory - it should produce the same effect as "git reset --hard
> && git reset HEAD@{1}". It should also delete
> locally-created files that aren't present in , such that the
> final working tree is exactly identical to what was committed in that
> tree-ish.
>
> git checkout-files foo -- myfile.txt
> should delete myfile.txt if it is present locally but not present in foo.
>
> git checkout-files foo -- .
> should recursively checkout all files in the current folder and all
> subfolders, and delete any locally-created files if they're not
> present in foo.
I think all these are the same as the non-overlay mode Thomas
mentioned [1]. Once he implements that in git-checkout, we can make it
default in checkout-files.
Two things though. I plan to get rid of "--" in checkout-files. The
main use case (I think) is reset from index, so you can just write
git checkout-files somefiles
and if you want to get it from the tree(-ish) "foo", you do
git checkout-files --from=foo somefiles
This form is easier to read (and even guess before you read man pages)
and leaves no room for ambiguation.
The second thing is, I plan to forbid "git checkout-files" without
arguments. If you want to reset the entire worktree, do
git checkout-files :/
or just current dir
git checkout-files .
Which brings us back to your "git checkout-files " use case
above. It should be treat the same way in my opinion, so we either do
git checkout-files --from=tree-ish :/
or
git checkout-files --from=tree-ish .
But "git checkout-files --from=tree-ish" alone is rejected.
[1]
https://public-inbox.org/git/xmqqwoowo1fk@gitster-ct.c.googlers.com/T/#mdb076d178ccf0ae3dba0fd63143f99278047da93
> Suggestion:
> If git checkout-files overwrites or deletes any locally-modified files
> from the workspace or index, those files could be auto-stashed. That
> would make it easy to restore them in the event of a mistyped command.
> Auto-stashing could be suppressed with a command-line argument (with
> alternate behaviors being fail-if-modified or always-overwrite).
Stashing I think is not the right tool for this. When you stash, you
plan to retrieve it back later but here you should rarely ever need to
unstash until the accident. For recovery from accidents like this, I
have another thing in queue to achieve the same (I'm calling it
"backup log" now). So we will have the same functionality, just with
different means.
> Idea:
> If git checkout-files modifies the submodules file, it could also
> auto-update the submodules. (For example, with something like "git
> submodule update --init --recursive --progress").
This one is tricky because we should deal with submodule autoupdate
consistently across all porcelain commands (or at least common ones),
not just checkout-files. I'd prefer to delay this until later. Once we
figure out what to do with other commands, then we can still change
defaults for checkout-files.
--
Duy
Re: [PATCH 2/2] format-patch: allow for independent diff & range-diff options
Hi Ævar,
On Thu, 29 Nov 2018, Ævar Arnfjörð Bjarmason wrote:
> On Thu, Nov 29 2018, Johannes Schindelin wrote:
>
> > On Thu, 29 Nov 2018, Ævar Arnfjörð Bjarmason wrote:
> >
> >> On Thu, Nov 29 2018, Johannes Schindelin wrote:
> >>
> >> > On Wed, 28 Nov 2018, Ævar Arnfjörð Bjarmason wrote:
> >> >
> >> >> Change the semantics of the "--range-diff" option so that the regular
> >> >> diff options can be provided separately for the range-diff and the
> >> >> patch. This allows for supplying e.g. --range-diff-U0 and -U1 to
> >> >> "format-patch" to provide different context for the range-diff and the
> >> >> patch. This wasn't possible before.
> >> >
> >> > I really, really dislike the `--range-diff-`. We have
> >> > precedent for passing optional arguments that are passed to some other
> >> > command, so a much more logical and consistent convention would be to use
> >> > `--range-diff[=..]`, allowing all of the diff options that
> >> > you might want to pass to the outer diff in one go rather than having a
> >> > lengthy string of `--range-diff-this` and `--range-diff-that` options.
> >>
> >> Where do we pass those sorts of arguments?
> >>
> >> Reasons I did it this way:
> >>
> >> a) Passing it as one option will require the user to double-quote those
> >> options that take quoted arguments (e.g. --word-diff-regex), which I
> >> thought sucked more than the prefix. On the implementation side we
> >> couldn't leave the parsing of the command-line to the shell anymore.
> >>
> >> b) I think people will want to tweak this very rarely, much more rarely
> >> than e.g. -U10 in format-patch itself, so having something long-ish
> >> doesn't sound bad.
> >
> > Hmm. I still don't like it. It sets a precedent, and we simply do not do
> > it that way in other circumstances (most obvious would be the -X merge
> > options). The more divergent user interfaces for the same sort of thing
> > are, the more brain cycles you force users to spend on navigating said
> > interfaces.
>
> Yeah it sucks, I just think it sucks less than the alternative :)
> I.e. I'm not picky about --range-diff-* prefix the name, but I think
> doing our own shell parsing would be nasty.
What prevents you from using `sq_dequote_to_argv()`?
> >> > I only had time to skim the patch, and I have to wonder why you pass
> >> > around full-blown `rev_info` structs for range diff (and with that really
> >> > awful name `rd_rev`) rather than just the `diff_options` that you
> >> > *actually* care about?
> >>
> >> Because setup_revisions() which does all the command-line parsing needs
> >> a rev_info, so even if we only need the diffopt in the end we need to
> >> initiate the whole thing.
> >>
> >> Suggestions for a better varibale name most welcome.
> >
> > `range_diff_revs`
> >
> > And you do not need to pass around the whole thing. You can easily pass
> > `_diff_revs.diffopt`.
> >
> > Don't pass around what you do not need to pass around.
>
> Ah, you mean internally in log.c, yes that makes sense. I thought you
> meant just pass diffopt to setup_revisions() (which needs the containing
> struct). Willdo.
Thanks,
Dscho
>
> > Ciao,
> > Dscho
> >
> >>
> >> > Ciao,
> >> > Dscho
> >> >
> >> >>
> >> >> Ever since the "--range-diff" option was added in
> >> >> 31e2617a5f ("format-patch: add --range-diff option to embed diff in
> >> >> cover letter", 2018-07-22) the "rev->diffopt" we pass down to the diff
> >> >> machinery has been the one we get from "format-patch"'s own
> >> >> setup_revisions().
> >> >>
> >> >> This sort of thing is unique among the log-like commands in
> >> >> builtin/log.c, no command than format-patch will embed the output of
> >> >> another log-like command. Since the "rev->diffopt" is reused we need
> >> >> to munge it before we pass it to show_range_diff(). See
> >> >> 43dafc4172 ("format-patch: don't include --stat with --range-diff
> >> >> output", 2018-11-22) for a related regression fix which is being
> >> >> mostly reverted here.
> >> >>
> >> >> Implementation notes: 1) We're not bothering with the full teardown
> >> >> around die() and will leak memory, but it's too much boilerplate to do
> >> >> all the frees with/without the die() and not worth it. 2) We call
> >> >> repo_init_revisions() for "rd_rev" even though we could get away with
> >> >> a shallow copy like the code we're replacing (and which
> >> >> show_range_diff() itself does). This is to make this code more easily
> >> >> understood.
> >> >>
> >> >> Signed-off-by: Ævar Arnfjörð Bjarmason
> >> >> ---
> >> >> Documentation/git-format-patch.txt | 10 ++-
> >> >> builtin/log.c | 42 +++---
> >> >> t/t3206-range-diff.sh | 41 +
> >> >> 3 files changed, 82 insertions(+), 11 deletions(-)
> >> >>
> >> >> diff --git a/Documentation/git-format-patch.txt
> >> >> b/Documentation/git-format-patch.txt
> >> >> index aba4c5febe..6c048f415f 100644
> >> >> ---
Re: [PATCH] rebase: mark the C reimplementation as an experimental opt-in feature (was Re: [ANNOUNCE] Git v2.20.0-rc1)
Hi Ian,
On Thu, 29 Nov 2018, Ian Jackson wrote:
> Johannes Schindelin writes ("Re: [PATCH] rebase: mark the C reimplementation
> as an experimental opt-in feature (was Re: [ANNOUNCE] Git v2.20.0-rc1)"):
> > if you could pry more information (or better information) out of that bug
> > reporter, that would be nice. Apparently my email address is blacklisted
> > by his mail provider, so he is unlikely to have received my previous mail
> > (nor will he receive this one, I am sure).
>
> (I did receive this mail. Sorry for the inconvenience, which sadly is
> inevitable occasionally in the modern email world. FTR in future feel
> free to send the bounce to postmaster@chiark and I will make a
> you-shaped hole in my spamfilter. Also with Debian bugs you can
> launder your messages by, eg, emailing 914695-submitter@bugs.)
Right. I myself have plenty of email-related problems that seem to crop up
this year in particular.
> > > > At https://bugs.debian.org/914695 is a report of a test regression in
> > > > an outside project that is very likely to have been triggered by the
> > > > new faster rebase code.
>
> As I wrote in the bug report last night:
>
> https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=914695#15
>
> I have investigated and the bug seems to be that git-rebase --onto now
> fails to honour GIT_REFLOG_ACTION for the initial checkout.
>
> In a successful run with older git I get a reflog like this:
>
>4833d74 HEAD@{0}: rebase finished: returning to refs/heads/with-preexisting
>4833d74 HEAD@{1}: debrebase new-upstream 2.1-1: rebase: Add another new
> upstream file
>cabd5ec HEAD@{2}: debrebase new-upstream 2.1-1: rebase: Edit the .c file
>0b362ce HEAD@{3}: debrebase new-upstream 2.1-1: rebase: Add a new upstream
> file
>29653e5 HEAD@{4}: debrebase new-upstream 2.1-1: rebase: checkout
> 29653e5a17bee4ac23a68bba3e12bc1f52858ac3
>85e0c46 HEAD@{5}: debrebase: launder for new upstream
>
> With a newer git I get this:
>
>6d3fb91 HEAD@{0}: rebase finished: returning to refs/heads/master
>6d3fb91 HEAD@{1}: debrebase new-upstream 2.1-1: rebase: Add another new
> upstream file
>86c0721 HEAD@{2}: debrebase new-upstream 2.1-1: rebase: Edit the .c file
>50ba56c HEAD@{3}: debrebase new-upstream 2.1-1: rebase: Add a new upstream
> file
>8272825 HEAD@{4}: rebase: checkout 8272825bb4ff6eba89afa936e32b6460f963a36a
>c78db55 HEAD@{5}: debrebase: launder for new upstream
>
> This breaks the test because my test suite is checking that I set
> GIT_REFLOG_ACTION appropriately.
>
> If you want I can provide a minimal test case but this should suffice
> to see the bug I hope...
This should be plenty for me to get going. Thank you!
Ciao,
Johannes
>
> Regards
> Ian.
>
> --
> Ian JacksonThese opinions are my own.
>
> If I emailed you from an address @fyvzl.net or @evade.org.uk, that is
> a private address which bypasses my fierce spamfilter.
>
Re: Simple git push --tags deleted all branches
On Thu, Nov 29 2018, Mateusz Loskot wrote: > On Thu, 29 Nov 2018 at 16:03, Ævar Arnfjörð Bjarmason > wrote: >> On Wed, Nov 28 2018, Mateusz Loskot wrote: >> > >> > (using git version 2.19.2.windows.1) >> > >> > I've just encountered one of those WTH moments. >> > >> > I have a bare repository >> > >> > core.git (BARE:master) $ git branch >> > 1.0 >> > 2.0 >> > * master >> > >> > core.git (BARE:master) $ git tag >> > 1.0.1651 >> > 1.0.766 >> > 2.0.1103 >> > 2.0.1200 >> > >> > I published the repo using: git push --all --follow-tags >> > >> > This succeeded, but there seem to be no tags pushed, just branches. >> > So, I followed with >> > >> > core.git (BARE:master) $ git push --tags >> > To XXX >> > - [deleted] 1.0 >> > - [deleted] 2.0 >> > ! [remote rejected] master (refusing to delete the current >> > branch: refs/heads/master) >> > error: failed to push some refs to 'XXX' >> > >> > And, I've found out that all branches and tags have been >> > wiped in both, local repo and remote :) >> > >> > I restored the repo and tried out >> > >> > git push origin 1.0 >> > git push origin --tags >> > >> > and this time both succeeded, without wiping out any refs. >> > >> > Could anyone help me to understand why remote-less >> > >> > git push --tags >> > >> > is/was so dangerous and unforgiving?! >> >> Since nobody's replied yet, I can't see what's going on here from the >> info you've provided. My guess is that you have something "mirror" set >> on the remote. > > Thank you for responding. > > The git push --tags hugely surprised me, and here is genuine screenshot > https://twitter.com/mloskot/status/1068072285846859776 > >> It seems you can't share the repo or its URL, but could you share the >> scrubbed output of 'git config -l --show-origin' when run inside this >> repository? > > Here is complete output. I have not stripped the basics like aliases, > just in case. Right, it's because you used --mirror, the important bit: > file:config remote.origin.url=https://xxx.com/core-external-metadata.git > file:config remote.origin.fetch=+refs/*:refs/* > file:config remote.origin.mirror=true > file:config I.e. you have cloned with the --mirror flag, this is what it's supposed to do: https://git-scm.com/docs/git-clone#git-clone---mirror https://git-scm.com/docs/git-fetch#git-fetch---prune I.e. you push and git tries to mirror the refs you have locally to the remote, including pruning stuff in the remote. This is useful, but not what you wanted here. It's used for e.g. making an up-to-date copy of a repo from server A to server B in HA setups where you'd like to fail over to server B and get the same refs you had on A.
Re: [PATCH v1] mem_pool: add GIT_TRACE_MEMPOOL support
Hi Ben,
On Thu, 29 Nov 2018, Ben Peart wrote:
> On 11/28/2018 4:37 AM, Johannes Schindelin wrote:
> > Hi Ben,
> >
> > On Tue, 27 Nov 2018, Ben Peart wrote:
> >
> > > From: Ben Peart
> > >
> > > Add tracing around initializing and discarding mempools. In discard report
> > > on the amount of memory unused in the current block to help tune setting
> > > the initial_size.
> > >
> > > Signed-off-by: Ben Peart
> > > ---
> > Looks good.
> >
> > My only question: should we also trace calls to _alloc(), _calloc() and
> > _combine()?
>
> I was trying to tune the initial size in my use of the mem_pool and so found
> this tracing useful to see how much memory was actually being used. I'm
> inclined to only add tracing as it is needed rather that proactively because
> we think it _might_ be needed. I suspect _alloc() and _calloc() would get
> very noisy and not add much value.
In other words, YAGNI. Makes sense.
Thanks,
Johannes
>
> >
> > Ciao,
> > Johannes
> >
> > > Notes:
> > > Base Ref: * git-trace-mempool
> > > Web-Diff: https://github.com/benpeart/git/commit/9ac84bbca2
> > > Checkout: git fetch https://github.com/benpeart/git
> > > git-trace-mempool-v1 && git checkout 9ac84bbca2
> > >
> > > mem-pool.c | 5 +
> > > 1 file changed, 5 insertions(+)
> > >
> > > diff --git a/mem-pool.c b/mem-pool.c
> > > index a2841a4a9a..065389aaec 100644
> > > --- a/mem-pool.c
> > > +++ b/mem-pool.c
> > > @@ -5,6 +5,7 @@
> > > #include "cache.h"
> > > #include "mem-pool.h"
> > >
> > > +static struct trace_key trace_mem_pool = TRACE_KEY_INIT(MEMPOOL);
> > > #define BLOCK_GROWTH_SIZE 1024*1024 - sizeof(struct mp_block);
> > >
> > > /*
> > > @@ -48,12 +49,16 @@ void mem_pool_init(struct mem_pool **mem_pool, size_t
> > > initial_size)
> > > mem_pool_alloc_block(pool, initial_size, NULL);
> > >
> > > *mem_pool = pool;
> > > + trace_printf_key(_mem_pool, "mem_pool (%p): init (%"PRIuMAX")
> > > initial size\n",
> > > + pool, (uintmax_t)initial_size);
> > > }
> > >
> > > void mem_pool_discard(struct mem_pool *mem_pool, int invalidate_memory)
> > > {
> > >struct mp_block *block, *block_to_free;
> > > + trace_printf_key(_mem_pool, "mem_pool (%p): discard
> > > (%"PRIuMAX")
> > > unused\n",
> > > + mem_pool, (uintmax_t)(mem_pool->mp_block->end -
> > > mem_pool->mp_block->next_free));
> > >block = mem_pool->mp_block;
> > >while (block)
> > >{
> > >
> > > base-commit: bb75be6cb916297f271c846f2f9caa3daaaec718
> > > --
> > > 2.18.0.windows.1
> > >
> > >
>
>
Re: [PATCH v2 6/7] checkout: split into switch-branch and checkout-files
On Thu, Nov 29, 2018 at 6:59 AM Junio C Hamano wrote: > > Stefan Xenos writes: > > > Although I have no problem with "switch-branch" as a command name, > > some alternative names we might consider for switch-branch might be: > > > > chbranch > > swbranch > > Please never go in that direction. So far, we made a conscious > effort to keep the names of most frequently used subcommand to > proper words that can be understood by coders (IOW, I expect they > know what 'grep' is, even though that may not be a 'proper word'). > > > switch > > branch change (as a subcommand for the "branch" command) > > It is more like moving to the branch to work on. I think 'switch' > is something SVN veterans may find familiar. Both are much better > than ch/swbranch that are overly cryptic. OK I'll go with switch-branch and restore-files in the next round. And perhaps consider just 'switch' and 'restore' later. -- Duy
Re: [PATCH/RFC v2 0/7] Introduce new commands switch-branch and checkout-files
On Wed, Nov 28, 2018 at 9:30 PM Stefan Beller wrote: > > On Wed, Nov 28, 2018 at 12:09 PM Duy Nguyen wrote: > > > > On Wed, Nov 28, 2018 at 9:01 PM Duy Nguyen wrote: > > > should we do > > > something about detached HEAD in this switch-branch command (or > > > whatever its name will be)? > > > > > > This is usually a confusing concept to new users > > > > And it just occurred to me that perhaps we should call this "unnamed > > branch" (at least at high UI level) instead of detached HEAD. It is > > technically not as accurate, but much better to understand. > > or 'direct' branch? makes me think, what is an indirect branch? > I mean 'detached HEAD' itself is also not correct > as the HEAD points to a valid commit/tag usually, so it is attached to > that content. The detachment comes from the implicit "from a branch". Yeah I guess it's short for "HEAD that is detached from a branch" -- Duy
Re: Simple git push --tags deleted all branches
On Thu, 29 Nov 2018 at 16:03, Ævar Arnfjörð Bjarmason wrote: > On Wed, Nov 28 2018, Mateusz Loskot wrote: > > > > (using git version 2.19.2.windows.1) > > > > I've just encountered one of those WTH moments. > > > > I have a bare repository > > > > core.git (BARE:master) $ git branch > > 1.0 > > 2.0 > > * master > > > > core.git (BARE:master) $ git tag > > 1.0.1651 > > 1.0.766 > > 2.0.1103 > > 2.0.1200 > > > > I published the repo using: git push --all --follow-tags > > > > This succeeded, but there seem to be no tags pushed, just branches. > > So, I followed with > > > > core.git (BARE:master) $ git push --tags > > To XXX > > - [deleted] 1.0 > > - [deleted] 2.0 > > ! [remote rejected] master (refusing to delete the current > > branch: refs/heads/master) > > error: failed to push some refs to 'XXX' > > > > And, I've found out that all branches and tags have been > > wiped in both, local repo and remote :) > > > > I restored the repo and tried out > > > > git push origin 1.0 > > git push origin --tags > > > > and this time both succeeded, without wiping out any refs. > > > > Could anyone help me to understand why remote-less > > > > git push --tags > > > > is/was so dangerous and unforgiving?! > > Since nobody's replied yet, I can't see what's going on here from the > info you've provided. My guess is that you have something "mirror" set > on the remote. Thank you for responding. The git push --tags hugely surprised me, and here is genuine screenshot https://twitter.com/mloskot/status/1068072285846859776 > It seems you can't share the repo or its URL, but could you share the > scrubbed output of 'git config -l --show-origin' when run inside this > repository? Here is complete output. I have not stripped the basics like aliases, just in case. ``` core-external-metadata.git (BARE:master) $ git config -l --show-origin file:"C:\\ProgramData/Git/config" core.symlinks=false file:"C:\\ProgramData/Git/config" core.autocrlf=true file:"C:\\ProgramData/Git/config" color.diff=auto file:"C:\\ProgramData/Git/config" color.status=auto file:"C:\\ProgramData/Git/config" color.branch=auto file:"C:\\ProgramData/Git/config" color.interactive=true file:"C:\\ProgramData/Git/config" help.format=html file:"C:\\ProgramData/Git/config" http.sslcainfo=C:/Program Files/Git/mingw64/ssl/certs/ca-bundle.crt file:"C:\\ProgramData/Git/config" diff.astextplain.textconv=astextplain file:"C:\\ProgramData/Git/config" rebase.autosquash=true file:C:/Program Files/Git/mingw64/etc/gitconfig http.sslcainfo=C:/Program Files/Git/mingw64/ssl/certs/ca-bundle.crt file:C:/Program Files/Git/mingw64/etc/gitconfig http.sslbackend=openssl file:C:/Program Files/Git/mingw64/etc/gitconfig diff.astextplain.textconv=astextplain file:C:/Program Files/Git/mingw64/etc/gitconfig filter.lfs.clean=git-lfs clean -- %f file:C:/Program Files/Git/mingw64/etc/gitconfig filter.lfs.smudge=git-lfs smudge -- %f file:C:/Program Files/Git/mingw64/etc/gitconfig filter.lfs.process=git-lfs filter-process file:C:/Program Files/Git/mingw64/etc/gitconfig filter.lfs.required=true file:C:/Program Files/Git/mingw64/etc/gitconfig credential.helper=manager file:C:/Users/mateuszl/.gitconfig user.name=Mateusz Łoskot file:C:/Users/mateuszl/.gitconfig user.email=mate...@loskot.net file:C:/Users/mateuszl/.gitconfig github.user=mloskot file:C:/Users/mateuszl/.gitconfig core.editor=vim file:C:/Users/mateuszl/.gitconfig color.branch=auto file:C:/Users/mateuszl/.gitconfig color.diff=auto file:C:/Users/mateuszl/.gitconfig color.status=auto file:C:/Users/mateuszl/.gitconfig color.ui=auto file:C:/Users/mateuszl/.gitconfig alias.br=branch file:C:/Users/mateuszl/.gitconfig alias.bv=branch -vv file:C:/Users/mateuszl/.gitconfig alias.brv=branch -vv file:C:/Users/mateuszl/.gitconfig alias.bra=branch -a file:C:/Users/mateuszl/.gitconfig alias.ci=commit --verbose file:C:/Users/mateuszl/.gitconfig alias.cia=commit --verbose --amend file:C:/Users/mateuszl/.gitconfig alias.ciae=commit --verbose --amend --no-edit file:C:/Users/mateuszl/.gitconfig alias.co=checkout file:C:/Users/mateuszl/.gitconfig alias.dc=diff --cached file:C:/Users/mateuszl/.gitconfig alias.df=diff file:C:/Users/mateuszl/.gitconfig alias.gf=flow file:C:/Users/mateuszl/.gitconfig alias.lg=log --color --graph --pretty=format:'%Cred%h%Creset -%C(yellow)%d%Creset %s %Cgreen(%cr) %C(bold blue)<%an>%Creset' --abbrev-commit file:C:/Users/mateuszl/.gitconfig alias.lol=log --graph --decorate --pretty=oneline --abbrev-commit file:C:/Users/mateuszl/.gitconfig alias.lola=log --graph --decorate --pretty=oneline --abbrev-commit --all file:C:/Users/mateuszl/.gitconfig alias.ls=ls-files file:C:/Users/mateuszl/.gitconfig alias.lst=lfs status file:C:/Users/mateuszl/.gitconfig alias.rt=remote
Re: Git Tags
On Thu, Nov 29 2018, Stefanie Leisestreichler wrote: > Hi. > > I have done this (on box A): > > git commit -m "Message" > git tag -a 0.9.0 > git push origin master > > In my local repository, when I run "git tag" it is showing me "0.9.0". > > Then I did (on box B) > git clone ssh://user@host:/path/project.git > cd project > git tag > > Now git tag is showing nothing. > > Why is the tag only available in my local repository? > > Also when I try to > git clone --branch 0.9.0 ssh://user@host:/path/project.git > it tells me: fatal:remote branch not found in upstream repository origin Because --branch means get refs/heads/, tags are not branches. However, because we're apparently quite loose about this in the clone/fetch code this does give you the tag if it exists, but probably not in the way you expect. We interpret the argument as a branch, and will get not only this tag but "follow" (see --no-tags in git-fetch(1)) the tag as though it were a branch and give you all tags leading up to that one. This would give you a single tag: git clone --no-tags --branch v2.19.0 --single-branch https://github.com/git/git.git But this is a more direct way to do it: git init git; git -C git fetch --no-tags https://github.com/git/git.git tag v2.19.0 Which'll since you said it failed that's because you haven't pushed the tag. Try 'git ls-remote ' to see if it's there (it's not).
Re: Simple git push --tags deleted all branches
On Wed, Nov 28 2018, Mateusz Loskot wrote: > Hi, > > (using git version 2.19.2.windows.1) > > I've just encountered one of those WTH moments. > > I have a bare repository > > core.git (BARE:master) $ git branch > 1.0 > 2.0 > * master > > core.git (BARE:master) $ git tag > 1.0.1651 > 1.0.766 > 2.0.1103 > 2.0.1200 > > I published the repo using: git push --all --follow-tags > > This succeeded, but there seem to be no tags pushed, just branches. > So, I followed with > > core.git (BARE:master) $ git push --tags > To XXX > - [deleted] 1.0 > - [deleted] 2.0 > ! [remote rejected] master (refusing to delete the current > branch: refs/heads/master) > error: failed to push some refs to 'XXX' > > And, I've found out that all branches and tags have been > wiped in both, local repo and remote :) > > I restored the repo and tried out > > git push origin 1.0 > git push origin --tags > > and this time both succeeded, without wiping out any refs. > > Could anyone help me to understand why remote-less > > git push --tags > > is/was so dangerous and unforgiving?! Since nobody's replied yet, I can't see what's going on here from the info you've provided. My guess is that you have something "mirror" set on the remote. It seems you can't share the repo or its URL, but could you share the scrubbed output of 'git config -l --show-origin' when run inside this repository?
Re: [PATCH] rebase: mark the C reimplementation as an experimental opt-in feature (was Re: [ANNOUNCE] Git v2.20.0-rc1)
Johannes Schindelin writes ("Re: [PATCH] rebase: mark the C reimplementation as
an experimental opt-in feature (was Re: [ANNOUNCE] Git v2.20.0-rc1)"):
> if you could pry more information (or better information) out of that bug
> reporter, that would be nice. Apparently my email address is blacklisted
> by his mail provider, so he is unlikely to have received my previous mail
> (nor will he receive this one, I am sure).
(I did receive this mail. Sorry for the inconvenience, which sadly is
inevitable occasionally in the modern email world. FTR in future feel
free to send the bounce to postmaster@chiark and I will make a
you-shaped hole in my spamfilter. Also with Debian bugs you can
launder your messages by, eg, emailing 914695-submitter@bugs.)
> > > At https://bugs.debian.org/914695 is a report of a test regression in
> > > an outside project that is very likely to have been triggered by the
> > > new faster rebase code.
As I wrote in the bug report last night:
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=914695#15
I have investigated and the bug seems to be that git-rebase --onto now
fails to honour GIT_REFLOG_ACTION for the initial checkout.
In a successful run with older git I get a reflog like this:
4833d74 HEAD@{0}: rebase finished: returning to refs/heads/with-preexisting
4833d74 HEAD@{1}: debrebase new-upstream 2.1-1: rebase: Add another new
upstream file
cabd5ec HEAD@{2}: debrebase new-upstream 2.1-1: rebase: Edit the .c file
0b362ce HEAD@{3}: debrebase new-upstream 2.1-1: rebase: Add a new upstream
file
29653e5 HEAD@{4}: debrebase new-upstream 2.1-1: rebase: checkout
29653e5a17bee4ac23a68bba3e12bc1f52858ac3
85e0c46 HEAD@{5}: debrebase: launder for new upstream
With a newer git I get this:
6d3fb91 HEAD@{0}: rebase finished: returning to refs/heads/master
6d3fb91 HEAD@{1}: debrebase new-upstream 2.1-1: rebase: Add another new
upstream file
86c0721 HEAD@{2}: debrebase new-upstream 2.1-1: rebase: Edit the .c file
50ba56c HEAD@{3}: debrebase new-upstream 2.1-1: rebase: Add a new upstream
file
8272825 HEAD@{4}: rebase: checkout 8272825bb4ff6eba89afa936e32b6460f963a36a
c78db55 HEAD@{5}: debrebase: launder for new upstream
This breaks the test because my test suite is checking that I set
GIT_REFLOG_ACTION appropriately.
If you want I can provide a minimal test case but this should suffice
to see the bug I hope...
Regards
Ian.
--
Ian JacksonThese opinions are my own.
If I emailed you from an address @fyvzl.net or @evade.org.uk, that is
a private address which bypasses my fierce spamfilter.
[PATCH v2 5/6] pack-objects: create pack.useSparse setting
From: Derrick Stolee
The '--sparse' flag in 'git pack-objects' changes the algorithm
used to enumerate objects to one that is faster for individual
users pushing new objects that change only a small cone of the
working directory. The sparse algorithm is not recommended for a
server, which likely sends new objects that appear across the
entire working directory.
Create a 'pack.useSparse' setting that enables this new algorithm.
This allows 'git push' to use this algorithm without passing a
'--sparse' flag all the way through four levels of run_command()
calls.
If the '--no-sparse' flag is set, then this config setting is
overridden.
Signed-off-by: Derrick Stolee
---
builtin/pack-objects.c | 4
t/t5322-pack-objects-sparse.sh | 15 +++
2 files changed, 19 insertions(+)
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 7d5b0735e3..124b1bafc4 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -2711,6 +2711,10 @@ static int git_pack_config(const char *k, const char *v,
void *cb)
use_bitmap_index_default = git_config_bool(k, v);
return 0;
}
+ if (!strcmp(k, "pack.usesparse")) {
+ sparse = git_config_bool(k, v);
+ return 0;
+ }
if (!strcmp(k, "pack.threads")) {
delta_search_threads = git_config_int(k, v);
if (delta_search_threads < 0)
diff --git a/t/t5322-pack-objects-sparse.sh b/t/t5322-pack-objects-sparse.sh
index 45dba6e014..8f5699bd91 100755
--- a/t/t5322-pack-objects-sparse.sh
+++ b/t/t5322-pack-objects-sparse.sh
@@ -121,4 +121,19 @@ test_expect_success 'sparse pack-objects' '
test_cmp expect_sparse_objects.txt sparse_objects.txt
'
+test_expect_success 'pack.useSparse enables algorithm' '
+ git config pack.useSparse true &&
+ git pack-objects --stdout --revs sparse.pack &&
+ git index-pack -o sparse.idx sparse.pack &&
+ git show-index sparse_objects.txt &&
+ test_cmp expect_sparse_objects.txt sparse_objects.txt
+'
+
+test_expect_success 'pack.useSparse overridden' '
+ git pack-objects --stdout --revs --no-sparse sparse.pack &&
+ git index-pack -o sparse.idx sparse.pack &&
+ git show-index sparse_objects.txt &&
+ test_cmp expect_objects.txt sparse_objects.txt
+'
+
test_done
--
gitgitgadget
[PATCH v2 0/6] Add a new "sparse" tree walk algorithm
One of the biggest remaining pain points for users of very large
repositories is the time it takes to run 'git push'. We inspected some slow
pushes by our developers and found that the "Enumerating Objects" phase of a
push was very slow. This is unsurprising, because this is why reachability
bitmaps exist. However, reachability bitmaps are not available to us because
of the single pack-file requirement. The bitmap approach is intended for
servers anyway, and clients have a much different behavior pattern.
Specifically, clients are normally pushing a very small number of objects
compared to the entire working directory. A typical user changes only a
small cone of the working directory, so let's use that to our benefit.
Create a new "sparse" mode for 'git pack-objects' that uses the paths that
introduce new objects to direct our search into the reachable trees. By
collecting trees at each path, we can then recurse into a path only when
there are uninteresting and interesting trees at that path. This gains a
significant performance boost for small topics while presenting a
possibility of packing extra objects.
The main algorithm change is in patch 4, but is set up a little bit in
patches 1 and 2.
As demonstrated in the included test script, we see that the existing
algorithm can send extra objects due to the way we specify the "frontier".
But we can send even more objects if a user copies objects from one folder
to another. I say "copy" because a rename would (usually) change the
original folder and trigger a walk into that path, discovering the objects.
In order to benefit from this approach, the user can opt-in using the
pack.useSparse config setting. This setting can be overridden using the
'--no-sparse' option.
Update in V2:
* Added GIT_TEST_PACK_SPARSE test option.
* Fixed test breakages when GIT_TEST_PACK_SPARSE is enabled by adding null
checks.
Derrick Stolee (6):
revision: add mark_tree_uninteresting_sparse
list-objects: consume sparse tree walk
pack-objects: add --sparse option
revision: implement sparse algorithm
pack-objects: create pack.useSparse setting
pack-objects: create GIT_TEST_PACK_SPARSE
Documentation/git-pack-objects.txt | 9 +-
bisect.c | 2 +-
builtin/pack-objects.c | 10 ++-
builtin/rev-list.c | 2 +-
http-push.c| 2 +-
list-objects.c | 55 +++-
list-objects.h | 4 +-
revision.c | 121 +
revision.h | 2 +
t/README | 4 +
t/t5322-pack-objects-sparse.sh | 139 +
11 files changed, 340 insertions(+), 10 deletions(-)
create mode 100755 t/t5322-pack-objects-sparse.sh
base-commit: a1598010f775d82b5adf12c29d0f5bc9b41434c6
Published-As:
https://github.com/gitgitgadget/git/releases/tags/pr-89%2Fderrickstolee%2Fpush%2Fsparse-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git
pr-89/derrickstolee/push/sparse-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/89
Range-diff vs v1:
1: b73b8de98c = 1: 60617681f7 revision: add mark_tree_uninteresting_sparse
2: 9bf04c748b ! 2: 4527addacb list-objects: consume sparse tree walk
@@ -116,6 +116,10 @@
+ for (parents = commit->parents; parents; parents = parents->next) {
+ struct commit *parent = parents->item;
+ struct tree *tree = get_commit_tree(parent);
++
++ if (!tree)
++ continue;
++
+ oidset_insert(set, >object.oid);
+
+ if (!(parent->object.flags & UNINTERESTING))
@@ -142,14 +146,14 @@
+
for (list = revs->commits; list; list = list->next) {
struct commit *commit = list->item;
-+
+
+- if (commit->object.flags & UNINTERESTING) {
+ if (sparse) {
+ struct tree *tree = get_commit_tree(commit);
-+
++
+ if (commit->object.flags & UNINTERESTING)
+ tree->object.flags |= UNINTERESTING;
-
-- if (commit->object.flags & UNINTERESTING) {
++
+ oidset_insert(, >object.oid);
+ add_edge_parents(commit, revs, show_edge, );
+ } else if (commit->object.flags & UNINTERESTING) {
@@ -189,3 +193,17 @@
struct oidset;
struct list_objects_filter_options;
+
+diff --git a/revision.c b/revision.c
+--- a/revision.c
b/revision.c
+@@
+ while ((oid = oidset_iter_next())) {
+ struct tree *tree = lookup_tree(r, oid);
+
++ if (!tree)
++ continue;
++
+ if (tree->object.flags & UNINTERESTING) {
+ /*
+ * Remove
[PATCH v2 1/6] revision: add mark_tree_uninteresting_sparse
From: Derrick Stolee
In preparation for a new algorithm that walks fewer trees when
creating a pack from a set of revisions, create a method that
takes an oidset of tree oids and marks reachable objects as
UNINTERESTING.
The current implementation uses the existing
mark_tree_uninteresting to recursively walk the trees and blobs.
This will walk the same number of trees as the old mechanism.
There is one new assumption in this approach: we are also given
the oids of the interesting trees. This implementation does not
use those trees at the moment, but we will use them in a later
rewrite of this method.
Signed-off-by: Derrick Stolee
---
revision.c | 22 ++
revision.h | 2 ++
2 files changed, 24 insertions(+)
diff --git a/revision.c b/revision.c
index 13e0519c02..3a62c7c187 100644
--- a/revision.c
+++ b/revision.c
@@ -99,6 +99,28 @@ void mark_tree_uninteresting(struct repository *r, struct
tree *tree)
mark_tree_contents_uninteresting(r, tree);
}
+void mark_trees_uninteresting_sparse(struct repository *r,
+struct oidset *set)
+{
+ struct object_id *oid;
+ struct oidset_iter iter;
+
+ oidset_iter_init(set, );
+ while ((oid = oidset_iter_next())) {
+ struct tree *tree = lookup_tree(r, oid);
+
+ if (tree->object.flags & UNINTERESTING) {
+ /*
+* Remove the flag so the next call
+* is not a no-op. The flag is added
+* in mark_tree_unintersting().
+*/
+ tree->object.flags ^= UNINTERESTING;
+ mark_tree_uninteresting(r, tree);
+ }
+ }
+}
+
struct commit_stack {
struct commit **items;
size_t nr, alloc;
diff --git a/revision.h b/revision.h
index 7987bfcd2e..f828e91ae9 100644
--- a/revision.h
+++ b/revision.h
@@ -67,6 +67,7 @@ struct rev_cmdline_info {
#define REVISION_WALK_NO_WALK_SORTED 1
#define REVISION_WALK_NO_WALK_UNSORTED 2
+struct oidset;
struct topo_walk_info;
struct rev_info {
@@ -327,6 +328,7 @@ void put_revision_mark(const struct rev_info *revs,
void mark_parents_uninteresting(struct commit *commit);
void mark_tree_uninteresting(struct repository *r, struct tree *tree);
+void mark_trees_uninteresting_sparse(struct repository *r, struct oidset *set);
void show_object_with_name(FILE *, struct object *, const char *);
--
gitgitgadget
[PATCH v2 2/6] list-objects: consume sparse tree walk
From: Derrick Stolee
When creating a pack-file using 'git pack-objects --revs' we provide
a list of interesting and uninteresting commits. For example, a push
operation would make the local topic branch be interesting and the
known remote refs as uninteresting. We want to discover the set of
new objects to send to the server as a thin pack.
We walk these commits until we discover a frontier of commits such
that every commit walk starting at interesting commits ends in a root
commit or unintersting commit. We then need to discover which
non-commit objects are reachable from uninteresting commits.
The mark_edges_unintersting() method in list-objects.c iterates on
the commit list and does the following:
* If the commit is UNINTERSTING, then mark its root tree and every
object it can reach as UNINTERESTING.
* If the commit is interesting, then mark the root tree of every
UNINTERSTING parent (and all objects that tree can reach) as
UNINTERSTING.
At the very end, we repeat the process on every commit directly
given to the revision walk from stdin. This helps ensure we properly
cover shallow commits that otherwise were not included in the
frontier.
The logic to recursively follow trees is in the
mark_tree_uninteresting() method in revision.c. The algorithm avoids
duplicate work by not recursing into trees that are already marked
UNINTERSTING.
Add a new 'sparse' option to the mark_edges_uninteresting() method
that performs this logic in a slightly new way. As we iterate over
the commits, we add all of the root trees to an oidset. Then, call
mark_trees_uninteresting_sparse() on that oidset. Note that we
include interesting trees in this process. The current implementation
of mark_trees_unintersting_sparse() will walk the same trees as
the old logic, but this will be replaced in a later change.
The sparse option is not used by any callers at the moment, but
will be wired to 'git pack-objects' in the next change.
Signed-off-by: Derrick Stolee
---
bisect.c | 2 +-
builtin/pack-objects.c | 2 +-
builtin/rev-list.c | 2 +-
http-push.c| 2 +-
list-objects.c | 55 +++---
list-objects.h | 4 ++-
revision.c | 3 +++
7 files changed, 61 insertions(+), 9 deletions(-)
diff --git a/bisect.c b/bisect.c
index 487675c672..842f8b4b8f 100644
--- a/bisect.c
+++ b/bisect.c
@@ -656,7 +656,7 @@ static void bisect_common(struct rev_info *revs)
if (prepare_revision_walk(revs))
die("revision walk setup failed");
if (revs->tree_objects)
- mark_edges_uninteresting(revs, NULL);
+ mark_edges_uninteresting(revs, NULL, 0);
}
static void exit_if_skipped_commits(struct commit_list *tried,
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 411aefd687..5f70d840a7 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -3135,7 +3135,7 @@ static void get_object_list(int ac, const char **av)
if (prepare_revision_walk())
die(_("revision walk setup failed"));
- mark_edges_uninteresting(, show_edge);
+ mark_edges_uninteresting(, show_edge, 0);
if (!fn_show_object)
fn_show_object = show_object;
diff --git a/builtin/rev-list.c b/builtin/rev-list.c
index 2880ed37e3..9663cbfae0 100644
--- a/builtin/rev-list.c
+++ b/builtin/rev-list.c
@@ -543,7 +543,7 @@ int cmd_rev_list(int argc, const char **argv, const char
*prefix)
if (prepare_revision_walk())
die("revision walk setup failed");
if (revs.tree_objects)
- mark_edges_uninteresting(, show_edge);
+ mark_edges_uninteresting(, show_edge, 0);
if (bisect_list) {
int reaches, all;
diff --git a/http-push.c b/http-push.c
index cd48590912..ea52d6f9f6 100644
--- a/http-push.c
+++ b/http-push.c
@@ -1933,7 +1933,7 @@ int cmd_main(int argc, const char **argv)
pushing = 0;
if (prepare_revision_walk())
die("revision walk setup failed");
- mark_edges_uninteresting(, NULL);
+ mark_edges_uninteresting(, NULL, 0);
objects_to_send = get_delta(, ref_lock);
finish_all_active_slots();
diff --git a/list-objects.c b/list-objects.c
index c41cc80db5..4fbdeca0a4 100644
--- a/list-objects.c
+++ b/list-objects.c
@@ -222,25 +222,72 @@ static void mark_edge_parents_uninteresting(struct commit
*commit,
}
}
-void mark_edges_uninteresting(struct rev_info *revs, show_edge_fn show_edge)
+static void add_edge_parents(struct commit *commit,
+struct rev_info *revs,
+show_edge_fn show_edge,
+struct oidset *set)
+{
+ struct commit_list *parents;
+
+ for (parents = commit->parents; parents; parents = parents->next) {
+ struct commit *parent = parents->item;
+
Re: [PATCH 2/2] format-patch: allow for independent diff & range-diff options
On Thu, Nov 29 2018, Johannes Schindelin wrote:
> Hi Ævar,
>
> On Thu, 29 Nov 2018, Ævar Arnfjörð Bjarmason wrote:
>
>> On Thu, Nov 29 2018, Johannes Schindelin wrote:
>>
>> > On Wed, 28 Nov 2018, Ævar Arnfjörð Bjarmason wrote:
>> >
>> >> Change the semantics of the "--range-diff" option so that the regular
>> >> diff options can be provided separately for the range-diff and the
>> >> patch. This allows for supplying e.g. --range-diff-U0 and -U1 to
>> >> "format-patch" to provide different context for the range-diff and the
>> >> patch. This wasn't possible before.
>> >
>> > I really, really dislike the `--range-diff-`. We have
>> > precedent for passing optional arguments that are passed to some other
>> > command, so a much more logical and consistent convention would be to use
>> > `--range-diff[=..]`, allowing all of the diff options that
>> > you might want to pass to the outer diff in one go rather than having a
>> > lengthy string of `--range-diff-this` and `--range-diff-that` options.
>>
>> Where do we pass those sorts of arguments?
>>
>> Reasons I did it this way:
>>
>> a) Passing it as one option will require the user to double-quote those
>> options that take quoted arguments (e.g. --word-diff-regex), which I
>> thought sucked more than the prefix. On the implementation side we
>> couldn't leave the parsing of the command-line to the shell anymore.
>>
>> b) I think people will want to tweak this very rarely, much more rarely
>> than e.g. -U10 in format-patch itself, so having something long-ish
>> doesn't sound bad.
>
> Hmm. I still don't like it. It sets a precedent, and we simply do not do
> it that way in other circumstances (most obvious would be the -X merge
> options). The more divergent user interfaces for the same sort of thing
> are, the more brain cycles you force users to spend on navigating said
> interfaces.
Yeah it sucks, I just think it sucks less than the alternative :)
I.e. I'm not picky about --range-diff-* prefix the name, but I think
doing our own shell parsing would be nasty.
>> > I only had time to skim the patch, and I have to wonder why you pass
>> > around full-blown `rev_info` structs for range diff (and with that really
>> > awful name `rd_rev`) rather than just the `diff_options` that you
>> > *actually* care about?
>>
>> Because setup_revisions() which does all the command-line parsing needs
>> a rev_info, so even if we only need the diffopt in the end we need to
>> initiate the whole thing.
>>
>> Suggestions for a better varibale name most welcome.
>
> `range_diff_revs`
>
> And you do not need to pass around the whole thing. You can easily pass
> `_diff_revs.diffopt`.
>
> Don't pass around what you do not need to pass around.
Ah, you mean internally in log.c, yes that makes sense. I thought you
meant just pass diffopt to setup_revisions() (which needs the containing
struct). Willdo.
> Ciao,
> Dscho
>
>>
>> > Ciao,
>> > Dscho
>> >
>> >>
>> >> Ever since the "--range-diff" option was added in
>> >> 31e2617a5f ("format-patch: add --range-diff option to embed diff in
>> >> cover letter", 2018-07-22) the "rev->diffopt" we pass down to the diff
>> >> machinery has been the one we get from "format-patch"'s own
>> >> setup_revisions().
>> >>
>> >> This sort of thing is unique among the log-like commands in
>> >> builtin/log.c, no command than format-patch will embed the output of
>> >> another log-like command. Since the "rev->diffopt" is reused we need
>> >> to munge it before we pass it to show_range_diff(). See
>> >> 43dafc4172 ("format-patch: don't include --stat with --range-diff
>> >> output", 2018-11-22) for a related regression fix which is being
>> >> mostly reverted here.
>> >>
>> >> Implementation notes: 1) We're not bothering with the full teardown
>> >> around die() and will leak memory, but it's too much boilerplate to do
>> >> all the frees with/without the die() and not worth it. 2) We call
>> >> repo_init_revisions() for "rd_rev" even though we could get away with
>> >> a shallow copy like the code we're replacing (and which
>> >> show_range_diff() itself does). This is to make this code more easily
>> >> understood.
>> >>
>> >> Signed-off-by: Ævar Arnfjörð Bjarmason
>> >> ---
>> >> Documentation/git-format-patch.txt | 10 ++-
>> >> builtin/log.c | 42 +++---
>> >> t/t3206-range-diff.sh | 41 +
>> >> 3 files changed, 82 insertions(+), 11 deletions(-)
>> >>
>> >> diff --git a/Documentation/git-format-patch.txt
>> >> b/Documentation/git-format-patch.txt
>> >> index aba4c5febe..6c048f415f 100644
>> >> --- a/Documentation/git-format-patch.txt
>> >> +++ b/Documentation/git-format-patch.txt
>> >> @@ -24,7 +24,8 @@ SYNOPSIS
>> >> [--to=] [--cc=]
>> >> [--[no-]cover-letter] [--quiet] [--notes[=]]
>> >> [--interdiff=]
>> >> -[--range-diff= [--creation-factor=]]
>> >> +[--range-diff=
[PATCH v2 6/6] pack-objects: create GIT_TEST_PACK_SPARSE
From: Derrick Stolee
Create a test variable GIT_TEST_PACK_SPARSE to enable the sparse
object walk algorithm by default during the test suite. Enabling
this variable ensures coverage in many interesting cases, such as
shallow clones, partial clones, and missing objects.
Signed-off-by: Derrick Stolee
---
builtin/pack-objects.c | 1 +
t/README | 4
t/t5322-pack-objects-sparse.sh | 6 +++---
3 files changed, 8 insertions(+), 3 deletions(-)
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 124b1bafc4..507d381153 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -3331,6 +3331,7 @@ int cmd_pack_objects(int argc, const char **argv, const
char *prefix)
read_replace_refs = 0;
+ sparse = git_env_bool("GIT_TEST_PACK_SPARSE", 0);
reset_pack_idx_option(_idx_opts);
git_config(git_pack_config, NULL);
diff --git a/t/README b/t/README
index 28711cc508..8b6dfe1864 100644
--- a/t/README
+++ b/t/README
@@ -342,6 +342,10 @@ GIT_TEST_INDEX_VERSION= exercises the index read/write
code path
for the index version specified. Can be set to any valid version
(currently 2, 3, or 4).
+GIT_TEST_PACK_SPARSE= if enabled will default the pack-objects
+builtin to use the sparse object walk. This can still be overridden by
+the --no-sparse command-line argument.
+
GIT_TEST_PRELOAD_INDEX= exercises the preload-index code path
by overriding the minimum number of cache entries required per thread.
diff --git a/t/t5322-pack-objects-sparse.sh b/t/t5322-pack-objects-sparse.sh
index 8f5699bd91..e8cf41d1c6 100755
--- a/t/t5322-pack-objects-sparse.sh
+++ b/t/t5322-pack-objects-sparse.sh
@@ -36,7 +36,7 @@ test_expect_success 'setup repo' '
'
test_expect_success 'non-sparse pack-objects' '
- git pack-objects --stdout --revs nonsparse.pack &&
+ git pack-objects --stdout --revs --no-sparse nonsparse.pack &&
git index-pack -o nonsparse.idx nonsparse.pack &&
git show-index nonsparse_objects.txt &&
test_cmp expect_objects.txt nonsparse_objects.txt
@@ -70,7 +70,7 @@ test_expect_success 'duplicate a folder from f3 and commit to
topic1' '
'
test_expect_success 'non-sparse pack-objects' '
- git pack-objects --stdout --revs nonsparse.pack &&
+ git pack-objects --stdout --revs --no-sparse nonsparse.pack &&
git index-pack -o nonsparse.idx nonsparse.pack &&
git show-index nonsparse_objects.txt &&
test_cmp expect_objects.txt nonsparse_objects.txt
@@ -102,7 +102,7 @@ test_expect_success 'non-sparse pack-objects' '
topic1 \
topic1^{tree} \
topic1:f3 | sort >expect_objects.txt &&
- git pack-objects --stdout --revs nonsparse.pack &&
+ git pack-objects --stdout --revs --no-sparse nonsparse.pack &&
git index-pack -o nonsparse.idx nonsparse.pack &&
git show-index nonsparse_objects.txt &&
test_cmp expect_objects.txt nonsparse_objects.txt
--
gitgitgadget
[PATCH v2 4/6] revision: implement sparse algorithm
From: Derrick Stolee
When enumerating objects to place in a pack-file during 'git
pack-objects --revs', we discover the "frontier" of commits
that we care about and the boundary with commit we find
uninteresting. From that point, we walk trees to discover which
trees and blobs are uninteresting. Finally, we walk trees to find
the interesting trees.
This commit introduces a new, "sparse" way to discover the
uninteresting trees. We use the perspective of a single user trying
to push their topic to a large repository. That user likely changed
a very small fraction of the paths in their working directory, but
we spend a lot of time walking all reachable trees.
The way to switch the logic to work in this sparse way is to start
caring about which paths introduce new trees. While it is not
possible to generate a diff between the frontier boundary and all
of the interesting commits, we can simulate that behavior by
inspecting all of the root trees as a whole, then recursing down
to the set of trees at each path.
We already had taken the first step by passing an oidset to
mark_trees_uninteresting_sparse(). We now create a dictionary
whose keys are paths and values are oidsets. We consider the set
of trees that appear at each path. While we inspect a tree, we
add its subtrees to the oidsets corresponding to the tree entry's
path. We also mark trees as UNINTERESTING if the tree we are
parsing is UNINTERESTING.
To actually improve the peformance, we need to terminate our
recursion unless the oidset contains some intersting trees and
some uninteresting trees. Technically, we only need one interesting
tree for this to speed up in most cases, but we also will not mark
anything UNINTERESTING if there are no uninteresting trees, so
that would be wasted effort.
There are a few ways that this is not a universally better option.
First, we can pack extra objects. If someone copies a subtree
from one tree to another, the first tree will appear UNINTERESTING
and we will not recurse to see that the subtree should also be
UNINTERESTING. We will walk the new tree and see the subtree as
a "new" object and add it to the pack. We add a test case that
demonstrates this as a way to prove that the --sparse option is
actually working.
Second, we can have extra memory pressure. If instead of being a
single user pushing a small topic we are a server sending new
objects from across the entire working directory, then we will
gain very little (the recursion will rarely terminate early) but
will spend extra time maintaining the path-oidset dictionaries.
Despite these potential drawbacks, the benefits of the algorithm
are clear. By adding a counter to 'add_children_by_path' and
'mark_tree_contents_uninteresting', I measured the number of
parsed trees for the two algorithms in a variety of repos.
For git.git, I used the following input:
v2.19.0
^v2.19.0~10
Objects to pack: 550
Walked (old alg): 282
Walked (new alg): 130
For the Linux repo, I used the following input:
v4.18
^v4.18~10
Objects to pack: 518
Walked (old alg): 4,836
Walked (new alg): 188
The two repos above are rather "wide and flat" compared to
other repos that I have used in the past. As a comparison,
I tested an old topic branch in the Azure DevOps repo, which
has a much deeper folder structure than the Linux repo.
Objects to pack:220
Walked (old alg): 22,804
Walked (new alg):129
I used the number of walked trees the main metric above because
it is consistent across multiple runs. When I ran my tests, the
performance of the pack-objects command with the same options
could change the end-to-end time by 10x depending on the file
system being warm. However, by repeating the same test on repeat
I could get more consistent timing results. The git.git and
Linux tests were too fast overall (less than 0.5s) to measure
an end-to-end difference. The Azure DevOps case was slow enough
to see the time improve from 15s to 1s in the warm case. The
cold case was 90s to 9s in my testing.
These improvements will have even larger benefits in the super-
large Windows repository. In our experiments, we see the
"Enumerate objects" phase of pack-objects taking 60-80% of the
end-to-end time of non-trivial pushes, taking longer than the
network time to send the pack and the server time to verify the
pack.
Signed-off-by: Derrick Stolee
---
revision.c | 116 ++---
t/t5322-pack-objects-sparse.sh | 21 --
2 files changed, 121 insertions(+), 16 deletions(-)
diff --git a/revision.c b/revision.c
index f9eb6400f1..971f1bb095 100644
--- a/revision.c
+++ b/revision.c
@@ -99,29 +99,125 @@ void mark_tree_uninteresting(struct repository *r, struct
tree *tree)
mark_tree_contents_uninteresting(r, tree);
}
+struct paths_and_oids {
+ struct string_list list;
+};
+
+static void paths_and_oids_init(struct paths_and_oids *po)
+{
+ string_list_init(>list, 1);
+}
+
+static void
[PATCH v2 3/6] pack-objects: add --sparse option
From: Derrick Stolee
Add a '--sparse' option flag to the pack-objects builtin. This
allows the user to specify that they want to use the new logic
for walking trees. This logic currently does not differ from the
existing output, but will in a later change.
Create a new test script, t5322-pack-objects-sparse.sh, to ensure
the object list that is selected matches what we expect. When we
update the logic to walk in a sparse fashion, the final test will
be updated to show the extra objects that are added.
Signed-off-by: Derrick Stolee
---
Documentation/git-pack-objects.txt | 9 ++-
builtin/pack-objects.c | 5 +-
t/t5322-pack-objects-sparse.sh | 115 +
3 files changed, 127 insertions(+), 2 deletions(-)
create mode 100755 t/t5322-pack-objects-sparse.sh
diff --git a/Documentation/git-pack-objects.txt
b/Documentation/git-pack-objects.txt
index 40c825c381..ced2630eb3 100644
--- a/Documentation/git-pack-objects.txt
+++ b/Documentation/git-pack-objects.txt
@@ -14,7 +14,7 @@ SYNOPSIS
[--local] [--incremental] [--window=] [--depth=]
[--revs [--unpacked | --all]] [--keep-pack=]
[--stdout [--filter=] | base-name]
- [--shallow] [--keep-true-parents] < object-list
+ [--shallow] [--keep-true-parents] [--sparse] < object-list
DESCRIPTION
@@ -196,6 +196,13 @@ depth is 4095.
Add --no-reuse-object if you want to force a uniform compression
level on all data no matter the source.
+--sparse::
+ Use the "sparse" algorithm to determine which objects to include in
+ the pack. This can have significant performance benefits when computing
+ a pack to send a small change. However, it is possible that extra
+ objects are added to the pack-file if the included commits contain
+ certain types of direct renames.
+
--thin::
Create a "thin" pack by omitting the common objects between a
sender and a receiver in order to reduce network transfer. This
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 5f70d840a7..7d5b0735e3 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -84,6 +84,7 @@ static unsigned long pack_size_limit;
static int depth = 50;
static int delta_search_threads;
static int pack_to_stdout;
+static int sparse;
static int thin;
static int num_preferred_base;
static struct progress *progress_state;
@@ -3135,7 +3136,7 @@ static void get_object_list(int ac, const char **av)
if (prepare_revision_walk())
die(_("revision walk setup failed"));
- mark_edges_uninteresting(, show_edge, 0);
+ mark_edges_uninteresting(, show_edge, sparse);
if (!fn_show_object)
fn_show_object = show_object;
@@ -3292,6 +3293,8 @@ int cmd_pack_objects(int argc, const char **argv, const
char *prefix)
{ OPTION_CALLBACK, 0, "unpack-unreachable", NULL, N_("time"),
N_("unpack unreachable objects newer than "),
PARSE_OPT_OPTARG, option_parse_unpack_unreachable },
+ OPT_BOOL(0, "sparse", ,
+N_("use the sparse reachability algorithm")),
OPT_BOOL(0, "thin", ,
N_("create thin packs")),
OPT_BOOL(0, "shallow", ,
diff --git a/t/t5322-pack-objects-sparse.sh b/t/t5322-pack-objects-sparse.sh
new file mode 100755
index 00..81f6805bc3
--- /dev/null
+++ b/t/t5322-pack-objects-sparse.sh
@@ -0,0 +1,115 @@
+#!/bin/sh
+
+test_description='pack-objects object selection using sparse algorithm'
+. ./test-lib.sh
+
+test_expect_success 'setup repo' '
+ test_commit initial &&
+ for i in $(test_seq 1 3)
+ do
+ mkdir f$i &&
+ for j in $(test_seq 1 3)
+ do
+ mkdir f$i/f$j &&
+ echo $j >f$i/f$j/data.txt
+ done
+ done &&
+ git add . &&
+ git commit -m "Initialized trees" &&
+ for i in $(test_seq 1 3)
+ do
+ git checkout -b topic$i master &&
+ echo change-$i >f$i/f$i/data.txt &&
+ git commit -a -m "Changed f$i/f$i/data.txt"
+ done &&
+ cat >packinput.txt <<-EOF &&
+ topic1
+ ^topic2
+ ^topic3
+ EOF
+ git rev-parse \
+ topic1 \
+ topic1^{tree} \
+ topic1:f1 \
+ topic1:f1/f1\
+ topic1:f1/f1/data.txt | sort >expect_objects.txt
+'
+
+test_expect_success 'non-sparse pack-objects' '
+ git pack-objects --stdout --revs nonsparse.pack &&
+ git index-pack -o nonsparse.idx nonsparse.pack &&
+ git show-index nonsparse_objects.txt &&
+ test_cmp expect_objects.txt nonsparse_objects.txt
+'
+
+test_expect_success 'sparse pack-objects' '
+ git pack-objects --stdout --revs --sparse sparse.pack &&
+
Re: [PATCH 3/5] pack-objects: add --sparse option
On 11/28/2018 5:11 PM, Stefan Beller wrote: +--sparse:: + Use the "sparse" algorithm to determine which objects to include in + the pack. This can have significant performance benefits when computing + a pack to send a small change. However, it is possible that extra + objects are added to the pack-file if the included commits contain + certain types of direct renames. As a user, where do I find a discussion of these walking algorithms? (i.e. how can I tell if I can really expect to gain performance benefits, what are the tradeoffs? "If it were strictly better than the original, it would be on by default" might be a thought of a user) You're right that having this hidden as an opt-in config variable makes it hard to discover as a typical user. I would argue that we should actually make the config setting true by default, and recommend that servers opt-out. Here are my reasons: 1. The vast majority of users are clients. 2. Client users are not likely to know about and tweak these settings. 3. Server users are more likely to keep an eye on the different knobs they can tweak. 4. Servers should use the reachability bitmaps, which don't use this logic anyway. While _eventually_ we should make this opt-out, we shouldn't do that until it has cooked a while. Thanks, -Stolee
Re: [PATCH] rebase: mark the C reimplementation as an experimental opt-in feature (was Re: [ANNOUNCE] Git v2.20.0-rc1)
Hi Jonathan, if you could pry more information (or better information) out of that bug reporter, that would be nice. Apparently my email address is blacklisted by his mail provider, so he is unlikely to have received my previous mail (nor will he receive this one, I am sure). Thanks, Dscho On Wed, 28 Nov 2018, Johannes Schindelin wrote: > Hi Jonathan, > > On Tue, 27 Nov 2018, Jonathan Nieder wrote: > > > At https://bugs.debian.org/914695 is a report of a test regression in > > an outside project that is very likely to have been triggered by the > > new faster rebase code. > > From looking through that log.gz (without having a clue where the test > code lives, so I cannot say what it is supposed to do, and also: this is > the first time I hear about dgit...), it would appear that this must be a > regression in the reflog messages produced by `git rebase`. > > > The issue has not been triaged, so I don't know yet whether it's a > > problem in rebase-in-c or a manifestation of a bug in the test. > > It ends thusly: > > -- snip -- > [...] > + git reflog > + egrep 'debrebase new-upstream.*checkout' > + test 1 = 0 > + t-report-failure > + set +x > TEST FAILED > -- snap -- > > Which makes me think that the reflog we produce in *some* code path that > originally called `git checkout` differs from the scripted rebase's > generated reflog. > > > That said, Google has been running with the new rebase since ~1 month > > ago when it became the default, with no issues reported by users. As a > > result, I am confident that it can cope with what most users of "next" > > throw at it, which means that if we are to find more issues to polish it > > better, it will need all the exposure it can get. > > Right. And there are a few weeks before the holidays, which should give me > time to fix whatever bugs are discovered (I only half mind being the only > one who fixes these bugs). > > > In the Google deployment, we will keep using rebase-in-c even if it > > gets disabled by default, in order to help with that. > > > > From the Debian point of view, it's only a matter of time before > > rebase-in-c becomes the default: even if it's not the default in 2.20, > > it would presumably be so in 2.21 or 2.22. That means the community's > > attention when resolving security and reliability bugs would be on the > > rebase-in-c implementation. As a result, the Debian package will most > > likely enable rebase-in-c by default even if upstream disables it, in > > order to increase the package's shelf life (i.e. to ease the > > maintenance burden of supporting whichever version of the package ends > > up in the next Debian stable). > > > > So with either hat on, it doesn't matter whether you apply this patch > > upstream. > > > > Having two pretty different deployments end up with the same > > conclusion leads me to suspect that it's best for upstream not to > > apply the revert patch, unless either > > > > (a) we have a concrete regression to address and then try again, or > > (b) we have a test or other plan to follow before trying again. > > In this instance, I am more a fan of the "let's move fast and break > things, then move even faster fixing them" approach. > > Besides, the bug that Ævar discovered was a bug already in the scripted > rebase, but hidden by yet another bug (the missing error checking). > > I get the pretty firm impression that the common code paths are now pretty > robust, and only lesser-exercised features may expose a bug (or > regression, as in the case of the reflogs, where one could argue that the > exact reflog message is not something we promise not to fiddle with). > > Ciao, > Dscho
Re: [PATCH v1] teach git to support a virtual (partially populated) work directory
On 11/28/2018 8:31 AM, SZEDER Gábor wrote:
On Tue, Nov 27, 2018 at 02:50:57PM -0500, Ben Peart wrote:
diff --git a/t/t1092-virtualworkdir.sh b/t/t1092-virtualworkdir.sh
new file mode 100755
index 00..0cdfe9b362
--- /dev/null
+++ b/t/t1092-virtualworkdir.sh
@@ -0,0 +1,393 @@
+#!/bin/sh
+
+test_description='virtual work directory tests'
+
+. ./test-lib.sh
+
+# We need total control of the virtual work directory hook
+sane_unset GIT_TEST_VIRTUALWORKDIR
+
+clean_repo () {
+ rm .git/index &&
+ git -c core.virtualworkdir=false reset --hard HEAD &&
+ git -c core.virtualworkdir=false clean -fd &&
+ touch untracked.txt &&
We would usually run '>untracked.txt' instead, sparing the external
process.
A further nit is that a function called 'clean_repo' creates new
untracked files...
Thanks, all good suggestions I've incorporated for the next iteration.
+ touch dir1/untracked.txt &&
+ touch dir2/untracked.txt
+}
+
+test_expect_success 'setup' '
+ mkdir -p .git/hooks/ &&
+ cat > .gitignore <<-\EOF &&
CodingGuidelines suggest no space between redirection operator and
filename.
+ .gitignore
+ expect*
+ actual*
+ EOF
+ touch file1.txt &&
+ touch file2.txt &&
+ mkdir -p dir1 &&
+ touch dir1/file1.txt &&
+ touch dir1/file2.txt &&
+ mkdir -p dir2 &&
+ touch dir2/file1.txt &&
+ touch dir2/file2.txt &&
+ git add . &&
+ git commit -m "initial" &&
+ git config --local core.virtualworkdir true
+'
+test_expect_success 'verify files not listed are ignored by git clean -f -x' '
+ clean_repo &&
I find it odd to clean the repo right after setting it up; but then
again, 'clean_repo' not only cleans, but also creates new files.
Perhaps rename it to 'reset_repo'? Dunno.
+ write_script .git/hooks/virtual-work-dir <<-\EOF &&
+ printf "untracked.txt\0"
+ printf "dir1/\0"
+ EOF
+ mkdir -p dir3 &&
+ touch dir3/untracked.txt &&
+ git clean -f -x &&
+ test -f file1.txt &&
Please use the 'test_path_is_file', ...
+ test -f file2.txt &&
+ test ! -f untracked.txt &&
... 'test_path_is_missing', and ...
+ test -d dir1 &&
... 'test_path_is_dir' helpers, respectively, because they print
informative error messages on failure.
+ test -f dir1/file1.txt &&
+ test -f dir1/file2.txt &&
+ test ! -f dir1/untracked.txt &&
+ test -f dir2/file1.txt &&
+ test -f dir2/file2.txt &&
+ test -f dir2/untracked.txt &&
+ test -d dir3 &&
+ test -f dir3/untracked.txt
+'
Re: [PATCH v1] mem_pool: add GIT_TRACE_MEMPOOL support
On 11/28/2018 4:37 AM, Johannes Schindelin wrote:
Hi Ben,
On Tue, 27 Nov 2018, Ben Peart wrote:
From: Ben Peart
Add tracing around initializing and discarding mempools. In discard report
on the amount of memory unused in the current block to help tune setting
the initial_size.
Signed-off-by: Ben Peart
---
Looks good.
My only question: should we also trace calls to _alloc(), _calloc() and
_combine()?
I was trying to tune the initial size in my use of the mem_pool and so
found this tracing useful to see how much memory was actually being
used. I'm inclined to only add tracing as it is needed rather that
proactively because we think it _might_ be needed. I suspect _alloc()
and _calloc() would get very noisy and not add much value.
Ciao,
Johannes
Notes:
Base Ref: * git-trace-mempool
Web-Diff: https://github.com/benpeart/git/commit/9ac84bbca2
Checkout: git fetch https://github.com/benpeart/git git-trace-mempool-v1
&& git checkout 9ac84bbca2
mem-pool.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/mem-pool.c b/mem-pool.c
index a2841a4a9a..065389aaec 100644
--- a/mem-pool.c
+++ b/mem-pool.c
@@ -5,6 +5,7 @@
#include "cache.h"
#include "mem-pool.h"
+static struct trace_key trace_mem_pool = TRACE_KEY_INIT(MEMPOOL);
#define BLOCK_GROWTH_SIZE 1024*1024 - sizeof(struct mp_block);
/*
@@ -48,12 +49,16 @@ void mem_pool_init(struct mem_pool **mem_pool, size_t
initial_size)
mem_pool_alloc_block(pool, initial_size, NULL);
*mem_pool = pool;
+ trace_printf_key(_mem_pool, "mem_pool (%p): init (%"PRIuMAX") initial
size\n",
+ pool, (uintmax_t)initial_size);
}
void mem_pool_discard(struct mem_pool *mem_pool, int invalidate_memory)
{
struct mp_block *block, *block_to_free;
+ trace_printf_key(_mem_pool, "mem_pool (%p): discard (%"PRIuMAX") unused\n",
+ mem_pool, (uintmax_t)(mem_pool->mp_block->end -
mem_pool->mp_block->next_free));
block = mem_pool->mp_block;
while (block)
{
base-commit: bb75be6cb916297f271c846f2f9caa3daaaec718
--
2.18.0.windows.1
Re: [PATCH v11 00/22] Convert "git stash" to C builtin
Hi Junio, On Thu, 29 Nov 2018, Johannes Schindelin wrote: > On Mon, 26 Nov 2018, Junio C Hamano wrote: > > > Junio C Hamano writes: > > > > > Thomas Gummerer writes: > > > > > >> Thanks for your work on this! I have read through the range-diff and > > >> the new patch of this last round, and this addresses all the comments > > >> I had on v10 (and some more :)). I consider it > > >> Reviewed-by: Thomas Gummerer > > > > > > Thanks. > > > > > > One thing that bothers me is that this seems to have been rebased on > > > 'master', but as long as we are rebasing, the updated series must > > > also take into account of the sd/stash-wo-user-name topic, i.e. if > > > we are rebasing it, it should be rebased on top of the result of > > > > > > git checkout -B ps/rebase-in-c master > > > git merge --no-ff sd/stash-wo-user-name > > > > > > I think. > > > > https://travis-ci.org/git/git/builds/459619672 would show that this > > C reimplementation now regresses from the scripted version due to > > lack of such rebasing (i.e. porting a correction from scripted one). > > Oh, you know, at first I *mis-read* your mail to mean "don't you rebase > all the time!", but in this case (in contrast to earlier statements about > rebasing between iterations of patch series), you *do* want Paul to > rebase. > > Let me see what I can come up with in my `git-stash` branch on > https://github.com/dscho/git There. I force-pushed an update that is based on sd/stash-wo-user-name and adds a `prepare_fallback_ident(name, email)` to `ident.c` for use in the built-in stash: https://github.com/dscho/git/commit/d37ce623fbd32e4345c701dea822e56de1a5417f It passes t3903 in a little over a minute with GIT_TEST_STASH_USE_BUILTIN=true and in a little less than seven minutes with GIT_TEST_STASH_USE_BUILTIN=false. Ciao, Dscho
Re: Git Tags
On Thu, 29 Nov 2018 at 14:40, Randall S. Becker wrote: > On November 29, 2018 6:56, Mateusz Loskot wrote: > > On Thu, 29 Nov 2018 at 12:50, Stefanie Leisestreichler > > wrote: > > > > > > git tag -a 0.9.0 > > > git push origin master > > > > > > In my local repository, when I run "git tag" it is showing me "0.9.0". > > > > > > Then I did (on box B) > > > git clone ssh://user@host:/path/project.git cd project git tag > > > > > > Now git tag is showing nothing. > > > > > > Why is the tag only available in my local repository? > > > > >From https://git-scm.com/book/en/v2/Git-Basics-Tagging > > "By default, the git push command doesn’t transfer tags to remote servers. > > You will have to explicitly push tags to a shared server after you have > > created > > them." > > git push --tags > After my yesterday experience [1], I'd be careful with git push --tags :) [1] genuine screenshot and link to related thread at https://twitter.com/mloskot/status/1068072285846859776 Best regards, -- Mateusz Loskot, http://mateusz.loskot.net
RE: Git Tags
On November 29, 2018 6:56, Mateusz Loskot wrote: > On Thu, 29 Nov 2018 at 12:50, Stefanie Leisestreichler > wrote: > > > > git tag -a 0.9.0 > > git push origin master > > > > In my local repository, when I run "git tag" it is showing me "0.9.0". > > > > Then I did (on box B) > > git clone ssh://user@host:/path/project.git cd project git tag > > > > Now git tag is showing nothing. > > > > Why is the tag only available in my local repository? > > >From https://git-scm.com/book/en/v2/Git-Basics-Tagging > "By default, the git push command doesn’t transfer tags to remote servers. > You will have to explicitly push tags to a shared server after you have > created > them." git push --tags and git fetch --tags to be symmetrical Cheers, Randall -- Brief whoami: NonStop developer since approximately 2112884442 UNIX developer since approximately 421664400 -- In my real life, I talk too much.
Re: [bug report] git-gui child windows are blank
Just checked gitk, it seems to work fine including children windows. This problem seems to affect git-gui only. On Thu, Nov 29, 2018 at 5:16 AM Eric Sunshine wrote: > > On Wed, Nov 28, 2018 at 2:29 PM Stefan Beller wrote: > > On Wed, Nov 28, 2018 at 6:13 AM Kenn Sebesta wrote: > > > v2.19.2, installed from brew on macOS Mojave 14.2.1. > > > > > > `git-gui` is my much beloved go-to tool for everything git. > > > Unfortunately, on my new Macbook Air it seems to have a bug. When I > > > first load the program, the parent window populates normally with the > > > stage/unstaged and diff panes. However, when I click Push, the child > > > window is completely blank. The frame is there, but there is no > > > content. > > > > Looking through the mailing list archive, this > > seemed to be one of the last relevant messges > > https://public-inbox.org/git/20180724065120.7664-1-sunsh...@sunshineco.com/ > > I was hoping that Junio would patch-monkey that one since that > crash-at-launch makes gitk unusable for Mojave users, but apparently > it never got picked up. > > Anyhow, the issue fixed by that patch has to do with 'osascript' on > Apple, and it's the only such invocation in the source code of > gitk/git-gui. The 'osascript' invocation merely attempts to foreground > the application at launch, so is almost certainly unrelated to the > above reported behavior. Somebody running Mojave will likely need to > spend some time debugging it. (Unfortunately, I'm a couple major > releases behind and don't plan on upgrading.)
[PATCH v2 0/1] Fix git rebase --stat -i
We're really entering obscure territory here, I would say.
To trigger the bug, two things have to come together: the user must have
asked for a diffstat afterwards, and the commits need to have been rebased
onto a completely unrelated commit history (i.e. there must exist no merge
base between the pre-rebase HEAD and the post-rebase HEAD).
Please note that this bug existed already in the scripted rebase, but it was
never detected because the scripted version would not even perform any error
checking.
It will make Junio very happy that I squashed the regression test in to the
patch that fixes it. The reason, however, was not to make Junio happy (I
hope to make him happy by fixing bugs), but simply that an earlier iteration
of test would only fail with the built-in rebase, but not with the scripted
version. The current version would fail with the scripted version, but I'll
save the time to split the patch again now.
Changes since v1:
* The commit message now talks more about what we should do in case that
there is no merge base, rather than stressing the differences between the
way scripted vs built-in rebase handled it (both buggy, and fixed by this
patch).
* In case that there is no merge base, it no longer reports "Changes from
(empty) to ..." but "Changes to ...", which should be a lot less
controversial.
Johannes Schindelin (1):
rebase --stat: fix when rebasing to an unrelated history
builtin/rebase.c | 18 --
git-legacy-rebase.sh | 10 --
t/t3406-rebase-message.sh | 10 ++
3 files changed, 30 insertions(+), 8 deletions(-)
base-commit: a1598010f775d82b5adf12c29d0f5bc9b41434c6
Published-As:
https://github.com/gitgitgadget/git/releases/tags/pr-88%2Fdscho%2Frebase-stat-fix-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git
pr-88/dscho/rebase-stat-fix-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/88
Range-diff vs v1:
1: 680385e4bd ! 1: 190c7856ad rebase --stat: fix when rebasing to an
unrelated history
@@ -3,22 +3,21 @@
rebase --stat: fix when rebasing to an unrelated history
When rebasing to a commit history that has no common commits with the
-current branch, there is no merge base. The scripted version of the
`git
-rebase` command was not prepared for that and spewed out
+current branch, there is no merge base. In diffstat mode, this means
+that we cannot compare to the merge base, but we have to compare to
the
+empty tree instead.
-fatal: ambiguous argument '': unknown revision or path not in
-the working tree.
+Also, if running in verbose diffstat mode, we should not output
-but then continued (due to lack of error checking).
+Changes from to
-The built-in version of the `git rebase` command blindly translated
that
-shell script code, assuming that there is no need to test whether
there
-*was* a merge base, and due to its better error checking, exited with
a
-fatal error (because it tried to read the object with hash ...
-as a tree).
+as that does not make sense without any merge base.
-Fix both scripted and built-in versions to output something sensibly,
-and add a regression test to keep this working in all eternity.
+Note: neither scripted nor built-in versoin of `git rebase` were
+prepared for this situation well. We use this opportunity not only to
+fix the bug(s), but also to make both versions' output consistent in
+this instance. And add a regression test to keep this working in all
+eternity.
Reported-by: Ævar Arnfjörð Bjarmason
Signed-off-by: Johannes Schindelin
@@ -27,15 +26,25 @@
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@
+ if (options.flags & REBASE_DIFFSTAT) {
+ struct diff_options opts;
- if (options.flags & REBASE_VERBOSE)
- printf(_("Changes from %s to %s:\n"),
+- if (options.flags & REBASE_VERBOSE)
+- printf(_("Changes from %s to %s:\n"),
- oid_to_hex(_base),
-+ is_null_oid(_base) ?
-+ "(empty)" : oid_to_hex(_base),
- oid_to_hex(>object.oid));
+- oid_to_hex(>object.oid));
++ if (options.flags & REBASE_VERBOSE) {
++ if (is_null_oid(_base))
++ printf(_("Changes to %s:\n"),
++oid_to_hex(>object.oid));
++ else
++ printf(_("Changes from %s to %s:\n"),
++oid_to_hex(_base),
++
[PATCH v2 1/1] rebase --stat: fix when rebasing to an unrelated history
From: Johannes Schindelin
When rebasing to a commit history that has no common commits with the
current branch, there is no merge base. In diffstat mode, this means
that we cannot compare to the merge base, but we have to compare to the
empty tree instead.
Also, if running in verbose diffstat mode, we should not output
Changes from to
as that does not make sense without any merge base.
Note: neither scripted nor built-in versoin of `git rebase` were
prepared for this situation well. We use this opportunity not only to
fix the bug(s), but also to make both versions' output consistent in
this instance. And add a regression test to keep this working in all
eternity.
Reported-by: Ævar Arnfjörð Bjarmason
Signed-off-by: Johannes Schindelin
---
builtin/rebase.c | 18 --
git-legacy-rebase.sh | 10 --
t/t3406-rebase-message.sh | 10 ++
3 files changed, 30 insertions(+), 8 deletions(-)
diff --git a/builtin/rebase.c b/builtin/rebase.c
index 5b3e5baec8..1c6f817f4b 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -1481,10 +1481,15 @@ int cmd_rebase(int argc, const char **argv, const char
*prefix)
if (options.flags & REBASE_DIFFSTAT) {
struct diff_options opts;
- if (options.flags & REBASE_VERBOSE)
- printf(_("Changes from %s to %s:\n"),
- oid_to_hex(_base),
- oid_to_hex(>object.oid));
+ if (options.flags & REBASE_VERBOSE) {
+ if (is_null_oid(_base))
+ printf(_("Changes to %s:\n"),
+ oid_to_hex(>object.oid));
+ else
+ printf(_("Changes from %s to %s:\n"),
+ oid_to_hex(_base),
+ oid_to_hex(>object.oid));
+ }
/* We want color (if set), but no pager */
diff_setup();
@@ -1494,8 +1499,9 @@ int cmd_rebase(int argc, const char **argv, const char
*prefix)
DIFF_FORMAT_SUMMARY | DIFF_FORMAT_DIFFSTAT;
opts.detect_rename = DIFF_DETECT_RENAME;
diff_setup_done();
- diff_tree_oid(_base, >object.oid,
- "", );
+ diff_tree_oid(is_null_oid(_base) ?
+ the_hash_algo->empty_tree : _base,
+ >object.oid, "", );
diffcore_std();
diff_flush();
}
diff --git a/git-legacy-rebase.sh b/git-legacy-rebase.sh
index b97ffdc9dd..b4c7dbfa57 100755
--- a/git-legacy-rebase.sh
+++ b/git-legacy-rebase.sh
@@ -718,10 +718,16 @@ if test -n "$diffstat"
then
if test -n "$verbose"
then
- echo "$(eval_gettext "Changes from \$mb to \$onto:")"
+ if test -z "$mb"
+ then
+ echo "$(eval_gettext "Changes to \$onto:")"
+ else
+ echo "$(eval_gettext "Changes from \$mb to \$onto:")"
+ fi
fi
+ mb_tree="${mb:-$(git hash-object -t tree /dev/null)}"
# We want color (if set), but no pager
- GIT_PAGER='' git diff --stat --summary "$mb" "$onto"
+ GIT_PAGER='' git diff --stat --summary "$mb_tree" "$onto"
fi
test -n "$interactive_rebase" && run_specific_rebase
diff --git a/t/t3406-rebase-message.sh b/t/t3406-rebase-message.sh
index 38bd876cab..c2c9950568 100755
--- a/t/t3406-rebase-message.sh
+++ b/t/t3406-rebase-message.sh
@@ -91,4 +91,14 @@ test_expect_success 'error out early upon -C or
--whitespace=' '
test_i18ngrep "Invalid whitespace option" err
'
+test_expect_success 'rebase -i onto unrelated history' '
+ git init unrelated &&
+ test_commit -C unrelated 1 &&
+ git -C unrelated remote add -f origin "$PWD" &&
+ git -C unrelated branch --set-upstream-to=origin/master &&
+ git -C unrelated -c core.editor=true rebase -i -v --stat >actual &&
+ test_i18ngrep "Changes to " actual &&
+ test_i18ngrep "5 files changed" actual
+'
+
test_done
--
gitgitgadget
Re: [PATCH v11 00/22] Convert "git stash" to C builtin
Hi Junio, On Mon, 26 Nov 2018, Junio C Hamano wrote: > Junio C Hamano writes: > > > Thomas Gummerer writes: > > > >> Thanks for your work on this! I have read through the range-diff and > >> the new patch of this last round, and this addresses all the comments > >> I had on v10 (and some more :)). I consider it > >> Reviewed-by: Thomas Gummerer > > > > Thanks. > > > > One thing that bothers me is that this seems to have been rebased on > > 'master', but as long as we are rebasing, the updated series must > > also take into account of the sd/stash-wo-user-name topic, i.e. if > > we are rebasing it, it should be rebased on top of the result of > > > > git checkout -B ps/rebase-in-c master > > git merge --no-ff sd/stash-wo-user-name > > > > I think. > > https://travis-ci.org/git/git/builds/459619672 would show that this > C reimplementation now regresses from the scripted version due to > lack of such rebasing (i.e. porting a correction from scripted one). Oh, you know, at first I *mis-read* your mail to mean "don't you rebase all the time!", but in this case (in contrast to earlier statements about rebasing between iterations of patch series), you *do* want Paul to rebase. Let me see what I can come up with in my `git-stash` branch on https://github.com/dscho/git Ciao, Dscho
Re: [PATCH 1/1] rebase --stat: fix when rebasing to an unrelated history
Hi Junio,
On Thu, 29 Nov 2018, Junio C Hamano wrote:
> "Johannes Schindelin via GitGitGadget"
> writes:
>
> > The built-in version of the `git rebase` command blindly translated that
> > shell script code, assuming that there is no need to test whether there
> > *was* a merge base, and due to its better error checking, exited with a
> > fatal error (because it tried to read the object with hash ...
> > as a tree).
>
> And the scripted version produced a wrong result because it did not
> check the lack of merge base and fed an empty string (presumably, as
> that is what you would get from mb=$(merge-base A B) when A and B
> are unrelated) to later steps? If that is the case, then it *is* a
> very significant thing to record here. As it was not merely "failed
> to stop due to lack of error checking", but a lot worse. It was
> producing a wrong result.
Indeed. But it was only in the `--stat` reporting, so it did not produce
an incorrect rebased history.
> A more faithful reimplementation would not have exited with fatal
> error, but would have produced the same wrong result, so "a better
> error checking caused the reimplementation die where the original
> did not die" may be correct but is much less important than the fact
> that "the logic used in the original to produce diffstat forgot that
> there may not be a merge base and would not have worked correctly in
> such a case, and that is what we are correcting" is more important.
True.
> Please descibe the issue as such, if that is the case (I cannot read
> from the description in the proposed log message if that is the
> case---but I came to the conclusion that it is what you wanted to
> fix reading the code).
Indeed, my commit message is a bit too close to what I fixed, and not
enough about what needed to be fixed.
> > if (options.flags & REBASE_VERBOSE)
> > printf(_("Changes from %s to %s:\n"),
> > - oid_to_hex(_base),
> > + is_null_oid(_base) ?
> > + "(empty)" : oid_to_hex(_base),
>
> I am not sure (empty) is a good idea for two reasons. It is going
> to be inserted into an translated string without translation.
Oooops.
> Also it is too similar to the fallback string used by some printf
> implementations when NULL was given to %s by mistake.
You mean `(null)`? That was actually intentional, I just thought that
`(empty)` would be different enough...
> If there weren't i18n issues, I would suggest to use "empty merge
> base" or "empty tree" instead, but the old one would have shown
> 0{40}, so perhaps empty_tree_oid_hex() is a good substitute?
As this is a user-facing message, I'd rather avoid something like
`empty_tree_oid_hex()`, which to every Git user who does not happen to be
a Git developer, too, must sound very cryptic.
But I guess that I should not be so lazy and really use two different
messages here:
Changes from to
and if there is no merge base,
Changes in
Will fix.
> > @@ -1494,8 +1495,9 @@ int cmd_rebase(int argc, const char **argv, const
> > char *prefix)
> > DIFF_FORMAT_SUMMARY | DIFF_FORMAT_DIFFSTAT;
> > opts.detect_rename = DIFF_DETECT_RENAME;
> > diff_setup_done();
> > - diff_tree_oid(_base, >object.oid,
> > - "", );
> > + diff_tree_oid(is_null_oid(_base) ?
> > + the_hash_algo->empty_tree : _base,
> > + >object.oid, "", );
>
> The original would have run "git diff '' $onto", and died with an
> error message, so both the original and the reimplementation were
> failing. Just in different ways ;-)
Right.
> > diff --git a/git-legacy-rebase.sh b/git-legacy-rebase.sh
> > index b97ffdc9dd..be3b241676 100755
> > --- a/git-legacy-rebase.sh
> > +++ b/git-legacy-rebase.sh
> > @@ -718,10 +718,12 @@ if test -n "$diffstat"
> > then
> > if test -n "$verbose"
> > then
> > - echo "$(eval_gettext "Changes from \$mb to \$onto:")"
> > + mb_display="${mb:-(empty)}"
> > + echo "$(eval_gettext "Changes from \$mb_display to \$onto:")"
> > fi
> > + mb_tree="${mb:-$(git hash-object -t tree /dev/null)}"
> > # We want color (if set), but no pager
> > - GIT_PAGER='' git diff --stat --summary "$mb" "$onto"
> > + GIT_PAGER='' git diff --stat --summary "$mb_tree" "$onto"
>
> Code fix for diff-tree invocation, both in the builtin/ version and
> this one, looks correct.
Okay. I fixed the two things you pointed out, just waiting for the Linux
phase to finish (I don't think there is anything OS-specific in this
patch, so I trust macOS and Windows to pass if Linux passes):
https://git-for-windows.visualstudio.com/git/_build/results?buildId=26116
Ciao,
Dscho
Hello
Good day, My Name is Johann Reimann and i have something important to discuss with you but only with your permission i will proceed. Regards J. Reimann
Re: [PATCH 2/2] format-patch: allow for independent diff & range-diff options
Hi Ævar,
On Thu, 29 Nov 2018, Ævar Arnfjörð Bjarmason wrote:
> On Thu, Nov 29 2018, Johannes Schindelin wrote:
>
> > On Wed, 28 Nov 2018, Ævar Arnfjörð Bjarmason wrote:
> >
> >> Change the semantics of the "--range-diff" option so that the regular
> >> diff options can be provided separately for the range-diff and the
> >> patch. This allows for supplying e.g. --range-diff-U0 and -U1 to
> >> "format-patch" to provide different context for the range-diff and the
> >> patch. This wasn't possible before.
> >
> > I really, really dislike the `--range-diff-`. We have
> > precedent for passing optional arguments that are passed to some other
> > command, so a much more logical and consistent convention would be to use
> > `--range-diff[=..]`, allowing all of the diff options that
> > you might want to pass to the outer diff in one go rather than having a
> > lengthy string of `--range-diff-this` and `--range-diff-that` options.
>
> Where do we pass those sorts of arguments?
>
> Reasons I did it this way:
>
> a) Passing it as one option will require the user to double-quote those
> options that take quoted arguments (e.g. --word-diff-regex), which I
> thought sucked more than the prefix. On the implementation side we
> couldn't leave the parsing of the command-line to the shell anymore.
>
> b) I think people will want to tweak this very rarely, much more rarely
> than e.g. -U10 in format-patch itself, so having something long-ish
> doesn't sound bad.
Hmm. I still don't like it. It sets a precedent, and we simply do not do
it that way in other circumstances (most obvious would be the -X merge
options). The more divergent user interfaces for the same sort of thing
are, the more brain cycles you force users to spend on navigating said
interfaces.
> > I only had time to skim the patch, and I have to wonder why you pass
> > around full-blown `rev_info` structs for range diff (and with that really
> > awful name `rd_rev`) rather than just the `diff_options` that you
> > *actually* care about?
>
> Because setup_revisions() which does all the command-line parsing needs
> a rev_info, so even if we only need the diffopt in the end we need to
> initiate the whole thing.
>
> Suggestions for a better varibale name most welcome.
`range_diff_revs`
And you do not need to pass around the whole thing. You can easily pass
`_diff_revs.diffopt`.
Don't pass around what you do not need to pass around.
Ciao,
Dscho
>
> > Ciao,
> > Dscho
> >
> >>
> >> Ever since the "--range-diff" option was added in
> >> 31e2617a5f ("format-patch: add --range-diff option to embed diff in
> >> cover letter", 2018-07-22) the "rev->diffopt" we pass down to the diff
> >> machinery has been the one we get from "format-patch"'s own
> >> setup_revisions().
> >>
> >> This sort of thing is unique among the log-like commands in
> >> builtin/log.c, no command than format-patch will embed the output of
> >> another log-like command. Since the "rev->diffopt" is reused we need
> >> to munge it before we pass it to show_range_diff(). See
> >> 43dafc4172 ("format-patch: don't include --stat with --range-diff
> >> output", 2018-11-22) for a related regression fix which is being
> >> mostly reverted here.
> >>
> >> Implementation notes: 1) We're not bothering with the full teardown
> >> around die() and will leak memory, but it's too much boilerplate to do
> >> all the frees with/without the die() and not worth it. 2) We call
> >> repo_init_revisions() for "rd_rev" even though we could get away with
> >> a shallow copy like the code we're replacing (and which
> >> show_range_diff() itself does). This is to make this code more easily
> >> understood.
> >>
> >> Signed-off-by: Ævar Arnfjörð Bjarmason
> >> ---
> >> Documentation/git-format-patch.txt | 10 ++-
> >> builtin/log.c | 42 +++---
> >> t/t3206-range-diff.sh | 41 +
> >> 3 files changed, 82 insertions(+), 11 deletions(-)
> >>
> >> diff --git a/Documentation/git-format-patch.txt
> >> b/Documentation/git-format-patch.txt
> >> index aba4c5febe..6c048f415f 100644
> >> --- a/Documentation/git-format-patch.txt
> >> +++ b/Documentation/git-format-patch.txt
> >> @@ -24,7 +24,8 @@ SYNOPSIS
> >> [--to=] [--cc=]
> >> [--[no-]cover-letter] [--quiet] [--notes[=]]
> >> [--interdiff=]
> >> - [--range-diff= [--creation-factor=]]
> >> + [--range-diff= [--creation-factor=]
> >> +[--range-diff]]
> >> [--progress]
> >> []
> >> [ | ]
> >> @@ -257,6 +258,13 @@ feeding the result to `git send-email`.
> >>creation/deletion cost fudge factor. See linkgit:git-range-diff[1])
> >>for details.
> >>
> >> +--range-diff::
> >> + Other options prefixed with `--range-diff` are stripped of
> >> + that prefix and passed as-is to the diff machinery used to
> >> + generate the range-diff,
Re: Git Tags
On Thu, 29 Nov 2018 at 12:50, Stefanie Leisestreichler wrote: > > git tag -a 0.9.0 > git push origin master > > In my local repository, when I run "git tag" it is showing me "0.9.0". > > Then I did (on box B) > git clone ssh://user@host:/path/project.git > cd project > git tag > > Now git tag is showing nothing. > > Why is the tag only available in my local repository? >From https://git-scm.com/book/en/v2/Git-Basics-Tagging "By default, the git push command doesn’t transfer tags to remote servers. You will have to explicitly push tags to a shared server after you have created them." Best regards, -- Mateusz Loskot, http://mateusz.loskot.net
Git Tags
Hi. I have done this (on box A): git commit -m "Message" git tag -a 0.9.0 git push origin master In my local repository, when I run "git tag" it is showing me "0.9.0". Then I did (on box B) git clone ssh://user@host:/path/project.git cd project git tag Now git tag is showing nothing. Why is the tag only available in my local repository? Also when I try to git clone --branch 0.9.0 ssh://user@host:/path/project.git it tells me: fatal:remote branch not found in upstream repository origin Why is the tag not available in origin? Thanks, Stefanie
Re: [PATCH] rebase -i: introduce the 'test' command
Hi Paul, On Thu, 29 Nov 2018, Johannes Schindelin wrote: > I already added a test... See the reschedule-failed-exec branch on > https://github.com/dscho/git. And now I put up a proper Pull Request, to be submitted via GitGitGadget right after Git v2.20.0 will be released (technically, we are in a feature freeze period right now, I just could no resist, as I found your reasoning for rescheduling failed `exec` commands compelling, and there were no `rebase` bugs for me to fix). https://github.com/gitgitgadget/git/pull/90 Ciao, Johannes
Re: [PATCH 2/2] format-patch: allow for independent diff & range-diff options
On Thu, Nov 29 2018, Johannes Schindelin wrote:
> Hi Ævar,
>
> On Wed, 28 Nov 2018, Ævar Arnfjörð Bjarmason wrote:
>
>> Change the semantics of the "--range-diff" option so that the regular
>> diff options can be provided separately for the range-diff and the
>> patch. This allows for supplying e.g. --range-diff-U0 and -U1 to
>> "format-patch" to provide different context for the range-diff and the
>> patch. This wasn't possible before.
>
> I really, really dislike the `--range-diff-`. We have
> precedent for passing optional arguments that are passed to some other
> command, so a much more logical and consistent convention would be to use
> `--range-diff[=..]`, allowing all of the diff options that
> you might want to pass to the outer diff in one go rather than having a
> lengthy string of `--range-diff-this` and `--range-diff-that` options.
Where do we pass those sorts of arguments?
Reasons I did it this way:
a) Passing it as one option will require the user to double-quote those
options that take quoted arguments (e.g. --word-diff-regex), which I
thought sucked more than the prefix. On the implementation side we
couldn't leave the parsing of the command-line to the shell anymore.
b) I think people will want to tweak this very rarely, much more rarely
than e.g. -U10 in format-patch itself, so having something long-ish
doesn't sound bad.
> I only had time to skim the patch, and I have to wonder why you pass
> around full-blown `rev_info` structs for range diff (and with that really
> awful name `rd_rev`) rather than just the `diff_options` that you
> *actually* care about?
Because setup_revisions() which does all the command-line parsing needs
a rev_info, so even if we only need the diffopt in the end we need to
initiate the whole thing.
Suggestions for a better varibale name most welcome.
> Ciao,
> Dscho
>
>>
>> Ever since the "--range-diff" option was added in
>> 31e2617a5f ("format-patch: add --range-diff option to embed diff in
>> cover letter", 2018-07-22) the "rev->diffopt" we pass down to the diff
>> machinery has been the one we get from "format-patch"'s own
>> setup_revisions().
>>
>> This sort of thing is unique among the log-like commands in
>> builtin/log.c, no command than format-patch will embed the output of
>> another log-like command. Since the "rev->diffopt" is reused we need
>> to munge it before we pass it to show_range_diff(). See
>> 43dafc4172 ("format-patch: don't include --stat with --range-diff
>> output", 2018-11-22) for a related regression fix which is being
>> mostly reverted here.
>>
>> Implementation notes: 1) We're not bothering with the full teardown
>> around die() and will leak memory, but it's too much boilerplate to do
>> all the frees with/without the die() and not worth it. 2) We call
>> repo_init_revisions() for "rd_rev" even though we could get away with
>> a shallow copy like the code we're replacing (and which
>> show_range_diff() itself does). This is to make this code more easily
>> understood.
>>
>> Signed-off-by: Ævar Arnfjörð Bjarmason
>> ---
>> Documentation/git-format-patch.txt | 10 ++-
>> builtin/log.c | 42 +++---
>> t/t3206-range-diff.sh | 41 +
>> 3 files changed, 82 insertions(+), 11 deletions(-)
>>
>> diff --git a/Documentation/git-format-patch.txt
>> b/Documentation/git-format-patch.txt
>> index aba4c5febe..6c048f415f 100644
>> --- a/Documentation/git-format-patch.txt
>> +++ b/Documentation/git-format-patch.txt
>> @@ -24,7 +24,8 @@ SYNOPSIS
>> [--to=] [--cc=]
>> [--[no-]cover-letter] [--quiet] [--notes[=]]
>> [--interdiff=]
>> - [--range-diff= [--creation-factor=]]
>> + [--range-diff= [--creation-factor=]
>> + [--range-diff]]
>> [--progress]
>> []
>> [ | ]
>> @@ -257,6 +258,13 @@ feeding the result to `git send-email`.
>> creation/deletion cost fudge factor. See linkgit:git-range-diff[1])
>> for details.
>>
>> +--range-diff::
>> +Other options prefixed with `--range-diff` are stripped of
>> +that prefix and passed as-is to the diff machinery used to
>> +generate the range-diff, e.g. `--range-diff-U0` and
>> +`--range-diff--no-color`. This allows for adjusting the format
>> +of the range-diff independently from the patch itself.
>> +
>> --notes[=]::
>> Append the notes (see linkgit:git-notes[1]) for the commit
>> after the three-dash line.
>> diff --git a/builtin/log.c b/builtin/log.c
>> index 02d88fa233..7658e56ecc 100644
>> --- a/builtin/log.c
>> +++ b/builtin/log.c
>> @@ -1023,7 +1023,8 @@ static void show_diffstat(struct rev_info *rev,
>> fprintf(rev->diffopt.file, "\n");
>> }
>>
>> -static void make_cover_letter(struct rev_info *rev, int use_stdout,
>> +static void make_cover_letter(struct rev_info *rev, struct rev_info *rd_rev,
>>
My Humble Request
Dear Friend, I am Abel Brent, a NATO soldier serving in Afghanistan. I and my Comrades, we are seeking your assistance to help us receive/invest our funds in your country in any lucrative business. Please if this proposal is acceptable by you, kindly respond back to me for more details. Thanks and waiting to hear from you. Abel.
Re: [bug report] git-gui child windows are blank
On Wed, Nov 28, 2018 at 2:29 PM Stefan Beller wrote: > On Wed, Nov 28, 2018 at 6:13 AM Kenn Sebesta wrote: > > v2.19.2, installed from brew on macOS Mojave 14.2.1. > > > > `git-gui` is my much beloved go-to tool for everything git. > > Unfortunately, on my new Macbook Air it seems to have a bug. When I > > first load the program, the parent window populates normally with the > > stage/unstaged and diff panes. However, when I click Push, the child > > window is completely blank. The frame is there, but there is no > > content. > > Looking through the mailing list archive, this > seemed to be one of the last relevant messges > https://public-inbox.org/git/20180724065120.7664-1-sunsh...@sunshineco.com/ I was hoping that Junio would patch-monkey that one since that crash-at-launch makes gitk unusable for Mojave users, but apparently it never got picked up. Anyhow, the issue fixed by that patch has to do with 'osascript' on Apple, and it's the only such invocation in the source code of gitk/git-gui. The 'osascript' invocation merely attempts to foreground the application at launch, so is almost certainly unrelated to the above reported behavior. Somebody running Mojave will likely need to spend some time debugging it. (Unfortunately, I'm a couple major releases behind and don't plan on upgrading.)
Re: in 2.19.2 t0061-run-command FAILs if . is in $PATH
Hi Merijn and Junio, On Thu, 29 Nov 2018, Junio C Hamano wrote: > Johannes Schindelin writes: > > > -test_expect_success 'run_command is restricted to PATH' ' > > +test_lazy_prereq DOT_IN_PATH ' > > + case ":$PATH:" in > > + *:.:*) true;; > > + *) false;; > > + esac > > +' > > An empty element in the colon-separated list also serves as an > instruction to pick up executable from $cwd, so > > case ":$PATH:" in > *:.:** | *::*) true ;; > *) false ;; > esac > > perhaps. Good point. Merijn, please be sure to squash this fix in before you submit the final thing. Thanks, Johannes > > > +test_expect_success !DOT_IN_PATH 'run_command is restricted to PATH' ' > > write_script should-not-run <<-\EOF && > > echo yikes > > EOF > > -- snap -- > > > > If so, can you please provide a commit message for it (you can add my > > Signed-off-by: line and your Tested-by: line). > > > > Thanks, > > Johannes >
Re: [PATCH 2/2] format-patch: allow for independent diff & range-diff options
Hi Ævar,
On Wed, 28 Nov 2018, Ævar Arnfjörð Bjarmason wrote:
> Change the semantics of the "--range-diff" option so that the regular
> diff options can be provided separately for the range-diff and the
> patch. This allows for supplying e.g. --range-diff-U0 and -U1 to
> "format-patch" to provide different context for the range-diff and the
> patch. This wasn't possible before.
I really, really dislike the `--range-diff-`. We have
precedent for passing optional arguments that are passed to some other
command, so a much more logical and consistent convention would be to use
`--range-diff[=..]`, allowing all of the diff options that
you might want to pass to the outer diff in one go rather than having a
lengthy string of `--range-diff-this` and `--range-diff-that` options.
I only had time to skim the patch, and I have to wonder why you pass
around full-blown `rev_info` structs for range diff (and with that really
awful name `rd_rev`) rather than just the `diff_options` that you
*actually* care about?
Ciao,
Dscho
>
> Ever since the "--range-diff" option was added in
> 31e2617a5f ("format-patch: add --range-diff option to embed diff in
> cover letter", 2018-07-22) the "rev->diffopt" we pass down to the diff
> machinery has been the one we get from "format-patch"'s own
> setup_revisions().
>
> This sort of thing is unique among the log-like commands in
> builtin/log.c, no command than format-patch will embed the output of
> another log-like command. Since the "rev->diffopt" is reused we need
> to munge it before we pass it to show_range_diff(). See
> 43dafc4172 ("format-patch: don't include --stat with --range-diff
> output", 2018-11-22) for a related regression fix which is being
> mostly reverted here.
>
> Implementation notes: 1) We're not bothering with the full teardown
> around die() and will leak memory, but it's too much boilerplate to do
> all the frees with/without the die() and not worth it. 2) We call
> repo_init_revisions() for "rd_rev" even though we could get away with
> a shallow copy like the code we're replacing (and which
> show_range_diff() itself does). This is to make this code more easily
> understood.
>
> Signed-off-by: Ævar Arnfjörð Bjarmason
> ---
> Documentation/git-format-patch.txt | 10 ++-
> builtin/log.c | 42 +++---
> t/t3206-range-diff.sh | 41 +
> 3 files changed, 82 insertions(+), 11 deletions(-)
>
> diff --git a/Documentation/git-format-patch.txt
> b/Documentation/git-format-patch.txt
> index aba4c5febe..6c048f415f 100644
> --- a/Documentation/git-format-patch.txt
> +++ b/Documentation/git-format-patch.txt
> @@ -24,7 +24,8 @@ SYNOPSIS
> [--to=] [--cc=]
> [--[no-]cover-letter] [--quiet] [--notes[=]]
> [--interdiff=]
> -[--range-diff= [--creation-factor=]]
> +[--range-diff= [--creation-factor=]
> + [--range-diff]]
> [--progress]
> []
> [ | ]
> @@ -257,6 +258,13 @@ feeding the result to `git send-email`.
> creation/deletion cost fudge factor. See linkgit:git-range-diff[1])
> for details.
>
> +--range-diff::
> + Other options prefixed with `--range-diff` are stripped of
> + that prefix and passed as-is to the diff machinery used to
> + generate the range-diff, e.g. `--range-diff-U0` and
> + `--range-diff--no-color`. This allows for adjusting the format
> + of the range-diff independently from the patch itself.
> +
> --notes[=]::
> Append the notes (see linkgit:git-notes[1]) for the commit
> after the three-dash line.
> diff --git a/builtin/log.c b/builtin/log.c
> index 02d88fa233..7658e56ecc 100644
> --- a/builtin/log.c
> +++ b/builtin/log.c
> @@ -1023,7 +1023,8 @@ static void show_diffstat(struct rev_info *rev,
> fprintf(rev->diffopt.file, "\n");
> }
>
> -static void make_cover_letter(struct rev_info *rev, int use_stdout,
> +static void make_cover_letter(struct rev_info *rev, struct rev_info *rd_rev,
> + int use_stdout,
> struct commit *origin,
> int nr, struct commit **list,
> const char *branch_name,
> @@ -1095,13 +1096,9 @@ static void make_cover_letter(struct rev_info *rev,
> int use_stdout,
> }
>
> if (rev->rdiff1) {
> - struct diff_options opts;
> - memcpy(, >diffopt, sizeof(opts));
> - opts.output_format &= ~(DIFF_FORMAT_DIFFSTAT |
> DIFF_FORMAT_SUMMARY);
> -
> fprintf_ln(rev->diffopt.file, "%s", rev->rdiff_title);
> show_range_diff(rev->rdiff1, rev->rdiff2,
> - rev->creation_factor, 1, );
> + rev->creation_factor, 1, _rev->diffopt);
> }
> }
>
> @@ -1485,6 +1482,7 @@ int cmd_format_patch(int argc, const char
Re: [PATCH] pack-protocol.txt: accept error packets in any context
Masaya Suzuki writes: > In the Git pack protocol definition, an error packet may appear only in > a certain context. However, servers can face a runtime error (e.g. I/O > error) at an arbitrary timing. This patch changes the protocol to allow > an error packet to be sent instead of any packet. > > Following this protocol spec change, the error packet handling code is > moved to pkt-line.c. > > Signed-off-by: Masaya Suzuki > --- Have you run tests with this applied? I noticed 5703.9 now has stale expectations, but there may be others.
Re: [PATCH] rebase -i: introduce the 'test' command
Hi Paul,
On Wed, 28 Nov 2018, Paul Morelle wrote:
> On 28/11/18 16:19, Johannes Schindelin wrote:
> >
> > On Wed, 28 Nov 2018, Paul Morelle wrote:
> >
> >> The 'exec' command can be used to run tests on a set of commits,
> >> interrupting on failing commits to let the user fix the tests.
> >>
> >> However, the 'exec' line has been consumed, so it won't be ran again by
> >> 'git rebase --continue' is ran, even if the tests weren't fixed.
> >>
> >> This commit introduces a new command 'test' equivalent to 'exec', except
> >> that it is automatically rescheduled in the todo list if it fails.
> >>
> >> Signed-off-by: Paul Morelle
> > Would it not make more sense to add a command-line option (and a config
> > setting) to re-schedule failed `exec` commands? Like so:
>
> Your proposition would do in most cases, however it is not possible to
> make a distinction between reschedulable and non-reschedulable commands.
True. But I don't think that's so terrible.
What I think is something to avoid is two commands that do something very,
very similar, but with two very, very different names.
In reality, I think that it would even make sense to change the default to
reschedule failed `exec` commands. Which is why I suggested to also add a
config option.
> Do you think that we can ignore the distinction between reschedulable
> and non-reschedulable commands in the same script?
Yes, I think that there *should not* be any distinction. It would just
make it harder to use the feature (users would have to keep in mind that
one version reschedules, one version does not).
> In this case, I would add some tests to your patch below, and propose
> the result on this mailing list.
I already added a test... See the reschedule-failed-exec branch on
https://github.com/dscho/git.
And I had another idea. To make this feature more useful for *myself*, I
would like to introduce the `-X` option as a shortcut for
`--reschedule-failed-exec -x`, e.g.
git rebase -X "make -j15 test"
What do you think?
Johannes
>
> > -- snip --
> > diff --git a/builtin/rebase--interactive.c b/builtin/rebase--interactive.c
> > index a2ab68ed0632..a9ab009fdbca 100644
> > --- a/builtin/rebase--interactive.c
> > +++ b/builtin/rebase--interactive.c
> > @@ -192,6 +192,8 @@ int cmd_rebase__interactive(int argc, const char
> > **argv, const char *prefix)
> > OPT_STRING(0, "onto-name", _name, N_("onto-name"),
> > N_("onto name")),
> > OPT_STRING(0, "cmd", , N_("cmd"), N_("the command to run")),
> > OPT_RERERE_AUTOUPDATE(_rerere_auto),
> > + OPT_BOOL(0, "reschedule-failed-exec",
> > _failed_exec,
> > +N_("automatically re-schedule any `exec` that fails")),
> > OPT_END()
> > };
> >
> > diff --git a/builtin/rebase.c b/builtin/rebase.c
> > index 5b3e5baec8a0..700cbc4e150e 100644
> > --- a/builtin/rebase.c
> > +++ b/builtin/rebase.c
> > @@ -104,6 +104,7 @@ struct rebase_options {
> > int rebase_merges, rebase_cousins;
> > char *strategy, *strategy_opts;
> > struct strbuf git_format_patch_opt;
> > + int reschedule_failed_exec;
> > };
> >
> > static int is_interactive(struct rebase_options *opts)
> > @@ -415,6 +416,8 @@ static int run_specific_rebase(struct rebase_options
> > *opts)
> > argv_array_push(, opts->gpg_sign_opt);
> > if (opts->signoff)
> > argv_array_push(, "--signoff");
> > + if (opts->reschedule_failed_exec)
> > + argv_array_push(,
> > "--reschedule-failed-exec");
> >
> > status = run_command();
> > goto finished_rebase;
> > @@ -903,6 +906,9 @@ int cmd_rebase(int argc, const char **argv, const char
> > *prefix)
> >"strategy")),
> > OPT_BOOL(0, "root", ,
> > N_("rebase all reachable commits up to the root(s)")),
> > + OPT_BOOL(0, "reschedule-failed-exec",
> > +_failed_exec,
> > +N_("automatically re-schedule any `exec` that fails")),
> > OPT_END(),
> > };
> > int i;
> > @@ -1195,6 +1201,9 @@ int cmd_rebase(int argc, const char **argv, const
> > char *prefix)
> > break;
> > }
> >
> > + if (options.reschedule_failed_exec && !is_interactive())
> > + die(_("--reschedule-failed-exec requires an interactive
> > rebase"));
> > +
> > if (options.git_am_opts.argc) {
> > /* all am options except -q are compatible only with --am */
> > for (i = options.git_am_opts.argc - 1; i >= 0; i--)
> > diff --git a/sequencer.c b/sequencer.c
> > index e1a4dd15f1a8..69bee63e116f 100644
> > --- a/sequencer.c
> > +++ b/sequencer.c
> > @@ -158,6 +158,7 @@ static GIT_PATH_FUNC(rebase_path_strategy,
> > "rebase-merge/strategy")
> > static GIT_PATH_FUNC(rebase_path_strategy_opts,
> > "rebase-merge/strategy_opts")
> > static
