This is an archive of the discontinued Mercurial Phabricator instance.

Differential D7714

subrepo: fix a crash when archiving an svn or git subrepo
ClosedPublic

Authored by mharbison72 on Dec 24 2019, 2:03 AM.

Details

Reviewers
None
Group Reviewers
hg-reviewers
Commits
rHG7ca8aa8840c0: subrepo: fix a crash when archiving an svn or git subrepo
Summary

Only hgsubrepos have a repository attribute. This is pretty hacky, but probably
the best we can do on stable. Pushing the lfstatus check down into the wrapper
for hgsubrepo (and dropping the check for lfstatus at the top of
hgsubrepoarchive()) resulted in various test failures because:

  1. hgsubrepoarchive isn't returning the number of files archived at the bottom, resulting in an error about += NoneType
  2. These copypasta archive wrappers don't use progress bars
  3. Largefiles are *not* currently archived when using extdiff (68822b7cdd01), but pushing this context manager down into the subrepo resulted in it apparently doing so (as evidenced by progress bars being dropped)

The other uses of lfstatus() are not in the substate processing loop, so they
shouldn't be an issue.

I initially put testcases in this test for largefiles-{on,off}, and it flagged
a bunch of exit code differences for cat and diff, so I backed that off.

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

mharbison72 created this revision.Dec 24 2019, 2:03 AM
mharbison72 added a commit: rHG7ca8aa8840c0: subrepo: fix a crash when archiving an svn or git subrepo.Jan 8 2020, 2:43 PM
This revision was not accepted when it landed; it landed in state Needs Review.
Closed by commit rHG7ca8aa8840c0: subrepo: fix a crash when archiving an svn or git subrepo (authored by mharbison72). · Explain Why
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Diff 19089

hgext/largefiles/overrides.py

getdata = lambda: util.readfile(path) getdata = lambda: util.readfile(path)
write(f, b'x' in ff and 0o755 or 0o644, b'l' in ff, getdata) write(f, b'x' in ff and 0o755 or 0o644, b'l' in ff, getdata)
if subrepos: if subrepos:
for subpath in sorted(ctx.substate): for subpath in sorted(ctx.substate):
sub = ctx.workingsub(subpath) sub = ctx.workingsub(subpath)
submatch = matchmod.subdirmatcher(subpath, match) submatch = matchmod.subdirmatcher(subpath, match)
subprefix = prefix + subpath + b'/' subprefix = prefix + subpath + b'/'
with lfstatus(sub._repo):
# TODO: Only hgsubrepo instances have `_repo`, so figure out how to
# infer and possibly set lfstatus in hgsubrepoarchive. That would
# allow only hgsubrepos to set this, instead of the current scheme
# where the parent sets this for the child.
with (
util.safehasattr(sub, '_repo')
and lfstatus(sub._repo)
or util.nullcontextmanager()
):
sub.archive(archiver, subprefix, submatch) sub.archive(archiver, subprefix, submatch)
archiver.done() archiver.done()
@eh.wrapfunction(subrepo.hgsubrepo, b'archive') @eh.wrapfunction(subrepo.hgsubrepo, b'archive')
def hgsubrepoarchive(orig, repo, archiver, prefix, match=None, decode=True): def hgsubrepoarchive(orig, repo, archiver, prefix, match=None, decode=True):
lfenabled = util.safehasattr(repo._repo, b'_largefilesenabled') lfenabled = util.safehasattr(repo._repo, b'_largefilesenabled')
getdata = lambda: util.readfile(os.path.join(prefix, path)) getdata = lambda: util.readfile(os.path.join(prefix, path))
write(f, b'x' in ff and 0o755 or 0o644, b'l' in ff, getdata) write(f, b'x' in ff and 0o755 or 0o644, b'l' in ff, getdata)
for subpath in sorted(ctx.substate): for subpath in sorted(ctx.substate):
sub = ctx.workingsub(subpath) sub = ctx.workingsub(subpath)
submatch = matchmod.subdirmatcher(subpath, match) submatch = matchmod.subdirmatcher(subpath, match)
subprefix = prefix + subpath + b'/' subprefix = prefix + subpath + b'/'
with lfstatus(sub._repo): # TODO: Only hgsubrepo instances have `_repo`, so figure out how to
# infer and possibly set lfstatus at the top of this function. That
# would allow only hgsubrepos to set this, instead of the current scheme
# where the parent sets this for the child.
with (
util.safehasattr(sub, '_repo')
and lfstatus(sub._repo)
or util.nullcontextmanager()
):
sub.archive(archiver, subprefix, submatch, decode) sub.archive(archiver, subprefix, submatch, decode)
# If a largefile is modified, the change is not reflected in its # If a largefile is modified, the change is not reflected in its
# standin until a commit. cmdutil.bailifchanged() raises an exception # standin until a commit. cmdutil.bailifchanged() raises an exception
# if the repo has uncommitted changes. Wrap it to also check if # if the repo has uncommitted changes. Wrap it to also check if
# largefiles were changed. This is used by bisect, backout and fetch. # largefiles were changed. This is used by bisect, backout and fetch.
@eh.wrapfunction(cmdutil, b'bailifchanged') @eh.wrapfunction(cmdutil, b'bailifchanged')

tests/test-subrepo-git.t

$ hg -R ../tc archive -S ../archive.tgz --prefix '.' 2>/dev/null $ hg -R ../tc archive -S ../archive.tgz --prefix '.' 2>/dev/null
$ tar -tzf ../archive.tgz | sort | grep -v pax_global_header $ tar -tzf ../archive.tgz | sort | grep -v pax_global_header
.hg_archival.txt .hg_archival.txt
.hgsub .hgsub
.hgsubstate .hgsubstate
a a
s/g s/g
$ hg -R ../tc archive -S ../lf_archive.tgz --prefix '.' \
> --config extensions.largefiles= 2>/dev/null
$ tar -tzf ../lf_archive.tgz | sort | grep -v pax_global_header
.hg_archival.txt
.hgsub
.hgsubstate
a
s/g
create nested repo create nested repo
$ cd .. $ cd ..
$ hg init outer $ hg init outer
$ cd outer $ cd outer
$ echo b>b $ echo b>b
$ hg add b $ hg add b
$ hg commit -m b $ hg commit -m b