Submitter | phabricator |
---|---|
Date | Jan. 17, 2020, 6:25 p.m. |
Message ID | <differential-rev-PHID-DREV-l7v5q2gitjstftlm74lk-req@mercurial-scm.org> |
Download | mbox | patch |
Permalink | /patch/44508/ |
State | Superseded |
Headers | show |
Comments
durin42 added a comment. I'm +1 on having this functionality in core, FYI. REPOSITORY rHG Mercurial CHANGES SINCE LAST ACTION https://phab.mercurial-scm.org/D7932/new/ REVISION DETAIL https://phab.mercurial-scm.org/D7932 To: pulkit, #hg-reviewers Cc: durin42, mercurial-devel
marmoute added a comment. Can we call it `debugbackupbundle` for clarify? REPOSITORY rHG Mercurial CHANGES SINCE LAST ACTION https://phab.mercurial-scm.org/D7932/new/ REVISION DETAIL https://phab.mercurial-scm.org/D7932 To: pulkit, #hg-reviewers Cc: marmoute, durin42, mercurial-devel
This revision now requires changes to proceed. durin42 added a comment. durin42 requested changes to this revision. I think I'd prefer debugbackupbundle per marmoute's suggestion. REPOSITORY rHG Mercurial CHANGES SINCE LAST ACTION https://phab.mercurial-scm.org/D7932/new/ REVISION DETAIL https://phab.mercurial-scm.org/D7932 To: pulkit, #hg-reviewers, durin42 Cc: marmoute, durin42, mercurial-devel
martinvonz added a comment. I know this has already been queue and I'm just slow, but could you send follow-up patches to address my comments? INLINE COMMENTS > debugcommands.py:3416 > + ] > + + cmdutil.logopts, > + _(b"hg debugbackupbundle [--recover HASH]"), Do we really want to support all of these? (FYI, they include `--stat`, `--graph`, `--style`, `--patch`, and a few more.) > debugcommands.py:3432 > + backups = filter( > + os.path.isfile, glob.glob(repo.vfs.join("strip-backup") + "/*.hg") > + ) I think Windows doesn't care much about `/` vs `\`, but should we ideally use `os.path.join(..., ".hg")` here? > debugcommands.py:3436 > + > + opts["bundle"] = "" > + opts["force"] = None Clearer to pass `bundlename=""` in the call to `getremotechanges()`? > debugcommands.py:3437 > + opts["bundle"] = "" > + opts["force"] = None > + limit = logcmdutil.getlimit(opts) Clearer to pass `force=None` (or `False`?) in the call to `getremotechanges()`? > debugcommands.py:3441 > + def display(other, chlist, displayer): > + if opts.get("newest_first"): > + chlist.reverse() Always false, so delete it? > debugcommands.py:3518-3528 > + backupdate = time.strftime( > + "%a %H:%M, %Y-%m-%d", > + time.localtime(os.path.getmtime(source)), > + ) > + ui.status("\n%s\n" % (backupdate.ljust(50))) > + if ui.verbose: > + ui.status("%s%s\n" % ("bundle:".ljust(13), source)) Perhaps the template here should apply at the bundle level instead of the changeset level, so the user can also template the date (and maybe bundle name). > debugcommands.py:3528 > + "template" > + ] = "{label('status.modified', node|short)} {desc|firstline}\n" > + displayer = logcmdutil.changesetdisplayer( heh, `status.modified` seems like quite an abuse of the labeling system (I assume it was picked because the author thought that the color they had configured for that looked good in this context too). Could you switch to a different label? > debugcommands.py:3530 > + displayer = logcmdutil.changesetdisplayer( > + ui, other, opts, False > + ) Does `False` (for the `differ` argument) produce different output than the default (`None`) would? If not, remove the `False`? If it does, could you pass it as a keyword argument instead (`differ=False`)? REPOSITORY rHG Mercurial CHANGES SINCE LAST ACTION https://phab.mercurial-scm.org/D7932/new/ REVISION DETAIL https://phab.mercurial-scm.org/D7932 To: pulkit, #hg-reviewers, durin42 Cc: martinvonz, marmoute, durin42, mercurial-devel
mharbison72 added inline comments. INLINE COMMENTS > martinvonz wrote in debugcommands.py:3432 > I think Windows doesn't care much about `/` vs `\`, but should we ideally use `os.path.join(..., ".hg")` here? I thought we were trying to avoid `os.path`, so maybe just `repo.vfs.join("strip-backup", ".hg")`? (And don't these need to be `b''` prefixed for py3?) REPOSITORY rHG Mercurial CHANGES SINCE LAST ACTION https://phab.mercurial-scm.org/D7932/new/ REVISION DETAIL https://phab.mercurial-scm.org/D7932 To: pulkit, #hg-reviewers, durin42 Cc: mharbison72, martinvonz, marmoute, durin42, mercurial-devel
marmoute added inline comments. INLINE COMMENTS > mharbison72 wrote in debugcommands.py:3432 > I thought we were trying to avoid `os.path`, so maybe just `repo.vfs.join("strip-backup", ".hg")`? > > (And don't these need to be `b''` prefixed for py3?) +1 on using `vfs` here, this is the abstraction we use everywhere else. REPOSITORY rHG Mercurial CHANGES SINCE LAST ACTION https://phab.mercurial-scm.org/D7932/new/ REVISION DETAIL https://phab.mercurial-scm.org/D7932 To: pulkit, #hg-reviewers, durin42 Cc: mharbison72, martinvonz, marmoute, durin42, mercurial-devel
Patch
diff --git a/tests/test-debugbackup.t b/tests/test-debugbackup.t new file mode 100644 --- /dev/null +++ b/tests/test-debugbackup.t @@ -0,0 +1,36 @@ + $ cat >> $HGRCPATH << EOF + > [extensions] + > strip= + > EOF + +Setup repo + + $ hg init repo + $ cd repo + +Test backups list and recover + + $ mkcommit() { + > echo "$1" > "$1" + > hg add "$1" + > hg ci -l $1 + > } + $ mkcommit a + $ mkcommit b + $ hg strip . + 0 files updated, 0 files merged, 1 files removed, 0 files unresolved + saved backup bundle to $TESTTMP/repo/.hg/strip-backup/d2ae7f538514-2953539b-backup.hg (glob) + $ hg debugbackups + Recover changesets using: hg debugbackups --recover <changeset hash> + + Available backup changesets: + * (glob) + d2ae7f538514 b + + $ hg debugbackups --recover d2ae7f538514 + Unbundling d2ae7f538514 + adding changesets + adding manifests + adding file changes + added 1 changesets with 1 changes to 1 files + new changesets d2ae7f538514 (1 drafts) diff --git a/mercurial/debugcommands.py b/mercurial/debugcommands.py --- a/mercurial/debugcommands.py +++ b/mercurial/debugcommands.py @@ -11,6 +11,7 @@ import collections import difflib import errno +import glob import operator import os import random @@ -38,6 +39,7 @@ ) from . import ( bundle2, + bundlerepo, changegroup, cmdutil, color, @@ -130,6 +132,137 @@ @command( + "debugbackups", + [ + ( + "", + "recover", + "", + "brings the specified changeset back into the repository", + ) + ] + + cmdutil.logopts, + _("hg debugbackups [--recover HASH]"), +) +def debugbackups(ui, repo, *pats, **opts): + """lists the changesets available in backup bundles + + Without any arguments, this command prints a list of the changesets in each + backup bundle. + + --recover takes a changeset hash and unbundles the first bundle that + contains that hash, which puts that changeset back in your repository. + + --verbose will print the entire commit message and the bundle path for that + backup. + """ + backuppath = repo.vfs.join("strip-backup") + backups = filter(os.path.isfile, glob.glob(backuppath + "/*.hg")) + backups.sort(key=lambda x: os.path.getmtime(x), reverse=True) + + opts["bundle"] = "" + opts["force"] = None + + def display(other, chlist, displayer): + limit = logcmdutil.getlimit(opts) + if opts.get("newest_first"): + chlist.reverse() + count = 0 + for n in chlist: + if limit is not None and count >= limit: + break + parents = [p for p in other.changelog.parents(n) if p != nullid] + if opts.get("no_merges") and len(parents) == 2: + continue + count += 1 + displayer.show(other[n]) + + recovernode = opts.get("recover") + if recovernode: + if scmutil.isrevsymbol(repo, recovernode): + ui.warn(_("%s already exists in the repo\n") % recovernode) + return + else: + msg = _( + "Recover changesets using: hg debugbackups --recover " + "<changeset hash>\n\nAvailable backup changesets:" + ) + ui.status(msg, label="status.removed") + + for backup in backups: + # Much of this is copied from the hg incoming logic + source = os.path.relpath(backup, encoding.getcwd()) + source = ui.expandpath(source) + source, branches = hg.parseurl(source, opts.get("branch")) + try: + other = hg.peer(repo, opts, source) + except error.LookupError as ex: + msg = _("\nwarning: unable to open bundle %s") % source + hint = _("\n(missing parent rev %s)\n") % short(ex.name) + ui.warn(msg) + ui.warn(hint) + continue + revs, checkout = hg.addbranchrevs( + repo, other, branches, opts.get("rev") + ) + + if revs: + revs = [other.lookup(rev) for rev in revs] + + quiet = ui.quiet + try: + ui.quiet = True + other, chlist, cleanupfn = bundlerepo.getremotechanges( + ui, repo, other, revs, opts["bundle"], opts["force"] + ) + except error.LookupError: + continue + finally: + ui.quiet = quiet + + try: + if chlist: + if recovernode: + tr = lock = None + try: + lock = repo.lock() + if scmutil.isrevsymbol(other, recovernode): + ui.status(_("Unbundling %s\n") % (recovernode)) + f = hg.openpath(ui, source) + gen = exchange.readbundle(ui, f, source) + tr = repo.transaction("unbundle") + if not isinstance(gen, bundle2.unbundle20): + gen.apply(repo, "unbundle", "bundle:" + source) + if isinstance(gen, bundle2.unbundle20): + bundle2.applybundle( + repo, + gen, + tr, + source="unbundle", + url="bundle:" + source, + ) + tr.close() + break + finally: + lockmod.release(lock, tr) + else: + backupdate = os.path.getmtime(source) + backupdate = time.strftime( + "%a %H:%M, %Y-%m-%d", time.localtime(backupdate) + ) + ui.status("\n%s\n" % (backupdate.ljust(50))) + if not ui.verbose: + opts["template"] = "{label('status.modified', node|short)} {desc|firstline}\n" + else: + ui.status("%s%s\n" % ("bundle:".ljust(13), source)) + displayer = logcmdutil.changesetdisplayer(ui, other, opts, False) + display(other, chlist, displayer) + displayer.close() + finally: + cleanupfn() + + +@command( b'debugbuilddag', [ (