This is an archive of the discontinued Mercurial Phabricator instance.

Differential D9675

upgrade: remove unnecessary `is None` check
ClosedPublic

Authored by pulkit on Dec 31 2020, 11:43 AM.

Details

Reviewers
mharbison72
Group Reviewers
hg-reviewers
Commits
rHG0ca98ed828f9: upgrade: remove unnecessary `is None` check
Summary

upgrade_engine.upgrade() always return the backuppath value and there are
not early returns. Hence I don't see how backuppath can be None.

Adding extra unncessary safe checks hides unknown bugs. Hence removing it.

Diff Detail

Repository
rHG Mercurial
Branch
default
Lint
No Linters Available
Unit
No Unit Test Coverage

Event Timeline

pulkit created this revision.Dec 31 2020, 11:43 AM
marmoute added a subscriber: marmoute.Jan 7 2021, 9:41 PM

If we did nothing, we might not have a backup ?

pulkit added a comment.Jan 8 2021, 11:34 AM
In D9675#146240, @marmoute wrote:

If we did nothing, we might not have a backup ?

For now, upgrade_engine.upgrade always does something. IOW, if we run hg debugupgraderepo --run, even if there is nothing to upgrade, we process everything. This will be fixed in an upcoming patch.

mharbison72 accepted this revision.Jan 12 2021, 8:51 PM
This revision is now accepted and ready to land.Jan 12 2021, 8:51 PM
pulkit added a commit: rHG0ca98ed828f9: upgrade: remove unnecessary `is None` check.Jan 12 2021, 8:54 PM
Closed by commit rHG0ca98ed828f9: upgrade: remove unnecessary `is None` check (authored by pulkit). · Explain Why
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mmercurial/upgrade.py (2 lines)

Diff 24562

mercurial/upgrade.py

# clone ui without using ui.copy because repo.ui is protected # clone ui without using ui.copy because repo.ui is protected
repoui = repo.ui.__class__(repo.ui) repoui = repo.ui.__class__(repo.ui)
dstrepo = hg.repository(repoui, path=tmppath, create=True) dstrepo = hg.repository(repoui, path=tmppath, create=True)
with dstrepo.wlock(), dstrepo.lock(): with dstrepo.wlock(), dstrepo.lock():
backuppath = upgrade_engine.upgrade( backuppath = upgrade_engine.upgrade(
ui, repo, dstrepo, upgrade_op ui, repo, dstrepo, upgrade_op
) )
if not (backup or backuppath is None): if not backup:
ui.status( ui.status(
_(b'removing old repository content %s\n') % backuppath _(b'removing old repository content %s\n') % backuppath
) )
repo.vfs.rmtree(backuppath, forcibly=True) repo.vfs.rmtree(backuppath, forcibly=True)
backuppath = None backuppath = None
finally: finally:
ui.status(_(b'removing temporary repository %s\n') % tmppath) ui.status(_(b'removing temporary repository %s\n') % tmppath)