bug?: git reset --mixed ignores deinitialized submodules

bug?: git reset --mixed ignores deinitialized submodules

[David Turner]

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
)

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

[Stefan Beller]

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/

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

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

[David Turner]

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.

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

[Stefan Beller]

>> 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 :)

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

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

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

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

[David Turner]

On Mon, 2017-03-13 at 14:19 -0700, Stefan Beller wrote:
> > > 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?

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

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

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

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