public inbox for git@vger.kernel.org 
 help / color / mirror / Atom feed
From: Justin Spahr-Summers <justin.spahrsummers@gmail•com>
To: Junio C Hamano <gitster@pobox•com>
Cc: Jens Lehmann <Jens.Lehmann@web•de>, git@vger•kernel.org
Subject: Re: Bug: Recursive submodules fail when the repo path contains spaces
Date: Tue, 24 Jul 2012 13:33:44 -0700	[thread overview]
Message-ID: <9ADC1C4EF43A4237BC7971884C50C0EB@gmail.com> (raw)
In-Reply-To: <7va9yogb37.fsf@alter.siamese.dyndns.org>

On Tuesday, 24. July 2012 at 13:26, Junio C Hamano wrote:
> Jens Lehmann <Jens.Lehmann@web•de (http://web.de)> writes:
> 
> > Am 24.07.2012 21:01, schrieb Justin Spahr-Summers:
> > > This occurs on Mac OS X 10.7.4, on git versions 1.7.10.2 (Apple Git-33) and 1.7.11.3.
> > > 
> > > Steps:
> > > 1. Create or clone a repository to an absolute path that contains spaces.
> > > 2. Add a submodule to the repository, if it does not already have one.
> > > 3. Within that submodule, attempt to add another submodule.
> > > 
> > > The result is an error "fatal: Not a git repository", followed by the relative path to the submodule directory within .git/modules of the top-level repository.
> > > 
> > > Similarly, using "git submodule update --init --recursive" in a freshly-cloned repository that matches the above configuration will fail with the same error. "git clone --recursive" does not seem to suffer from the same problem at clone time, but will still fail to add recursive submodules.
> > 
> > Hmm, I don't understand how that is different from what t7407 does, it uses
> > "git submodule update --init --recursive" in to populate recursive submodules
> > in a freshly cloned repository whose path contains a space (in the trash
> > directory name) in test number 8.
> 
> 
> 
> I can see one codepath that would behave incorrectly, especially if
> the submodule path relative to the superproject has whitespaces in
> it. In module_clone(), you have:
> 
> # We already are at the root of the work tree but cd_to_toplevel will
> # resolve any symlinks that might be present in $PWD
> a=$(cd_to_toplevel && cd "$gitdir" && pwd)/
> b=$(cd_to_toplevel && cd "$sm_path" && pwd)/
> ...
> # Turn each leading "*/" component into "../"
> rel=$(echo $b | sed -e 's|[^/][^/]*|..|g')
> echo "gitdir: $rel/$a" >"$sm_path/.git"
> 
> I _think_ $sm_path is computed correctly by the codeflow leading to
> this place, and $b is also computed correctly, but notice the lack
> of quoting around $b when you echo it? It will be split at $IFS, so
> if b='/Program Files/My Stupidity/', the sed script will see a
> single SP between My and Stupidity, which is different from what you
> wanted to feed, I presume.
> 
> Having said that, I do not think git-submodule is prepared to take
> paths with path-unsafe characters in it, given that many part of it
> has loops like "while read mode sha1 stage sm_path" that reads from
> ls-files/ls-tree output without -z (which means it cannot handle
> pathnames with LF in them).
> 
> My recommendation at this point (i.e. not a long term) for people
> with problems Justin saw is "Don't do it then".

I appreciate the debugging work. Unfortunately, none of the relative submodule paths have had whitespace in them, so I'm not sure that's the issue. 

Here's some real output, with a couple specific names removed, starting from the root of the top-level repository (where External/twui is a submodule):

$ cd External/twui
$ git submodule add git://github.com/petejkim/expecta.git TwUITests/expecta
Cloning into 'TwUITests/expecta'...
remote: Counting objects: 988, done.
remote: Compressing objects: 100% (404/404), done.
remote: Total 988 (delta 680), reused 842 (delta 535)
Receiving objects: 100% (988/988), 156.30 KiB, done.
Resolving deltas: 100% (680/680), done.
fatal: Not a git repository: ../../../../../../../../Volumes/drive name with spaces/Users/justin/Documents/Programming/project name with spaces/.git/modules/External/twui/modules/TwUITests/expecta

  reply	other threads:[~2012-07-24 20:33 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-07-24 19:01 Bug: Recursive submodules fail when the repo path contains spaces Justin Spahr-Summers
2012-07-24 20:08 ` Jens Lehmann
2012-07-24 20:26   ` Junio C Hamano
2012-07-24 20:33     ` Justin Spahr-Summers [this message]
2012-07-24 20:37       ` Junio C Hamano
2012-07-28 16:21       ` Heiko Voigt
2012-07-28 17:53         ` Justin Spahr-Summers
2012-07-29 18:31       ` Phil Hord
2012-07-29 19:30         ` Justin Spahr-Summers
2012-07-29 21:10           ` Phil Hord
2012-07-29 21:13             ` Justin Spahr-Summers
2012-07-24 20:30   ` Justin Spahr-Summers

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=9ADC1C4EF43A4237BC7971884C50C0EB@gmail.com \
    --to=justin.spahrsummers@gmail$(echo .)com \
    --cc=Jens.Lehmann@web$(echo .)de \
    --cc=git@vger$(echo .)kernel.org \
    --cc=gitster@pobox$(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