git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: David Turner <novalis@novalis.org>
To: Stefan Beller <sbeller@google.com>
Cc: "git@vger.kernel.org" <git@vger.kernel.org>
Subject: Re: bug?: git reset --mixed ignores deinitialized submodules
Date: Mon, 13 Mar 2017 14:37:13 -0400	[thread overview]
Message-ID: <1489430233.10192.56.camel@novalis.org> (raw)
In-Reply-To: <CAGZ79kZmWaobW9e4iPY05y0N6PLcFphGnZmDHtrGKeV0Up70vg@mail.gmail.com>

On Mon, 2017-03-13 at 10:51 -0700, Stefan Beller wrote:
> On Fri, Mar 10, 2017 at 1:06 PM, David Turner <novalis@novalis.org>
> wrote:
> > Git reset --mixed ignores submodules which are not
> > initialized.  I've
> > attached a demo script.
> > 
> > On one hand, this matches the documentation ("Resets the index but
> > not
> > the working tree").  But on the other hand, it kind of doesn't:
> > "(i.e.,
> > the changed files are preserved but not marked for commit)".
> > 
> > It's hard to figure out what a mixed reset should do.  It would be
> > weird for it to initialize the submodule.  Maybe it should just
> > refuse
> > to run?  Maybe there should be an option for it to initialize the
> > submodule for you?  Maybe it should drop a special-purpose file
> > that
> > git understands to be a submodule change?  For instance (and this
> > is
> > insane, but, like, maybe worth considering) it could use extended
> > filesystem attributes, where available.
> > 
> > #!/bin/bash
> > mkdir demo
> > cd demo
> > 
> > git init main
> > 
> > (
> >         git init sub1 &&
> >         cd sub1 &&
> >         dd if=/dev/urandom of=f bs=40 count=1 &&
> 
> We prefer reproducability in tests, so if we take this into
> a (regression) test later we may want to
>     s/dd.../echo "determinism!" >f/

Yeah, that was leftover from some previous version of this script, I
think.   This wasn't intended to be a t/ test, since I don't know what
the right answer is -- just a demo in case my prose was unclear.

> > # commit that change on main,  deinit the submodule and do a mixed
> > reset
> > (
> >         cd main &&
> >         git add sub1 &&
> >         git commit -m 'update sub1' &&
> >         git submodule deinit sub1 &&
> >         git reset --mixed HEAD^ &&
> 
> As of now most commands (including reset)
> are unaware of submodules to begin with.
> They are ignored in most cases, i.e. git-status
> has some tack-on to (pseudo-)report changes
> in submodules, but it's not really builtin.
> 
> A submodule that is not initialized
> ( i.e. submodule.<name>.url is unset) ought
> to not be touched at all. This is spelled out in
> the man page for "submodule update" only at this
> point.
> 
> 
> >         git status # change to sub1 is lost
> 
> The change is not really lost, as you can get it via
> 
>     git checkout HEAD@{1}
>     git submodule update --init

Sure, the commit isn't lost entirely.  But a mixed reset is often used
to mean "go back to before I committed", and here, that's not precisely
what's happening.  In other words, it's not confusing to the user.

> This works most of the time, but it is unreliable as the
> submodule may have had some gc inbetween which
> threw away important objects.

Sure; that's a separate issue.

> Steping back a bit, rereading the subject line,
> what do you think is the bug here?
> 
> * git-status not reporting about uninitialized submodules?

Here, I think git-status is correctly reporting the state of the repo.

> * git reset --mixed not touching the submodule worktree

Yes, possibly.

> * lack of --recurse-submodules in git-reset? (and that not
>   being default, see prior point)

Or possibly this.

> * submodules being in detached HEAD all the time?

In this case, the submodule is not initialized, so it is not in any
state at all.


For me, the bug (if any) is the bad user experience of doing a mixed
reset and expecting to be able to commit (possibly with some git-add
operations) from there and get back something like the commit to which
the user had git-reset.  

That's why I have the question mark there -- it's not clear that this
is a reasonable expectation.


  reply	other threads:[~2017-03-13 18:37 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-03-10 21:06 bug?: git reset --mixed ignores deinitialized submodules David Turner
2017-03-13 17:51 ` Stefan Beller
2017-03-13 18:37   ` David Turner [this message]
2017-03-13 21:19     ` Stefan Beller
2017-03-13 21:36       ` David Turner

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=1489430233.10192.56.camel@novalis.org \
    --to=novalis@novalis.org \
    --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).