git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Stefan Beller <sbeller@google.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH] RFC: A new type of symbolic refs
Date: Mon, 17 Jul 2017 14:48:17 -0700	[thread overview]
Message-ID: <xmqqvamqg2fy.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <20170711010639.31398-1-sbeller@google.com> (Stefan Beller's message of "Mon, 10 Jul 2017 18:06:39 -0700")

Stefan Beller <sbeller@google.com> writes:

> +int read_external_symref(struct strbuf *from, struct strbuf *out)
> +{
> +	struct child_process cp = CHILD_PROCESS_INIT;
> +	const char *repo, *gitlink;
> +	int hint, code;
> +	struct strbuf **split = strbuf_split(from, 0);
> +	struct strbuf cmd_out = STRBUF_INIT;
> +
> +	if (!split[0] || !split[1])
> +		return -1;
> +
> +	repo = split[0]->buf + 5; /* skip 'repo:' */
> +	gitlink = split[1]->buf;
> +
> +	argv_array_pushl(&cp.args,
> +			"ignored-first-arg",
> +			"-C", repo,
> +			"ls-tree", "-z", "HEAD", "--", gitlink, NULL);
> +
> +	/*
> +	 * 17 accounts for '160000 commit ',
> +	 * the \t before path and trailing \0.
> +	 */
> +	hint = 17 + GIT_SHA1_HEXSZ + split[1]->len;
> +	code = capture_command(&cp, &cmd_out, hint);
> +
> +	strbuf_release(split[0]);
> +	strbuf_release(split[1]);
> +
> +	if (!code) {
> +		strbuf_reset(out);
> +		strbuf_add(out, cmd_out.buf + strlen("160000 commit "),
> +			   GIT_SHA1_HEXSZ);
> +	} else
> +		return -1;
> +
> +	return 0;
> +}

This may help the initial checkout, but to be useful after that, we
need to define what happens when an equivalent of "git update-ref
HEAD" is done in the submodule repository, when HEAD is pointing
elsewhere.  The above only shows read-only operation, which is not
all that interesting.

I _think_ a symbolic HEAD that points upwards to the gitlink entry in
the superproject's index is the easiest to understand and it is
something we can define a clear and useful semantics for.

When a recursive checkout of a branch 'foo' is made in the
superproject, the index in the superproject would name the commit in
the submodule to be checked out.  We traditionally detech the HEAD
at the submodule to that commit, but instead we could say "check the
index of the superproject to see where the HEAD should be pointing
at" in the submodule.  Either way, immediately after such a
recursive checkout, "git status" inside the submodule would find
that the HEAD points at the commit recorded in the 'foo' branch of
the superproject and things are clean.  

After you work in the submodule and make a commit, an equivalent of
"git update-ref HEAD" is done behind the scene to update HEAD in the
submodule.  In the traditional world, that is done to detached HEAD
and nothing else changes, but if the symref HEAD points upwards into
the index of the superproject, what needs to be done is very obvious;
we do "git add submodule" in the superproject.  And this is not just
limited to creating a commit in the submodule.  "reset --hard HEAD~2"
in the submodule to rewind the HEAD by two commits would also be an
update to HEAD and through the symref-ness of the HEAD should result
in an update to the index of the superproject.

However, I do not think a good explanation of what should mean when
this new-style symbolic HEAD points at a commit in the superproject,
whether its limited to its HEAD or a tip of an arbitrary branch that
may not even be checked out.  These are not something we can easily
change without affecting wider context.  Our submodule, when we make
a new commit, may be ready to advance, but our superproject and
other submodules may not be ready to be included in a new commit in
the superproject.

So I think the idea this patch illustrates is on to something
interesting and potentially useful, but I am not sure if it makes
sense to tie it to anything but the index of the superproject.

Even if we limit ourselves to pointing at the index of the
superproject, there probably are a handful of interesting issues
that need to be clarified (not in the sense of "this and that issues
exist, so this won't be a useful feature", but in the sense of "we'd
be able to do these useful things using this feature, and we need to
fill in more details"), such as:

 - Making new commits in the submodule available to the upstream.
   Just like a detached HEAD in the submodule, this is not tied to
   any concrete branch, and it is unclear how a recursive "push"
   from the superproject should propagate the changes to the
   upstream of the submodule;

 - Switching between branches that bind the same commit for the
   submodule in the superproject would work just like switching
   between branches that record the same blob for a path, i.e. it
   will carry forward a local modification.

 - The index entry in the superproject may now have to get involved
   in fsck and reachability study in the submodule as reachability
   root.  A corollary to this is that submodules behave more
   similarly to regular blobs wrt "git reset --hard" in the
   superproject, which is a good thing.  "git -C submodule commit &&
   git reset --hard" will create a new commit in the submodule, add
   it to the index of the superproject, and then lose that change
   from the index of the superproject, making the commit
   unreachable, just like "edit file && git add file && git reset
   --hard" in the superproject will make the blob that records the
   updated content of the file unreachable.

Thanks.

  parent reply	other threads:[~2017-07-17 21:48 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-07-11  1:06 [PATCH] RFC: A new type of symbolic refs Stefan Beller
2017-07-16 13:04 ` Philip Oakley
2017-07-16 14:20   ` Philip Oakley
2017-07-17 19:21   ` Stefan Beller
2017-07-17 21:48 ` Junio C Hamano [this message]
2017-07-17 23:22   ` Stefan Beller
2017-07-18 19:03     ` 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

  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=xmqqvamqg2fy.fsf@gitster.mtv.corp.google.com \
    --to=gitster@pobox.com \
    --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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).