public inbox for git@vger.kernel.org 
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox•com>
To: Stefan Beller <sbeller@google•com>
Cc: git@vger•kernel.org, pclouds@gmail•com, Jens.Lehmann@web•de,
	jacob.keller@gmail•com, sunshine@sunshineco•com
Subject: Re: [PATCHv3 5/5] submodule--helper clone: lose the extra prefix option
Date: Fri, 25 Mar 2016 12:41:56 -0700	[thread overview]
Message-ID: <xmqqfuve2vln.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <1458931156-29125-6-git-send-email-sbeller@google.com> (Stefan Beller's message of "Fri, 25 Mar 2016 11:39:16 -0700")

Stefan Beller <sbeller@google•com> writes:

> In the rewrite from shell to C (ee8838d157761, 2015-09-08, submodule:
> rewrite `module_clone` shell function in C), we never made use of the
> prefix. Probably it sneaked in as module_list which was converted in the
> same series had the prefix as well.
>
> Signed-off-by: Stefan Beller <sbeller@google•com>
> ---

Hmph.

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.

  reply	other threads:[~2016-03-25 19:42 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-03-25 18:39 [PATCHv3 0/5] submodule helper: cleanup prefix passing Stefan Beller
2016-03-25 18:39 ` [PATCHv3 1/5] submodule: prepare recursive path from non root directory Stefan Beller
2016-03-25 19:21   ` Junio C Hamano
2016-03-25 18:39 ` [PATCHv3 2/5] submodule--helper list: lose the extra prefix option Stefan Beller
2016-03-25 18:39 ` [PATCHv3 3/5] submodule update: add test for recursive from non root dir Stefan Beller
2016-03-25 18:39 ` [PATCHv3 4/5] submodule sync: test syncing one submodule Stefan Beller
2016-03-25 18:39 ` [PATCHv3 5/5] submodule--helper clone: lose the extra prefix option Stefan Beller
2016-03-25 19:41   ` Junio C Hamano [this message]
2016-03-25 20:54     ` Junio C Hamano
2016-03-25 22:09       ` Stefan Beller
2016-03-25 22:39         ` Junio C Hamano
2016-03-25 23:02           ` Stefan Beller
2016-03-25 23:15             ` Junio C Hamano
2016-03-25 23:51               ` Stefan Beller
2016-03-28 16:56               ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqfuve2vln.fsf@gitster.mtv.corp.google.com \
    --to=gitster@pobox$(echo .)com \
    --cc=Jens.Lehmann@web$(echo .)de \
    --cc=git@vger$(echo .)kernel.org \
    --cc=jacob.keller@gmail$(echo .)com \
    --cc=pclouds@gmail$(echo .)com \
    --cc=sbeller@google$(echo .)com \
    --cc=sunshine@sunshineco$(echo .)com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox