* Re: RFC: Another proposed hash function transition plan
From: Johannes Schindelin @ 2017-03-08 15:40 UTC (permalink / raw)
To: Ian Jackson
Cc: Linus Torvalds, Jonathan Nieder, Git Mailing List, Stefan Beller,
bmwill, Jonathan Tan, Jeff King
In-Reply-To: <22719.59633.269164.986923@chiark.greenend.org.uk>
Hi Ian,
On Wed, 8 Mar 2017, Ian Jackson wrote:
> Linus Torvalds writes ("Re: RFC: Another proposed hash function transition plan"):
> > Of course, having written that, I now realize how it would cause
> > problems for the usual shit-for-brains case-insensitive filesystems.
> > So I guess base64 encoding doesn't work well for that reason.
>
> AFAIAA object names occur in publicly-visible filenames only in notes
> tree objects, which are manipulated by git internally and do not
> necessarily need to appear in the filesystem.
>
> The filenames in .git/objects/ can be in whatever encoding we like, so
> are not an obstacle.
Given that the idea was to encode the new hash in base64 or base85, we
*are* talking about an encoding. In that respect, yes, it can be whatever
encoding we like, and Linus just made a good point (with unnecessary foul
language) of explaining why base64/base85 is not that encoding.
Ciao,
Johannes
^ permalink raw reply
* Re: RFC: Another proposed hash function transition plan
From: Johannes Schindelin @ 2017-03-08 15:37 UTC (permalink / raw)
To: Ian Jackson
Cc: Linus Torvalds, Jonathan Nieder, Git Mailing List, Stefan Beller,
bmwill, Jonathan Tan, Jeff King
In-Reply-To: <22719.59633.269164.986923@chiark.greenend.org.uk>
Hi Ian,
On Wed, 8 Mar 2017, Ian Jackson wrote:
> Few people use uppercase in ref names because of the case-insensitive
> filesystem problem;
Not true.
Ciao,
Johannes
^ permalink raw reply
* Re: Crash on MSYS2 with GIT_WORK_TREE
From: Johannes Schindelin @ 2017-03-08 15:34 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Brandon Williams, Stefan Beller, git
In-Reply-To: <xmqqa88w4bbp.fsf@gitster.mtv.corp.google.com>
Hi Junio,
On Tue, 7 Mar 2017, Junio C Hamano wrote:
> Junio C Hamano <gitster@pobox.com> writes:
>
> > OK, so it appears that we'd better audit all the callsites of
> > real_pathdup() and see if anybody _assumes_ that the return values are
> > not NULL. They all need fixing.
Indeed.
> I just looked at 4ac9006f ("real_path: have callers use real_pathdup and
> strbuf_realpath", 2016-12-12) and it seems all hunks that replaces
> xstrdup(real_path(...)) with real_pathdup(...) in the commit share the
> same issue.
Right, I tried to convey that information in my email to which you
replied.
> The one in canonicalize_ceiling_entry() looks OK, though.
Yes, it immediately tests whether NULL was returned.
> ec9629b3 ("submodule absorbing: fix worktree/gitdir pointers
> recursively for non-moves", 2017-01-25) introduces a new use of
> real_pathdup() and the result is immediately used to call
> connect_work_tree_and_git_dir() without checking its NULL-ness, but
> the argument to new_git_dir is something that came from git_path()
> that was successfully passed to safe_create_leading_directories(),
> so this one should be OK.
>
> 1c16df23 ("Merge branch 'bw/realpath-wo-chdir'", 2017-01-18) turns a
> few xstrdup(real_path(...)) in dir.c without thinking. I think that
> evil merge probably should be reverted.
Rather than a heavy-handed reversal, I would really prefer to perform a
diligent audit of all real_pathdup() callers and adjust them
appropriately.
Turns out that the canonicalize_ceiling_entry() caller is *the only one*
handling NULL correctly. All other callers need to be changed.
Will send something out in a moment.
Ciao,
Johannes
^ permalink raw reply
* [PATCH] t2027: avoid using pipes
From: Prathamesh Chavan @ 2017-03-08 15:13 UTC (permalink / raw)
To: git
The exit code of the upstream of a pipe is ignored thus we should avoid
using it. By writing out the output of the git command to a file, we
can test the exit codes of both the commands.
Signed-off-by: Prathamesh <pc44800@gmail.com>
---
t/t2027-worktree-list.sh | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/t/t2027-worktree-list.sh b/t/t2027-worktree-list.sh
index 848da5f..daa7a04 100755
--- a/t/t2027-worktree-list.sh
+++ b/t/t2027-worktree-list.sh
@@ -31,7 +31,7 @@ test_expect_success '"list" all worktrees from main' '
test_when_finished "rm -rf here && git worktree prune" &&
git worktree add --detach here master &&
echo "$(git -C here rev-parse --show-toplevel) $(git rev-parse --short HEAD) (detached HEAD)" >>expect &&
- git worktree list | sed "s/ */ /g" >actual &&
+ git worktree list >out && sed "s/ */ /g" <out >actual &&
test_cmp expect actual
'
@@ -40,7 +40,7 @@ test_expect_success '"list" all worktrees from linked' '
test_when_finished "rm -rf here && git worktree prune" &&
git worktree add --detach here master &&
echo "$(git -C here rev-parse --show-toplevel) $(git rev-parse --short HEAD) (detached HEAD)" >>expect &&
- git -C here worktree list | sed "s/ */ /g" >actual &&
+ git -C here worktree list >out && sed "s/ */ /g" <out >actual &&
test_cmp expect actual
'
@@ -73,7 +73,7 @@ test_expect_success '"list" all worktrees from bare main' '
git -C bare1 worktree add --detach ../there master &&
echo "$(pwd)/bare1 (bare)" >expect &&
echo "$(git -C there rev-parse --show-toplevel) $(git -C there rev-parse --short HEAD) (detached HEAD)" >>expect &&
- git -C bare1 worktree list | sed "s/ */ /g" >actual &&
+ git -C bare1 worktree list >out && sed "s/ */ /g" <out >actual &&
test_cmp expect actual
'
@@ -96,7 +96,7 @@ test_expect_success '"list" all worktrees from linked with a bare main' '
git -C bare1 worktree add --detach ../there master &&
echo "$(pwd)/bare1 (bare)" >expect &&
echo "$(git -C there rev-parse --show-toplevel) $(git -C there rev-parse --short HEAD) (detached HEAD)" >>expect &&
- git -C there worktree list | sed "s/ */ /g" >actual &&
+ git -C there worktree list >out && sed "s/ */ /g" <out >actual &&
test_cmp expect actual
'
@@ -118,9 +118,9 @@ test_expect_success 'broken main worktree still at the top' '
cd linked &&
echo "worktree $(pwd)" >expected &&
echo "ref: .broken" >../.git/HEAD &&
- git worktree list --porcelain | head -n 3 >actual &&
+ git worktree list --porcelain >out && head -n 3 out >actual &&
test_cmp ../expected actual &&
- git worktree list | head -n 1 >actual.2 &&
+ git worktree list >out && head -n 1 out >actual.2 &&
grep -F "(error)" actual.2
)
'
@@ -134,7 +134,7 @@ test_expect_success 'linked worktrees are sorted' '
test_commit new &&
git worktree add ../first &&
git worktree add ../second &&
- git worktree list --porcelain | grep ^worktree >actual
+ git worktree list --porcelain >out && grep ^worktree out >actual
) &&
cat >expected <<-EOF &&
worktree $(pwd)/sorted/main
--
https://github.com/git/git/pull/336
^ permalink raw reply related
* Re: diff.ignoreSubmoudles config setting broken?
From: Sebastian Schuberth @ 2017-03-08 15:07 UTC (permalink / raw)
To: Jeff King; +Cc: Git Mailing List, Stefan Beller, Jens.Lehmann
In-Reply-To: <20170308140110.wgdedquqwm75zws2@sigill.intra.peff.net>
On Wed, Mar 8, 2017 at 3:01 PM, Jeff King <peff@peff.net> wrote:
>> > Hrm. Isn't "all" the default? That's what git-diff(1) says (but I've
>> > never used the feature myself).
>> >
>> > That would imply to me that there's another config option set somewhere
>> > (perhaps in the repo-level config). What does:
>> >
>> > git config --show-origin --get-all diff.ignoresubmodules
>> >
>> > say?
>>
>> It says:
>>
>> file:/home/seschube/.gitconfig all
>
> OK, that looks right, so my guess is probably the wrong direction.
> Peeking at the code, it looks like there may be some per-submodule
> magic, but I don't know how it all works. So I'll stop looking and wait
> for somebody more clueful to respond.
+ Jens
--
Sebastian Schuberth
^ permalink raw reply
* Re: [PATCH] t*: avoid using pipes
From: Prathamesh Chavan @ 2017-03-08 13:32 UTC (permalink / raw)
To: Jeff King; +Cc: Stefan Beller, Johannes Sixt, git@vger.kernel.org, Pranit Bauva
In-Reply-To: <20170308060334.6ilcjgaxgycuhpxu@sigill.intra.peff.net>
On Wed, Mar 8, 2017 at 11:33 AM, Jeff King <peff@peff.net> wrote:
> On Tue, Mar 07, 2017 at 12:52:49PM -0800, Stefan Beller wrote:
>
>> On Tue, Mar 7, 2017 at 12:39 PM, Johannes Sixt <j6t@kdbg.org> wrote:
>>
>> > Welcome to the Git community!
>>
>> >
>> > Actually, being a *micro* project, it should stay so. Not doing all of the
>> > changes would leave some tasks for other apprentices to get warm with our
>> > review process.
>>
>> right, so just pick one file.
>
> I also wonder if we really want all invocations of git to be marked up
> in this way. If the primary goal of the test is checking that a certain
> git command runs successfully and generates the expected output, then I
> think it is a good candidate for conversion.
>
> So in a hunk like this:
>
> test_expect_success 'git commit-tree records the correct tree in a commit' '
> commit0=$(echo NO | git commit-tree $P) &&
> - tree=$(git show --pretty=raw $commit0 |
> - sed -n -e "s/^tree //p" -e "/^author /q") &&
> + tree=$(git show --pretty=raw $commit0 >out &&
> + sed -n -e "s/^tree //p" -e "/^author /q" <out) &&
> test "z$tree" = "z$P"
>
> we are interested in testing commit-tree, not "git show". Is it worth
> avoiding pipes there? I admit the cost to using the intermediate file is
> not huge there, but it feels more awkward and un-shell-like to me as a
> reader.
>
> -Peff
Thank you everyone, for reviewing my changes. And as said in the
reviews, I'll send a single patch file as my microproject, leaving the other
files as low hanging fruit for the others to look at. Also, I try to include as
many suggested improvements as possible and will also remember them for
my future patches.
^ permalink raw reply
* Re: diff.ignoreSubmoudles config setting broken?
From: Jeff King @ 2017-03-08 13:33 UTC (permalink / raw)
To: Sebastian Schuberth; +Cc: git, Stefan Beller
In-Reply-To: <5e5b1b92-f7c6-2987-356e-1aab2bff557e@gmail.com>
On Wed, Mar 08, 2017 at 01:54:02PM +0100, Sebastian Schuberth wrote:
> I'm getting
>
> $ git config --global diff.ignoreSubmodules all
> $ git diff
> diff --git a/scanners/scancode-toolkit b/scanners/scancode-toolkit
> index 65e5c9c..6b021a8 160000
> --- a/scanners/scancode-toolkit
> +++ b/scanners/scancode-toolkit
> @@ -1 +1 @@
> -Subproject commit 65e5c9c9508441c5f62beff4749cf455c6eadc30
> +Subproject commit 6b021a8addf6d3c5f2a6ef1af6245e095c21d8ec
>
> but with
>
> $ git diff --ignore-submodules=all
Hrm. Isn't "all" the default? That's what git-diff(1) says (but I've
never used the feature myself).
That would imply to me that there's another config option set somewhere
(perhaps in the repo-level config). What does:
git config --show-origin --get-all diff.ignoresubmodules
say?
-Peff
^ permalink raw reply
* Re: diff.ignoreSubmoudles config setting broken?
From: Jeff King @ 2017-03-08 14:01 UTC (permalink / raw)
To: Sebastian Schuberth; +Cc: Git Mailing List, Stefan Beller
In-Reply-To: <CAHGBnuM3iM-kHdxdox_1i56uLbv7gQ5ZUY9Xqf4BG7G_kTf+jQ@mail.gmail.com>
On Wed, Mar 08, 2017 at 02:43:00PM +0100, Sebastian Schuberth wrote:
> > Hrm. Isn't "all" the default? That's what git-diff(1) says (but I've
> > never used the feature myself).
> >
> > That would imply to me that there's another config option set somewhere
> > (perhaps in the repo-level config). What does:
> >
> > git config --show-origin --get-all diff.ignoresubmodules
> >
> > say?
>
> It says:
>
> file:/home/seschube/.gitconfig all
OK, that looks right, so my guess is probably the wrong direction.
Peeking at the code, it looks like there may be some per-submodule
magic, but I don't know how it all works. So I'll stop looking and wait
for somebody more clueful to respond.
-Peff
^ permalink raw reply
* Re: [PATCH v3 0/9] Fix the early config
From: Jeff King @ 2017-03-08 7:30 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git, Junio C Hamano, Duy Nguyen
In-Reply-To: <alpine.DEB.2.20.1703071314180.3767@virtualbox>
On Tue, Mar 07, 2017 at 03:31:43PM +0100, Johannes Schindelin wrote:
> > /*
> > * Find GIT_DIR of the repository that contains the current working
> > * directory, without changing the working directory or other global
> > * state. The result is appended to gitdir. The return value is NULL
> > * if no repository was found, or gitdir->buf otherwise.
> > */
>
> I changed it a little bit more. In particular, I changed the
> discover_git_directory() function to return the pointer to the path
> itself: it provides additional value, and if that is not what the caller
> wants, they can use git_dir->buf just as well.
Yes, that makes much more sense.
> There is one more thing I included in v4: when I (re-)implemented that
> pre-command/post-command hook I was hinting at earlier, the test suite
> identified a problem where an invalid .git file would prevent even `git
> init` from working (it was actually much more complicated than that, but
> the gist is that `git -p init` would fail, no matter how much sense it
> may make to you to paginate an `init` run, it should still not fail,
> right?). So I added a patch on top to fix that.
Good catch. Another "non-gentle" thing I noticed here while looking at
another thread: the repository-format version check uses the config
parser, which will die() in certain circumstances. So for instance:
$ git init
$ git rev-parse && echo ok
ok
$ echo '[core]repositoryformatversion = 10' >.git/config
$ git rev-parse && echo ok
fatal: Expected git repo version <= 1, found 10
$ echo '[core]repositoryformatversion = foobar' >.git/config
$ git rev-parse && echo ok
fatal: bad numeric config value 'foobar' for 'core.repositoryformatversion' in file .git/config: invalid unit
$ echo '[co' >.git/config
$ git rev-parse && echo ok
fatal: bad config line 1 in file .git/config
Your series correctly avoids the first failure by calling the
read/verify_repository_format functions correctly. But I think it would
get tripped up by the other two.
Fixing the first one is probably not too hard; check_repo_format()
should use a more forgiving parser than git_config_int().
The second one I thought would be tricky, but it looks like we added a
die_on_error flag in b2dc09455. That does what we want, but it needs to
be plumbed through to git_config_from_file().
> And another change: the GIT_DIR_NONE value was handled incorrectly in
> discover_git_directory().
This is the "if (setup_git_directory_1() <= 0)" change from the
interdiff? That's subtle. The compiler could have noticed if we used a
switch statement here. But then any new error conditions would have to
be added to that switch statement.
> I am slightly disappointed that the these additional problems were not
> spotted in any review but my own. And I had not even included a Duck.
Get used to being disappointed, I guess. A non-zero number of bugs will
slip through when writing code _and_ when reviewing it.
> [ceil_offset]
> Hopefully that clears up the picture?
Yes, it does. Thanks.
-Peff
^ permalink raw reply
* diff.ignoreSubmoudles config setting broken?
From: Sebastian Schuberth @ 2017-03-08 12:54 UTC (permalink / raw)
To: git; +Cc: Stefan Beller
Hi,
with
$ git --version
git version 2.12.0.windows.1
I'm getting
$ git config --global diff.ignoreSubmodules all
$ git diff
diff --git a/scanners/scancode-toolkit b/scanners/scancode-toolkit
index 65e5c9c..6b021a8 160000
--- a/scanners/scancode-toolkit
+++ b/scanners/scancode-toolkit
@@ -1 +1 @@
-Subproject commit 65e5c9c9508441c5f62beff4749cf455c6eadc30
+Subproject commit 6b021a8addf6d3c5f2a6ef1af6245e095c21d8ec
but with
$ git diff --ignore-submodules=all
I'm getting the expected empty output.
I can reproduce the same on Linux with "git version 2.11.0". Am I missing something, or is this a bug?
Regards,
Sebastian
^ permalink raw reply related
* Re: RFC: Another proposed hash function transition plan
From: Ian Jackson @ 2017-03-08 11:20 UTC (permalink / raw)
To: Linus Torvalds
Cc: Jonathan Nieder, Git Mailing List, Stefan Beller, bmwill,
Jonathan Tan, Jeff King
In-Reply-To: <CA+55aFyyi0vBBApf9grYQzF2PRZMjtCzkB4LzYvLpqQ-Z7QfJQ@mail.gmail.com>
Linus Torvalds writes ("Re: RFC: Another proposed hash function transition plan"):
> Also, since 256 isn't evenly divisible by 6, and because you'd want
> some way to explictly disambiguate the new hashes, the rule *could* be
> that the ASCII representation of a new hash is the base64 encoding of
> the 258-bit value that has "10" prepended to it as padding.
>
> That way the first character of the hash would be guaranteed to not be
> a hex digit, because it would be in the range [g-v] (indexes 32..47).
We should arrange for this to be an uppercase, not a lowercase,
letter, for the reasons I explained in my own proposal. To summarise:
It would be undesirable to further increase the overlap between object
names and ref names. Few people use uppercase in ref names because of
the case-insensitive filesystem problem; so object names starting with
uppercase ascii are distinct from most object names.
> Of course, having written that, I now realize how it would cause
> problems for the usual shit-for-brains case-insensitive filesystems.
> So I guess base64 encoding doesn't work well for that reason.
AFAIAA object names occur in publicly-visible filenames only in notes
tree objects, which are manipulated by git internally and do not
necessarily need to appear in the filesystem.
The filenames in .git/objects/ can be in whatever encoding we like, so
are not an obstacle.
Ian.
--
Ian Jackson <ijackson@chiark.greenend.org.uk> These 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.
^ permalink raw reply
* Re: [PATCH 4/6] send-pack: improve unpack-status error messages
From: Jeff King @ 2017-03-08 5:45 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Horst Schirmeier, git
In-Reply-To: <xmqq37eo66hw.fsf@gitster.mtv.corp.google.com>
On Tue, Mar 07, 2017 at 02:56:27PM -0800, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
>
> > When the remote tells us that the "unpack" step failed, we
> > show an error message. However, unless you are familiar with
> > the internals of send-pack and receive-pack, it was not
> > clear that this represented an error on the remote side.
> > Let's re-word to make that more obvious.
> >
> > Likewise, when we got an unexpected packet from the other
> > end, we complained with a vague message but did not actually
> > show the packet. Let's fix that.
>
> Both make sense.
>
> > And finally, neither message was marked for translation. The
> > message from the remote probably won't be translated, but
> > there's no reason we can't do better for the local half.
>
> Hmm, OK.
I'll admit that I don't actually use the translations myself, being a
native English speaker. So I am just guessing that somebody for whom
English is a second language would rather see the first half in a more
intelligible format. That at least tells them what the second half _is_,
so they might be able to search for the error with more context.
If my guess is wrong, though, I'm happy to retract that part or bump it
out to a separate patch.
-Peff
^ permalink raw reply
* Re: Crash on MSYS2 with GIT_WORK_TREE
From: Johannes Schindelin @ 2017-03-08 12:03 UTC (permalink / raw)
To: valtron; +Cc: git, Brandon Williams
In-Reply-To: <CAFKRc7ysOAOVx-7ww7MLF1qKpuKdJQqAtAhLJcYh3yMD3G2ncA@mail.gmail.com>
Hi valtron,
On Tue, 7 Mar 2017, valtron wrote:
> I only ran into this because of git-gui, where I eventually tracked it
> down to line 1330:
>
> set env(GIT_WORK_TREE) $_gitworktree
As git-gui is a Tcl script, which in turn runs as a pure Windows
application, the path should use backslashes.
> With that line commented out, it works. I'll look into why git-gui
> sets it to a windows-path-with-forward-slashes, but that's a separate
> issue from the crash.
It is... please do contribute your fix when you have one.
> Also, from the stack trace, I think git is still able to understand the
> path, since it appears to correctly convert it to /c/repo, but I might
> be wrong since I haven't look at the code.
Git does not convert the path at all. It is the *MSYS2 runtime* that
converts Windows paths to POSIX paths, if any. And it does so selectively.
The current working directory is always transformed. PATH is always
transformed. Environment variables are transformed *when they look like
Windows paths*. And I am fairly certain that a GIT_WORK_TREE with a colon
and forward-slashes fails that test: the MSYS2 runtime thinks this is not
a Windows path and leaves it alone. Enter your problem.
Ciao,
Johannes
^ permalink raw reply
* Re: Crash on MSYS2 with GIT_WORK_TREE
From: Johannes Schindelin @ 2017-03-08 11:59 UTC (permalink / raw)
To: Brandon Williams; +Cc: valtron, git
In-Reply-To: <20170308020918.GA1650@google.com>
Hi Brandon,
On Tue, 7 Mar 2017, Brandon Williams wrote:
> On 03/08, Johannes Schindelin wrote:
> >
> > [...] On *Linux*, this happens:
> >
> > $ GIT_WORK_TREE=c:/invalid git rev-parse HEAD
> > Segmentation fault (core dumped)
> >
> > The reason is this: when set_git_work_tree() was converted from using
> > xstrdup(real_path()) to real_pathdup(), we completely missed the fact
> > that the former passed die_on_error = 1 to strbuf_realpath(), while
> > the latter passed die_on_error = 0. As a consequence, work_tree can be
> > NULL now, and the current code does not expect set_git_work_tree() to
> > return successfully after setting work_tree to NULL.
> >
> > I Cc:ed Brandon, the author of 4ac9006f832 (real_path: have callers
> > use real_pathdup and strbuf_realpath, 2016-12-12).
> >
> > Brandon, I have a hunch that pretty much all of the
> > xstrdup(real_path()) -> real_pathdup() sites have a problem now. The
> > previous contract was that real_path() would die() if the passed path
> > is invalid. The new contract is that real_pathdup() returns NULL in
> > such a case. I believe that the following call sites are problematic
> > in particular:
>
> Welp, looks like I missed that when I made the conversion. You're
> right, the semantics of getting the real_path were changed which would
> cause a NULL to be returned instead of the program exiting with a call
> to die().
>
> After a cursory look at your patch, I think all of your changes look
> sane. I would have to take a closer look at the call sites to see if
> each caller would need to die or not. I'm assuming you took a quick
> glace to make your decision about each call site?
I did take a quick glance, but did you have a look at the time of day I
sent this patch? You do not want to trust my judgement after that.
Another thing: may I ask you to delete the quoted parts of the mail that
you are actually not responding to? Junio also often simply keeps the rest
of the mail quoted, and I always have to scroll all the way to the end
just to verify that nothing more has been said, which can be slightly
annoying when you are tired. I do plan to read your mails in the future,
so culling the quoted-yet-unanswered part would save me trouble.
Thanks,
Dscho
^ permalink raw reply
* [PATCH] diff: allow "-" as a short-hand for "last branch"
From: mash @ 2017-03-08 9:50 UTC (permalink / raw)
To: Git Mailing List
Cc: Junio C Hamano, Vegard Nossum, Štěpán Němec
Just like "git merge -" is a short-hand for "git merge @{-1}" to
conveniently merge the previous branch, "git diff -" is a short-hand for
"git diff @{-1}" to conveniently diff against the previous branch.
Allow the usage of "-" in the dot dot notation to allow the use of
"git diff -..HEAD^" as a short-hand for "git diff @{-1}..HEAD^".
Signed-off-by: mash <mash+git@crossperf.com>
---
This is a GSoC microproject. I'm not sure how useful this change is.
Please review it and take it apart.
I'm not very happy with the change of handle_revision_arg.
Maybe I should teach sha1_name.c:get_sha1_basic how to handle a dash
instead.
Documentation was not updated. I could only think of updating revisions.txt but
that might be misleading since the use of dash does not work everywhere.
revision.c | 22 ++++++++++++++++++--
t/t4063-diff-last.sh | 58 ++++++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 78 insertions(+), 2 deletions(-)
create mode 100755 t/t4063-diff-last.sh
diff --git a/revision.c b/revision.c
index b37dbec..c331bd5 100644
--- a/revision.c
+++ b/revision.c
@@ -1439,6 +1439,7 @@ int handle_revision_arg(const char *arg_, struct rev_info *revs, int flags, unsi
const char *arg = arg_;
int cant_be_filename = revarg_opt & REVARG_CANNOT_BE_FILENAME;
unsigned get_sha1_flags = 0;
+ static const char previous_branch[] = "@{-1}";
flags = flags & UNINTERESTING ? flags | BOTTOM : flags & ~BOTTOM;
@@ -1457,6 +1458,8 @@ int handle_revision_arg(const char *arg_, struct rev_info *revs, int flags, unsi
if (!*next)
next = head_by_default;
+ else if (!strcmp(next, "-"))
+ next = previous_branch;
if (dotdot == arg)
this = head_by_default;
if (this == head_by_default && next == head_by_default &&
@@ -1469,6 +1472,8 @@ int handle_revision_arg(const char *arg_, struct rev_info *revs, int flags, unsi
*dotdot = '.';
return -1;
}
+ } else if (!strcmp(this, "-")) {
+ this = previous_branch;
}
if (!get_sha1_committish(this, from_sha1) &&
!get_sha1_committish(next, sha1)) {
@@ -1568,6 +1573,8 @@ int handle_revision_arg(const char *arg_, struct rev_info *revs, int flags, unsi
if (revarg_opt & REVARG_COMMITTISH)
get_sha1_flags = GET_SHA1_COMMITTISH;
+ if (!strcmp(arg, "-"))
+ arg = previous_branch;
if (get_sha1_with_context(arg, get_sha1_flags, sha1, &oc))
return revs->ignore_missing ? 0 : -1;
if (!cant_be_filename)
@@ -1578,6 +1585,15 @@ int handle_revision_arg(const char *arg_, struct rev_info *revs, int flags, unsi
return 0;
}
+/*
+ * Check if the argument is supposed to be a revision argument instead of an
+ * option even though it starts with a dash.
+ */
+static int is_revision_arg(const char *arg)
+{
+ return *arg == '\0' || starts_with(arg, "..");
+}
+
struct cmdline_pathspec {
int alloc;
int nr;
@@ -1621,7 +1637,9 @@ static void read_revisions_from_stdin(struct rev_info *revs,
seen_dashdash = 1;
break;
}
- die("options not supported in --stdin mode");
+ if (!is_revision_arg(sb.buf + 1)) {
+ die("options not supported in --stdin mode");
+ }
}
if (handle_revision_arg(sb.buf, revs, 0,
REVARG_CANNOT_BE_FILENAME))
@@ -2205,7 +2223,7 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s
read_from_stdin = 0;
for (left = i = 1; i < argc; i++) {
const char *arg = argv[i];
- if (*arg == '-') {
+ if (*arg == '-' && !is_revision_arg(arg + 1)) {
int opts;
opts = handle_revision_pseudo_opt(submodule,
diff --git a/t/t4063-diff-last.sh b/t/t4063-diff-last.sh
new file mode 100755
index 0000000..1f635cb
--- /dev/null
+++ b/t/t4063-diff-last.sh
@@ -0,0 +1,58 @@
+#!/bin/sh
+
+test_description='diff against last branch'
+
+. ./test-lib.sh
+
+test_expect_success 'setup' '
+ echo hello >world &&
+ git add world &&
+ git commit -m initial &&
+ git branch other &&
+ echo "hello again" >>world &&
+ git add world &&
+ git commit -m second
+'
+
+test_expect_success '"diff -" does not work initially' '
+ test_must_fail git diff -
+'
+
+test_expect_success '"diff -" diffs against previous branch' '
+ git checkout other &&
+
+ cat <<-\EOF >expect &&
+ diff --git a/world b/world
+ index c66f159..ce01362 100644
+ --- a/world
+ +++ b/world
+ @@ -1,2 +1 @@
+ hello
+ -hello again
+ EOF
+
+ git diff - >out &&
+ test_cmp expect out
+'
+
+test_expect_success '"diff -.." diffs against previous branch' '
+ git diff -.. >out &&
+ test_cmp expect out
+'
+
+test_expect_success '"diff ..-" diffs inverted' '
+ cat <<-\EOF >expect &&
+ diff --git a/world b/world
+ index ce01362..c66f159 100644
+ --- a/world
+ +++ b/world
+ @@ -1 +1,2 @@
+ hello
+ +hello again
+ EOF
+
+ git diff ..- >out &&
+ test_cmp expect out
+'
+
+test_done
--
2.9.3
^ permalink raw reply related
* Re: [PATCHv2] rev-parse: add --show-superproject-working-tree
From: Junio C Hamano @ 2017-03-08 1:30 UTC (permalink / raw)
To: Stefan Beller
Cc: SZEDER Gábor, Benjamin Fuchs, Git Mailing List,
brian m. carlson, Ville Skyttä
In-Reply-To: <20170308005615.20321-1-sbeller@google.com>
Looks more or less right but invoke "ls-files -z" and reading the \0
delimited output would be easier; otherwise you would have to worry
about c-unquoting the pathname when the submodule is bound at a path
with funny character (like a double-quote) in it.
Also, returning the exact string of the path from the API function is
absolutely the right thing. I however have to wonder if rev-parse need
to do the c-quoting unless it is told to show pathnames in its output
without quoting (perhaps with "-z")? Or are paths from "rev-parse"
(like "--git-dir", "--show-toplevel", etc.) already excempt from the
usual quoting rules---if so, doing puts() and nothing else is fine to
be consistent with the existing practice (in the longer term, I am
sure we would need to revisit so that scripts can handle paths with
funny characters sensibly, but that would be a different topic if
existing ones like "--git-dir" are already unsafe).
Sorry for top-posting (I am not on a terminal right now).
On Tue, Mar 7, 2017 at 4:56 PM, Stefan Beller <sbeller@google.com> wrote:
> In some situations it is useful to know if the given repository
> is a submodule of another repository.
>
> Add the flag --show-superproject-working-tree to git-rev-parse
> to make it easy to find out if there is a superproject.
>
> Signed-off-by: Stefan Beller <sbeller@google.com>
> ---
>
> * not RFC anymore, but for real this time; including a test and docs :)
>
> * Following Junios advice: there is only one function
> (superproject_exists was dropped) using ls-files.
> (the test actually tests for a staged submodule)
>
> Thanks,
> Stefan
>
>
> Documentation/git-rev-parse.txt | 5 +++
> builtin/rev-parse.c | 7 ++++
> submodule.c | 83 +++++++++++++++++++++++++++++++++++++++++
> submodule.h | 8 ++++
> t/t1500-rev-parse.sh | 14 +++++++
> 5 files changed, 117 insertions(+)
>
> diff --git a/Documentation/git-rev-parse.txt b/Documentation/git-rev-parse.txt
> index 91c02b8c85..b841bad7c7 100644
> --- a/Documentation/git-rev-parse.txt
> +++ b/Documentation/git-rev-parse.txt
> @@ -261,6 +261,11 @@ print a message to stderr and exit with nonzero status.
> --show-toplevel::
> Show the absolute path of the top-level directory.
>
> +--show-superproject-working-tree
> + Show the absolute path of the top-level directory of
> + the superproject. A superproject is a repository that records
> + this repository as a submodule.
> +
> --shared-index-path::
> Show the path to the shared index file in split index mode, or
> empty if not in split-index mode.
> diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
> index e08677e559..2549643267 100644
> --- a/builtin/rev-parse.c
> +++ b/builtin/rev-parse.c
> @@ -12,6 +12,7 @@
> #include "diff.h"
> #include "revision.h"
> #include "split-index.h"
> +#include "submodule.h"
>
> #define DO_REVS 1
> #define DO_NOREV 2
> @@ -779,6 +780,12 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)
> puts(work_tree);
> continue;
> }
> + if (!strcmp(arg, "--show-superproject-working-tree")) {
> + const char *superproject = get_superproject_working_tree();
> + if (superproject)
> + puts(superproject);
> + continue;
> + }
> if (!strcmp(arg, "--show-prefix")) {
> if (prefix)
> puts(prefix);
> diff --git a/submodule.c b/submodule.c
> index 3b98766a6b..06473d3646 100644
> --- a/submodule.c
> +++ b/submodule.c
> @@ -1514,3 +1514,86 @@ void absorb_git_dir_into_superproject(const char *prefix,
> strbuf_release(&sb);
> }
> }
> +
> +const char *get_superproject_working_tree(void)
> +{
> + struct child_process cp = CHILD_PROCESS_INIT;
> + struct strbuf sb = STRBUF_INIT;
> + const char *one_up = real_path_if_valid("../");
> + const char *cwd = xgetcwd();
> + const char *ret = NULL;
> + const char *subpath;
> + int code;
> + ssize_t len;
> +
> + if (!is_inside_work_tree())
> + /*
> + * FIXME:
> + * We might have a superproject, but it is harder
> + * to determine.
> + */
> + return NULL;
> +
> + if (!one_up)
> + return NULL;
> +
> + subpath = relative_path(cwd, one_up, &sb);
> +
> + prepare_submodule_repo_env(&cp.env_array);
> + argv_array_pop(&cp.env_array);
> +
> + argv_array_pushl(&cp.args, "--literal-pathspecs", "-C", "..",
> + "ls-files", "--stage", "--full-name", "--", subpath, NULL);
> + strbuf_reset(&sb);
> +
> + cp.no_stdin = 1;
> + cp.no_stderr = 1;
> + cp.out = -1;
> + cp.git_cmd = 1;
> +
> + if (start_command(&cp))
> + die(_("could not start ls-files in .."));
> +
> + len = strbuf_read(&sb, cp.out, PATH_MAX);
> + close(cp.out);
> +
> + if (starts_with(sb.buf, "160000")) {
> + int super_sub_len;
> + int cwd_len = strlen(cwd);
> + char *super_sub, *super_wt;
> +
> + /*
> + * There is a superproject having this repo as a submodule.
> + * The format is <mode> SP <hash> SP <stage> TAB <full name> LF,
> + * First remove LF, then skip up to \t.
> + */
> + strbuf_rtrim(&sb);
> + super_sub = strchr(sb.buf, '\t') + 1;
> +
> + super_sub_len = sb.buf + sb.len - super_sub;
> + if (super_sub_len > cwd_len ||
> + strcmp(&cwd[cwd_len - super_sub_len], super_sub))
> + die (_("BUG: returned path string doesn't match cwd?"));
> +
> + super_wt = xstrdup(cwd);
> + super_wt[cwd_len - super_sub_len] = '\0';
> +
> + ret = real_path(super_wt);
> +
> + free(super_wt);
> + }
> + strbuf_release(&sb);
> +
> + code = finish_command(&cp);
> +
> + if (code == 128)
> + /* '../' is not a git repository */
> + return NULL;
> + if (code == 0 && len == 0)
> + /* There is an unrelated git repository at '../' */
> + return NULL;
> + if (code)
> + die(_("ls-tree returned unexpected return code %d"), code);
> +
> + return ret;
> +}
> diff --git a/submodule.h b/submodule.h
> index 05ab674f06..c8a0c9cb29 100644
> --- a/submodule.h
> +++ b/submodule.h
> @@ -93,4 +93,12 @@ extern void prepare_submodule_repo_env(struct argv_array *out);
> extern void absorb_git_dir_into_superproject(const char *prefix,
> const char *path,
> unsigned flags);
> +
> +/*
> + * Return the absolute path of the working tree of the superproject, which this
> + * project is a submodule of. If this repository is not a submodule of
> + * another repository, return NULL.
> + */
> +extern const char *get_superproject_working_tree(void);
> +
> #endif
> diff --git a/t/t1500-rev-parse.sh b/t/t1500-rev-parse.sh
> index 9ed8b8ccba..03d3c7f6d6 100755
> --- a/t/t1500-rev-parse.sh
> +++ b/t/t1500-rev-parse.sh
> @@ -116,4 +116,18 @@ test_expect_success 'git-path inside sub-dir' '
> test_cmp expect actual
> '
>
> +test_expect_success 'showing the superproject correctly' '
> + git rev-parse --show-superproject-working-tree >out &&
> + test_must_be_empty out &&
> +
> + test_create_repo super &&
> + test_commit -C super test_commit &&
> + test_create_repo sub &&
> + test_commit -C sub test_commit &&
> + git -C super submodule add ../sub dir/sub &&
> + echo $(pwd)/super >expect &&
> + git -C super/dir/sub rev-parse --show-superproject-working-tree >out &&
> + test_cmp expect out
> +'
> +
> test_done
> --
> 2.12.0.190.g6a12a61b77.dirty
>
^ permalink raw reply
* Re: [PATCH] t*: avoid using pipes
From: Jeff King @ 2017-03-08 6:03 UTC (permalink / raw)
To: Stefan Beller
Cc: Johannes Sixt, Prathamesh Chavan, git@vger.kernel.org,
Pranit Bauva
In-Reply-To: <CAGZ79kZhfRiUQndEGB=b34WMCPv0KDjpDix0Ly85aFeyOQAwWA@mail.gmail.com>
On Tue, Mar 07, 2017 at 12:52:49PM -0800, Stefan Beller wrote:
> On Tue, Mar 7, 2017 at 12:39 PM, Johannes Sixt <j6t@kdbg.org> wrote:
>
> > Welcome to the Git community!
>
> >
> > Actually, being a *micro* project, it should stay so. Not doing all of the
> > changes would leave some tasks for other apprentices to get warm with our
> > review process.
>
> right, so just pick one file.
I also wonder if we really want all invocations of git to be marked up
in this way. If the primary goal of the test is checking that a certain
git command runs successfully and generates the expected output, then I
think it is a good candidate for conversion.
So in a hunk like this:
test_expect_success 'git commit-tree records the correct tree in a commit' '
commit0=$(echo NO | git commit-tree $P) &&
- tree=$(git show --pretty=raw $commit0 |
- sed -n -e "s/^tree //p" -e "/^author /q") &&
+ tree=$(git show --pretty=raw $commit0 >out &&
+ sed -n -e "s/^tree //p" -e "/^author /q" <out) &&
test "z$tree" = "z$P"
we are interested in testing commit-tree, not "git show". Is it worth
avoiding pipes there? I admit the cost to using the intermediate file is
not huge there, but it feels more awkward and un-shell-like to me as a
reader.
-Peff
^ permalink raw reply
* Re: [PATCHv2] rev-parse: add --show-superproject-working-tree
From: Junio C Hamano @ 2017-03-08 6:01 UTC (permalink / raw)
To: Stefan Beller; +Cc: szeder.dev, email, git, sandals, ville.skytta
In-Reply-To: <20170308005615.20321-1-sbeller@google.com>
Stefan Beller <sbeller@google.com> writes:
> + if (!strcmp(arg, "--show-superproject-working-tree")) {
> + const char *superproject = get_superproject_working_tree();
> + if (superproject)
> + puts(superproject);
> + continue;
> + }
Returning the exact string of the path from the API function is
absolutely the right thing. I however have to wonder if rev-parse
need to do the c-quoting unless it is told to show pathnames in its
output without quoting (perhaps with "-z"). Paths from "rev-parse"
(like "--git-dir", "--show-toplevel", etc.) already are excempt from
the usual quoting rules, so doing puts() and nothing else is fine to
be consistent with the existing practice, but in the longer term, I
am sure we would need to revisit so that scripts can handle paths
with funny characters sensibly, but that would be a different topic
if existing ones like "--git-dir" are already unsafe.
> if (!strcmp(arg, "--show-prefix")) {
> if (prefix)
> puts(prefix);
> diff --git a/submodule.c b/submodule.c
> index 3b98766a6b..06473d3646 100644
> --- a/submodule.c
> +++ b/submodule.c
> @@ -1514,3 +1514,86 @@ void absorb_git_dir_into_superproject(const char *prefix,
> strbuf_release(&sb);
> }
> }
> +
> +const char *get_superproject_working_tree(void)
> +{
> +...
> + argv_array_pushl(&cp.args, "--literal-pathspecs", "-C", "..",
> + "ls-files", "--stage", "--full-name", "--", subpath, NULL);
> + strbuf_reset(&sb);
> +...
> + if (starts_with(sb.buf, "160000")) {
> + int super_sub_len;
> + int cwd_len = strlen(cwd);
> + char *super_sub, *super_wt;
> +
> + /*
> + * There is a superproject having this repo as a submodule.
> + * The format is <mode> SP <hash> SP <stage> TAB <full name> LF,
> + * First remove LF, then skip up to \t.
> + */
Looks more or less right but invoke "ls-files -z" and reading the \0
delimited output would be easier; otherwise you would have to worry
about c-unquoting the pathname when the submodule is bound at a path
with funny character (like a double-quote) in it.
^ permalink raw reply
* Re: [Request for Documentation] Differentiate signed (commits/tags/pushes)
From: Jeff King @ 2017-03-08 5:41 UTC (permalink / raw)
To: Stefan Beller; +Cc: tom, Matthieu Moy, git@vger.kernel.org, Junio C Hamano
In-Reply-To: <CAGZ79kb=ZwaMeGAu_R1Bjt4KyxKHYnP4U-RgA1of7F05E5CCQg@mail.gmail.com>
On Tue, Mar 07, 2017 at 02:19:19PM -0800, Stefan Beller wrote:
> On Tue, Mar 7, 2017 at 1:23 AM, Jeff King <peff@peff.net> wrote:
> > On Mon, Mar 06, 2017 at 11:59:24AM -0800, Stefan Beller wrote:
> >
> >> What is the difference between signed commits and tags?
> >> (Not from a technical perspective, but for the end user)
> >
> > I think git has never really tried to assign any _meaning_ to the
> > signatures. It implements the technical bits and leaves it up to the
> > user and their workflows to decide what a given signature means.
>
> That is a nihilistic approach? ;)
To some degree. :) I think it is less that we believe in nothing, but
that Git has traditionally been a toolkit on which you could build a
variety of workflows. We wouldn't want to constrain the low-level tools
by building too much policy logic into them. And IMHO, nobody has really
written the high-level tools for signing.
I would be happy if somebody wanted to do so. Either separate tools, or
a suite of options and config that could be used to make the existing
tools help enforce certain policies. E.g., if "git log --show-signature"
were to complain that the signer id does not match the committer ident,
when a certain config option is set.
Having a git-trust-model(7) manpage is probably a good idea, but I think
it's nicer still if users can adopt the trust model with the flip of a
switch.
> Off list I was told
> "just look at Documentation/technical/signature-format.txt;
> it explains all different things that you can sign or have signed
> stuff". But as an end user I refuse to look at that. ;)
I agree that end users should not have to look at it. But I also think
it doesn't actually discuss the policy options (i.e., what the
signatures could "mean"). AFAIK there is not a good discussion of that
anywhere.
> > Signing individual commits _could_ convey meaning (2), but "all history
> > leading up" part is unlikely to be something that the signer needs to
> > enforce on every commit.
>
> I was told signed commits could act as a poor mans
> push certificate (again off list :/).
Yeah, they could. But you should probably just use push certificates.
Which _also_ are missing the high-level workflow bits; I'd be happy to
get GitHub to start recording push signatures somewhere if people know
what they would want to do with them.
The idea was discussed of sticking them in git-notes. But I wonder if it
would be useful to hand them out during upload-pack, so that somebody
fetching has a cryptographic chain back to the original pusher (rather
than trusting the intermediate server).
I think that is slightly complicated because each push certificate only
mentions the refs that were pushed. So verifying the ref state for N
refs might involve up to N separate push certificates (i.e., the last
one that touched each ref, not just the last one overall).
> > In my opinion, the most useful meaning for commit-signing is simply to
> > cryptographically certify the identity of the committer. We don't
> > compare the GPG key ident to the "committer" field, but IMHO that would
> > be a reasonable optional feature for verify-commit (I say optional
> > because we're starting to assign semantics now).
>
> So the signed commit focuses on the committer instead of the content
> (which is what tags are rather used for) ?
I think it's more subtle than that, and gets into the "is a commit a
snapshot or a diff" question. We all know that technically the commit
object points to a snapshot of the tree. But it also points to a parent,
and I think what is interesting here is the change between the parent's
tree and the commit's tree.
So I would take a commit signature to mean that you are the author of
the changes moving from $commit^ to $commit, without making any specific
attestation of the content in $commit^. IOW, you are really signing the
changes and your commit message.
> > I think one of the reasons kernel (and git) developers aren't that
> > interested in signed commits is that they're not really that interesting
> > in a patch workflow. You're verifying the committer, who in this project
> > is invariably Junio, and we just take his word that whatever is in the
> > "author" field is reasonable.
>
> Well in such a workflow Junio could also sign the tip-commits of
> pu/next before pushing, such that we can trust it was really him doing
> the maintenance work and not his evil twin.
Yes, he could. He could also force-push a tag for the current value
(although git does not handle tag-overwriting very well). That's sort of
what push-certificates were meant to do a better job of.
> > So I don't think it's just a checkbox feature. It's a useful thing for
> > certain workflows that really want to be able to attribute individual
> > commits with cryptographic strength.
>
> "certain workflows". :(
>
> See, I really like reading e.g. the "On Re-tagging" section of git-tag
> as it doesn't hand wave around the decisions to make.
OK, less hand-waving. Imagine you are a company with a finite set of
developers, and you issue each developer a gpg key. They all turn on
commit.gpgsign. They all clone from a central GitHub repository, push
back to topic branches in that repository, and then merge via the
website.
What do the commit signatures buy you? If you treat them as "I certify
that I made the changes in this commit", then it gives you a
cryptographic audit trail. When you later find a backdoor in the
software, you are not left guessing as to who pushed it (since they
could easily forge the committer ident in the object header), or even
whether it entered the repository via a push or if somebody broke in to
the server and modified the repository. You find which commit introduced
it and check the signature.
Of course a clever attacker would just not sign the backdoor commit. So
you'd probably want some policy to notice unsigned commits early
(e.g., perhaps reject them on push and fetch).
> Now as a user I may already have a workflow that I like. And I might
> want to "bring in more security". Then I have to figure out possible
> attack scenarios and which sort of signing can prevent such an attack.
>
> And each organisation has to do that themselves, but we as the provider
> of the tool might have this knowledge because we implemented all
> these shiny "sign here, please" parts.
Sure. I absolutely think we need better documentation and tools here.
-Peff
^ permalink raw reply
* [PATCH] t*: avoid using pipes
From: Prathamesh Chavan @ 2017-03-07 16:18 UTC (permalink / raw)
Cc: git, Pranit Bauva
Hi,
I'm Prathamesh Chavan. As a part of my micropraoject I have been working on
"Avoid pipes for git related commands in test suites". I tried sending the
patch, but it got blocked since the mail contained more than 100 000
characters.
Hence, I have made the required changes in branch "micro-proj" and you may
find it in 'git' repository on my github account. My user name is: pratham-pc.
Please review the changes.
Thanks.
^ permalink raw reply
* Re: Crash on MSYS2 with GIT_WORK_TREE
From: Junio C Hamano @ 2017-03-08 2:36 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: valtron, git, Brandon Williams
In-Reply-To: <alpine.DEB.2.20.1703080104580.3767@virtualbox>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> The problem is actually even worse: On *Linux*, this happens:
>
> $ GIT_WORK_TREE=c:/invalid git rev-parse HEAD
> Segmentation fault (core dumped)
>
> The reason is this: when set_git_work_tree() was converted from using
> xstrdup(real_path()) to real_pathdup(), we completely missed the fact that
> the former passed die_on_error = 1 to strbuf_realpath(), while the latter
> passed die_on_error = 0. As a consequence, work_tree can be NULL now, and
> the current code does not expect set_git_work_tree() to return
> successfully after setting work_tree to NULL.
Ouch.
> Brandon, I have a hunch that pretty much all of the xstrdup(real_path())
> -> real_pathdup() sites have a problem now. The previous contract was that
> real_path() would die() if the passed path is invalid. The new contract is
> that real_pathdup() returns NULL in such a case.
OK, so it appears that we'd better audit all the callsites of
real_pathdup() and see if anybody _assumes_ that the return values
are not NULL. They all need fixing.
Thanks for digging it through to the root cause.
^ permalink raw reply
* [PATCH 0/6] deadlock regression in v2.11.0 with failed mkdtemp
From: Jeff King @ 2017-03-07 13:34 UTC (permalink / raw)
To: Horst Schirmeier; +Cc: git
In-Reply-To: <20170307111406.GB1955@quickstop.soohrt.org>
On Tue, Mar 07, 2017 at 12:14:06PM +0100, Horst Schirmeier wrote:
> On Tue, 07 Mar 2017, Horst Schirmeier wrote:
> > I observe a regression that seems to have been introduced between
> > v2.10.0 and v2.11.0. When I try to push into a repository on the local
> > filesystem that belongs to another user and has not explicitly been
> > prepared for shared use, v2.11.0 shows some of the usual diagnostic
> > output and then freezes instead of announcing why it failed to push.
>
> Bisecting points to v2.10.1-373-g722ff7f:
>
> 722ff7f876c8a2ad99c42434f58af098e61b96e8 is the first bad commit
> commit 722ff7f876c8a2ad99c42434f58af098e61b96e8
> Author: Jeff King <peff@peff.net>
> Date: Mon Oct 3 16:49:14 2016 -0400
>
> receive-pack: quarantine objects until pre-receive accepts
Thanks, I was able to reproduce easily with:
git init --bare foo.git
chown -R nobody foo.git
git push foo.git HEAD
Here's a series to fix it. The first patch addresses the deadlock. The
rest try to improve the output on the client side. With v2.10.0, this
case looks like:
$ git push ~/tmp/foo.git HEAD
Counting objects: 209837, done.
Delta compression using up to 8 threads.
Compressing objects: 100% (52180/52180), done.
remote: fatal: Unable to create temporary file '/home/peff/tmp/foo.git/./objects/pack/tmp_pack_XXXXXX': Permission denied
error: failed to push some refs to '/home/peff/tmp/foo.git'
With just the first patch applied, you get just:
$ git push ~/tmp/foo.git HEAD
Counting objects: 210973, done.
Delta compression using up to 8 threads.
Compressing objects: 100% (52799/52799), done.
error: failed to push some refs to '/home/peff/tmp/foo.git'
Yuck. In the original, the error was generated by the child index-pack,
and we relayed it over the sideband. But we don't even get as far as
running index-pack in the newer version; we fail trying to make the
tmpdir. The error ends up in the "unpack" protocol field, but the client
side does a bad job of showing that. With the rest of the patches, it
looks like:
$ git push ~/tmp/foo.git HEAD
Counting objects: 210973, done.
Delta compression using up to 8 threads.
Compressing objects: 100% (52799/52799), done.
error: remote unpack failed: unable to create temporary object directory
error: failed to push some refs to '/home/peff/tmp/foo.git'
Compared to v2.10.0, that omits the name of the directory and the errno
value (because receive-pack does not send them). That potentially makes
debugging harder, but arguably it's the right thing to do. On a remote
site, those details aren't really the client's business. But if people
feel strongly, we could add them back into this error code path.
So the first patch is a no-brainer and should go to maint. The rest can
be applied separately if need be.
[1/6]: receive-pack: fix deadlock when we cannot create tmpdir
[2/6]: send-pack: extract parsing of "unpack" response
[3/6]: send-pack: use skip_prefix for parsing unpack status
[4/6]: send-pack: improve unpack-status error messages
[5/6]: send-pack: read "unpack" status even on pack-objects failure
[6/6]: send-pack: report signal death of pack-objects
builtin/receive-pack.c | 5 ++++-
send-pack.c | 46 ++++++++++++++++++++++++++++++++++++----------
2 files changed, 40 insertions(+), 11 deletions(-)
-Peff
^ permalink raw reply
* Re: [PATCH v4 04/10] setup_git_directory_1(): avoid changing global state
From: Brandon Williams @ 2017-03-08 2:10 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git, Junio C Hamano, Jeff King, Duy Nguyen
In-Reply-To: <alpine.DEB.2.20.1703080156040.3767@virtualbox>
On 03/08, Johannes Schindelin wrote:
> Hi Brandon,
>
> On Tue, 7 Mar 2017, Brandon Williams wrote:
>
> > On 03/07, Johannes Schindelin wrote:
> > > const char *setup_git_directory_gently(int *nongit_ok)
> > > {
> > > + struct strbuf cwd = STRBUF_INIT, dir = STRBUF_INIT, gitdir = STRBUF_INIT;
> >
> > I couldn't see any strbuf_release() calls for these strbufs so there may
> > be some memory leaking here.
>
> You are correct, of course. Something like this may work:
Yep that should fix it!
>
> -- snipsnap --
> diff --git a/setup.c b/setup.c
> index 9118b48590a..c822582b96e 100644
> --- a/setup.c
> +++ b/setup.c
> @@ -1027,6 +1027,8 @@ const char *setup_git_directory_gently(int *nongit_ok)
> case GIT_DIR_HIT_MOUNT_POINT:
> if (nongit_ok) {
> *nongit_ok = 1;
> + strbuf_release(&cwd);
> + strbuf_release(&dir);
> return NULL;
> }
> die(_("Not a git repository (or any parent up to mount point %s)\n"
> @@ -1044,6 +1046,10 @@ const char *setup_git_directory_gently(int *nongit_ok)
> startup_info->have_repository = !nongit_ok || !*nongit_ok;
> startup_info->prefix = prefix;
>
> + strbuf_release(&cwd);
> + strbuf_release(&dir);
> + strbuf_release(&gitdir);
> +
> return prefix;
> }
>
--
Brandon Williams
^ permalink raw reply
* Re: Crash on MSYS2 with GIT_WORK_TREE
From: Brandon Williams @ 2017-03-08 2:09 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: valtron, git
In-Reply-To: <alpine.DEB.2.20.1703080104580.3767@virtualbox>
On 03/08, Johannes Schindelin wrote:
> Hi valtron,
>
> On Wed, 8 Mar 2017, Johannes Schindelin wrote:
>
> > On Tue, 7 Mar 2017, valtron wrote:
> >
> > > When GIT_WORK_TREE contains a drive-letter and forward-slashes, some git
> > > commands crash:
> > >
> > > C:\repo>set GIT_WORK_TREE=C:/repo
> > > C:\repo>git rev-parse HEAD
> > > 1 [main] git 2332 cygwin_exception::open_stackdumpfile: Dumping
> > > stack trace to git.exe.stackdump
> >
> > [...]
> >
> > In any case, this problem is squarely within the MSYS2 runtime. It has
> > nothing to do with Git except for the motivation to set an environment
> > variable to an absolute path as you outlined.
>
> Oh boy was I *wrong*! I take that back and apologize for my premature
> verdict.
>
> It is true that you should not set GIT_WORKTREE=c:/repo if you want to
> work with MSYS2 Git because MSYS2 expects pseudo Unix paths, i.e. /c/repo,
> and it will simply try to guess correctly and convert Windows paths with
> drive letters and backslashes to that form.
>
> But that does not excuse a crash.
>
> The problem is actually even worse: On *Linux*, this happens:
>
> $ GIT_WORK_TREE=c:/invalid git rev-parse HEAD
> Segmentation fault (core dumped)
>
> The reason is this: when set_git_work_tree() was converted from using
> xstrdup(real_path()) to real_pathdup(), we completely missed the fact that
> the former passed die_on_error = 1 to strbuf_realpath(), while the latter
> passed die_on_error = 0. As a consequence, work_tree can be NULL now, and
> the current code does not expect set_git_work_tree() to return
> successfully after setting work_tree to NULL.
>
> I Cc:ed Brandon, the author of 4ac9006f832 (real_path: have callers use
> real_pathdup and strbuf_realpath, 2016-12-12).
>
> Brandon, I have a hunch that pretty much all of the xstrdup(real_path())
> -> real_pathdup() sites have a problem now. The previous contract was that
> real_path() would die() if the passed path is invalid. The new contract is
> that real_pathdup() returns NULL in such a case. I believe that the
> following call sites are problematic in particular:
Welp, looks like I missed that when I made the conversion. You're
right, the semantics of getting the real_path were changed which would
cause a NULL to be returned instead of the program exiting with a call
to die().
After a cursory look at your patch, I think all of your changes look
sane. I would have to take a closer look at the call sites to see if
each caller would need to die or not. I'm assuming you took a quick
glace to make your decision about each call site?
>
> builtin/init-db.c: init_db():
> char *original_git_dir = real_pathdup(git_dir);
>
> builtin/init-db.c: cmd_init_db():
> real_git_dir = real_pathdup(real_git_dir);
> ...
> git_work_tree_cfg = real_pathdup(rel);
>
> environment.c: set_git_work_tree():
> work_tree = real_pathdup(new_work_tree);
>
> setup.c: setup_discovered_git_dir():
> gitdir = real_pathdup(gitdir);
>
> submodule.c: connect_work_tree_and_git_dir():
> const char *real_work_tree = real_pathdup(work_tree);
>
> transport.c: refs_from_alternate_cb():
> other = real_pathdup(e->path);
>
> worktree.c: find_worktree():
> path = real_pathdup(arg);
>
> I verified that all calls are still there, except for the submodule.c one
> which simply moved to dir.c and the transport.c one which apparently now
> no longer die()s but simply ignores non-existing paths now.
>
> That leaves six places to patch, methinks... This diff may serve as an
> initial version, but I have not really had a deep look at all call sites
> (and it is an unwise idea to trust me at this hour anyway, look at the
> time when I sent this mail):
>
> -- snipsnap --
> diff --git a/abspath.c b/abspath.c
> index 2f0c26e0e2c..b02e068aa34 100644
> --- a/abspath.c
> +++ b/abspath.c
> @@ -214,12 +214,12 @@ const char *real_path_if_valid(const char *path)
> return strbuf_realpath(&realpath, path, 0);
> }
>
> -char *real_pathdup(const char *path)
> +char *real_pathdup(const char *path, int die_on_error)
> {
> struct strbuf realpath = STRBUF_INIT;
> char *retval = NULL;
>
> - if (strbuf_realpath(&realpath, path, 0))
> + if (strbuf_realpath(&realpath, path, die_on_error))
> retval = strbuf_detach(&realpath, NULL);
>
> strbuf_release(&realpath);
> diff --git a/builtin/init-db.c b/builtin/init-db.c
> index 1d4d6a00789..8a6acb0ec69 100644
> --- a/builtin/init-db.c
> +++ b/builtin/init-db.c
> @@ -338,7 +338,7 @@ int init_db(const char *git_dir, const char *real_git_dir,
> {
> int reinit;
> int exist_ok = flags & INIT_DB_EXIST_OK;
> - char *original_git_dir = real_pathdup(git_dir);
> + char *original_git_dir = real_pathdup(git_dir, 1);
>
> if (real_git_dir) {
> struct stat st;
> @@ -489,7 +489,7 @@ int cmd_init_db(int argc, const char **argv, const char *prefix)
> argc = parse_options(argc, argv, prefix, init_db_options, init_db_usage, 0);
>
> if (real_git_dir && !is_absolute_path(real_git_dir))
> - real_git_dir = real_pathdup(real_git_dir);
> + real_git_dir = real_pathdup(real_git_dir, 1);
>
> if (argc == 1) {
> int mkdir_tried = 0;
> @@ -560,7 +560,7 @@ int cmd_init_db(int argc, const char **argv, const char *prefix)
> const char *git_dir_parent = strrchr(git_dir, '/');
> if (git_dir_parent) {
> char *rel = xstrndup(git_dir, git_dir_parent - git_dir);
> - git_work_tree_cfg = real_pathdup(rel);
> + git_work_tree_cfg = real_pathdup(rel, 1);
> free(rel);
> }
> if (!git_work_tree_cfg)
> diff --git a/cache.h b/cache.h
> index e7b57457e73..7168c1e5ff0 100644
> --- a/cache.h
> +++ b/cache.h
> @@ -1160,7 +1160,7 @@ char *strbuf_realpath(struct strbuf *resolved, const char *path,
> int die_on_error);
> const char *real_path(const char *path);
> const char *real_path_if_valid(const char *path);
> -char *real_pathdup(const char *path);
> +char *real_pathdup(const char *path, int die_on_error);
> const char *absolute_path(const char *path);
> char *absolute_pathdup(const char *path);
> const char *remove_leading_path(const char *in, const char *prefix);
> diff --git a/dir.c b/dir.c
> index 4541f9e1460..aeeb5ce1049 100644
> --- a/dir.c
> +++ b/dir.c
> @@ -2730,8 +2730,8 @@ void connect_work_tree_and_git_dir(const char *work_tree_, const char *git_dir_)
> {
> struct strbuf file_name = STRBUF_INIT;
> struct strbuf rel_path = STRBUF_INIT;
> - char *git_dir = real_pathdup(git_dir_);
> - char *work_tree = real_pathdup(work_tree_);
> + char *git_dir = real_pathdup(git_dir_, 1);
> + char *work_tree = real_pathdup(work_tree_, 1);
>
> /* Update gitfile */
> strbuf_addf(&file_name, "%s/.git", work_tree);
> diff --git a/environment.c b/environment.c
> index c07fb17fb70..42dc3106d2f 100644
> --- a/environment.c
> +++ b/environment.c
> @@ -259,7 +259,7 @@ void set_git_work_tree(const char *new_work_tree)
> return;
> }
> git_work_tree_initialized = 1;
> - work_tree = real_pathdup(new_work_tree);
> + work_tree = real_pathdup(new_work_tree, 1);
> }
>
> const char *get_git_work_tree(void)
> diff --git a/setup.c b/setup.c
> index 9118b48590a..d51549a6de3 100644
> --- a/setup.c
> +++ b/setup.c
> @@ -698,7 +698,7 @@ static const char *setup_discovered_git_dir(const char *gitdir,
> /* --work-tree is set without --git-dir; use discovered one */
> if (getenv(GIT_WORK_TREE_ENVIRONMENT) || git_work_tree_cfg) {
> if (offset != cwd->len && !is_absolute_path(gitdir))
> - gitdir = real_pathdup(gitdir);
> + gitdir = real_pathdup(gitdir, 1);
> if (chdir(cwd->buf))
> die_errno("Could not come back to cwd");
> return setup_explicit_git_dir(gitdir, cwd, nongit_ok);
> @@ -808,7 +808,7 @@ static int canonicalize_ceiling_entry(struct string_list_item *item,
> /* Keep entry but do not canonicalize it */
> return 1;
> } else {
> - char *real_path = real_pathdup(ceil);
> + char *real_path = real_pathdup(ceil, 0);
> if (!real_path) {
> return 0;
> }
> diff --git a/submodule.c b/submodule.c
> index 3b98766a6bc..1d4c0ce86ee 100644
> --- a/submodule.c
> +++ b/submodule.c
> @@ -1403,7 +1403,7 @@ static void relocate_single_git_dir_into_superproject(const char *prefix,
> /* If it is an actual gitfile, it doesn't need migration. */
> return;
>
> - real_old_git_dir = real_pathdup(old_git_dir);
> + real_old_git_dir = real_pathdup(old_git_dir, 0);
>
> sub = submodule_from_path(null_sha1, path);
> if (!sub)
> @@ -1412,7 +1412,7 @@ static void relocate_single_git_dir_into_superproject(const char *prefix,
> new_git_dir = git_path("modules/%s", sub->name);
> if (safe_create_leading_directories_const(new_git_dir) < 0)
> die(_("could not create directory '%s'"), new_git_dir);
> - real_new_git_dir = real_pathdup(new_git_dir);
> + real_new_git_dir = real_pathdup(new_git_dir, 0);
>
> if (!prefix)
> prefix = get_super_prefix();
> @@ -1472,14 +1472,14 @@ void absorb_git_dir_into_superproject(const char *prefix,
> new_git_dir = git_path("modules/%s", sub->name);
> if (safe_create_leading_directories_const(new_git_dir) < 0)
> die(_("could not create directory '%s'"), new_git_dir);
> - real_new_git_dir = real_pathdup(new_git_dir);
> + real_new_git_dir = real_pathdup(new_git_dir, 0);
> connect_work_tree_and_git_dir(path, real_new_git_dir);
>
> free(real_new_git_dir);
> } else {
> /* Is it already absorbed into the superprojects git dir? */
> - char *real_sub_git_dir = real_pathdup(sub_git_dir);
> - char *real_common_git_dir = real_pathdup(get_git_common_dir());
> + char *real_sub_git_dir = real_pathdup(sub_git_dir, 0);
> + char *real_common_git_dir = real_pathdup(get_git_common_dir(), 0);
>
> if (!starts_with(real_sub_git_dir, real_common_git_dir))
> relocate_single_git_dir_into_superproject(prefix, path);
> diff --git a/worktree.c b/worktree.c
> index d633761575b..0486e31ad4a 100644
> --- a/worktree.c
> +++ b/worktree.c
> @@ -255,7 +255,7 @@ struct worktree *find_worktree(struct worktree **list,
> return wt;
>
> arg = prefix_filename(prefix, strlen(prefix), arg);
> - path = real_pathdup(arg);
> + path = real_pathdup(arg, 1);
> for (; *list; list++)
> if (!fspathcmp(path, real_path((*list)->path)))
> break;
--
Brandon Williams
^ permalink raw reply
* Re: Crash on MSYS2 with GIT_WORK_TREE
From: Johannes Schindelin @ 2017-03-08 0:51 UTC (permalink / raw)
To: valtron; +Cc: git, Brandon Williams
In-Reply-To: <alpine.DEB.2.20.1703072345530.3767@virtualbox>
Hi valtron,
On Wed, 8 Mar 2017, Johannes Schindelin wrote:
> On Tue, 7 Mar 2017, valtron wrote:
>
> > When GIT_WORK_TREE contains a drive-letter and forward-slashes, some git
> > commands crash:
> >
> > C:\repo>set GIT_WORK_TREE=C:/repo
> > C:\repo>git rev-parse HEAD
> > 1 [main] git 2332 cygwin_exception::open_stackdumpfile: Dumping
> > stack trace to git.exe.stackdump
>
> [...]
>
> In any case, this problem is squarely within the MSYS2 runtime. It has
> nothing to do with Git except for the motivation to set an environment
> variable to an absolute path as you outlined.
Oh boy was I *wrong*! I take that back and apologize for my premature
verdict.
It is true that you should not set GIT_WORKTREE=c:/repo if you want to
work with MSYS2 Git because MSYS2 expects pseudo Unix paths, i.e. /c/repo,
and it will simply try to guess correctly and convert Windows paths with
drive letters and backslashes to that form.
But that does not excuse a crash.
The problem is actually even worse: On *Linux*, this happens:
$ GIT_WORK_TREE=c:/invalid git rev-parse HEAD
Segmentation fault (core dumped)
The reason is this: when set_git_work_tree() was converted from using
xstrdup(real_path()) to real_pathdup(), we completely missed the fact that
the former passed die_on_error = 1 to strbuf_realpath(), while the latter
passed die_on_error = 0. As a consequence, work_tree can be NULL now, and
the current code does not expect set_git_work_tree() to return
successfully after setting work_tree to NULL.
I Cc:ed Brandon, the author of 4ac9006f832 (real_path: have callers use
real_pathdup and strbuf_realpath, 2016-12-12).
Brandon, I have a hunch that pretty much all of the xstrdup(real_path())
-> real_pathdup() sites have a problem now. The previous contract was that
real_path() would die() if the passed path is invalid. The new contract is
that real_pathdup() returns NULL in such a case. I believe that the
following call sites are problematic in particular:
builtin/init-db.c: init_db():
char *original_git_dir = real_pathdup(git_dir);
builtin/init-db.c: cmd_init_db():
real_git_dir = real_pathdup(real_git_dir);
...
git_work_tree_cfg = real_pathdup(rel);
environment.c: set_git_work_tree():
work_tree = real_pathdup(new_work_tree);
setup.c: setup_discovered_git_dir():
gitdir = real_pathdup(gitdir);
submodule.c: connect_work_tree_and_git_dir():
const char *real_work_tree = real_pathdup(work_tree);
transport.c: refs_from_alternate_cb():
other = real_pathdup(e->path);
worktree.c: find_worktree():
path = real_pathdup(arg);
I verified that all calls are still there, except for the submodule.c one
which simply moved to dir.c and the transport.c one which apparently now
no longer die()s but simply ignores non-existing paths now.
That leaves six places to patch, methinks... This diff may serve as an
initial version, but I have not really had a deep look at all call sites
(and it is an unwise idea to trust me at this hour anyway, look at the
time when I sent this mail):
-- snipsnap --
diff --git a/abspath.c b/abspath.c
index 2f0c26e0e2c..b02e068aa34 100644
--- a/abspath.c
+++ b/abspath.c
@@ -214,12 +214,12 @@ const char *real_path_if_valid(const char *path)
return strbuf_realpath(&realpath, path, 0);
}
-char *real_pathdup(const char *path)
+char *real_pathdup(const char *path, int die_on_error)
{
struct strbuf realpath = STRBUF_INIT;
char *retval = NULL;
- if (strbuf_realpath(&realpath, path, 0))
+ if (strbuf_realpath(&realpath, path, die_on_error))
retval = strbuf_detach(&realpath, NULL);
strbuf_release(&realpath);
diff --git a/builtin/init-db.c b/builtin/init-db.c
index 1d4d6a00789..8a6acb0ec69 100644
--- a/builtin/init-db.c
+++ b/builtin/init-db.c
@@ -338,7 +338,7 @@ int init_db(const char *git_dir, const char *real_git_dir,
{
int reinit;
int exist_ok = flags & INIT_DB_EXIST_OK;
- char *original_git_dir = real_pathdup(git_dir);
+ char *original_git_dir = real_pathdup(git_dir, 1);
if (real_git_dir) {
struct stat st;
@@ -489,7 +489,7 @@ int cmd_init_db(int argc, const char **argv, const char *prefix)
argc = parse_options(argc, argv, prefix, init_db_options, init_db_usage, 0);
if (real_git_dir && !is_absolute_path(real_git_dir))
- real_git_dir = real_pathdup(real_git_dir);
+ real_git_dir = real_pathdup(real_git_dir, 1);
if (argc == 1) {
int mkdir_tried = 0;
@@ -560,7 +560,7 @@ int cmd_init_db(int argc, const char **argv, const char *prefix)
const char *git_dir_parent = strrchr(git_dir, '/');
if (git_dir_parent) {
char *rel = xstrndup(git_dir, git_dir_parent - git_dir);
- git_work_tree_cfg = real_pathdup(rel);
+ git_work_tree_cfg = real_pathdup(rel, 1);
free(rel);
}
if (!git_work_tree_cfg)
diff --git a/cache.h b/cache.h
index e7b57457e73..7168c1e5ff0 100644
--- a/cache.h
+++ b/cache.h
@@ -1160,7 +1160,7 @@ char *strbuf_realpath(struct strbuf *resolved, const char *path,
int die_on_error);
const char *real_path(const char *path);
const char *real_path_if_valid(const char *path);
-char *real_pathdup(const char *path);
+char *real_pathdup(const char *path, int die_on_error);
const char *absolute_path(const char *path);
char *absolute_pathdup(const char *path);
const char *remove_leading_path(const char *in, const char *prefix);
diff --git a/dir.c b/dir.c
index 4541f9e1460..aeeb5ce1049 100644
--- a/dir.c
+++ b/dir.c
@@ -2730,8 +2730,8 @@ void connect_work_tree_and_git_dir(const char *work_tree_, const char *git_dir_)
{
struct strbuf file_name = STRBUF_INIT;
struct strbuf rel_path = STRBUF_INIT;
- char *git_dir = real_pathdup(git_dir_);
- char *work_tree = real_pathdup(work_tree_);
+ char *git_dir = real_pathdup(git_dir_, 1);
+ char *work_tree = real_pathdup(work_tree_, 1);
/* Update gitfile */
strbuf_addf(&file_name, "%s/.git", work_tree);
diff --git a/environment.c b/environment.c
index c07fb17fb70..42dc3106d2f 100644
--- a/environment.c
+++ b/environment.c
@@ -259,7 +259,7 @@ void set_git_work_tree(const char *new_work_tree)
return;
}
git_work_tree_initialized = 1;
- work_tree = real_pathdup(new_work_tree);
+ work_tree = real_pathdup(new_work_tree, 1);
}
const char *get_git_work_tree(void)
diff --git a/setup.c b/setup.c
index 9118b48590a..d51549a6de3 100644
--- a/setup.c
+++ b/setup.c
@@ -698,7 +698,7 @@ static const char *setup_discovered_git_dir(const char *gitdir,
/* --work-tree is set without --git-dir; use discovered one */
if (getenv(GIT_WORK_TREE_ENVIRONMENT) || git_work_tree_cfg) {
if (offset != cwd->len && !is_absolute_path(gitdir))
- gitdir = real_pathdup(gitdir);
+ gitdir = real_pathdup(gitdir, 1);
if (chdir(cwd->buf))
die_errno("Could not come back to cwd");
return setup_explicit_git_dir(gitdir, cwd, nongit_ok);
@@ -808,7 +808,7 @@ static int canonicalize_ceiling_entry(struct string_list_item *item,
/* Keep entry but do not canonicalize it */
return 1;
} else {
- char *real_path = real_pathdup(ceil);
+ char *real_path = real_pathdup(ceil, 0);
if (!real_path) {
return 0;
}
diff --git a/submodule.c b/submodule.c
index 3b98766a6bc..1d4c0ce86ee 100644
--- a/submodule.c
+++ b/submodule.c
@@ -1403,7 +1403,7 @@ static void relocate_single_git_dir_into_superproject(const char *prefix,
/* If it is an actual gitfile, it doesn't need migration. */
return;
- real_old_git_dir = real_pathdup(old_git_dir);
+ real_old_git_dir = real_pathdup(old_git_dir, 0);
sub = submodule_from_path(null_sha1, path);
if (!sub)
@@ -1412,7 +1412,7 @@ static void relocate_single_git_dir_into_superproject(const char *prefix,
new_git_dir = git_path("modules/%s", sub->name);
if (safe_create_leading_directories_const(new_git_dir) < 0)
die(_("could not create directory '%s'"), new_git_dir);
- real_new_git_dir = real_pathdup(new_git_dir);
+ real_new_git_dir = real_pathdup(new_git_dir, 0);
if (!prefix)
prefix = get_super_prefix();
@@ -1472,14 +1472,14 @@ void absorb_git_dir_into_superproject(const char *prefix,
new_git_dir = git_path("modules/%s", sub->name);
if (safe_create_leading_directories_const(new_git_dir) < 0)
die(_("could not create directory '%s'"), new_git_dir);
- real_new_git_dir = real_pathdup(new_git_dir);
+ real_new_git_dir = real_pathdup(new_git_dir, 0);
connect_work_tree_and_git_dir(path, real_new_git_dir);
free(real_new_git_dir);
} else {
/* Is it already absorbed into the superprojects git dir? */
- char *real_sub_git_dir = real_pathdup(sub_git_dir);
- char *real_common_git_dir = real_pathdup(get_git_common_dir());
+ char *real_sub_git_dir = real_pathdup(sub_git_dir, 0);
+ char *real_common_git_dir = real_pathdup(get_git_common_dir(), 0);
if (!starts_with(real_sub_git_dir, real_common_git_dir))
relocate_single_git_dir_into_superproject(prefix, path);
diff --git a/worktree.c b/worktree.c
index d633761575b..0486e31ad4a 100644
--- a/worktree.c
+++ b/worktree.c
@@ -255,7 +255,7 @@ struct worktree *find_worktree(struct worktree **list,
return wt;
arg = prefix_filename(prefix, strlen(prefix), arg);
- path = real_pathdup(arg);
+ path = real_pathdup(arg, 1);
for (; *list; list++)
if (!fspathcmp(path, real_path((*list)->path)))
break;
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox