This is an archive of the discontinued Mercurial Phabricator instance.

Differential D11400

narrow: fix commits of empty files
ClosedPublic

Authored by valentin.gatienbaron on Sep 10 2021, 3:13 PM.

Details

Reviewers
durin42
Alphare
Group Reviewers
hg-reviewers
Commits
rHG5b9de38a0356: narrow: fix commits of empty files
Summary

The problem is that when committing a new file with empty contents (or
in general empty file with filelog p1 = -1), hg commit with narrow
doesn't create a filelog revision at all, which causes failures in
further commands.

The problem seems to be that:

  • hg thinks that instead of creating a new filelog revision, it can use the filelog's p1 (the nullrev)
  • because it thinks the file contents is the same in that revision and in p1
  • because narrowfilelog.cmp(nullrev, b'') is True (unlike with filelog.cmp)

It's not clear to me which cmp behaves better. But I think it makes
sense to change the commit code to not to "reuse" the null rev when
adding an empty file with filelog p1 == filelog p2 == -1. This is
consistent with never writing the null rev in the manifest, which `hg
verify` claims is an invariant:

inside/c@4: manifest refers to unknown revision 000000000000

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

Alphare requested changes to this revision.Sep 13 2021, 6:07 AM
Alphare added a subscriber: Alphare.

Other than my request in the test, the change seems like a good idea to me.

tests/test-narrow-commit.t
110–111

I would add a check that the file has actually been added by the commit and not just silently creating an empty commit somehow.

This revision now requires changes to proceed.Sep 13 2021, 6:07 AM
valentin.gatienbaron marked an inline comment as done.Sep 13 2021, 9:22 AM
Alphare accepted this revision.Sep 13 2021, 9:23 AM

Thanks. I'll queue this on stable.

This revision is now accepted and ready to land.Sep 13 2021, 9:23 AM
Alphare updated this revision to Diff 30216.Sep 13 2021, 11:35 AM
valentin.gatienbaron added a commit: rHG5b9de38a0356: narrow: fix commits of empty files.Sep 13 2021, 11:44 AM
Closed by commit rHG5b9de38a0356: narrow: fix commits of empty files (authored by valentin.gatienbaron). · Explain Why
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mmercurial/commit.py (1 line)
Mtests/test-narrow-commit.t (11 lines)

Diff 30221

mercurial/commit.py

and ms.active() and ms.active()
and ms.extras(fname).get(b'merge-removal-candidate') == b'yes' and ms.extras(fname).get(b'merge-removal-candidate') == b'yes'
): ):
force_new_node = True force_new_node = True
# is the file changed? # is the file changed?
text = fctx.data() text = fctx.data()
if ( if (
fparent2 != repo.nullid fparent2 != repo.nullid
or fparent1 == repo.nullid
or meta or meta
or flog.cmp(fparent1, text) or flog.cmp(fparent1, text)
or force_new_node or force_new_node
): ):
if touched is None: # do not overwrite added if touched is None: # do not overwrite added
if fparent2 == repo.nullid: if fparent2 == repo.nullid:
touched = 'modified' touched = 'modified'
else: else:

tests/test-narrow-commit.t

the timestamps of files stored by dirstate will appear older than the the timestamps of files stored by dirstate will appear older than the
dirstate file, and therefore we'll be able to get stable output from dirstate file, and therefore we'll be able to get stable output from
debugdirstate. If we don't do this, the test can be slightly flaky. debugdirstate. If we don't do this, the test can be slightly flaky.
$ sleep 3 $ sleep 3
$ hg status $ hg status
$ hg debugdirstate --no-dates $ hg debugdirstate --no-dates
n 644 10 set inside/f1 n 644 10 set inside/f1
Can't commit empty files Can commit empty files
$ touch inside/c; hg add inside/c; hg commit -qm _; hg verify -q $ touch inside/c; hg add inside/c; hg commit -qm _; hg verify -q
warning: revlog 'data/inside/c.i' not in fncache! $ hg cat -r . inside/c
AlphareUnsubmitted
Done

I would add a check that the file has actually been added by the commit and not just silently creating an empty commit somehow.

Alphare: I would add a check that the file has actually been added by the commit and not just silently…
4: empty or missing inside/c
inside/c@4: manifest refers to unknown revision 000000000000
1 warnings encountered!
hint: run "hg debugrebuildfncache" to recover from corrupt fncache
2 integrity errors encountered!
(first damaged changeset appears to be 4)
[1]