threads / discuss / 51288
Subject: Git status parse error after v.2.22.0 upgrade
Seeing an error after upgrading to git v2.22.0.
git status shows “error: could not parse ‘*a commit message from 2012*’”
Downgrading to v.2.21.0 does not yield this message for git status.
I can only replicate this in a private repo I have so far. Unable to share the repo but can debug if you need more info.
Johannes SchindelinRe: Git status parse error after v.2.22.0 upgrade
Hi Espen,
On Tue, 11 Jun 2019, Espen Antonsen wrote:
> Seeing an error after upgrading to git v2.22.0. > > git status shows “error: could not parse ‘*a commit message from 2012*’” > > Downgrading to v.2.21.0 does not yield this message for git status. > > I can only replicate this in a private repo I have so far. Unable to > share the repo but can debug if you need more info.
Sounds like a hook gone haywire. Find out more by setting `GIT_TRACE=1` before repeating the command.
Ciao, Johannes
Re: Git status parse error after v.2.22.0 upgrade
I don't have any hooks, just the default sample scripts. GIT_TRACE doesn't show anything else.
On Tue, 11 Jun 2019, at 21:28, Johannes Schindelin wrote:
> Hi Espen, > > On Tue, 11 Jun 2019, Espen Antonsen wrote: > > > Seeing an error after upgrading to git v2.22.0. > > > > git status shows “error: could not parse ‘*a commit message from 2012*’” > > > > Downgrading to v.2.21.0 does not yield this message for git status. > > > > I can only replicate this in a private repo I have so far. Unable to > > share the repo but can debug if you need more info. > > Sounds like a hook gone haywire. Find out more by setting `GIT_TRACE=1` > before repeating the command. > > Ciao, > Johannes >
Johannes SchindelinRe: Git status parse error after v.2.22.0 upgrade
Hi Espen,
On Wed, 12 Jun 2019, Espen Antonsen wrote:
> I don't have any hooks, just the default sample scripts. GIT_TRACE > doesn't show anything else.
Okay, it was just a guess.
And here is another guess: unless you impart with more details about your test case, I guess there is little anybody else can do to help you.
Ciao, Johannes
Re: Git status parse error after v.2.22.0 upgrade
Hello,
Managed to repro this problem on my private repo too.
Bisect points to b51a0fdc3822c2ef260f6d496b6df6d33b101e8a However, I think the real culprit is 4a72486de97b5c6b0979b2b51e50c268bdb0d4f6, specifically `parse_insn_line` call.
My repo indeed contains a ".git/sequencer/todo" file which contains references to commits long-gone (i.e., rebased). Renaming or deleting this file stops whines about "error: could not parse".
-- Regards, Aleksandrs Ļedovskis
On Wed, Jun 12, 2019 at 08:47:27PM +0200, Johannes Schindelin wrote:
> Hi Espen, > > On Wed, 12 Jun 2019, Espen Antonsen wrote: > > > I don't have any hooks, just the default sample scripts. GIT_TRACE > > doesn't show anything else. > > Okay, it was just a guess. > > And here is another guess: unless you impart with more details about your > test case, I guess there is little anybody else can do to help you. > > Ciao, > Johannes
Junio C HamanoRe: Git status parse error after v.2.22.0 upgrade
aleksandrs@ledovskis.lv writes:
> My repo indeed contains a ".git/sequencer/todo" file which > contains references to commits long-gone (i.e., rebased). > Renaming or deleting this file stops whines about "error: could > not parse".
Interesting. So in short, when the repository has leftover sequencer state file that is not in use, "git status parse" thing (whatever it is---are you getting it when you run "git status" command???)---is not careful enough to notice that it does not matter even if that leftover file is unusable.
Two issues "the sequencer" folks may want to address are
(1) make the one that reads an irrelevant/stale 'todo' file more
careful to ignore errors in such a file; (2) make the sequencer machinery more careful to clean up after it
is done or it is aborted (for example, "git reset --hard"
could remove these state files preemptively even when a rebase
is not in progress, I would think).I think we already had some patches toward the latter recently.
Thanks for a report.
Jeff KingRe: Git status parse error after v.2.22.0 upgrade
On Thu, Jun 13, 2019 at 09:05:16AM -0700, Junio C Hamano wrote:
> aleksandrs@ledovskis.lv writes: > > > My repo indeed contains a ".git/sequencer/todo" file which > > contains references to commits long-gone (i.e., rebased). > > Renaming or deleting this file stops whines about "error: could > > not parse". > > Interesting. So in short, when the repository has leftover > sequencer state file that is not in use, "git status parse" thing > (whatever it is---are you getting it when you run "git status" > command???)---is not careful enough to notice that it does not > matter even if that leftover file is unusable. > > Two issues "the sequencer" folks may want to address are > > (1) make the one that reads an irrelevant/stale 'todo' file more > careful to ignore errors in such a file; > > (2) make the sequencer machinery more careful to clean up after it > is done or it is aborted (for example, "git reset --hard" > could remove these state files preemptively even when a rebase > is not in progress, I would think). > > I think we already had some patches toward the latter recently.
Maybe I am not understanding it correctly, but do you mean in (2) that "git reset --hard" would always clear sequencer state? That seems undesirable to me, as I often use "git reset" to move the head around during a rebase. (e.g., when using "rebase -i" to split apart I commit, I stop on that commit, "git reset" back to the parent, and then selectively "add -p" the various parts).
Direction (1) seems quite sensible to me, though.
-Peff
Re: Git status parse error after v.2.22.0 upgrade
On 13/06/2019 17:24, Jeff King wrote:
> On Thu, Jun 13, 2019 at 09:05:16AM -0700, Junio C Hamano wrote: > >> aleksandrs@ledovskis.lv writes: >> >>> My repo indeed contains a ".git/sequencer/todo" file which >>> contains references to commits long-gone (i.e., rebased). >>> Renaming or deleting this file stops whines about "error: could >>> not parse".
I think this bug report is a result of 4a72486de9 ("fix cherry-pick/revert status after commit", 2019-04-16) which tries to read the sequencer todo file if it is present and there is no CHERRY_PICK_HEAD/REVERT_HEAD. Before that if the user committed a conflict resolution in the middle of a sequence of picks then status would forget that there was a cherry-pick in progress. It just reuses the code that cherry-pick uses to parse the first todo list item which expects the commits to exist.
>> Interesting. So in short, when the repository has leftover >> sequencer state file that is not in use, "git status parse" thing >> (whatever it is---are you getting it when you run "git status" >> command???)---is not careful enough to notice that it does not >> matter even if that leftover file is unusable. >> >> Two issues "the sequencer" folks may want to address are >> >> (1) make the one that reads an irrelevant/stale 'todo' file more >> careful to ignore errors in such a file; >> >> (2) make the sequencer machinery more careful to clean up after it >> is done or it is aborted (for example, "git reset --hard" >> could remove these state files preemptively even when a rebase >> is not in progress, I would think). >> >> I think we already had some patches toward the latter recently. > > Maybe I am not understanding it correctly, but do you mean in (2) that > "git reset --hard" would always clear sequencer state?
I think the commit Junio is referring to is b07d9bfd17 ("commit/reset: try to clean up sequencer state", 2019-04-16) which will only remove the sequencer directory if it stops after the pick was the last one in the series. The idea is that if cherry-pick stops for a conflict resolution on the last pick user commits the result directly or run reset without running `cherry-pick --continue` afterwards the sequencer state gets cleaned up properly.
> That seems > undesirable to me, as I often use "git reset" to move the head around > during a rebase. (e.g., when using "rebase -i" to split apart I commit, > I stop on that commit, "git reset" back to the parent, and then > selectively "add -p" the various parts). > > Direction (1) seems quite sensible to me, though.
Now that we try harder to clean up the sequencer state I wonder if that would just cover up bugs where the state has not been removed when it should have been. That can lead to unpleasant problems if the user aborts a single revert/cherry-pick when there is stale sequencer state around as it rewinds HEAD to the commit when the stale cherry-pick/revert was started as explained in the message to b07d9bfd17 ("commit/reset: try to clean up sequencer state", 2019-04-16)
If we do want to do something then maybe teaching gc not to collect commits listed in .git/sequencer/todo and .git/rebase-merge/git-rebase-todo would be useful.
Best Wishes
Phillip
> -Peff >
Jeff KingRe: Git status parse error after v.2.22.0 upgrade
On Thu, Jun 13, 2019 at 06:43:41PM +0100, Phillip Wood wrote:
> >> (2) make the sequencer machinery more careful to clean up after it
> >> is done or it is aborted (for example, "git reset --hard"
> >> could remove these state files preemptively even when a rebase
> >> is not in progress, I would think).
> >>
> >> I think we already had some patches toward the latter recently.
> >
> > Maybe I am not understanding it correctly, but do you mean in (2) that
> > "git reset --hard" would always clear sequencer state?
>
> I think the commit Junio is referring to is
> b07d9bfd17 ("commit/reset: try to clean up sequencer state", 2019-04-16)
> which will only remove the sequencer directory if it stops after the
> pick was the last one in the series. The idea is that if cherry-pick
> stops for a conflict resolution on the last pick user commits the result
> directly or run reset without running `cherry-pick --continue`
> afterwards the sequencer state gets cleaned up properly.OK, that makes a lot more sense to me. It did make me wonder if doing an "e"dit on the final commit of a rebase followed by "reset HEAD^" would clear the sequencer state. But looking at that commit, this kicks in only for the cherry-pick and revert cases.
Thanks for explaining.
> If we do want to do something then maybe teaching gc not to collect > commits listed in .git/sequencer/todo and > .git/rebase-merge/git-rebase-todo would be useful.
IMHO that opens up a can of worms, because the reachability rules get more complicated (so for example people trying to prune away unwanted history get confused about what is still mentioning the objects). That's a reasonable rare case. OTOH, so is mentioning commits in an ongoing rebase that are not still reachable from the branch you are rebasing, nor the HEAD reflog.
-Peff
Re: Git status parse error after v.2.22.0 upgrade
On 13/06/2019 18:43, Phillip Wood wrote:
> On 13/06/2019 17:24, Jeff King wrote:
>> On Thu, Jun 13, 2019 at 09:05:16AM -0700, Junio C Hamano wrote:
>>>
>>> Two issues "the sequencer" folks may want to address are
>>>
>>> (1) make the one that reads an irrelevant/stale 'todo' file more
>>> careful to ignore errors in such a file;
>>>
>>> (2) make the sequencer machinery more careful to clean up after it
>>> is done or it is aborted (for example, "git reset --hard"
>>> could remove these state files preemptively even when a rebase
>>> is not in progress, I would think).
>>>
>>> I think we already had some patches toward the latter recently.
>>
>> Maybe I am not understanding it correctly, but do you mean in (2) that
>> "git reset --hard" would always clear sequencer state?
>
> I think the commit Junio is referring to is
> b07d9bfd17 ("commit/reset: try to clean up sequencer state", 2019-04-16)
> which will only remove the sequencer directory if it stops after the
> pick was the last one in the series. The idea is that if cherry-pick
> stops for a conflict resolution on the last pick user commits the result
> directly or run reset without running `cherry-pick --continue`
> afterwards the sequencer state gets cleaned up properly.
>
>> That seems
>> undesirable to me, as I often use "git reset" to move the head around
>> during a rebase. (e.g., when using "rebase -i" to split apart I commit,
>> I stop on that commit, "git reset" back to the parent, and then
>> selectively "add -p" the various parts).
>>
>> Direction (1) seems quite sensible to me, though.
>
> Now that we try harder to clean up the sequencer state I wonder if that
> would just cover up bugs where the state has not been removed when it
> should have been.When I wrote that it hadn't dawned on me that if there is an error the status will not tell the user that a cherry-pick is in progress which is what we really want so they are alerted to the stale sequencer state. I've posted a series [1] to address this (sadly gitgitgadget wont let me post it on this thread).
Best Wishes
Phillip
[1] https://public-inbox.org/git/pull.275.git.gitgitgadget@gmail.com/T/#mf57a4ab95ba907fbf2d06ec64e9b676db158eace
> That can lead to unpleasant problems if the user
> aborts a single revert/cherry-pick when there is stale sequencer state
> around as it rewinds HEAD to the commit when the stale
> cherry-pick/revert was started as explained in the message to b07d9bfd17
> ("commit/reset: try to clean up sequencer state", 2019-04-16)
>
> If we do want to do something then maybe teaching gc not to collect
> commits listed in .git/sequencer/todo and
> .git/rebase-merge/git-rebase-todo would be useful.
>
> Best Wishes
>
> Phillip
>
>> -Peff
>>
> 
SZEDER GáborRe: Git status parse error after v.2.22.0 upgrade
On Thu, Jun 13, 2019 at 09:05:16AM -0700, Junio C Hamano wrote:
> aleksandrs@ledovskis.lv writes: > > > My repo indeed contains a ".git/sequencer/todo" file which > > contains references to commits long-gone (i.e., rebased). > > Renaming or deleting this file stops whines about "error: could > > not parse". > > Interesting. So in short, when the repository has leftover > sequencer state file that is not in use, "git status parse" thing > (whatever it is---are you getting it when you run "git status" > command???)---is not careful enough to notice that it does not > matter even if that leftover file is unusable. > > Two issues "the sequencer" folks may want to address are > > (1) make the one that reads an irrelevant/stale 'todo' file more > careful to ignore errors in such a file; > > (2) make the sequencer machinery more careful to clean up after it > is done or it is aborted
It may or may not be related, but... Some weeks (months?) ago I run into a situation where 'git rebase' didn't clean up after itself following a well-timed ctrl-C, and got confused to the point that not even a subsequent 'git rebase --abort' was able to rectify the situation.
So, I wanted to rebase just a couple of commits to somewhere else, but messed up the command's parameters, and it then tried to rebase a couple hundred commits. Upon noticing the unexpectedly large numbers in the "Generating patches" progress line I hit ctrl-C, and then the aborting 'git rebase' apparently left an incomplete/corrupted '.git/rebase-apply/' directory behind. Unfortunately I didn't think about saving the precious corrupted state, and deleted that dir right away... only saved the transcript from the terminal later:
~/src/git (test-atexit)$ git rebase js/test-atexit First, rewinding head to replay your work on top of it... Generating patches: 100% (509/509), done. ^C ~/src/git ((42e6cb0046...)|REBASE 1/509)$ gitk js/test-atexit ~/src/git ((42e6cb0046...)|REBASE 1/509)$ git rebase --abort error: could not read '.git/rebase-apply/head-name': No such file or directory ~/src/git ((42e6cb0046...)|REBASE 1/509)$ git reset --hard HEAD is now at 42e6cb0046 git-p4: use `test_atexit` to kill the daemon ~/src/git ((42e6cb0046...)|REBASE 1/509)$ rm -rf .git/re rebase-apply/ rebased-patches refs/ ~/src/git ((42e6cb0046...)|REBASE 1/509)$ rm -rf .git/rebase-apply/ ~/src/git ((42e6cb0046...))$ git checkout -
Re: Git status parse error after v.2.22.0 upgrade
On Thu, Jun 13, 2019 at 09:05:16AM -0700, Junio C Hamano wrote:
> are you getting it when you run "git status" command???
Yes, the regular old porcelain `git-status(1)`.
$ git status error: could not parse '<<full contents of .git/sequencer/todo here>>' On branch <<some-branch>> nothing to commit, working tree clean
-- Best, Aleksandrs Ļedovskis