Patchwork largefiles: fix support for local largefiles while using share extension

login
register
mail settings
Submitter Henrik Stuart
Date June 3, 2016, 7:55 a.m.
Message ID <59465e5e6b059e848e2e.1464940548@DESKTOP-HENRIKS.hq.unity3d.com>
Download mbox | patch
Permalink /patch/15368/
State Superseded
Commit f359cdc91e21bb120d2c28049c403d995225ea70
Headers show

Comments

Henrik Stuart - June 3, 2016, 7:55 a.m.
# HG changeset patch
# User Henrik Stuart <henriks@unity3d.com>
# Date 1464938050 -7200
#      Fri Jun 03 09:14:10 2016 +0200
# Branch stable
# Node ID 59465e5e6b059e848e2e127f263288bceaa49015
# Parent  e82ca7d0967cd10d92562820297c3413abe6fc29
largefiles: fix support for local largefiles while using share extension

Prior to revision 2a3f24786d09, largefiles were saved in the local repository,
even if it was using the share extension. After that change, all largefiles are
now stored in the shared repository. However, the backward compatibility for
existing largefiles already placed in the local repository was never tested,
and has been broken since.
Henrik Stuart - June 3, 2016, 8 a.m.
On 03-06-2016 09:55, Henrik Stuart wrote:
> # HG changeset patch
> # User Henrik Stuart <henriks@unity3d.com>
> # Date 1464938050 -7200
> #      Fri Jun 03 09:14:10 2016 +0200
> # Branch stable
> # Node ID 59465e5e6b059e848e2e127f263288bceaa49015
> # Parent  e82ca7d0967cd10d92562820297c3413abe6fc29
> largefiles: fix support for local largefiles while using share extension
>
> Prior to revision 2a3f24786d09, largefiles were saved in the local repository,
> even if it was using the share extension. After that change, all largefiles are
> now stored in the shared repository. However, the backward compatibility for
> existing largefiles already placed in the local repository was never tested,
> and has been broken since.

Note that this probably qualifies for stable as some users are unable to 
push their changes when using largefiles.
Augie Fackler - June 4, 2016, 2:33 a.m.
On Fri, Jun 03, 2016 at 09:55:48AM +0200, Henrik Stuart wrote:
> # HG changeset patch
> # User Henrik Stuart <henriks@unity3d.com>
> # Date 1464938050 -7200
> #      Fri Jun 03 09:14:10 2016 +0200
> # Branch stable
> # Node ID 59465e5e6b059e848e2e127f263288bceaa49015
> # Parent  e82ca7d0967cd10d92562820297c3413abe6fc29
> largefiles: fix support for local largefiles while using share extension

I agree this makes sense for stable.

>
> Prior to revision 2a3f24786d09, largefiles were saved in the local repository,
> even if it was using the share extension. After that change, all largefiles are
> now stored in the shared repository. However, the backward compatibility for
> existing largefiles already placed in the local repository was never tested,
> and has been broken since.
>
> diff --git a/hgext/largefiles/lfutil.py b/hgext/largefiles/lfutil.py
> --- a/hgext/largefiles/lfutil.py
> +++ b/hgext/largefiles/lfutil.py
> @@ -204,7 +204,7 @@
>      if instore(repo, hash):
>          return (path, True)
>      elif repo.shared() and instore(repo, hash, True):
> -        return storepath(repo, hash, True)
> +        return storepath(repo, hash, True), True
>
>      return (path, False)
>
> diff --git a/tests/test-largefiles-shared-repo.t b/tests/test-largefiles-shared-repo.t
> new file mode 100644
> --- /dev/null
> +++ b/tests/test-largefiles-shared-repo.t

However, could this be bolted on to the end of an existing largefiles
test? That'd let us avoid duplicating a lot of repository setup work.
Henrik Stuart - June 6, 2016, 6:35 a.m.
On 04-06-2016 04:33, Augie Fackler wrote:
> On Fri, Jun 03, 2016 at 09:55:48AM +0200, Henrik Stuart wrote:
>> # HG changeset patch
>> # User Henrik Stuart <henriks@unity3d.com>
>> # Date 1464938050 -7200
>> #      Fri Jun 03 09:14:10 2016 +0200
>> # Branch stable
>> # Node ID 59465e5e6b059e848e2e127f263288bceaa49015
>> # Parent  e82ca7d0967cd10d92562820297c3413abe6fc29
>> largefiles: fix support for local largefiles while using share extension
>
> I agree this makes sense for stable.
>
>>
>> Prior to revision 2a3f24786d09, largefiles were saved in the local repository,
>> even if it was using the share extension. After that change, all largefiles are
>> now stored in the shared repository. However, the backward compatibility for
>> existing largefiles already placed in the local repository was never tested,
>> and has been broken since.
>>
>> diff --git a/hgext/largefiles/lfutil.py b/hgext/largefiles/lfutil.py
>> --- a/hgext/largefiles/lfutil.py
>> +++ b/hgext/largefiles/lfutil.py
>> @@ -204,7 +204,7 @@
>>      if instore(repo, hash):
>>          return (path, True)
>>      elif repo.shared() and instore(repo, hash, True):
>> -        return storepath(repo, hash, True)
>> +        return storepath(repo, hash, True), True
>>
>>      return (path, False)
>>
>> diff --git a/tests/test-largefiles-shared-repo.t b/tests/test-largefiles-shared-repo.t
>> new file mode 100644
>> --- /dev/null
>> +++ b/tests/test-largefiles-shared-repo.t
>
> However, could this be bolted on to the end of an existing largefiles
> test? That'd let us avoid duplicating a lot of repository setup work.

