threads / bug / 45346

bug?: git reset --mixed ignores deinitialized submodules

Subject: bug?: git reset --mixed ignores deinitialized submodules

## tl;dr

5 messages between Mar 10, 2017 and Mar 13, 2017.

replies: 4people: 2as markdown or json

David Turner· Mar 10, 2017, 21:06 UTC · lore

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 &&
	git add f &&
	git commit -m f
) &&

(
	cd main &&
	git submodule add ../sub1 sub1 &&
	git commit -m 'add submodule' &&
	git tag start
) &&

# add a commit on sub1
(
	cd main/sub1 &&
	echo morx > f &&
	git add f &&
	git commit -m 'a commit'
) &&

# 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^ &&
	git status # change to sub1 is lost
)

Stefan Beller· Mar 13, 2017, 17:51 UTC · re: David Turner · lore

Re: bug?: git reset --mixed ignores deinitialized submodules

On Fri, Mar 10, 2017 at 1:06 PM, David Turner <novalis@novalis.org> wrote:

Show 25 quoted lines

> 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/

Show 8 quoted lines

> # 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

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

Steping back a bit, rereading the subject line, what do you think is the bug here?

* git-status not reporting about uninitialized submodules?
* git reset --mixed not touching the submodule worktree
* lack of --recurse-submodules in git-reset? (and that not
  being default, see prior point)
* submodules being in detached HEAD all the time?

Thanks, Stefan

David Turner· Mar 13, 2017, 18:37 UTC · re: Stefan Beller · lore

Re: bug?: git reset --mixed ignores deinitialized submodules

On Mon, 2017-03-13 at 10:51 -0700, Stefan Beller wrote:

Show 37 quoted lines

> 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.

Show 28 quoted lines

> > # 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.

Stefan Beller· Mar 13, 2017, 21:19 UTC · re: David Turner · lore

Re: bug?: git reset --mixed ignores deinitialized submodules

Show 8 quoted lines

>> 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.

Well, you ran the deinit after the commit, so I don't think we expect to undo everything up to "just before the commit".

> In other words, it's not confusing to the user.

ok, great :)

Show 22 quoted lines

>
>> 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.

Well even in this case a reset recursing into submodules doesn't change the (de-)init status of a submodule. All it would alter is the submodule HEAD pointing to, IMHO.

Show 5 quoted lines

>
>> * 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.

Oh right.

Show 6 quoted lines

>
>
> 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.

Technically this is also doable,

    git reset --mixed HEAD^ # as in the original email
    git update-index --add --cacheinfo \
        160000,$(git -C .git/modules/sub1 rev-parse HEAD),sub1
    git add another_file
    git commit -m "recreate the commit"

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

Fuzzing around with gitlinks that are non-populated submodules is a messy business.

So another way would be to populate the submodule manually

    GIT_DIR=.git/modules/sub1 \
    GIT_WORKTREE=sub1 \
    git checkout -f # implies last HEAD

and then continue in the superproject.

I am making up excuses for poor UX here, though. I do agree that the expectations may vary wildly because of poor UX in submodules.

Stefan

David Turner· Mar 13, 2017, 21:36 UTC · re: Stefan Beller · lore

Re: bug?: git reset --mixed ignores deinitialized submodules

On Mon, 2017-03-13 at 14:19 -0700, Stefan Beller wrote:

Show 13 quoted lines

> > > 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.
> 
> Well, you ran the deinit after the commit, so I don't think
> we expect to undo everything up to "just before the commit".

Sure, but other workdir changes after the commit would have been blown away; so why not this one?

Show 8 quoted lines

> > > * lack of --recurse-submodules in git-reset? (and that not
> > >   being default, see prior point)
> > 
> > Or possibly this.
> 
> Well even in this case a reset recursing into submodules doesn't
> change the (de-)init status of a submodule. All it would alter is the
> submodule HEAD pointing to, IMHO.

That's OK with me.  It's weird, but at least it's explicable.

Show 15 quoted lines

> > 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.
> 
> Technically this is also doable,
> 
>     git reset --mixed HEAD^ # as in the original email
>     git update-index --add --cacheinfo \
>         160000,$(git -C .git/modules/sub1 rev-parse HEAD),sub1
>     git add another_file
>     git commit -m "recreate the commit"

Yeah, unless the user had done various other operations that messed with .git/modules/sub1 (e.g. if they had first checked out another branch with --recurse-submodules).

Also, this updates the index, which a mixed reset isn't supposed to touch.

Show 6 quoted lines

> > That's why I have the question mark there -- it's not clear that
> > this
> > is a reasonable expectation.
> 
> Fuzzing around with gitlinks that are non-populated submodules is
> a messy business.

Agreed.

Show 7 quoted lines

> So another way would be to populate the submodule manually
> 
>     GIT_DIR=.git/modules/sub1 \
>     GIT_WORKTREE=sub1 \
>     git checkout -f # implies last HEAD
> 
> and then continue in the superproject.

(see above for why this is not a general solution)

> I am making up excuses for poor UX here, though.
> I do agree that the expectations may vary wildly because of poor UX
> in submodules.

I agree that it's not quite clear what to expect. I can just say that I was quite surprised when my colleague demoed this one for me.