Page MenuHomePhabricator

sparse: add local files to temporaryfiles if they exist out of sparse
ClosedPublic

Authored by pulkit on Aug 21 2018, 9:21 AM.

Details

Summary

We get the f1 from args if it's merge and check that whether that exists in
sparse checkout or not. If that does not, we add that for merging.

The error comes from very low-level where we try to read data of a
working-filectx which does not exists in the working directory. It will be
extremely ugly to plug in logic to update sparse copy with new file at such a
low level.

We already have logic related to updating the checkout with required files in
calculateupdates() and let's handle this case there only. calculateupdates()
call sparse.filterupdatesactions() and the logic is added into the latter
function.

To get the exact traceback, this patch can be backed out and
test-sparse-merges.t can be run with ui.traceback=True.

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.Aug 21 2018, 9:21 AM
pulkit updated this revision to Diff 10490.Aug 21 2018, 10:03 AM
pulkit updated this revision to Diff 10503.Aug 22 2018, 7:10 AM

Maybe the current state is not very helpful in reviewing. The traceback which I am trying to fix is this:

   $ hg merge
+  Traceback (most recent call last):
+    File "/place/vartmp/hgtests.nx0HIy/install/lib/python/mercurial/scmutil.py", line 161, in callcatch
+      return func()
+    File "/place/vartmp/hgtests.nx0HIy/install/lib/python/mercurial/dispatch.py", line 344, in _runcatchfunc
+      return _dispatch(req)
+    File "/place/vartmp/hgtests.nx0HIy/install/lib/python/mercurial/dispatch.py", line 982, in _dispatch
+      cmdpats, cmdoptions)
+    File "/place/vartmp/hgtests.nx0HIy/install/lib/python/mercurial/dispatch.py", line 728, in runcommand
+      ret = _runcommand(ui, options, cmd, d)
+    File "/place/vartmp/hgtests.nx0HIy/install/lib/python/mercurial/dispatch.py", line 990, in _runcommand
+      return cmdfunc()
+    File "/place/vartmp/hgtests.nx0HIy/install/lib/python/mercurial/dispatch.py", line 979, in <lambda>
+      d = lambda: util.checksignature(func)(ui, *args, **strcmdopt)
+    File "/place/vartmp/hgtests.nx0HIy/install/lib/python/mercurial/util.py", line 1531, in check
+      return func(*args, **kwargs)
+    File "/place/vartmp/hgtests.nx0HIy/install/lib/python/mercurial/commands.py", line 3785, in merge
+      labels=labels, abort=abort)
+    File "/place/vartmp/hgtests.nx0HIy/install/lib/python/mercurial/hg.py", line 904, in merge
+      labels=labels)
+    File "/place/vartmp/hgtests.nx0HIy/install/lib/python/mercurial/merge.py", line 2177, in update
+      stats = applyupdates(repo, actions, wc, p2, overwrite, labels=labels)
+    File "/place/vartmp/hgtests.nx0HIy/install/lib/python/mercurial/merge.py", line 1584, in applyupdates
+      ms.add(fcl, fco, fca, f)
+    File "/place/vartmp/hgtests.nx0HIy/install/lib/python/mercurial/merge.py", line 493, in add
+      self._repo.vfs.write('merge/' + hash, fcl.data())
+    File "/place/vartmp/hgtests.nx0HIy/install/lib/python/mercurial/context.py", line 1685, in data
+      return self._repo.wread(self._path)
+    File "/place/vartmp/hgtests.nx0HIy/install/lib/python/mercurial/localrepo.py", line 1212, in wread
+      data = self.wvfs.read(filename)
+    File "/place/vartmp/hgtests.nx0HIy/install/lib/python/mercurial/vfs.py", line 78, in read
+      with self(path, 'rb') as fp:
+    File "/place/vartmp/hgtests.nx0HIy/install/lib/python/mercurial/vfs.py", line 409, in __call__
+      fp = util.posixfile(f, mode)
+  IOError: [Errno 2] $ENOENT$: '$TESTTMP/mvtest/a'
   abort: $ENOENT$: $TESTTMP/mvtest/a
   [255]

The error comes when we try to read a working-fctx which is not present in working directory. I don't think applyupdates is the right place to add logic about updating sparse checkouts. The best way I could figure out is that explicitly take care that f2 is loaded while merging.

gentle ping for review.

pulkit added a comment.Sep 7 2018, 8:20 AM

gentle ping for review. If something is missing from this patch or if there is a way I can make this review easy, let me know. I will be happy to do that.

In D4341#67539, @pulkit wrote:

Maybe the current state is not very helpful in reviewing. The traceback which I am trying to fix is this:

   $ hg merge
+  Traceback (most recent call last):
+    File "/place/vartmp/hgtests.nx0HIy/install/lib/python/mercurial/scmutil.py", line 161, in callcatch
+      return func()
+    File "/place/vartmp/hgtests.nx0HIy/install/lib/python/mercurial/dispatch.py", line 344, in _runcatchfunc
+      return _dispatch(req)
+    File "/place/vartmp/hgtests.nx0HIy/install/lib/python/mercurial/dispatch.py", line 982, in _dispatch
+      cmdpats, cmdoptions)
+    File "/place/vartmp/hgtests.nx0HIy/install/lib/python/mercurial/dispatch.py", line 728, in runcommand
+      ret = _runcommand(ui, options, cmd, d)
+    File "/place/vartmp/hgtests.nx0HIy/install/lib/python/mercurial/dispatch.py", line 990, in _runcommand
+      return cmdfunc()
+    File "/place/vartmp/hgtests.nx0HIy/install/lib/python/mercurial/dispatch.py", line 979, in <lambda>
+      d = lambda: util.checksignature(func)(ui, *args, **strcmdopt)
+    File "/place/vartmp/hgtests.nx0HIy/install/lib/python/mercurial/util.py", line 1531, in check
+      return func(*args, **kwargs)
+    File "/place/vartmp/hgtests.nx0HIy/install/lib/python/mercurial/commands.py", line 3785, in merge
+      labels=labels, abort=abort)
+    File "/place/vartmp/hgtests.nx0HIy/install/lib/python/mercurial/hg.py", line 904, in merge
+      labels=labels)
+    File "/place/vartmp/hgtests.nx0HIy/install/lib/python/mercurial/merge.py", line 2177, in update
+      stats = applyupdates(repo, actions, wc, p2, overwrite, labels=labels)
+    File "/place/vartmp/hgtests.nx0HIy/install/lib/python/mercurial/merge.py", line 1584, in applyupdates
+      ms.add(fcl, fco, fca, f)
+    File "/place/vartmp/hgtests.nx0HIy/install/lib/python/mercurial/merge.py", line 493, in add
+      self._repo.vfs.write('merge/' + hash, fcl.data())
+    File "/place/vartmp/hgtests.nx0HIy/install/lib/python/mercurial/context.py", line 1685, in data
+      return self._repo.wread(self._path)
+    File "/place/vartmp/hgtests.nx0HIy/install/lib/python/mercurial/localrepo.py", line 1212, in wread
+      data = self.wvfs.read(filename)
+    File "/place/vartmp/hgtests.nx0HIy/install/lib/python/mercurial/vfs.py", line 78, in read
+      with self(path, 'rb') as fp:
+    File "/place/vartmp/hgtests.nx0HIy/install/lib/python/mercurial/vfs.py", line 409, in __call__
+      fp = util.posixfile(f, mode)
+  IOError: [Errno 2] $ENOENT$: '$TESTTMP/mvtest/a'
   abort: $ENOENT$: $TESTTMP/mvtest/a
   [255]

The error comes when we try to read a working-fctx which is not present in working directory. I don't think applyupdates is the right place to add logic about updating sparse checkouts. The best way I could figure out is that explicitly take care that f2 is loaded while merging.

Could you add that to the commit message (probably not including the entire stack trace)?

martinvonz requested changes to this revision.Sep 10 2018, 11:41 AM
martinvonz added inline comments.
mercurial/sparse.py
365

This seems both cryptic and brittle. Can we instead do something like this?

if type in (merge.ACTION_MOVE, merge.ACTION_...):
  f1, f2, fa, move, anc = args
This revision now requires changes to proceed.Sep 10 2018, 11:41 AM
pulkit edited the summary of this revision. (Show Details)Sep 11 2018, 8:16 AM
pulkit updated this revision to Diff 10877.
martinvonz accepted this revision.Sep 11 2018, 12:38 PM
This revision is now accepted and ready to land.Sep 11 2018, 12:38 PM
This revision was automatically updated to reflect the committed changes.