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
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

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 24766

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)