This is an archive of the discontinued Mercurial Phabricator instance.

Differential D11869

upgrade: explicitly warn when a `--no-xxx` flag is overwritten
ClosedPublic

Authored by marmoute on Dec 7 2021, 3:53 AM.

Details

Reviewers
Alphare
Group Reviewers
hg-reviewers
Commits
rHG1d0978cfe968: upgrade: explicitly warn when a `--no-xxx` flag is overwritten
Summary

Some format upgrade/downgrades -needs- revlog to be recomputed. So we now
detect that individually and warn when it contradict explicitly passed flag.

This is part of a larger series to make debugupgraderepo smarter about which
revlog it picks by default.

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

marmoute created this revision.Dec 7 2021, 3:53 AM
marmoute updated this revision to Diff 31339.Dec 7 2021, 4:13 AM
Alphare requested changes to this revision.Dec 10 2021, 4:54 AM
Alphare added a subscriber: Alphare.

There is also a typo in the commit message: s/server/series/

mercurial/upgrade.py
107

I think it would be simpler (and technically faster even though it most likely doesn't matter here) to say if not specified_revlogs.get(FL) (same for the other two similar places)

This revision now requires changes to proceed.Dec 10 2021, 4:54 AM
marmoute added inline comments.Dec 10 2021, 5:14 AM
mercurial/upgrade.py
107

There is a semantic difference between the lack of value (None, no flag passed) and an explicite "False" (--no-xxx) flag passed.

So this does not seems an option here.

Alphare added inline comments.Dec 10 2021, 5:16 AM
mercurial/upgrade.py
107

Would if specified_revlogs.get(FL) == False work?

marmoute added inline comments.Dec 10 2021, 5:18 AM
mercurial/upgrade.py
107

This is a forbidden construct. I'll have to report your proposal to the local python inquisition.

Alphare added inline comments.Dec 10 2021, 5:20 AM
mercurial/upgrade.py
107

*sighs in dynamic typing*

marmoute edited the summary of this revision. (Show Details)Dec 10 2021, 6:24 AM
marmoute added a commit: rHG1d0978cfe968: upgrade: explicitly warn when a `--no-xxx` flag is overwritten.Dec 10 2021, 10:47 AM
This revision was not accepted when it landed; it landed in state Needs Revision.
Closed by commit rHG1d0978cfe968: upgrade: explicitly warn when a `--no-xxx` flag is overwritten (authored by marmoute). · Explain Why
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mmercurial/upgrade.py (37 lines)
Mtests/test-upgrade-repo.t (16 lines)

Diff 31420

mercurial/upgrade.py

) )
removed_actions = upgrade_actions.find_format_downgrades(repo) removed_actions = upgrade_actions.find_format_downgrades(repo)
removedreqs = repo.requirements - newreqs removedreqs = repo.requirements - newreqs
addedreqs = newreqs - repo.requirements addedreqs = newreqs - repo.requirements
# check if we need to touch revlog and if so, which ones # check if we need to touch revlog and if so, which ones
touched_revlogs = set()
overwrite_msg = _(b'warning: ignoring %14s, as upgrade is changing: %s\n')
msg_issued = False
FL = upgrade_engine.UPGRADE_FILELOGS
MN = upgrade_engine.UPGRADE_MANIFEST
CL = upgrade_engine.UPGRADE_CHANGELOG
for action in sorted(up_actions + removed_actions, key=lambda a: a.name):
# optimisation does not "requires anything, they just needs it.
if action.type != upgrade_actions.FORMAT_VARIANT:
continue
if action.touches_filelogs and FL not in touched_revlogs:
if FL in specified_revlogs:
AlphareUnsubmitted
Not Done

