After a rebase -i I ended up with a file marked as clean while it still had uncommitted changes, I'll try to build a repro soon
Critical because invalide dirstate lead to data loss
I can reproduce in a shell, but not in the test suite. $ cat >> $HGRCPATH << EOF > [ui] > interactive=True > EOF $ hg init issue4592 $ cd issue4592 $ echo a > a $ hg add a $ hg commit -m a $ echo e > a $ hg st M a $ hg revert -i a << EOF > y > n > EOF diff -r cb9a9f314b8b a 1 hunks, 1 lines changed examine changes to 'a'? [Ynesfdaq?] y @@ -1,1 +1,1 @@ -e +a record this change to 'a'? [Ynesfdaq?] n st and diff are buggy $ hg st ? a.orig $ hg diff The rest is fine $ cat a e Touching file fixes it. $ touch a $ hg st M a ? a.orig There is either timing issue, that my shell hits but not the test, or something is weird in my interactive setup. Can someone try the same step manual on their own ends?
Looking a the code, we appears to be marking everything as "normal" even if we actually only partially revert them. I'm not sure why the test pass at all. Blindly falling back to the more prudent "normallookup" fixes it for me. I'll prepare a proper patch tomorrow. diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py --- a/mercurial/cmdutil.py +++ b/mercurial/cmdutil.py @@ -3099,10 +3099,13 @@ def _performrevert(repo, parents, ctx, a normal = repo.dirstate.normallookup else: normal = repo.dirstate.normal if interactive: + normallookup = None + if normal: + normallookup = repo.dirstate.normallookup # Prompt the user for changes to revert torevert = [repo.wjoin(f) for f in actions['revert'][0]] m = scmutil.match(ctx, torevert, {}) diff = patch.diff(repo, None, ctx.node(), m) originalchunks = patch.parsepatch(diff) @@ -3123,12 +3126,12 @@ def _performrevert(repo, parents, ctx, a except patch.PatchError, err: raise util.Abort(str(err)) del fp for f in actions['revert'][0]: - if normal: - normal(f) + if normallookup: + normallookup(f) else: for f in actions['revert'][0]: checkout(f) if normal:
FYI, I could reproduce the problem on my machine. Since we don't check if all or no chunks of a file are reverted, I agree that normallookup seems right. And it's probably not worth checking for those cases either.
I've actually dropped all the normal call, This is what happen when reverting againts an arbitrary rev and just lets the status logic do the work.
Fixed by http://selenic.com/repo/hg/rev/ab6e3729747e Pierre-Yves David <pierre-yves.david@fb.com> revert: stop marking files clean after interactive revert (issue4592) The goal of 'hg revert --interactive' is usually to keep some change in the revert file, so the files -must-not- be marked as clean. We want the status logic to do its usual job here. For unclear reasons (probably timing related), I was unable to build an automated test that reproduced issue4592 but manual testing shows this is fixed. (please test the fix)
Bug was set to TESTING for 7 days, resolving
Fixed by http://selenic.com/repo/hg/rev/45bd336e3991 Martin von Zweigbergk <martinvonz@google.com> revert: accept just -I/-X without paths or -a/-i (issue4592) 'hg revert -I foo' currently fails with abort: no files or directories specified (use --all to revert all files, or 'hg update 1' to update) It doesn't seem intentional that -I/-X without other paths or --all/--interactive should fail, and it doesn't seem that harmful to allow it either, so let's just do that. (please test the fix)
Sorry, 45bd336e3991 did not fix this issue, but issue 4618.