Patchwork [STABLE,V2] largefiles: prevent committing a missing largefile

login
register
mail settings
Submitter Matt Harbison
Date Jan. 27, 2016, 1:11 a.m.
Message ID <70f7e55f661904541966.1453857069@Envy>
Download mbox | patch
Permalink /patch/12894/
State Accepted
Delegated to: Yuya Nishihara
Headers show

Comments

Matt Harbison - Jan. 27, 2016, 1:11 a.m.
# HG changeset patch
# User Matt Harbison <matt_harbison@yahoo.com>
# Date 1453612219 18000
#      Sun Jan 24 00:10:19 2016 -0500
# Branch stable
# Node ID 70f7e55f661904541966e115843e6753524a7664
# Parent  f20852b3f6c8f9ef04cdfb902dafb3e8923a4a20
largefiles: prevent committing a missing largefile

Previously, if the largefile was deleted at the time of a commit, the standin
was silently not updated and its current state (possibly garbage) was recorded.
The test makes it look like this is somewhat of an edge case, but the same thing
happens when an `hg revert` followed by `rm` changes the standin.

Aside from the second invocation of this in lfutil.updatestandinsbymatch()
(which is what triggers this test case), the three other uses are guarded by
dirstate checks for added or modified, or an existence check in the filesystem.
So aborting in lfutil.updatestandins() should be safe, and will avoid silent
skips in the future if this is used elsewhere.
Yuya Nishihara - Jan. 28, 2016, 2:26 p.m.
On Tue, 26 Jan 2016 20:11:09 -0500, Matt Harbison wrote:
> # HG changeset patch
> # User Matt Harbison <matt_harbison@yahoo.com>
> # Date 1453612219 18000
> #      Sun Jan 24 00:10:19 2016 -0500
> # Branch stable
> # Node ID 70f7e55f661904541966e115843e6753524a7664
> # Parent  f20852b3f6c8f9ef04cdfb902dafb3e8923a4a20
> largefiles: prevent committing a missing largefile
> 
> Previously, if the largefile was deleted at the time of a commit, the standin
> was silently not updated and its current state (possibly garbage) was recorded.
> The test makes it look like this is somewhat of an edge case, but the same thing
> happens when an `hg revert` followed by `rm` changes the standin.
> 
> Aside from the second invocation of this in lfutil.updatestandinsbymatch()
> (which is what triggers this test case), the three other uses are guarded by
> dirstate checks for added or modified, or an existence check in the filesystem.
> So aborting in lfutil.updatestandins() should be safe, and will avoid silent
> skips in the future if this is used elsewhere.

Looks good, pushed to the clowncopter, thanks.

> --- a/tests/test-largefiles-cache.t
> +++ b/tests/test-largefiles-cache.t
> @@ -19,6 +19,12 @@
>    $ hg rm large
>    $ hg commit -m 'branchhead without largefile' large
>    $ hg up -qr 0
> +  $ rm large
> +  $ echo "0000000000000000000000000000000000000000" > .hglf/large
> +  $ hg commit -m 'commit missing file with corrupt standin' large
> +  abort: large: file not found!

We'll need another workaround for "hg commit" (with no filename).
Matt Harbison - Jan. 29, 2016, 3:49 a.m.
On Thu, 28 Jan 2016 09:26:04 -0500, Yuya Nishihara <yuya@tcha.org> wrote:

> On Tue, 26 Jan 2016 20:11:09 -0500, Matt Harbison wrote:
>> # HG changeset patch
>> # User Matt Harbison <matt_harbison@yahoo.com>
>> # Date 1453612219 18000
>> #      Sun Jan 24 00:10:19 2016 -0500
>> # Branch stable
>> # Node ID 70f7e55f661904541966e115843e6753524a7664
>> # Parent  f20852b3f6c8f9ef04cdfb902dafb3e8923a4a20
>> largefiles: prevent committing a missing largefile
>
>> --- a/tests/test-largefiles-cache.t
>> +++ b/tests/test-largefiles-cache.t
>> @@ -19,6 +19,12 @@
>>    $ hg rm large
>>    $ hg commit -m 'branchhead without largefile' large
>>    $ hg up -qr 0
>> +  $ rm large
>> +  $ echo "0000000000000000000000000000000000000000" > .hglf/large
>> +  $ hg commit -m 'commit missing file with corrupt standin' large
>> +  abort: large: file not found!
>
> We'll need another workaround for "hg commit" (with no filename).

I thought 4511e8dac4c7 would cover this, but it looks like trashing the  
standin is distinct case.  I'll see what I can cobble together.

Patch

diff --git a/hgext/largefiles/lfutil.py b/hgext/largefiles/lfutil.py
--- a/hgext/largefiles/lfutil.py
+++ b/hgext/largefiles/lfutil.py
@@ -319,6 +319,8 @@ 
         hash = hashfile(file)
         executable = getexecutable(file)
         writestandin(repo, standin, hash, executable)
+    else:
+        raise error.Abort(_('%s: file not found!') % splitstandin(standin))
 
 def readstandin(repo, filename, node=None):
     '''read hex hash from standin for filename at given node, or working
diff --git a/tests/test-largefiles-cache.t b/tests/test-largefiles-cache.t
--- a/tests/test-largefiles-cache.t
+++ b/tests/test-largefiles-cache.t
@@ -19,6 +19,12 @@ 
   $ hg rm large
   $ hg commit -m 'branchhead without largefile' large
   $ hg up -qr 0
+  $ rm large
+  $ echo "0000000000000000000000000000000000000000" > .hglf/large
+  $ hg commit -m 'commit missing file with corrupt standin' large
+  abort: large: file not found!
+  [255]
+  $ hg up -Cqr 0
   $ cd ..
 
 Discard all cached largefiles in USERCACHE