git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Brandon Williams <bmwill@google.com>
To: Stefan Beller <sbeller@google.com>
Cc: jrnieder@gmail.com, git@vger.kernel.org
Subject: Re: [PATCH 0/5] Some submodule bugfixes and "reattaching detached HEADs"
Date: Mon, 1 May 2017 11:24:05 -0700	[thread overview]
Message-ID: <20170501182405.GG39135@google.com> (raw)
In-Reply-To: <20170501180058.8063-1-sbeller@google.com>

On 05/01, Stefan Beller wrote:
> The first three patches fix bugs in existing submodule code,
> sort of independent from the last 2 patches, which are RFCs.
> 
> 
> 
> In submodules as of today you always end up with a detached HEAD,
> which may be scary to people used to working on branches. (I myself
> enjoy working with a detached HEAD).
> 
> The detached HEAD in a submodule is the correct in the submodule-as-a-lib
> case, but in case you actively want to work inside submodules, you
> may want to have proper branch support, e.g. when typing:
> 
>     git checkout --recuse-submodules master
> 
> you may want to have the submodules on the master branch as well.

I don't know why submodules were originally designed to be in a
detached HEAD state but I much prefer working on branches (as I'm sure
many other developers do) so the prospect of this becoming the norm is
exciting! :D
> 
> There are a couple of challenges though:
> * We'd probably want to pay attention to the branch configuration
>   (.gitmodules submodule.NAME.branch to be "." or "foo")

Yes, I agree.  If we have a particular name of a branch configured to be
tracked, then we should respect that and try to attach HEAD onto that
branch name.

> * What if the submodule does not have a master branch?
>   Jonathan proposed off list that we may just want to create the
>   branch in the submodule. This is not implemented in this series.

I think it would be fine to create the branch, just as long as it
doesn't already exist as I can't think of a negative impact of this
(aside from maybe having more branches in the submodules after a
prolonged time period?)

> * In case the branch exists in the submodule and doesn't point at the
>   the object as recorded in the superproject, we may either want to 
>   update the branch to point at the superproject record or we want to
>   reject the "reattaching HEAD". Patch 4 provides the later.

The later seems like the most correct thing to do.  It would be
unfortunate if I had done some work on top of the master branch in a
submodule (which wasn't reflected in the superproject), then done a
'checkout master' in the super project only to go back to the submodule
and find that all my work is gone! (Well not gone, but you'd have to go
into the scary reflog!)  It just makes sense to leave HEAD in a detached
state here as it prevents loss of work.

> Any other ideas on why this could be a bad idea or corner cases?
> 
> Thanks,
> Stefan
> 
> Stefan Beller (5):
>   submodule_move_head: fix leak of struct child_process
>   submodule_move_head: prepare env for child correctly
>   submodule: avoid auto-discovery in new working tree manipulator code
>   submodule--helper: reattach-HEAD
>   submodule_move_head: reattach submodule HEAD to branch if possible.
> 
>  builtin/submodule--helper.c | 12 ++++++
>  submodule.c                 | 93 ++++++++++++++++++++++++++++++++++++++++-----
>  submodule.h                 | 10 +++++
>  3 files changed, 106 insertions(+), 9 deletions(-)
> 
> -- 
> 2.13.0.rc1.1.gbc33f0f778
> 

-- 
Brandon Williams

  parent reply	other threads:[~2017-05-01 18:24 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-05-01 18:00 [PATCH 0/5] Some submodule bugfixes and "reattaching detached HEADs" Stefan Beller
2017-05-01 18:00 ` [PATCH 1/5] submodule_move_head: fix leak of struct child_process Stefan Beller
2017-05-01 18:06   ` Brandon Williams
2017-05-02  3:07     ` Junio C Hamano
2017-05-02  4:20       ` Stefan Beller
2017-05-01 18:00 ` [PATCH 2/5] submodule_move_head: prepare env for child correctly Stefan Beller
2017-05-02  2:04   ` Junio C Hamano
2017-05-02  4:18     ` Stefan Beller
2017-05-01 18:00 ` [PATCH 3/5] submodule: avoid auto-discovery in new working tree manipulator code Stefan Beller
2017-05-01 18:00 ` [RFC PATCH 4/5] submodule--helper: reattach-HEAD Stefan Beller
2017-05-01 18:36   ` Brandon Williams
2017-05-01 19:00     ` Stefan Beller
2017-05-01 18:00 ` [RFC PATCH 5/5] submodule_move_head: reattach submodule HEAD to branch if possible Stefan Beller
2017-05-01 18:24 ` Brandon Williams [this message]
2017-05-02  1:35   ` [PATCH 0/5] Some submodule bugfixes and "reattaching detached HEADs" Junio C Hamano
2017-05-02  4:04     ` Stefan Beller
2017-05-31 22:09       ` Stefan Beller
2017-05-02 19:32 ` [PATCHv2 0/3] Some submodule bugfixes Stefan Beller
2017-05-02 19:32   ` [PATCHv2 1/3] submodule_move_head: reuse child_process structure for futher commands Stefan Beller
2017-05-02 19:32   ` [PATCHv2 2/3] submodule: avoid auto-discovery in new working tree manipulator code Stefan Beller
2017-05-02 19:32   ` [PATCHv2 3/3] submodule: properly recurse for read-tree and checkout Stefan Beller
2017-05-02 19:42   ` [PATCHv2 0/3] Some submodule bugfixes Brandon Williams

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=20170501182405.GG39135@google.com \
    --to=bmwill@google.com \
    --cc=git@vger.kernel.org \
    --cc=jrnieder@gmail.com \
    --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).