{"thread":{"id":"45346","subject":"bug?: git reset --mixed ignores deinitialized submodules","startedAt":"2017-03-10T21:07:05Z","lastAt":"2017-03-13T21:36:40Z","messageCount":5,"participants":["David Turner","Stefan Beller"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"313797","messageId":"1489180018.10192.45.camel@novalis.org","threadId":"45346","inReplyTo":null,"subject":"bug?: git reset --mixed ignores deinitialized submodules","fromName":"David Turner","fromEmail":"novalis@novalis.org","sentAt":"2017-03-10T21:06:58Z","receivedAt":"2017-03-10T21:07:05Z","isPatch":false,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"Git reset --mixed ignores submodules which are not initialized.  I've\nattached a demo script.  \n\nOn one hand, this matches the documentation (\"Resets the index but not\nthe working tree\").  But on the other hand, it kind of doesn't: \"(i.e.,\nthe changed files are preserved but not marked for commit)\".\n\nIt's hard to figure out what a mixed reset should do.  It would be\nweird for it to initialize the submodule.  Maybe it should just refuse\nto run?  Maybe there should be an option for it to initialize the\nsubmodule for you?  Maybe it should drop a special-purpose file that\ngit understands to be a submodule change?  For instance (and this is\ninsane, but, like, maybe worth considering) it could use extended\nfilesystem attributes, where available.\n\n#!/bin/bash\nmkdir demo\ncd demo\n\ngit init main\n\n(\n\tgit init sub1 &&\n\tcd sub1 &&\n\tdd if=/dev/urandom of=f bs=40 count=1 &&\n\tgit add f &&\n\tgit commit -m f\n) &&\n\n(\n\tcd main &&\n\tgit submodule add ../sub1 sub1 &&\n\tgit commit -m 'add submodule' &&\n\tgit tag start\n) &&\n\n# add a commit on sub1\n(\n\tcd main/sub1 &&\n\techo morx > f &&\n\tgit add f &&\n\tgit commit -m 'a commit'\n) &&\n\n# commit that change on main,  deinit the submodule and do a mixed\nreset\n(\n\tcd main &&\n\tgit add sub1 &&\n\tgit commit -m 'update sub1' &&\n\tgit submodule deinit sub1 &&\n\tgit reset --mixed HEAD^ &&\n\tgit status # change to sub1 is lost\n)\n\n"},{"id":"313909","messageId":"CAGZ79kZmWaobW9e4iPY05y0N6PLcFphGnZmDHtrGKeV0Up70vg@mail.gmail.com","threadId":"45346","inReplyTo":"1489180018.10192.45.camel@novalis.org","subject":"Re: bug?: git reset --mixed ignores deinitialized submodules","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-03-13T17:51:05Z","receivedAt":"2017-03-13T17:51:13Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Fri, Mar 10, 2017 at 1:06 PM, David Turner <novalis@novalis.org> wrote:\n> Git reset --mixed ignores submodules which are not initialized.  I've\n> attached a demo script.\n>\n> On one hand, this matches the documentation (\"Resets the index but not\n> the working tree\").  But on the other hand, it kind of doesn't: \"(i.e.,\n> the changed files are preserved but not marked for commit)\".\n>\n> It's hard to figure out what a mixed reset should do.  It would be\n> weird for it to initialize the submodule.  Maybe it should just refuse\n> to run?  Maybe there should be an option for it to initialize the\n> submodule for you?  Maybe it should drop a special-purpose file that\n> git understands to be a submodule change?  For instance (and this is\n> insane, but, like, maybe worth considering) it could use extended\n> filesystem attributes, where available.\n>\n> #!/bin/bash\n> mkdir demo\n> cd demo\n>\n> git init main\n>\n> (\n>         git init sub1 &&\n>         cd sub1 &&\n>         dd if=/dev/urandom of=f bs=40 count=1 &&\n\nWe prefer reproducability in tests, so if we take this into\na (regression) test later we may want to\n    s/dd.../echo \"determinism!\" >f/\n\n> # commit that change on main,  deinit the submodule and do a mixed\n> reset\n> (\n>         cd main &&\n>         git add sub1 &&\n>         git commit -m 'update sub1' &&\n>         git submodule deinit sub1 &&\n>         git reset --mixed HEAD^ &&\n\nAs of now most commands (including reset)\nare unaware of submodules to begin with.\nThey are ignored in most cases, i.e. git-status\nhas some tack-on to (pseudo-)report changes\nin submodules, but it's not really builtin.\n\nA submodule that is not initialized\n( i.e. submodule.<name>.url is unset) ought\nto not be touched at all. This is spelled out in\nthe man page for \"submodule update\" only at this\npoint.\n\n\n>         git status # change to sub1 is lost\n\nThe change is not really lost, as you can get it via\n\n    git checkout HEAD@{1}\n    git submodule update --init\n\nThis works most of the time, but it is unreliable as the\nsubmodule may have had some gc inbetween which\nthrew away important objects.\n\nSteping back a bit, rereading the subject line,\nwhat do you think is the bug here?\n\n* git-status not reporting about uninitialized submodules?\n* git reset --mixed not touching the submodule worktree\n* lack of --recurse-submodules in git-reset? (and that not\n  being default, see prior point)\n* submodules being in detached HEAD all the time?\n\nThanks,\nStefan\n"},{"id":"313922","messageId":"1489430233.10192.56.camel@novalis.org","threadId":"45346","inReplyTo":"CAGZ79kZmWaobW9e4iPY05y0N6PLcFphGnZmDHtrGKeV0Up70vg@mail.gmail.com","subject":"Re: bug?: git reset --mixed ignores deinitialized submodules","fromName":"David Turner","fromEmail":"novalis@novalis.org","sentAt":"2017-03-13T18:37:13Z","receivedAt":"2017-03-13T18:37:20Z","isPatch":false,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"On Mon, 2017-03-13 at 10:51 -0700, Stefan Beller wrote:\n> On Fri, Mar 10, 2017 at 1:06 PM, David Turner <novalis@novalis.org>\n> wrote:\n> > Git reset --mixed ignores submodules which are not\n> > initialized.  I've\n> > attached a demo script.\n> > \n> > On one hand, this matches the documentation (\"Resets the index but\n> > not\n> > the working tree\").  But on the other hand, it kind of doesn't:\n> > \"(i.e.,\n> > the changed files are preserved but not marked for commit)\".\n> > \n> > It's hard to figure out what a mixed reset should do.  It would be\n> > weird for it to initialize the submodule.  Maybe it should just\n> > refuse\n> > to run?  Maybe there should be an option for it to initialize the\n> > submodule for you?  Maybe it should drop a special-purpose file\n> > that\n> > git understands to be a submodule change?  For instance (and this\n> > is\n> > insane, but, like, maybe worth considering) it could use extended\n> > filesystem attributes, where available.\n> > \n> > #!/bin/bash\n> > mkdir demo\n> > cd demo\n> > \n> > git init main\n> > \n> > (\n> >         git init sub1 &&\n> >         cd sub1 &&\n> >         dd if=/dev/urandom of=f bs=40 count=1 &&\n> \n> We prefer reproducability in tests, so if we take this into\n> a (regression) test later we may want to\n>     s/dd.../echo \"determinism!\" >f/\n\nYeah, that was leftover from some previous version of this script, I\nthink.   This wasn't intended to be a t/ test, since I don't know what\nthe right answer is -- just a demo in case my prose was unclear.\n\n> > # commit that change on main,  deinit the submodule and do a mixed\n> > reset\n> > (\n> >         cd main &&\n> >         git add sub1 &&\n> >         git commit -m 'update sub1' &&\n> >         git submodule deinit sub1 &&\n> >         git reset --mixed HEAD^ &&\n> \n> As of now most commands (including reset)\n> are unaware of submodules to begin with.\n> They are ignored in most cases, i.e. git-status\n> has some tack-on to (pseudo-)report changes\n> in submodules, but it's not really builtin.\n> \n> A submodule that is not initialized\n> ( i.e. submodule.<name>.url is unset) ought\n> to not be touched at all. This is spelled out in\n> the man page for \"submodule update\" only at this\n> point.\n> \n> \n> >         git status # change to sub1 is lost\n> \n> The change is not really lost, as you can get it via\n> \n>     git checkout HEAD@{1}\n>     git submodule update --init\n\nSure, the commit isn't lost entirely.  But a mixed reset is often used\nto mean \"go back to before I committed\", and here, that's not precisely\nwhat's happening.  In other words, it's not confusing to the user.\n\n> This works most of the time, but it is unreliable as the\n> submodule may have had some gc inbetween which\n> threw away important objects.\n\nSure; that's a separate issue.\n\n> Steping back a bit, rereading the subject line,\n> what do you think is the bug here?\n> \n> * git-status not reporting about uninitialized submodules?\n\nHere, I think git-status is correctly reporting the state of the repo.\n\n> * git reset --mixed not touching the submodule worktree\n\nYes, possibly.\n\n> * lack of --recurse-submodules in git-reset? (and that not\n>   being default, see prior point)\n\nOr possibly this.\n\n> * submodules being in detached HEAD all the time?\n\nIn this case, the submodule is not initialized, so it is not in any\nstate at all.\n\n\nFor me, the bug (if any) is the bad user experience of doing a mixed\nreset and expecting to be able to commit (possibly with some git-add\noperations) from there and get back something like the commit to which\nthe user had git-reset.  \n\nThat's why I have the question mark there -- it's not clear that this\nis a reasonable expectation.\n\n"},{"id":"313970","messageId":"CAGZ79ka=OwSWGDBqiZ5X9LvOXphFe5=QjmH9DoeWLu2svmVtyg@mail.gmail.com","threadId":"45346","inReplyTo":"1489430233.10192.56.camel@novalis.org","subject":"Re: bug?: git reset --mixed ignores deinitialized submodules","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-03-13T21:19:23Z","receivedAt":"2017-03-13T21:19:30Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":">> The change is not really lost, as you can get it via\n>>\n>>     git checkout HEAD@{1}\n>>     git submodule update --init\n>\n> Sure, the commit isn't lost entirely.  But a mixed reset is often used\n> to mean \"go back to before I committed\", and here, that's not precisely\n> what's happening.\n\nWell, you ran the deinit after the commit, so I don't think\nwe expect to undo everything up to \"just before the commit\".\n\n> In other words, it's not confusing to the user.\n\nok, great :)\n\n>\n>> This works most of the time, but it is unreliable as the\n>> submodule may have had some gc inbetween which\n>> threw away important objects.\n>\n> Sure; that's a separate issue.\n>\n>> Steping back a bit, rereading the subject line,\n>> what do you think is the bug here?\n>>\n>> * git-status not reporting about uninitialized submodules?\n>\n> Here, I think git-status is correctly reporting the state of the repo.\n>\n>> * git reset --mixed not touching the submodule worktree\n>\n> Yes, possibly.\n>\n>> * lack of --recurse-submodules in git-reset? (and that not\n>>   being default, see prior point)\n>\n> Or possibly this.\n\nWell even in this case a reset recursing into submodules doesn't change\nthe (de-)init status of a submodule. All it would alter is the\nsubmodule HEAD pointing to, IMHO.\n\n>\n>> * submodules being in detached HEAD all the time?\n>\n> In this case, the submodule is not initialized, so it is not in any\n> state at all.\n\nOh right.\n\n>\n>\n> For me, the bug (if any) is the bad user experience of doing a mixed\n> reset and expecting to be able to commit (possibly with some git-add\n> operations) from there and get back something like the commit to which\n> the user had git-reset.\n\nTechnically this is also doable,\n\n    git reset --mixed HEAD^ # as in the original email\n    git update-index --add --cacheinfo \\\n        160000,$(git -C .git/modules/sub1 rev-parse HEAD),sub1\n    git add another_file\n    git commit -m \"recreate the commit\"\n\n>\n> That's why I have the question mark there -- it's not clear that this\n> is a reasonable expectation.\n\nFuzzing around with gitlinks that are non-populated submodules is\na messy business.\n\nSo another way would be to populate the submodule manually\n\n    GIT_DIR=.git/modules/sub1 \\\n    GIT_WORKTREE=sub1 \\\n    git checkout -f # implies last HEAD\n\nand then continue in the superproject.\n\nI am making up excuses for poor UX here, though.\nI do agree that the expectations may vary wildly because of poor UX\nin submodules.\n\nStefan\n"},{"id":"313975","messageId":"1489440993.10192.58.camel@novalis.org","threadId":"45346","inReplyTo":"CAGZ79ka=OwSWGDBqiZ5X9LvOXphFe5=QjmH9DoeWLu2svmVtyg@mail.gmail.com","subject":"Re: bug?: git reset --mixed ignores deinitialized submodules","fromName":"David Turner","fromEmail":"novalis@novalis.org","sentAt":"2017-03-13T21:36:33Z","receivedAt":"2017-03-13T21:36:40Z","isPatch":false,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"On Mon, 2017-03-13 at 14:19 -0700, Stefan Beller wrote:\n> > > The change is not really lost, as you can get it via\n> > > \n> > >     git checkout HEAD@{1}\n> > >     git submodule update --init\n> > \n> > Sure, the commit isn't lost entirely.  But a mixed reset is often\n> > used\n> > to mean \"go back to before I committed\", and here, that's not\n> > precisely\n> > what's happening.\n> \n> Well, you ran the deinit after the commit, so I don't think\n> we expect to undo everything up to \"just before the commit\".\n\nSure, but other workdir changes after the commit would have been blown\naway; so why not this one?\n\n> > > * lack of --recurse-submodules in git-reset? (and that not\n> > >   being default, see prior point)\n> > \n> > Or possibly this.\n> \n> Well even in this case a reset recursing into submodules doesn't\n> change the (de-)init status of a submodule. All it would alter is the\n> submodule HEAD pointing to, IMHO.\n\nThat's OK with me.  It's weird, but at least it's explicable. \n\n> > For me, the bug (if any) is the bad user experience of doing a\n> > mixed\n> > reset and expecting to be able to commit (possibly with some git-\n> > add\n> > operations) from there and get back something like the commit to\n> > which\n> > the user had git-reset.\n> \n> Technically this is also doable,\n> \n>     git reset --mixed HEAD^ # as in the original email\n>     git update-index --add --cacheinfo \\\n>         160000,$(git -C .git/modules/sub1 rev-parse HEAD),sub1\n>     git add another_file\n>     git commit -m \"recreate the commit\"\n\nYeah, unless the user had done various other operations that messed\nwith .git/modules/sub1 (e.g. if they had first checked out another\nbranch with --recurse-submodules).\n\nAlso, this updates the index, which a mixed reset isn't supposed to\ntouch. \n\n> > That's why I have the question mark there -- it's not clear that\n> > this\n> > is a reasonable expectation.\n> \n> Fuzzing around with gitlinks that are non-populated submodules is\n> a messy business.\n\nAgreed.\n\n> So another way would be to populate the submodule manually\n> \n>     GIT_DIR=.git/modules/sub1 \\\n>     GIT_WORKTREE=sub1 \\\n>     git checkout -f # implies last HEAD\n> \n> and then continue in the superproject.\n\n(see above for why this is not a general solution)\n\n> I am making up excuses for poor UX here, though.\n> I do agree that the expectations may vary wildly because of poor UX\n> in submodules.\n\nI agree that it's not quite clear what to expect.  I can just say that\nI was quite surprised when my colleague demoed this one for me.\n\n"}]}