I think it would be simpler (and technically faster even though it most likely doesn't matter here) to say if not specified_revlogs.get(FL) (same for the other two similar places)

Alphare: I think it would be simpler (and technically faster even though it most likely doesn't matter…
marmouteAuthorUnsubmitted
Done

There is a semantic difference between the lack of value (None, no flag passed) and an explicite "False" (--no-xxx) flag passed.

So this does not seems an option here.

marmoute: There is a semantic difference between the lack of value (None, no flag passed) and an…
AlphareUnsubmitted
Not Done

Would if specified_revlogs.get(FL) == False work?

Alphare: Would `if specified_revlogs.get(FL) == False` work?
marmouteAuthorUnsubmitted
Done

This is a forbidden construct. I'll have to report your proposal to the local python inquisition.

marmoute: This is a forbidden construct. I'll have to report your proposal to the local python…
AlphareUnsubmitted
Not Done

*sighs in dynamic typing*

Alphare: *sighs in dynamic typing*
if not specified_revlogs[FL]:
msg = overwrite_msg % (b'--no-filelogs', action.name)
ui.warn(msg)
msg_issued = True
touched_revlogs.add(FL)
if action.touches_manifests and MN not in touched_revlogs:
if MN in specified_revlogs:
if not specified_revlogs[MN]:
msg = overwrite_msg % (b'--no-manifest', action.name)
ui.warn(msg)
msg_issued = True
touched_revlogs.add(MN)
if action.touches_changelog and CL not in touched_revlogs:
if CL in specified_revlogs:
if not specified_revlogs[CL]:
msg = overwrite_msg % (b'--no-changelog', action.name)
ui.warn(msg)
msg_issued = True
touched_revlogs.add(CL)
if msg_issued:
ui.warn((b"\n"))
revlogs = set(upgrade_engine.UPGRADE_ALL_REVLOGS) revlogs = set(upgrade_engine.UPGRADE_ALL_REVLOGS)
if specified_revlogs: if specified_revlogs:
# we have some limitation on revlogs to be recloned # we have some limitation on revlogs to be recloned
if any(specified_revlogs.values()): if any(specified_revlogs.values()):
revlogs = set() revlogs = set()
for upgrade, enabled in specified_revlogs.items(): for upgrade, enabled in specified_revlogs.items():
if enabled: if enabled:
revlogs.add(upgrade) revlogs.add(upgrade)

tests/test-upgrade-repo.t

checking files checking files
checked 3 changesets with 3 changes to 3 files checked 3 changesets with 3 changes to 3 files
Check you can't skip revlog clone during important format downgrade Check you can't skip revlog clone during important format downgrade
$ echo "[format]" > .hg/hgrc $ echo "[format]" > .hg/hgrc
$ echo "sparse-revlog=no" >> .hg/hgrc $ echo "sparse-revlog=no" >> .hg/hgrc
$ hg debugupgrade --optimize re-delta-parent --no-manifest --no-backup --quiet
warning: ignoring --no-manifest, as upgrade is changing: sparserevlog
ignoring revlogs selection flags, format requirements change: sparserevlog
requirements
preserved: dotencode, fncache, generaldelta, revlogv1, store (no-rust !)
preserved: dotencode, fncache, generaldelta, persistent-nodemap, revlogv1, store (rust !)
removed: sparserevlog
optimisations: re-delta-parent
processed revlogs:
- all-filelogs
- changelog
- manifest
$ hg debugupgrade --optimize re-delta-parent --run --manifest --no-backup --debug --traceback $ hg debugupgrade --optimize re-delta-parent --run --manifest --no-backup --debug --traceback
ignoring revlogs selection flags, format requirements change: sparserevlog ignoring revlogs selection flags, format requirements change: sparserevlog
upgrade will perform the following actions: upgrade will perform the following actions:
requirements requirements
preserved: dotencode, fncache, generaldelta, revlogv1, store (no-rust !) preserved: dotencode, fncache, generaldelta, revlogv1, store (no-rust !)
preserved: dotencode, fncache, generaldelta, persistent-nodemap, revlogv1, store (rust !) preserved: dotencode, fncache, generaldelta, persistent-nodemap, revlogv1, store (rust !)
removed: sparserevlog removed: sparserevlog