Re: [PATCHv3 5/5] submodule--helper clone: lose the extra prefix option

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Stefan Beller <sbeller@xxxxxxxxxx> writes:

> The replacement works fine in all tests except for the recursive
> tests as then the chdir is an important detail. In the submodule
> there is no $wt_prefix (as it is the parents' wt_prefix we passed in),

So the real reason is that we may tweak $wt_prefix to refer to a
non-existing directory, that submodule--helper is buggy and does not
notice it, and that using "-C $wt_prefix" would catch it because it
first tries to chdir to it.

The calling script "git submodule" first sets $wt_prefix to the
original directory, so there is no way chdir'ing back there would
not work, but if we update it (e.g. by appending a path to a
submodule we want to work in), it may very well end up referring to
a directory that does not exist (e.g. it may not have been
"init"ed).  Is that what is going on?

If that is the case, it makes a lot more sense as an explanation.
The justification for the main change 4/5 in the log message,
i.e. "-C $wt_prefix" is more familiar, was an irrelevant subjective
statement that only said "we could change it to use -C" without
explaining why "--prefix $wt_prefix" was wrong, and that was why I
was unhappy about it.

> So here is the example from before:
>         repo/ # a superproject repository
>         repo/untracked/ # an untracked dir in repo/
>         repo/sub/ # a submodule
>         repo/sub/subsub # a submodule of a submodule
>
> When calling "git submodule <cmd> --recursive" from within
> repo/untracked/, the recursed instance will end up in
> repo/sub/ with the parents prefix ("untracked/")
>
> In case of git -C $wt_prefix we would try to chdir into
> repo/sub/untracked/ which doesn't exist and our journey ends here.

That sounds like an unrelated bug, though.  Whether -C or --prefix
is used, we shouldn't be using "repo/sub/untracked" after going to
"repo/sub".  If the <cmd> somehow wanted to refer to a relative path
(e.g. "file") in the original directory, it should be using
../untracked as the base (e.g. it would use "../untracked/file").

Of course by using -C, you might notice that repo/sub/untracked does
not exist, but that is not a proper error checking---what if the
submodule at repo/sub does have a directory with that name?  IOW,
the computation that gave repo/sub/untracked instead of ../untracked
is wrong and using -C would not make it right.

And if you clear the prefix you originally obtained in calling
script "git submodule", which is "untracked/", even if <cmd> somehow
wanted to refer to the "file" in that directory, the only clue to do
so is forever lost.  Again, this is unrelated to -C/--prefix, but
that is what the patch 2 in the original series (which was rolled
into patch 1 in the update) was about.

So I am not sure what the value of using -C is.  At least that
"example from before" does not serve as a good justification.

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]