git@vger.kernel.org mailing list mirror (one of many)
 help / Atom feed
From: Jonathan Nieder <jrnieder@gmail.com>
To: Stefan Beller <sbeller@google.com>
Cc: allan.jensen@qt.io, git <git@vger.kernel.org>
Subject: Re: Old submodules broken in 2.19rc1 and 2.19rc2
Date: Fri, 7 Sep 2018 15:35:15 -0700
Message-ID: <20180907223515.GD103699@aiede.svl.corp.google.com> (raw)
In-Reply-To: <CAGZ79kaVQ0T=acpviOoD+8XVxYsefNkO7c5d+d0Wc0iCbr2Evw@mail.gmail.com>

Stefan Beller wrote:
> On Fri, Sep 7, 2018 at 2:53 AM Allan Sandfeld Jensen <allan.jensen@qt.io> wrote:

>> Submodules checked out with older versions of git not longer works in the
>> latest 2.19 releases. A "git submodule update --recursive" command wil fail
>> for each submodule with a line saying "fatal: could not open
>> '<submodule>/.git' for writing> Is a directory.
[...]
> I have the suspicion that e98317508c0 (submodule:
> ensure core.worktree is set after update, 2018-06-18)
> might be the offender.

I still was not able to reproduce it, but after a bit of staring at
the code, I'm pretty sure I just did something wrong in the
reproduction process.  That commit is indeed the offender.

It introduces the following code (rewrapped for clarity) in
git-submodule.sh:

	if ! $(
		git config -f \
			"$(git rev-parse --git-common-dir)/modules/$name/config" \
			core.worktree
	) 2>/dev/null
	then
		git submodule--helper connect-gitdir-workingtree "$name" "$sm_path"
	fi

Staring at it for a while, you can see one problem: the 'if ! $(git
config)' should be simply 'if ! git config'.  This ends up trying to
run the core.worktree value as a command, which would usually fail.

That brings us into connect_work_tree_and_git_dir, which does

	/* Prepare .git file */
	strbuf_addf(&gitfile_sb, "%s/.git", work_tree_);
	if (safe_create_leading_directories_const(gitfile_sb.buf))
		die(_("could not create directories for %s"), gitfile_sb.buf);

	/* Prepare config file */
	strbuf_addf(&cfg_sb, "%s/config", git_dir_);
	if (safe_create_leading_directories_const(cfg_sb.buf))
		die(_("could not create directories for %s"), cfg_sb.buf);

	git_dir = real_pathdup(git_dir_, 1);
	work_tree = real_pathdup(work_tree_, 1);

	/* Write .git file */
	write_file(gitfile_sb.buf, "gitdir: %s",
		   relative_path(git_dir, work_tree, &rel_path));

The write_file runs into .git already existing as a directory, failing
with the message Allan saw.

This would happen in at least two cases:

- if the submodule exists both in .git/modules/ *and* in the worktree
  (due to flipping between Git versions and branches with and without
  the submodule), the above will happen

- likewise if the submodule exists only in the worktree, like for Allan.

In "next" there is 74d4731d (submodule--helper: replace
connect-gitdir-workingtree by ensure-core-worktree, 2018-08-13) which
uses robust helpers in C that handle this much better.  I think we
should revert e98317508c0 in "master" (for 2.19) and keep making use
of that 'second try' in "next" (for 2.20).

I'll try to pin down a reproduction case and send a revert + testsuite
patch.

Thanks again,
Jonathan

  parent reply index

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-09-07  9:52 Allan Sandfeld Jensen
2018-09-07 15:03 ` Jeff King
2018-09-07 15:18   ` Allan Sandfeld Jensen
2018-09-07 20:14     ` Jonathan Nieder
2018-09-07 17:08 ` Stefan Beller
2018-09-07 20:20   ` Jonathan Nieder
2018-09-07 22:33   ` Allan Sandfeld Jensen
2018-09-07 22:35   ` Jonathan Nieder [this message]
2018-09-07 22:45     ` Stefan Beller
2018-09-08  0:09       ` [PATCH] Revert "Merge branch 'sb/submodule-core-worktree'" (was Re: Old submodules broken in 2.19rc1 and 2.19rc2) Jonathan Nieder
2018-09-08  2:04         ` Junio C Hamano
2018-09-08 18:39           ` Johannes Sixt
2018-09-10 17:11             ` Stefan Beller
2018-09-11 19:49             ` Junio C Hamano

Reply instructions:

You may reply publically 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

  List information: http://vger.kernel.org/majordomo-info.html

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

  git send-email \
    --in-reply-to=20180907223515.GD103699@aiede.svl.corp.google.com \
    --to=jrnieder@gmail.com \
    --cc=allan.jensen@qt.io \
    --cc=git@vger.kernel.org \
    --cc=sbeller@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

git@vger.kernel.org mailing list mirror (one of many)

Archives are clonable:
	git clone --mirror https://public-inbox.org/git
	git clone --mirror http://ou63pmih66umazou.onion/git
	git clone --mirror http://czquwvybam4bgbro.onion/git
	git clone --mirror http://hjrcffqmbrq6wope.onion/git

Newsgroups are available over NNTP:
	nntp://news.public-inbox.org/inbox.comp.version-control.git
	nntp://ou63pmih66umazou.onion/inbox.comp.version-control.git
	nntp://czquwvybam4bgbro.onion/inbox.comp.version-control.git
	nntp://hjrcffqmbrq6wope.onion/inbox.comp.version-control.git
	nntp://news.gmane.org/gmane.comp.version-control.git

 note: .onion URLs require Tor: https://www.torproject.org/
       or Tor2web: https://www.tor2web.org/

AGPL code for this site: git clone https://public-inbox.org/ public-inbox