Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow git difftool --no-index to run outside of a worktree #2175

Merged
merged 6 commits into from
May 8, 2019

Conversation

dscho
Copy link
Member

@dscho dscho commented Apr 30, 2019

This addresses #2123

(Well, maybe I should try to get git difftool --no-index --dir-diff -- ../1 ../2 to work, but it seems like that project would be quite a bit more involved than I can afford right now...)

We will always spawn something from `git difftool`, so we will always
have to set `GIT_DIR` and `GIT_WORK_TREE`.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
@dscho dscho added this to the v2.21.0(2) milestone Apr 30, 2019
@dscho
Copy link
Member Author

dscho commented May 6, 2019

@FDpellet thank you for approving these changes. However, as this seems to be your first contribution to Git, I am not exactly sure on which bases you reviewed them. Did you test a locally-built Git? Did you review the actual code? Did you leave your check mark only as a joke?

dscho added 5 commits May 8, 2019 21:01
`OPT_ARGUMENT()` is intended to keep the specified long option in `argv`
and not to do anything else.

However, it would make a lot of sense for the caller to know whether
this option was seen at all or not. For example, we want to teach `git
difftool` to work outside of any Git worktree, but only when
`--no-index` was specified.

Note: nothing in Git uses OPT_ARGUMENT(). Even worse, looking through
the commit history, one can easily see that nothing even
ever used it, apart from the regression test.

So not only do we make `OPT_ARGUMENT()` more useful, we are also about
to introduce its first real user!

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
As far as this developer can tell, the conversion from a Perl script to
a built-in caused the regression in the difftool that it no longer runs
outside of a Git worktree (with `--no-index`, of course).

It is a bit embarrassing that it took over two years after retiring the
Perl version to discover this regression, but at least we now know, and
can do something, about it.

This fixes git-for-windows#2123

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
In `--no-index` mode, we now no longer require a worktree nor a
repository. But some code paths in `difftool` expect those to be
present.

The most notable such code path is the `--dir-diff` one: we use the
existing checkout machinery to copy the files, and that machinery looks
up replacement refs, looks at alternate ODBs, wants to use the worktree
path, etc.

Rather than running into segmentation faults, let's die with an
informative error message.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
This fixes git-for-windows#2123

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
This patch addresses the segmentation faults in `git difftool --no-index
--dir-diff`: surprisingly, those two options don't make no sense
together.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
@dscho dscho force-pushed the difftool-no-index branch from 03a2c4a to 290a666 Compare May 8, 2019 19:02
@dscho dscho merged commit 65f1570 into git-for-windows:master May 8, 2019
@dscho dscho deleted the difftool-no-index branch May 8, 2019 21:54
git-for-windows-ci pushed a commit that referenced this pull request May 8, 2019
Allow `git difftool --no-index` to run outside of a worktree
dscho added a commit that referenced this pull request May 9, 2019
Allow `git difftool --no-index` to run outside of a worktree
dscho added a commit that referenced this pull request May 9, 2019
Allow `git difftool --no-index` to run outside of a worktree
dscho added a commit that referenced this pull request May 9, 2019
Allow `git difftool --no-index` to run outside of a worktree
dscho added a commit to git-for-windows/build-extra that referenced this pull request May 9, 2019
`git difftool --no-index` [can now be run outside of Git
worktrees](git-for-windows/git#2175).

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
git-for-windows-ci pushed a commit that referenced this pull request May 9, 2019
Allow `git difftool --no-index` to run outside of a worktree
dscho added a commit that referenced this pull request May 10, 2019
Allow `git difftool --no-index` to run outside of a worktree
dscho added a commit that referenced this pull request May 10, 2019
Allow `git difftool --no-index` to run outside of a worktree
dscho added a commit that referenced this pull request May 10, 2019
Allow `git difftool --no-index` to run outside of a worktree
git-for-windows-ci pushed a commit that referenced this pull request May 10, 2019
Allow `git difftool --no-index` to run outside of a worktree
git-for-windows-ci pushed a commit that referenced this pull request May 10, 2019
Allow `git difftool --no-index` to run outside of a worktree
git-for-windows-ci pushed a commit that referenced this pull request May 10, 2019
Allow `git difftool --no-index` to run outside of a worktree
dscho added a commit that referenced this pull request May 13, 2019
Allow `git difftool --no-index` to run outside of a worktree
dscho added a commit that referenced this pull request May 13, 2019
Allow `git difftool --no-index` to run outside of a worktree
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant