* Crash on MSYS2 with GIT_WORK_TREE
From: valtron @ 2017-03-07 21:28 UTC (permalink / raw)
To: git
Git 1.12.0. 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
C:\repo>set GIT_WORK_TREE=
C:\repo>git rev-parse HEAD
a394e40861e1012a96f9578a1f0cf0c5a49ede11
On the other hand, "C:\repo" and "/c/repo" don't have this issue.
Stacktrace from GDB (on git-rev-parse) is:
#0 0x000000018019634d in strcmp (s1=0x600062080 "/c/repo", s2=0x0)
at /msys_scripts/msys2-runtime/src/msys2-runtime/newlib/libc/string/strcmp.c:83
#1 0x00000001005239f1 in ?? ()
#2 0x0000000100523f36 in ?? ()
#3 0x000000010046c6fa in ?? ()
#4 0x0000000100401b6d in ?? ()
#5 0x0000000100401e4b in ?? ()
#6 0x0000000100563593 in ?? ()
#7 0x0000000180047c37 in _cygwin_exit_return ()
at /msys_scripts/msys2-runtime/src/msys2-runtime/winsup/cygwin/dcrt0.cc:1031
#8 0x0000000180045873 in _cygtls::call2 (this=0xffffce00,
func=0x180046bd0 <dll_crt0_1(void*)>, arg=0x0,
buf=buf@entry=0xffffcdf0) at
/msys_scripts/msys2-runtime/src/msys2-runtime/winsup/cygwin/cygtls.cc:40
#9 0x0000000180045924 in _cygtls::call (func=<optimized out>,
arg=<optimized out>)
at /msys_scripts/msys2-runtime/src/msys2-runtime/winsup/cygwin/cygtls.cc:27
#10 0x0000000000000000 in ?? ()
Backtrace stopped: previous frame inner to this frame (corrupt stack?)
It seems "C:/repo" was changed to "/c/repo", but it crashes because it
gets compared to a nullptr.
^ permalink raw reply
* [PULL] git svn branch authentication fix
From: Eric Wong @ 2017-03-07 21:32 UTC (permalink / raw)
To: Junio C Hamano, Hiroshi Shirosaki; +Cc: git
In-Reply-To: <1488779947-25264-1-git-send-email-h.shirosaki@gmail.com>
Thank you. I fixed spelling in the title (s/authenticaton/authentication/),
added my S-o-b, and pushed for Junio to pick up
The following changes since commit 3bc53220cb2dcf709f7a027a3f526befd021d858:
First batch after 2.12 (2017-02-27 14:04:24 -0800)
are available in the git repository at:
git://bogomips.org/git-svn.git svn-auth-branch
for you to fetch changes up to e0688e9b28f2c5ff711460ee8b62077be5df2360:
git svn: fix authentication with 'branch' (2017-03-07 21:29:03 +0000)
----------------------------------------------------------------
Hiroshi Shirosaki (1):
git svn: fix authentication with 'branch'
git-svn.perl | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
^ permalink raw reply
* Re: [PATCH 15/18] read-cache, remove_marked_cache_entries: wipe selected submodules.
From: Junio C Hamano @ 2017-03-07 22:42 UTC (permalink / raw)
To: Stefan Beller; +Cc: git, bmwill, novalis, sandals, hvoigt, jrnieder, ramsay
In-Reply-To: <20170306205919.9713-16-sbeller@google.com>
Stefan Beller <sbeller@google.com> writes:
> Signed-off-by: Stefan Beller <sbeller@google.com>
> ---
> read-cache.c | 27 +++++++++++++++++++++++++--
> 1 file changed, 25 insertions(+), 2 deletions(-)
>
> diff --git a/read-cache.c b/read-cache.c
> index 9054369dd0..9a2abacf7a 100644
> --- a/read-cache.c
> +++ b/read-cache.c
> @@ -18,6 +18,8 @@
> #include "varint.h"
> #include "split-index.h"
> #include "utf8.h"
> +#include "submodule.h"
> +#include "submodule-config.h"
>
> /* Mask for the name length in ce_flags in the on-disk index */
>
> @@ -520,6 +522,22 @@ int remove_index_entry_at(struct index_state *istate, int pos)
> return 1;
> }
>
> +static void remove_submodule_according_to_strategy(const struct submodule *sub)
> +{
> + switch (sub->update_strategy.type) {
> + case SM_UPDATE_UNSPECIFIED:
> + case SM_UPDATE_CHECKOUT:
> + case SM_UPDATE_REBASE:
> + case SM_UPDATE_MERGE:
> + submodule_move_head(sub->path, "HEAD", NULL, \
What is this backslash doing here?
> + SUBMODULE_MOVE_HEAD_FORCE);
> + break;
> + case SM_UPDATE_NONE:
> + case SM_UPDATE_COMMAND:
> + ; /* Do not touch the submodule. */
> + }
> +}
> +
> /*
> * Remove all cache entries marked for removal, that is where
> * CE_REMOVE is set in ce_flags. This is much more effective than
> @@ -532,8 +550,13 @@ void remove_marked_cache_entries(struct index_state *istate)
>
> for (i = j = 0; i < istate->cache_nr; i++) {
> if (ce_array[i]->ce_flags & CE_REMOVE) {
> - remove_name_hash(istate, ce_array[i]);
> - save_or_free_index_entry(istate, ce_array[i]);
> + const struct submodule *sub = submodule_from_ce(ce_array[i]);
> + if (sub) {
> + remove_submodule_according_to_strategy(sub);
> + } else {
> + remove_name_hash(istate, ce_array[i]);
> + save_or_free_index_entry(istate, ce_array[i]);
> + }
I cannot readily tell as the proposed log message is on the sketchy
side to explain the reasoning behind this, but this behaviour change
is done unconditionally (in other words, without introducing a flag
that is set by --recurse-submodules command line flag and switches
behaviour based on the flag) here. What is the end-user visible
effect with this change? Can we have a new test in this same patch
to demonstrate the desired behaviour introduced by this change (or
the bug this change fixes)?
> }
> else
> ce_array[j++] = ce_array[i];
^ permalink raw reply
* Re: [PATCH 16/18] entry.c: update submodules when interesting
From: Junio C Hamano @ 2017-03-07 22:42 UTC (permalink / raw)
To: Stefan Beller; +Cc: git, bmwill, novalis, sandals, hvoigt, jrnieder, ramsay
In-Reply-To: <20170306205919.9713-17-sbeller@google.com>
Stefan Beller <sbeller@google.com> writes:
> Signed-off-by: Stefan Beller <sbeller@google.com>
> ---
> entry.c | 30 ++++++++++++++++++++++++++++++
> 1 file changed, 30 insertions(+)
>
> diff --git a/entry.c b/entry.c
> index c6eea240b6..d2b512da90 100644
> --- a/entry.c
> +++ b/entry.c
> @@ -2,6 +2,7 @@
> #include "blob.h"
> #include "dir.h"
> #include "streaming.h"
> +#include "submodule.h"
>
> static void create_directories(const char *path, int path_len,
> const struct checkout *state)
> @@ -146,6 +147,7 @@ static int write_entry(struct cache_entry *ce,
> unsigned long size;
> size_t wrote, newsize = 0;
> struct stat st;
> + const struct submodule *sub;
>
> if (ce_mode_s_ifmt == S_IFREG) {
> struct stream_filter *filter = get_stream_filter(ce->name,
> @@ -203,6 +205,10 @@ static int write_entry(struct cache_entry *ce,
> return error("cannot create temporary submodule %s", path);
> if (mkdir(path, 0777) < 0)
> return error("cannot create submodule directory %s", path);
> + sub = submodule_from_ce(ce);
> + if (sub)
> + return submodule_move_head(ce->name,
> + NULL, oid_to_hex(&ce->oid), SUBMODULE_MOVE_HEAD_FORCE);
> break;
> default:
> return error("unknown file mode for %s in index", path);
> @@ -259,7 +265,31 @@ int checkout_entry(struct cache_entry *ce,
> strbuf_add(&path, ce->name, ce_namelen(ce));
>
> if (!check_path(path.buf, path.len, &st, state->base_dir_len)) {
> + const struct submodule *sub;
> unsigned changed = ce_match_stat(ce, &st, CE_MATCH_IGNORE_VALID|CE_MATCH_IGNORE_SKIP_WORKTREE);
> + /*
> + * Needs to be checked before !changed returns early,
> + * as the possibly empty directory was not changed
> + */
> + sub = submodule_from_ce(ce);
> + if (sub) {
> + int err;
> + if (!is_submodule_populated_gently(ce->name, &err)) {
> + struct stat sb;
> + if (lstat(ce->name, &sb))
> + die(_("could not stat file '%s'"), ce->name);
> + if (!(st.st_mode & S_IFDIR))
> + unlink_or_warn(ce->name);
We found that the path ce->name is supposed to be a submodule that
is checked out, we found that the submodule is not yet populated, we
tried to make sure what is on that path, and its failure would cause
us to die(). All that is sensible.
Then we want to make sure the filesystem entity at the path
currently is a directory and otherwise unlink (i.e. we may have an
unrelated file that is not tracked there, or perhaps the user earlier
decided that replacing the submodule with a single file is a good
idea, but then getting rid of the change with "checkout -f" before
doing "git add" on that file). That is also sensible.
But if that unlink fails, shouldn't we die, just like the case where
we cannot tell what is at the path ce->name?
And if that unlink succeeds, instead of having an empty directory,
we start the "move-head" call to switch from NULL to ce->oid without
having any directory. Wouldn't we want to mkdir() here (and remove
mkdir() in submodule_move_head() if there is one---if there isn't
then I do not think this codepath would work)?
If we had a directory here, but if that directory is not empty,
should we proceed? I am assuming (I haven't carefully read
"move-head") that it is OK because the "run an equivalent of
'checkout --detach ce->oid'" done by "move-head" would notice a
possible information loss to overwrite an untracked path (everything
is "untracked" as the head moves from NULL to ce->oid in this case),
and prevent it from happening.
Am I reading the design correctly?
> + return submodule_move_head(ce->name,
> + NULL, oid_to_hex(&ce->oid),
> + SUBMODULE_MOVE_HEAD_FORCE);
> + } else
> + return submodule_move_head(ce->name,
> + "HEAD", oid_to_hex(&ce->oid),
> + SUBMODULE_MOVE_HEAD_FORCE);
> + }
> +
> if (!changed)
> return 0;
> if (!state->force) {
^ permalink raw reply
* Re: [PATCH 05/18] lib-submodule-update.sh: define tests for recursing into submodules
From: Junio C Hamano @ 2017-03-07 22:26 UTC (permalink / raw)
To: Stefan Beller; +Cc: git, bmwill, novalis, sandals, hvoigt, jrnieder, ramsay
In-Reply-To: <20170306205919.9713-6-sbeller@google.com>
Stefan Beller <sbeller@google.com> writes:
> + # ... and ignored files are ignroed
ignored.
^ permalink raw reply
* Re: [PATCH 18/18] builtin/read-tree: add --recurse-submodules switch
From: Junio C Hamano @ 2017-03-07 22:42 UTC (permalink / raw)
To: Stefan Beller; +Cc: git, bmwill, novalis, sandals, hvoigt, jrnieder, ramsay
In-Reply-To: <20170306205919.9713-19-sbeller@google.com>
Stefan Beller <sbeller@google.com> writes:
> A new known failure mode is introduced[1], which is actually not
> a failure but a feature in read-tree. Unlike checkout for which
> the recursive submodule tests were originally written, read-tree does
> warn about ignored untracked files that would be overwritten.
> For the sake of keeping the test library for submodules genric, just
generic
^ permalink raw reply
* Re: [PULL] git svn branch authentication fix
From: Junio C Hamano @ 2017-03-07 22:43 UTC (permalink / raw)
To: Eric Wong; +Cc: Hiroshi Shirosaki, git
In-Reply-To: <20170307213217.GA20443@starla>
Eric Wong <e@80x24.org> writes:
> Thank you. I fixed spelling in the title (s/authenticaton/authentication/),
> added my S-o-b, and pushed for Junio to pick up
>
> The following changes since commit 3bc53220cb2dcf709f7a027a3f526befd021d858:
>
> First batch after 2.12 (2017-02-27 14:04:24 -0800)
>
> are available in the git repository at:
>
> git://bogomips.org/git-svn.git svn-auth-branch
>
> for you to fetch changes up to e0688e9b28f2c5ff711460ee8b62077be5df2360:
>
> git svn: fix authentication with 'branch' (2017-03-07 21:29:03 +0000)
>
> ----------------------------------------------------------------
> Hiroshi Shirosaki (1):
> git svn: fix authentication with 'branch'
>
> git-svn.perl | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
Thanks, both. Will pull.
^ permalink raw reply
* Re: [PATCH 4/6] send-pack: improve unpack-status error messages
From: Junio C Hamano @ 2017-03-07 22:56 UTC (permalink / raw)
To: Jeff King; +Cc: Horst Schirmeier, git
In-Reply-To: <20170307133736.4lpn7mgme26dqs3m@sigill.intra.peff.net>
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.
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
> send-pack.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/send-pack.c b/send-pack.c
> index 243633da1..83c23aef6 100644
> --- a/send-pack.c
> +++ b/send-pack.c
> @@ -134,9 +134,9 @@ static int receive_unpack_status(int in)
> {
> const char *line = packet_read_line(in, NULL);
> if (!skip_prefix(line, "unpack ", &line))
> - return error("did not receive remote status");
> + return error(_("unable to parse remote unpack status: %s"), line);
> if (strcmp(line, "ok"))
> - return error("unpack failed: %s", line);
> + return error(_("remote unpack failed: %s"), line);
> return 0;
> }
^ permalink raw reply
* Re: fatal error when diffing changed symlinks
From: Johannes Schindelin @ 2017-03-07 22:34 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jeff King, git, Christophe Macabiau
In-Reply-To: <xmqqfuip7y07.fsf@gitster.mtv.corp.google.com>
Hi Junio,
On Tue, 7 Mar 2017, Junio C Hamano wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
> >> > When viewing a working tree file, oid.hash could be 0{40} and
> >> > read_sha1_file() is not the right function to use to obtain the
> >> > contents.
> >> >
> >> > Both of these two need to pay attention to 0{40}, I think, as the
> >> > user may be running "difftool -R --dir-diff" in which case the
> >> > working tree would appear in the left hand side instead.
> >>
> >> As a side note, I think even outside of 0{40}, this should be checking
> >> the return value of read_sha1_file(). A corrupted repo should die(), not
> >> segfault.
> >
> > I agree. I am on it.
>
> Friendly ping, if only to make sure that we can keep a piece of this
> thread in the more "recent" pile.
>
> If you have other topics you need to perfect, I think it is OK to
> postpone the fix on this topic a bit longer, but I'd hate to ship
> two releases with a known breakage without an attempt to fix it, so
> if you are otherwise occupied, I may encourage others (including me)
> to take a look at this. The new "difftool" also has a reported
> regression somebody else expressed willingness to work on, which is
> sort of blocked by everybody else not knowing the timeline on this
> one. cf. <20170303212836.GB13790@arch-attack.localdomain>
>
> A patch series would be very welcome, but "Please go ahead if
> somebody else has time, and I'll help reviewing" would be also
> good.
Unfortunately, the obvious fix was not enough. My current work in progress
is in the `difftool-null-sha1` branch on https://gihub.com/dscho/git. I
still have a few other things to tend to, but hope to come back to the
difftool before the end of the week.
Ciao,
Johannes
^ permalink raw reply
* Re: [PATCH 16/18] entry.c: update submodules when interesting
From: Junio C Hamano @ 2017-03-07 23:04 UTC (permalink / raw)
To: Stefan Beller; +Cc: git, bmwill, novalis, sandals, hvoigt, jrnieder, ramsay
In-Reply-To: <xmqqtw746758.fsf@gitster.mtv.corp.google.com>
Junio C Hamano <gitster@pobox.com> writes:
>> + if (!is_submodule_populated_gently(ce->name, &err)) {
>> + struct stat sb;
>> + if (lstat(ce->name, &sb))
>> + die(_("could not stat file '%s'"), ce->name);
>> + if (!(st.st_mode & S_IFDIR))
>> + unlink_or_warn(ce->name);
>
> We found that the path ce->name is supposed to be a submodule that
> is checked out, we found that the submodule is not yet populated, we
> tried to make sure what is on that path, and its failure would cause
> us to die(). All that is sensible.
> ...
> But if that unlink fails, shouldn't we die, just like the case where
> we cannot tell what is at the path ce->name?
>
> And if that unlink succeeds, instead of having an empty directory,
> we start the "move-head" call to switch from NULL to ce->oid without
> having any directory. Wouldn't we want to mkdir() here (and remove
> mkdir() in submodule_move_head() if there is one---if there isn't
> then I do not think this codepath would work)?
In addition to mkdir(), would we also need the .git file that point
into superproject's .git/modules/ too?
^ permalink raw reply
* Re: [PATCH v4 04/10] setup_git_directory_1(): avoid changing global state
From: Junio C Hamano @ 2017-03-07 23:24 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git, Jeff King, Duy Nguyen
In-Reply-To: <2c8ab22700fb40c9e4e9b46f4981b45db7f2dcf2.1488897111.git.johannes.schindelin@gmx.de>
Johannes Schindelin <johannes.schindelin@gmx.de> writes:
> + switch (setup_git_directory_gently_1(&dir, &gitdir)) {
> + case GIT_DIR_NONE:
> + prefix = NULL;
> + break;
> + case GIT_DIR_EXPLICIT:
> + prefix = setup_explicit_git_dir(gitdir.buf, &cwd, nongit_ok);
> + break;
> + case GIT_DIR_DISCOVERED:
> + if (dir.len < cwd.len && chdir(dir.buf))
> + die(_("Cannot change to '%s'"), dir.buf);
> + prefix = setup_discovered_git_dir(gitdir.buf, &cwd, dir.len,
> + nongit_ok);
> + break;
> + case GIT_DIR_BARE:
> + if (dir.len < cwd.len && chdir(dir.buf))
> + die(_("Cannot change to '%s'"), dir.buf);
> + prefix = setup_bare_git_dir(&cwd, dir.len, nongit_ok);
> + break;
> + case GIT_DIR_HIT_CEILING:
> + prefix = setup_nongit(cwd.buf, nongit_ok);
> + break;
> + case GIT_DIR_HIT_MOUNT_POINT:
> + if (nongit_ok) {
> + *nongit_ok = 1;
> + return NULL;
> + }
> + die(_("Not a git repository (or any parent up to mount point %s)\n"
> + "Stopping at filesystem boundary (GIT_DISCOVERY_ACROSS_FILESYSTEM not set)."),
> + dir.buf);
> + default:
> + die("BUG: unhandled setup_git_directory_1() result");
> + }
I _might_ find niggles in other patches (and other parts of this
patch) that enables the above clean implementation, but this
switch() statement speaks of the value of this entire series ;-)
Very nicely done.
^ permalink raw reply
* Re: [PATCH 16/18] entry.c: update submodules when interesting
From: Stefan Beller @ 2017-03-07 23:08 UTC (permalink / raw)
To: Junio C Hamano
Cc: git@vger.kernel.org, Brandon Williams, David Turner,
brian m. carlson, Heiko Voigt, Jonathan Nieder, Ramsay Jones
In-Reply-To: <xmqqy3wg4rko.fsf@gitster.mtv.corp.google.com>
On Tue, Mar 7, 2017 at 3:04 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Junio C Hamano <gitster@pobox.com> writes:
>
>>> + if (!is_submodule_populated_gently(ce->name, &err)) {
>>> + struct stat sb;
>>> + if (lstat(ce->name, &sb))
>>> + die(_("could not stat file '%s'"), ce->name);
>>> + if (!(st.st_mode & S_IFDIR))
>>> + unlink_or_warn(ce->name);
>>
>> We found that the path ce->name is supposed to be a submodule that
>> is checked out, we found that the submodule is not yet populated, we
>> tried to make sure what is on that path, and its failure would cause
>> us to die(). All that is sensible.
>> ...
>> But if that unlink fails, shouldn't we die, just like the case where
>> we cannot tell what is at the path ce->name?
>>
>> And if that unlink succeeds, instead of having an empty directory,
>> we start the "move-head" call to switch from NULL to ce->oid without
>> having any directory. Wouldn't we want to mkdir() here (and remove
>> mkdir() in submodule_move_head() if there is one---if there isn't
>> then I do not think this codepath would work)?
>
> In addition to mkdir(), would we also need the .git file that point
> into superproject's .git/modules/ too?
The move_head function takes care of it
Both creation as well as deletion are handled in the move_head function,
when either new or old is NULL.
We can drag that out of that function and have it at the appropriate places
manually.
Why do you think it is better design to have mkdir here and not in move_head?
(For me having everything in move_head was easier to understand,
maybe it is not for others)
Thanks,
Stefan
^ permalink raw reply
* Re: [PATCH v4 04/10] setup_git_directory_1(): avoid changing global state
From: Brandon Williams @ 2017-03-07 23:35 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git, Junio C Hamano, Jeff King, Duy Nguyen
In-Reply-To: <2c8ab22700fb40c9e4e9b46f4981b45db7f2dcf2.1488897111.git.johannes.schindelin@gmx.de>
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.
> const char *prefix;
>
> - prefix = setup_git_directory_gently_1(nongit_ok);
> + /*
> + * We may have read an incomplete configuration before
> + * setting-up the git directory. If so, clear the cache so
> + * that the next queries to the configuration reload complete
> + * configuration (including the per-repo config file that we
> + * ignored previously).
> + */
> + git_config_clear();
> +
> + /*
> + * Let's assume that we are in a git repository.
> + * If it turns out later that we are somewhere else, the value will be
> + * updated accordingly.
> + */
> + if (nongit_ok)
> + *nongit_ok = 0;
> +
> + if (strbuf_getcwd(&cwd))
> + die_errno(_("Unable to read current working directory"));
> + strbuf_addbuf(&dir, &cwd);
> +
> + switch (setup_git_directory_gently_1(&dir, &gitdir)) {
> + case GIT_DIR_NONE:
> + prefix = NULL;
> + break;
> + case GIT_DIR_EXPLICIT:
> + prefix = setup_explicit_git_dir(gitdir.buf, &cwd, nongit_ok);
> + break;
> + case GIT_DIR_DISCOVERED:
> + if (dir.len < cwd.len && chdir(dir.buf))
> + die(_("Cannot change to '%s'"), dir.buf);
> + prefix = setup_discovered_git_dir(gitdir.buf, &cwd, dir.len,
> + nongit_ok);
> + break;
> + case GIT_DIR_BARE:
> + if (dir.len < cwd.len && chdir(dir.buf))
> + die(_("Cannot change to '%s'"), dir.buf);
> + prefix = setup_bare_git_dir(&cwd, dir.len, nongit_ok);
> + break;
> + case GIT_DIR_HIT_CEILING:
> + prefix = setup_nongit(cwd.buf, nongit_ok);
> + break;
> + case GIT_DIR_HIT_MOUNT_POINT:
> + if (nongit_ok) {
> + *nongit_ok = 1;
> + return NULL;
> + }
> + die(_("Not a git repository (or any parent up to mount point %s)\n"
> + "Stopping at filesystem boundary (GIT_DISCOVERY_ACROSS_FILESYSTEM not set)."),
> + dir.buf);
> + default:
> + die("BUG: unhandled setup_git_directory_1() result");
> + }
> +
> if (prefix)
> setenv(GIT_PREFIX_ENVIRONMENT, prefix, 1);
> else
> --
> 2.12.0.windows.1.7.g94dafc3b124
>
>
--
Brandon Williams
^ permalink raw reply
* Re: [PATCH 15/18] read-cache, remove_marked_cache_entries: wipe selected submodules.
From: Stefan Beller @ 2017-03-07 23:37 UTC (permalink / raw)
To: Junio C Hamano
Cc: git@vger.kernel.org, Brandon Williams, David Turner,
brian m. carlson, Heiko Voigt, Jonathan Nieder, Ramsay Jones
In-Reply-To: <xmqqfuio674n.fsf@gitster.mtv.corp.google.com>
On Tue, Mar 7, 2017 at 2:42 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Stefan Beller <sbeller@google.com> writes:
>
>> Signed-off-by: Stefan Beller <sbeller@google.com>
>> + submodule_move_head(sub->path, "HEAD", NULL, \
>
> What is this backslash doing here?
I am still bad at following coding conventions, apparently.
will remove.
>> @@ -532,8 +550,13 @@ void remove_marked_cache_entries(struct index_state *istate)
>>
>> for (i = j = 0; i < istate->cache_nr; i++) {
>> if (ce_array[i]->ce_flags & CE_REMOVE) {
>> - remove_name_hash(istate, ce_array[i]);
>> - save_or_free_index_entry(istate, ce_array[i]);
>> + const struct submodule *sub = submodule_from_ce(ce_array[i]);
>> + if (sub) {
>> + remove_submodule_according_to_strategy(sub);
>> + } else {
>> + remove_name_hash(istate, ce_array[i]);
>> + save_or_free_index_entry(istate, ce_array[i]);
>> + }
>
> I cannot readily tell as the proposed log message is on the sketchy
> side to explain the reasoning behind this, but this behaviour change
> is done unconditionally (in other words, without introducing a flag
> that is set by --recurse-submodules command line flag and switches
> behaviour based on the flag) here. What is the end-user visible
> effect with this change?
submodule_from_ce returns always NULL, when such flag is not given.
From 10/18:
+const struct submodule *submodule_from_ce(const struct cache_entry *ce)
+{
+ if (!S_ISGITLINK(ce->ce_mode))
+ return NULL;
+
+ if (!should_update_submodules())
+ return NULL;
+
+ return submodule_from_path(null_sha1, ce->name);
+}
should_update_submodules is always false if such a flag is not set,
such that we end up in the else case which is literally the same as
the removed lines (they are just indented).
> Can we have a new test in this same patch
> to demonstrate the desired behaviour introduced by this change (or
> the bug this change fixes)?
This doesn't fix a bug, but in case the flag is given (in patches 17,18)
this new code removes submodules that are no longer recorded in
the tree.
^ permalink raw reply
* Re: GSoC 2017
From: Valery Tolstov @ 2017-03-07 23:22 UTC (permalink / raw)
To: t.gummerer; +Cc: git, sbeller
Decided to rewrite module_clone to use connect_work_tree_and_git_dir
as my microproject by suggestion of Stefan Bellar.
https://public-inbox.org/git/CAGZ79kY+1E-wg0-uzGJmE+haOE+1WCmg0Eux7rWGtkU_aBDQ9g@mail.gmail.com/
It seems that connect_work_tree_and_git_dir needs to be slightly changed
to deal with submodules. Because path to config file for submodules lies
not directly in .git directory we need to know whether the path is
submodule or not. Can I use submodule_from_path to determine this?
I mean what if NULL result also indicates error sometimes?
Another possible solution is to pass the flag indicating
that path is submodule or not. But I think this solution is bad.
Regards,
Valery Tolstov
^ permalink raw reply
* Re: Crash on MSYS2 with GIT_WORK_TREE
From: Johannes Schindelin @ 2017-03-07 23:03 UTC (permalink / raw)
To: valtron; +Cc: git
In-Reply-To: <CAFKRc7y_kpCGNORENUZ2qw_4qBwjjyaaDFxAEQa52fTryj+w7A@mail.gmail.com>
Hi valtron,
On Tue, 7 Mar 2017, valtron wrote:
> Git 1.12.0.
I take it you meant 2.12.0. And you probably also meant to imply that you
are referring to MSYS2's git.exe in /usr/bin/.
> 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
This suggests that my assumption above is correct: it looks as if you are
executing <MSYS2>\usr\bin\git.exe here.
The proof lies in the pudding, though, and you are the only person who can
do that unless you post the contents of that git.exe.stackdump.
> Stacktrace from GDB (on git-rev-parse) is:
>
> #0 0x000000018019634d in strcmp (s1=0x600062080 "/c/repo", s2=0x0)
> at /msys_scripts/msys2-runtime/src/msys2-runtime/newlib/libc/string/strcmp.c:83
> #1 0x00000001005239f1 in ?? ()
> #2 0x0000000100523f36 in ?? ()
> #3 0x000000010046c6fa in ?? ()
> #4 0x0000000100401b6d in ?? ()
> #5 0x0000000100401e4b in ?? ()
> #6 0x0000000100563593 in ?? ()
> #7 0x0000000180047c37 in _cygwin_exit_return ()
> at /msys_scripts/msys2-runtime/src/msys2-runtime/winsup/cygwin/dcrt0.cc:1031
> #8 0x0000000180045873 in _cygtls::call2 (this=0xffffce00,
> func=0x180046bd0 <dll_crt0_1(void*)>, arg=0x0,
> buf=buf@entry=0xffffcdf0) at
> /msys_scripts/msys2-runtime/src/msys2-runtime/winsup/cygwin/cygtls.cc:40
> #9 0x0000000180045924 in _cygtls::call (func=<optimized out>,
> arg=<optimized out>)
> at /msys_scripts/msys2-runtime/src/msys2-runtime/winsup/cygwin/cygtls.cc:27
> #10 0x0000000000000000 in ?? ()
> Backtrace stopped: previous frame inner to this frame (corrupt stack?)
This may be the wrong thread, though. You can find out what other threads
there are with `info threads` and switch by `thread <id>`, the
backtrace(s) of the other thread(s) may be informative.
Please also note that installing the `msys2-runtime-devel` package via
Pacman may be able to get you nicer symbols.
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.
Having said that, the problem also occurs when using *MSYS2* git.exe (i.e.
/usr/bin/git.exe, not /mingw64/bin/git.exe) in the Git for Windows SDK.
Ciao,
Johannes
^ permalink raw reply
* Re: [Request for Documentation] Differentiate signed (commits/tags/pushes)
From: Stefan Beller @ 2017-03-07 22:19 UTC (permalink / raw)
To: Jeff King; +Cc: tom, Matthieu Moy, git@vger.kernel.org, Junio C Hamano
In-Reply-To: <20170307092353.ibirvitsxhzn3apz@sigill.intra.peff.net>
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? ;)
As a user I would like to know what I can do with such a signed commit.
And I happen to be an experienced user in the sense that I know
about git tag --sign as well as git verify-tag; further reading of
these two man pages tells me I can even use git-tag to verify a
tag. So looking for the verify option in git-commit for signed commits...
well, no. Ah! git-verify-commit it is.
I assumed to have most discussion in git-tag or at least a pointer
from there to further reading.
In an ideal world we might have a manpage git-trust-model(7),
that explains different workflows and when certain signing
mechanisms make sense and what they protect us from.
I might write such a man page (after I get the gitmodules
page done).
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. ;)
>
> People generally seem to take tag signatures to mean one (or both) of:
>
> 1. Certifying that the tree contents at that tag are the official,
> signed release contents (i.e., this is version 1.0, and I'm the
> maintainer, so believe me).
>
> 2. Certifying that all the history leading up to that tag is
> "official" in some sense (i.e., I'm the maintainer, and this was
> the tip of my git tree at the time I tagged the release).
>
> 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 :/).
> 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 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.
> But for a project whose workflow is based around pushing and pulling
> commits, I think it does make sense. The author field may not always
> match the committer (e.g., in a cherry-pick), but it still lets you
> trace that attestation of the author back to the committer. And it's up
> to UI to make that distinction clear (e.g., if you push a signed
> cherry-pick to GitHub, the commit is labeled with something like "A U
> Thor committed with C O Mitter", and then you get a little "Verified"
> tag for C O Mitter that gives you more information about the signature).
>
> 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.
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.
Thanks for the lively discussion,
Stefan
^ permalink raw reply
* Re: [RFC PATCH] rev-parse: add --show-superproject-working-tree
From: Junio C Hamano @ 2017-03-07 22:49 UTC (permalink / raw)
To: Stefan Beller
Cc: SZEDER Gábor, Benjamin Fuchs, git@vger.kernel.org,
brian m. carlson, ville.skytta
In-Reply-To: <CAGZ79kYMZk3sNNjWgp9acQG6z5Q5CnsJi+n7Bvr3EkfbSHasMA@mail.gmail.com>
Stefan Beller <sbeller@google.com> writes:
> On Tue, Mar 7, 2017 at 10:44 AM, Junio C Hamano <gitster@pobox.com> wrote:
>
>> So perhaps your superproject_exists() helper can be eliminated
>
> That is what I had originally, but I assumed a strict helper function
> for "existence of the superproject" would be interesting in the future,
> e.g. for get_superproject_git_dir, or on its own. There was an attempt
> to have the shell prompt indicate if you are in a submodule,
> which would not need to know the worktree or git dir of the
> superproject, but only its existence.
>
>> instead coded in get_superproject_working_tree() in place to do:
>>
>> - xgetcwd() to get "/local/repo/super/sub/dir".
>
> Did you mean .../super/dir/sub ?
I meant "/local/repo/super/sub/dir". I am using this case to
illustrate: the root of the superproject is at /local/repo/super,
its submodule we are interested in is at sub/dir, and the function
is working inside the submodule--after the repository discovery
moves the cwd, xgetcwd() would give the root of the working tree of
the submodule, which is at "/local/repo/super/sub/dir".
And that would give us "dir" by taking that as relative to its "../"
>> - relative_path() to get "dir".
>
> ok.
indeed.
>> - ask "ls-{tree,files} --full-name HEAD dir" to get "160000"
>> and "sub/dir".
>
> "ls-files --stage --full-name" to get
> 160000 ... dir/sub
Yeah, also when usihng ls-files you obviously do not give HEAD but
you do give "dir" as the pathspec. And you get "sub/dir" as the
result.
^ permalink raw reply
* Re: [PATCH v4 04/10] setup_git_directory_1(): avoid changing global state
From: Johannes Schindelin @ 2017-03-08 0:57 UTC (permalink / raw)
To: Brandon Williams; +Cc: git, Junio C Hamano, Jeff King, Duy Nguyen
In-Reply-To: <20170307233549.GA128647@google.com>
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:
-- 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;
}
^ permalink raw reply related
* [PATCHv2] rev-parse: add --show-superproject-working-tree
From: Stefan Beller @ 2017-03-08 0:56 UTC (permalink / raw)
To: szeder.dev, email, git, sandals, ville.skytta; +Cc: Stefan Beller
In-Reply-To: <xmqq7f4066t9.fsf@gitster.mtv.corp.google.com>
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 related
* Re: Crash on MSYS2 with GIT_WORK_TREE
From: valtron @ 2017-03-08 1:08 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git, Brandon Williams
In-Reply-To: <alpine.DEB.2.20.1703080104580.3767@virtualbox>
Hi Johannes,
Thanks for looking at this! Yes, it's 2.12.0, sorry for the typo.
I only ran into this because of git-gui, where I eventually tracked it
down to line 1330:
set env(GIT_WORK_TREE) $_gitworktree
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. 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.
On Tue, Mar 7, 2017 at 5:51 PM, Johannes Schindelin
<Johannes.Schindelin@gmx.de> 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:
>
> 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
* Re: [PATCH 16/18] entry.c: update submodules when interesting
From: Junio C Hamano @ 2017-03-08 1:08 UTC (permalink / raw)
To: Stefan Beller
Cc: git@vger.kernel.org, Brandon Williams, David Turner,
brian m. carlson, Heiko Voigt, Jonathan Nieder, Ramsay Jones
In-Reply-To: <CAGZ79kYmaAY-ASa672eXmQapxJns+a3dEC_BiEmH7ti==arLtw@mail.gmail.com>
Stefan Beller <sbeller@google.com> writes:
>> In addition to mkdir(), would we also need the .git file that point
>> into superproject's .git/modules/ too?
>
> The move_head function takes care of it
> Both creation as well as deletion are handled in the move_head function,
> when either new or old is NULL.
Oh, if it does the creation of directory and adjusting of .git
gitfile, and handles other anomalies, I have no problem with it.
It was just that this codepath tried to handle some kind of anomaly
(namely, the user originally thought the submodule needs to be
changed to a regular file and then changed mind and wants to restore
it from the index) and yet not doing the full anomaly handling (like
"not even an empty directory exists" case) that confused me.
^ permalink raw reply
* Re: Crash on MSYS2 with GIT_WORK_TREE
From: Johannes Schindelin @ 2017-03-08 1:05 UTC (permalink / raw)
To: valtron; +Cc: git
In-Reply-To: <alpine.DEB.2.20.1703072345530.3767@virtualbox>
Hi valtron,
just to set the record straight on a few of my suggestions that turned out
to be incorrect:
On Wed, 8 Mar 2017, Johannes Schindelin wrote:
> On Tue, 7 Mar 2017, valtron wrote:
>
> > Stacktrace from GDB (on git-rev-parse) is:
> >
> > #0 0x000000018019634d in strcmp (s1=0x600062080 "/c/repo", s2=0x0)
> > at /msys_scripts/msys2-runtime/src/msys2-runtime/newlib/libc/string/strcmp.c:83
> > #1 0x00000001005239f1 in ?? ()
> > #2 0x0000000100523f36 in ?? ()
> > #3 0x000000010046c6fa in ?? ()
> > #4 0x0000000100401b6d in ?? ()
> > #5 0x0000000100401e4b in ?? ()
> > #6 0x0000000100563593 in ?? ()
> > #7 0x0000000180047c37 in _cygwin_exit_return ()
> > at /msys_scripts/msys2-runtime/src/msys2-runtime/winsup/cygwin/dcrt0.cc:1031
> > #8 0x0000000180045873 in _cygtls::call2 (this=0xffffce00,
> > func=0x180046bd0 <dll_crt0_1(void*)>, arg=0x0,
> > buf=buf@entry=0xffffcdf0) at
> > /msys_scripts/msys2-runtime/src/msys2-runtime/winsup/cygwin/cygtls.cc:40
> > #9 0x0000000180045924 in _cygtls::call (func=<optimized out>,
> > arg=<optimized out>)
> > at /msys_scripts/msys2-runtime/src/msys2-runtime/winsup/cygwin/cygtls.cc:27
> > #10 0x0000000000000000 in ?? ()
> > Backtrace stopped: previous frame inner to this frame (corrupt stack?)
>
> This may be the wrong thread, though. You can find out what other threads
> there are with `info threads` and switch by `thread <id>`, the
> backtrace(s) of the other thread(s) may be informative.
It was actually the correct thread.
> Please also note that installing the `msys2-runtime-devel` package via
> Pacman may be able to get you nicer symbols.
I was wrong. The problem here is git.exe: you would have to uncomment the
'#options("debug","!strip")' line in the git/PKGBUILD file in
https://github.com/Alexpux/MSYS2-packages before rebuilding the package
[*1*], and then reinstalling it (which increases the installed size by
94.82MB to 119.63MB due to the unstripped symbols). But then gdb would
find the symbols alright and you'd see that the crash happens in
#1 0x000000010051aecb in setup_explicit_git_dir
(gitdirenv=gitdirenv@entry=0x1005a00cc <sign_off_header+556> ".git",
cwd=cwd@entry=0x10055e970 <cwd>, nongit_ok=nongit_ok@entry=0x0) at
setup.c:669
669 if (!strcmp(cwd->buf, worktree)) { /* cwd == worktree */
and worktree indeed was NULL.
Ciao,
Johannes
^ permalink raw reply
* Re: [PATCH 15/18] read-cache, remove_marked_cache_entries: wipe selected submodules.
From: Junio C Hamano @ 2017-03-08 1:14 UTC (permalink / raw)
To: Stefan Beller
Cc: git@vger.kernel.org, Brandon Williams, David Turner,
brian m. carlson, Heiko Voigt, Jonathan Nieder, Ramsay Jones
In-Reply-To: <CAGZ79kbdJa54YyKPHR9ycuJBVtyNY_2yaD7_5RPPu++Awiz5cA@mail.gmail.com>
Stefan Beller <sbeller@google.com> writes:
> submodule_from_ce returns always NULL, when such flag is not given.
> From 10/18:
>
> +const struct submodule *submodule_from_ce(const struct cache_entry *ce)
> +{
> + if (!S_ISGITLINK(ce->ce_mode))
> + return NULL;
> +
> + if (!should_update_submodules())
> + return NULL;
> +
> + return submodule_from_path(null_sha1, ce->name);
> +}
>
> should_update_submodules is always false if such a flag is not set,
> such that we end up in the else case which is literally the same as
> the removed lines (they are just indented).
I see.
I didn't think a function this deep in the callchain that does not
take any parameter could possibly change the behaviour based on the
end-user input. I was expecting that such a state (i.e. are we
recursive? are we allowed to forcibly update the working tree
files?) would be kept part of something like "struct checkout" and
passed around the callchain.
That was why I didn't look at how that function answers "should
update?" question, and got puzzled. Because it would imply there is
some hidden state that is accessible by everybody--a global variable
or something--which would point at a deeper design issue.
^ 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