It could potentially be rolled into test-largefiles-cache like Matt 
suggests in a separate mail, but I rather prefer to keep the interaction 
between largefiles and share in its own test file, building out 
test-largefiles-shared-repo for further scenarios that involve both 
extensions. However, I don't feel strongly about it, and we can easily 
go with Matt's suggestion. Just let me know if I should resend the patch 
with Matt's suggestion instead.
Augie Fackler - June 6, 2016, 3:01 p.m.
> On Jun 6, 2016, at 02:35, Henrik Stuart <henriks+hg@unity3d.com> wrote:
> 
> On 04-06-2016 04:33, Augie Fackler wrote:
>> On Fri, Jun 03, 2016 at 09:55:48AM +0200, Henrik Stuart wrote:
>>> # HG changeset patch
>>> # User Henrik Stuart <henriks@unity3d.com>
>>> # Date 1464938050 -7200
>>> #      Fri Jun 03 09:14:10 2016 +0200
>>> # Branch stable
>>> # Node ID 59465e5e6b059e848e2e127f263288bceaa49015
>>> # Parent  e82ca7d0967cd10d92562820297c3413abe6fc29
>>> largefiles: fix support for local largefiles while using share extension
>> 
>> I agree this makes sense for stable.
>> 
>>> 
>>> Prior to revision 2a3f24786d09, largefiles were saved in the local repository,
>>> even if it was using the share extension. After that change, all largefiles are
>>> now stored in the shared repository. However, the backward compatibility for
>>> existing largefiles already placed in the local repository was never tested,
>>> and has been broken since.
>>> 
>>> diff --git a/hgext/largefiles/lfutil.py b/hgext/largefiles/lfutil.py
>>> --- a/hgext/largefiles/lfutil.py
>>> +++ b/hgext/largefiles/lfutil.py
>>> @@ -204,7 +204,7 @@
>>>     if instore(repo, hash):
>>>         return (path, True)
>>>     elif repo.shared() and instore(repo, hash, True):
>>> -        return storepath(repo, hash, True)
>>> +        return storepath(repo, hash, True), True
>>> 
>>>     return (path, False)
>>> 
>>> diff --git a/tests/test-largefiles-shared-repo.t b/tests/test-largefiles-shared-repo.t
>>> new file mode 100644
>>> --- /dev/null
>>> +++ b/tests/test-largefiles-shared-repo.t
>> 
>> However, could this be bolted on to the end of an existing largefiles
>> test? That'd let us avoid duplicating a lot of repository setup work.
> 
> It could potentially be rolled into test-largefiles-cache like Matt suggests in a separate mail, but I rather prefer to keep the interaction between largefiles and share in its own test file, building out test-largefiles-shared-repo for further scenarios that involve both extensions. However, I don't feel strongly about it, and we can easily go with Matt's suggestion. Just let me know if I should resend the patch with Matt's suggestion instead.

Let's just go with mpm's suggestion for now.

AF

> 
> -- 
> Kind regards,
>  Henrik
Sean Farley - June 7, 2016, 12:31 a.m.
Augie Fackler <raf@durin42.com> writes:

