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

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

 



Junio C Hamano <gitster@xxxxxxxxx> writes:

> This helper is called from the root level of the superproject's
> working tree (after cd_to_toplevel is done), and has options like
> --url.  If the user named --url with a relative pathname to a local
> repository directory (or a bundle file), shouldn't it be adjusted,
> and wouldn't prefix the only clue what that given path is relative
> to?  Same for --reference repository's path.
>
> I am not sure removing "--prefix=$wt_prefix" without doing "git -C
> $wt_prefix" on the calling side is the right thing to do.  Even
> though the options list used by this function does not seem to use
> OPTION_FILENAME, parse-options API takes prefix exactly because
> relative pathnames need to be adjusted, and it smells like that the
> breakage brought in by this change is merely hidden by existing bugs
> in the code that does not use prefix to adjust relative paths.

As you may be able to guess from the above, I do not fundamentally
oppose to using "-C $wt_prefix" in place of "--prefix $wt_prefix".

These two should be equivalent, from the callers' point of view.

If you use the "--prefix $wt_prefix" thing, then the called program,
which starts at the root level of the working tree, just overrides
the prefix that it would get from the caller, as "prefix" its own
startup sequence computes when it is invoked by its caller is not
useful for adjusting the things that are relative to the directory
the caller originally was invoked at.

The resulting behaviour should be as if the called program were
originally started inside $wt_prefix directory, the start-up
sequence crawled up the directory hierarchy to find the root level
of the working tree and derived $wt_prefix as the prefix that is
given as the third parameter of any builtin cmd_foo().

And that is what would happen if you used "git -C $wt_prefix" to run
the program.  So "git -C $wt_prefix" should be functionally
equivalent to "--prefix $wt_prefix", even though the former is less
efficient by having to do the discovery and series of chdir(2) only
to discover the prefix the caller already has.

What bugs me the most is that, even though they should be
equivalent, you need 1/5 (or 1&2 in the old series) only when you
use "git -C $wt_prefix" but not "--prefix $wt_prefix".  That means
that they are not equivalent.  And it is not clear why, i.e. where
they differ.

It could be that the way "--prefix $wt_prefix" work is insufficient
and "git -C $wt_prefix" does more complete job, and the codepath
that implements "--prefix $wt_prefix" needs to do more to become
truly equivalent.  If that is a case, it means that there is a bug
in "--prefix $wt_prefix" codepath and the callers of the program may
be compensating for the bug by doing wrong things in order to work
around the bug.  When switching to "git -C $wt_prefix", you may be
seeing the effect of these wrong things the callers do, exposed as
bugs that need to be fixed with 1/5 (or 1&2 in the old series).

And I'd want to see the log message explain how the two ways are not
equivalent and what "--prefix $wt_prefix" and the current callers
that use that mechanism get wrong.

--
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]