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

login
register
mail settings
Submitter Matt Harbison
Date Jan. 24, 2016, 6:25 a.m.
Message ID <7df484912e12acea1fcb.1453616746@Envy>
Download mbox | patch
Permalink /patch/12882/
State Superseded
Commit 571ba161f6be9ad696efcdbcf8bdf0a8a6ba0766
Delegated to: Yuya Nishihara
Headers show

Comments

Matt Harbison - Jan. 24, 2016, 6:25 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 7df484912e12acea1fcb1fbe6bcf4efddead779c
# 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.
Yuya Nishihara - Jan. 25, 2016, 3:34 p.m.
On Sun, 24 Jan 2016 01:25:46 -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 7df484912e12acea1fcb1fbe6bcf4efddead779c
> # 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.
> 
> 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,9 @@
>          hash = hashfile(file)
>          executable = getexecutable(file)
>          writestandin(repo, standin, hash, executable)
> +        return True
> +
> +    return False
>  
>  def readstandin(repo, filename, node=None):
>      '''read hex hash from standin for filename at given node, or working
> @@ -552,11 +555,15 @@
>      # stay refreshed.  No harm done: the user modified them and
>      # asked to commit them, so sooner or later we're going to
>      # refresh the standins.  Might as well leave them refreshed.
> +    missing = set()
>      lfdirstate = openlfdirstate(ui, repo)
>      for fstandin in standins:
>          lfile = splitstandin(fstandin)
>          if lfdirstate[lfile] != 'r':
> -            updatestandin(repo, fstandin)
> +            if not updatestandin(repo, fstandin):
> +                missing.add(fstandin)
> +
> +    standins = [s for s in standins if s not in missing]
>  
>      # Cook up a new matcher that only matches regular files or
>      # standins corresponding to the big files requested by the
> 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
> +  nothing changed (1 missing files, see 'hg status')
> +  [1]

Can it abort? I'm afraid of a side effect of removing missing standins.
Also, "hg commit deleted-non-large" said "abort: ... file not found!".
Matt Harbison - Jan. 26, 2016, 3:38 a.m.
On Mon, 25 Jan 2016 10:34:50 -0500, Yuya Nishihara <yuya@tcha.org> wrote:

> On Sun, 24 Jan 2016 01:25:46 -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 7df484912e12acea1fcb1fbe6bcf4efddead779c
>> # 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.
>>
>> 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,9 @@
>>          hash = hashfile(file)
>>          executable = getexecutable(file)
>>          writestandin(repo, standin, hash, executable)
>> +        return True
>> +
>> +    return False
>>
>>  def readstandin(repo, filename, node=None):
>>      '''read hex hash from standin for filename at given node, or  
>> working
>> @@ -552,11 +555,15 @@
>>      # stay refreshed.  No harm done: the user modified them and
>>      # asked to commit them, so sooner or later we're going to
>>      # refresh the standins.  Might as well leave them refreshed.
>> +    missing = set()
>>      lfdirstate = openlfdirstate(ui, repo)
>>      for fstandin in standins:
>>          lfile = splitstandin(fstandin)
>>          if lfdirstate[lfile] != 'r':
>> -            updatestandin(repo, fstandin)
>> +            if not updatestandin(repo, fstandin):
>> +                missing.add(fstandin)
>> +
>> +    standins = [s for s in standins if s not in missing]
>>
>>      # Cook up a new matcher that only matches regular files or
>>      # standins corresponding to the big files requested by the
>> 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
>> +  nothing changed (1 missing files, see 'hg status')
>> +  [1]
>
> Can it abort?

Yep, that's a better idea, thanks.  I'll wait until 1 and 2 are upstream  
before resending.

> I'm afraid of a side effect of removing missing standins.

Can you elaborate?  It looks like `hg rm standin` is silently ignored, and  
I didn't notice any breakage with `rm standin`.

> Also, "hg commit deleted-non-large" said "abort: ... file not found!".
Yuya Nishihara - Jan. 26, 2016, 2:48 p.m.
On Mon, 25 Jan 2016 22:38:07 -0500, Matt Harbison wrote:
> On Mon, 25 Jan 2016 10:34:50 -0500, Yuya Nishihara <yuya@tcha.org> wrote:
> 
> > On Sun, 24 Jan 2016 01:25:46 -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 7df484912e12acea1fcb1fbe6bcf4efddead779c
> >> # 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.
> >>
> >> 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,9 @@
> >>          hash = hashfile(file)
> >>          executable = getexecutable(file)
> >>          writestandin(repo, standin, hash, executable)
> >> +        return True
> >> +
> >> +    return False
> >>
> >>  def readstandin(repo, filename, node=None):
> >>      '''read hex hash from standin for filename at given node, or  
> >> working
> >> @@ -552,11 +555,15 @@
> >>      # stay refreshed.  No harm done: the user modified them and
> >>      # asked to commit them, so sooner or later we're going to
> >>      # refresh the standins.  Might as well leave them refreshed.
> >> +    missing = set()
> >>      lfdirstate = openlfdirstate(ui, repo)
> >>      for fstandin in standins:
> >>          lfile = splitstandin(fstandin)
> >>          if lfdirstate[lfile] != 'r':
> >> -            updatestandin(repo, fstandin)
> >> +            if not updatestandin(repo, fstandin):
> >> +                missing.add(fstandin)
> >> +
> >> +    standins = [s for s in standins if s not in missing]
> >>
> >>      # Cook up a new matcher that only matches regular files or
> >>      # standins corresponding to the big files requested by the
> >> 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
> >> +  nothing changed (1 missing files, see 'hg status')
> >> +  [1]
> >
> > Can it abort?
> 
> Yep, that's a better idea, thanks.  I'll wait until 1 and 2 are upstream  
> before resending.

Pushed the first two to the clowncopter, thanks.

> > I'm afraid of a side effect of removing missing standins.
> 
> Can you elaborate?  It looks like `hg rm standin` is silently ignored, and  
> I didn't notice any breakage with `rm standin`.

I don't have a particular concern in mind. I was just afraid of touching
match._files which would be passed around.

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,9 @@ 
         hash = hashfile(file)
         executable = getexecutable(file)
         writestandin(repo, standin, hash, executable)
+        return True
+
+    return False
 
 def readstandin(repo, filename, node=None):
     '''read hex hash from standin for filename at given node, or working
@@ -552,11 +555,15 @@ 
     # stay refreshed.  No harm done: the user modified them and
     # asked to commit them, so sooner or later we're going to
     # refresh the standins.  Might as well leave them refreshed.
+    missing = set()
     lfdirstate = openlfdirstate(ui, repo)
     for fstandin in standins:
         lfile = splitstandin(fstandin)
         if lfdirstate[lfile] != 'r':
-            updatestandin(repo, fstandin)
+            if not updatestandin(repo, fstandin):
+                missing.add(fstandin)
+
+    standins = [s for s in standins if s not in missing]
 
     # Cook up a new matcher that only matches regular files or
     # standins corresponding to the big files requested by the
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
+  nothing changed (1 missing files, see 'hg status')
+  [1]
+  $ hg up -Cqr 0
   $ cd ..
 
 Discard all cached largefiles in USERCACHE