>> On Jun 6, 2016, at 02:35, Henrik Stuart <henriks+hg@unity3d.com> wrote:
>> 
>> On 04-06-2016 04:33, Augie Fackler wrote:
>>> On Fri, Jun 03, 2016 at 09:55:48AM +0200, Henrik Stuart wrote:
>>>> # HG changeset patch
>>>> # User Henrik Stuart <henriks@unity3d.com>
>>>> # Date 1464938050 -7200
>>>> #      Fri Jun 03 09:14:10 2016 +0200
>>>> # Branch stable
>>>> # Node ID 59465e5e6b059e848e2e127f263288bceaa49015
>>>> # Parent  e82ca7d0967cd10d92562820297c3413abe6fc29
>>>> largefiles: fix support for local largefiles while using share extension
>>> 
>>> I agree this makes sense for stable.
>>> 
>>>> 
>>>> Prior to revision 2a3f24786d09, largefiles were saved in the local repository,
>>>> even if it was using the share extension. After that change, all largefiles are
>>>> now stored in the shared repository. However, the backward compatibility for
>>>> existing largefiles already placed in the local repository was never tested,
>>>> and has been broken since.
>>>> 
>>>> diff --git a/hgext/largefiles/lfutil.py b/hgext/largefiles/lfutil.py
>>>> --- a/hgext/largefiles/lfutil.py
>>>> +++ b/hgext/largefiles/lfutil.py
>>>> @@ -204,7 +204,7 @@
>>>>     if instore(repo, hash):
>>>>         return (path, True)
>>>>     elif repo.shared() and instore(repo, hash, True):
>>>> -        return storepath(repo, hash, True)
>>>> +        return storepath(repo, hash, True), True
>>>> 
>>>>     return (path, False)
>>>> 
>>>> diff --git a/tests/test-largefiles-shared-repo.t b/tests/test-largefiles-shared-repo.t
>>>> new file mode 100644
>>>> --- /dev/null
>>>> +++ b/tests/test-largefiles-shared-repo.t
>>> 
>>> However, could this be bolted on to the end of an existing largefiles
>>> test? That'd let us avoid duplicating a lot of repository setup work.
>> 
>> It could potentially be rolled into test-largefiles-cache like Matt suggests in a separate mail, but I rather prefer to keep the interaction between largefiles and share in its own test file, building out test-largefiles-shared-repo for further scenarios that involve both extensions. However, I don't feel strongly about it, and we can easily go with Matt's suggestion. Just let me know if I should resend the patch with Matt's suggestion instead.
>
> Let's just go with mpm's suggestion for now.

Just FYI, it was Matt H. not mpm.

Patch

diff --git a/hgext/largefiles/lfutil.py b/hgext/largefiles/lfutil.py
--- a/hgext/largefiles/lfutil.py
+++ b/hgext/largefiles/lfutil.py
@@ -204,7 +204,7 @@ 
     if instore(repo, hash):
         return (path, True)
     elif repo.shared() and instore(repo, hash, True):
-        return storepath(repo, hash, True)
+        return storepath(repo, hash, True), True
 
     return (path, False)
 
diff --git a/tests/test-largefiles-shared-repo.t b/tests/test-largefiles-shared-repo.t
new file mode 100644
--- /dev/null
+++ b/tests/test-largefiles-shared-repo.t
@@ -0,0 +1,59 @@ 
+Tests for using largefiles with shared repositories
+
+  $ cat >> $HGRCPATH <<EOF
+  > [extensions]
+  > largefiles=
+  > share=
+  > [largefiles]
+  > minsize=2
+  > patterns=
+  >   *.dat
+  > EOF
+
+Set up "upstream" repository
+  $ hg init upstream
+  $ cd upstream
+  $ echo foo > ufile1.dat
+  $ hg add --large ufile1.dat
+  $ hg ci -mu0 -q
+  $ cd ..
+
+Set up shared repository and sharing
+  $ hg clone upstream share -q
+  $ cat >> $HGRCPATH <<EOF
+  > [share]
+  > pool=$TESTTMP/share
+  > EOF
+
+Clone to another repository and create a large file
+  $ hg clone share clone1 -r0
+  (sharing from new pooled repository 5cd2508cb5c0cf943579d0e2fad64bc31fcc541c)
+  adding changesets
+  adding manifests
+  adding file changes
+  added 1 changesets with 1 changes to 1 files
+  no changes found
+  updating working directory
+  getting changed largefiles
+  1 largefiles updated, 0 removed
+  1 files updated, 0 files merged, 0 files removed, 0 files unresolved
+  $ cd clone1
+  $ echo baz > file3.dat
+  $ hg add --large file3.dat
+  $ hg ci -m2 -q
+  $ cd ..
+
+Get clone1 into state from before revision 2a3f24786d09, i.e. with the largefile
+locally rather than in the shared repository
+  $ mkdir -p $TESTTMP/clone1/.hg/largefiles
+  $ cp $TESTTMP/share/5cd2508cb5c0cf943579d0e2fad64bc31fcc541c/.hg/largefiles/6eadeac2dade6347e87c0d24fd455feffa7069f0 $TESTTMP/clone1/.hg/largefiles/6eadeac2dade6347e87c0d24fd455feffa7069f0
+  $ rm $TESTTMP/share/5cd2508cb5c0cf943579d0e2fad64bc31fcc541c/.hg/largefiles/6eadeac2dade6347e87c0d24fd455feffa7069f0
+
+Push to upstream
+  $ hg -R clone1 push -b. upstream --force
+  pushing to upstream
+  searching for changes
+  adding changesets
+  adding manifests
+  adding file changes
+  added 1 changesets with 1 changes to 1 files