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
Branch
default
Lint
No Linters Available
Unit
No Unit Test Coverage

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 31339

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