Patchwork subrepo: config option to disable subrepositories

login
register
mail settings
Submitter Matt Harbison
Date Nov. 4, 2017, 5:06 a.m.
Message ID <op.y85xs7ca9lwrgf@envy>
Download mbox | patch
Permalink /patch/25387/
State Not Applicable
Headers show

Comments

Matt Harbison - Nov. 4, 2017, 5:06 a.m.
On Fri, 03 Nov 2017 20:28:27 -0400, Gregory Szorc  
<gregory.szorc@gmail.com> wrote:

> # HG changeset patch
> # User Gregory Szorc <gregory.szorc@gmail.com>
> # Date 1509755155 25200
> #      Fri Nov 03 17:25:55 2017 -0700
> # Branch stable
> # Node ID f2390c369bfebf32f26f5a2e4aa5620224a7c8ea
> # Parent  f445b10dc7fb3495d24d1c22b0996148864c77f7
> subrepo: config option to disable subrepositories
>

> One class of problems stems from the fact that the .hgsubstate file
> is managed automatically. This code still runs in this patch and
> the result is the content of .hgsubstate will get nuked by a client
> with the feature disabled. If you have this file in your repo and
> disable the feature and commit, you lose the file. That's obviously
> not good.

This seems to prevent the file from being nuked when no subrepo is present:


What is the bug around the subrepo() revset?
Gregory Szorc - Nov. 4, 2017, 5:43 a.m.
On Fri, Nov 3, 2017 at 10:06 PM, Matt Harbison <mharbison72@gmail.com>
wrote:

> On Fri, 03 Nov 2017 20:28:27 -0400, Gregory Szorc <gregory.szorc@gmail.com>
> wrote:
>
> # HG changeset patch
>> # User Gregory Szorc <gregory.szorc@gmail.com>
>> # Date 1509755155 25200
>> #      Fri Nov 03 17:25:55 2017 -0700
>> # Branch stable
>> # Node ID f2390c369bfebf32f26f5a2e4aa5620224a7c8ea
>> # Parent  f445b10dc7fb3495d24d1c22b0996148864c77f7
>> subrepo: config option to disable subrepositories
>>
>>
> One class of problems stems from the fact that the .hgsubstate file
>> is managed automatically. This code still runs in this patch and
>> the result is the content of .hgsubstate will get nuked by a client
>> with the feature disabled. If you have this file in your repo and
>> disable the feature and commit, you lose the file. That's obviously
>> not good.
>>
>
> This seems to prevent the file from being nuked when no subrepo is present:
>
> diff --git a/mercurial/subrepo.py b/mercurial/subrepo.py
> --- a/mercurial/subrepo.py
> +++ b/mercurial/subrepo.py
> @@ -182,6 +182,9 @@
>
>  def writestate(repo, state):
>      """rewrite .hgsubstate in (outer) repo with these subrepo states"""
> +    if not repo.ui.configbool('ui', 'enablesubrepos'):
> +        return
> +
>      lines = ['%s %s\n' % (state[s][1], s) for s in sorted(state)
>                                                  if state[s][1] !=
> nullstate[1]]
>      repo.wwrite('.hgsubstate', ''.join(lines), '')
> diff --git a/tests/test-subrepo-disable.t b/tests/test-subrepo-disable.t
> --- a/tests/test-subrepo-disable.t
> +++ b/tests/test-subrepo-disable.t
> @@ -123,15 +123,9 @@
>
>    $ cd with-substate-disabled
>    $ hg status
> -  M .hgsubstate
> -TODO buggy
>    $ hg diff
> -  diff -r 7645bb5a5a99 .hgsubstate
> -  --- a/.hgsubstate    Thu Jan 01 00:00:00 1970 +0000
> -  +++ b/.hgsubstate    Thu Jan 01 00:00:00 1970 +0000
> -  @@ -1,1 +0,0 @@
> -  -45cc468b8f18bee314935a4651bad80f9cb3b540 hgsub
>    $ cat .hgsubstate
> +  45cc468b8f18bee314935a4651bad80f9cb3b540 hgsub
>    $ hg up
>    0 files updated, 0 files merged, 0 files removed, 0 files unresolved
>
>
> What is the bug around the subrepo() revset?
>

The revset needs to read the parsed .hgsubstate file. That we can probably
fix easily by adding a flag to force parsing even if the subrepo feature is
disabled.

It is the magical .hgsubstate management that will be harder to fix with a
simple patch. I'm leaning towards putting most "do something with subrepos"
code behind an "if ui.configbool(...)" check. That is several dozen call
sites. But I think we'll need to do that in order to get the behavior we
desire.

Patch

diff --git a/mercurial/subrepo.py b/mercurial/subrepo.py
--- a/mercurial/subrepo.py
+++ b/mercurial/subrepo.py
@@ -182,6 +182,9 @@ 

  def writestate(repo, state):
      """rewrite .hgsubstate in (outer) repo with these subrepo states"""
+    if not repo.ui.configbool('ui', 'enablesubrepos'):
+        return
+
      lines = ['%s %s\n' % (state[s][1], s) for s in sorted(state)
                                                  if state[s][1] !=  
nullstate[1]]
      repo.wwrite('.hgsubstate', ''.join(lines), '')
diff --git a/tests/test-subrepo-disable.t b/tests/test-subrepo-disable.t
--- a/tests/test-subrepo-disable.t
+++ b/tests/test-subrepo-disable.t
@@ -123,15 +123,9 @@ 

    $ cd with-substate-disabled
    $ hg status
-  M .hgsubstate
-TODO buggy
    $ hg diff
-  diff -r 7645bb5a5a99 .hgsubstate
-  --- a/.hgsubstate    Thu Jan 01 00:00:00 1970 +0000
-  +++ b/.hgsubstate    Thu Jan 01 00:00:00 1970 +0000
-  @@ -1,1 +0,0 @@
-  -45cc468b8f18bee314935a4651bad80f9cb3b540 hgsub
    $ cat .hgsubstate
+  45cc468b8f18bee314935a4651bad80f9cb3b540 hgsub
    $ hg up
    0 files updated, 0 files merged, 0 files removed, 0 files unresolved