Re: [PATCH] subrepo: config option to disable subrepositories

2017-11-04 Thread Gregory Szorc
On Fri, Nov 3, 2017 at 11:57 PM, Yuya Nishihara  wrote:

> On Fri, 03 Nov 2017 17:28:27 -0700, Gregory Szorc wrote:
> > # HG changeset patch
> > # User Gregory Szorc 
> > # Date 1509755155 25200
> > #  Fri Nov 03 17:25:55 2017 -0700
> > # Branch stable
> > # Node ID f2390c369bfebf32f26f5a2e4aa5620224a7c8ea
> > # Parent  f445b10dc7fb3495d24d1c22b0996148864c77f7
> > subrepo: config option to disable subrepositories
>
> > +``enablesubrepos``
> > +Whether the subrepositories feature is enabled. If disabled,
> > +subrepositories are effectively ignored by the Mercurial client.
> > +(default: True)
>
> We might want to select subrepo types to be enabled since hg subrepo is
> more widely used and considered less broken.
>

I would like per-type controls as well.

I would prefer to start with a simple patch providing a master switch. We
can add per-type switches in later. But we should have a plan for the
option names so the end state has a reasonable UI.

I'll try to send out a V2 in the next few hours...


>
> > +TODO buggy because localrepository.commit() managed its file contents
> > +  $ hg commit -m 'manually add .hgsubstate'
> > +  nothing changed
> > +  [1]
> > +
> > +... but hg commit --amend works (this relies on a regression in 4.4)
> > +
> > +  $ hg commit --amend
> > +  saved backup bundle to $TESTTMP/testrepo0/.hg/strip-
> backup/addf99df3e66-ab5c9ff8-amend.hg (glob)
>
> Yeah, it's the issue5677.
>
> https://bz.mercurial-scm.org/show_bug.cgi?id=5677
>
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel


Re: [PATCH] subrepo: config option to disable subrepositories

2017-11-04 Thread Adrian Buehlmann
On 2017-11-04 07:57, Yuya Nishihara wrote:
> On Fri, 03 Nov 2017 17:28:27 -0700, Gregory Szorc wrote:
>> # HG changeset patch
>> # User Gregory Szorc 
>> # Date 1509755155 25200
>> #  Fri Nov 03 17:25:55 2017 -0700
>> # Branch stable
>> # Node ID f2390c369bfebf32f26f5a2e4aa5620224a7c8ea
>> # Parent  f445b10dc7fb3495d24d1c22b0996148864c77f7
>> subrepo: config option to disable subrepositories
> 
>> +``enablesubrepos``
>> +Whether the subrepositories feature is enabled. If disabled,
>> +subrepositories are effectively ignored by the Mercurial client.
>> +(default: True)
> 
> We might want to select subrepo types to be enabled since hg subrepo is
> more widely used and considered less broken.
> 

After looking at subrepo.gitsubrepo._checkversion in the mercurial sources:

Assuming the problem is limited to specific vulnerable versions of
external scm tools:

Perhaps, this might be narrowed to requiring specific versions of git
and svn using a config knob (which defaults to the hard-coded values).
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel


Re: [PATCH] subrepo: config option to disable subrepositories

2017-11-04 Thread Yuya Nishihara
On Fri, 03 Nov 2017 17:28:27 -0700, Gregory Szorc wrote:
> # HG changeset patch
> # User Gregory Szorc 
> # Date 1509755155 25200
> #  Fri Nov 03 17:25:55 2017 -0700
> # Branch stable
> # Node ID f2390c369bfebf32f26f5a2e4aa5620224a7c8ea
> # Parent  f445b10dc7fb3495d24d1c22b0996148864c77f7
> subrepo: config option to disable subrepositories

> +``enablesubrepos``
> +Whether the subrepositories feature is enabled. If disabled,
> +subrepositories are effectively ignored by the Mercurial client.
> +(default: True)

We might want to select subrepo types to be enabled since hg subrepo is
more widely used and considered less broken.

> +TODO buggy because localrepository.commit() managed its file contents
> +  $ hg commit -m 'manually add .hgsubstate'
> +  nothing changed
> +  [1]
> +
> +... but hg commit --amend works (this relies on a regression in 4.4)
> +
> +  $ hg commit --amend
> +  saved backup bundle to 
> $TESTTMP/testrepo0/.hg/strip-backup/addf99df3e66-ab5c9ff8-amend.hg (glob)

Yeah, it's the issue5677.

https://bz.mercurial-scm.org/show_bug.cgi?id=5677
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel


Re: [PATCH] subrepo: config option to disable subrepositories

2017-11-03 Thread Gregory Szorc
On Fri, Nov 3, 2017 at 10:06 PM, Matt Harbison 
wrote:

> On Fri, 03 Nov 2017 20:28:27 -0400, Gregory Szorc 
> wrote:
>
> # HG changeset patch
>> # User Gregory Szorc 
>> # 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/.hgsubstateThu Jan 01 00:00:00 1970 +
> -  +++ b/.hgsubstateThu Jan 01 00:00:00 1970 +
> -  @@ -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.
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel


Re: [PATCH] subrepo: config option to disable subrepositories

2017-11-03 Thread Matt Harbison
On Fri, 03 Nov 2017 20:28:27 -0400, Gregory Szorc  
 wrote:



# HG changeset patch
# User Gregory Szorc 
# 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/.hgsubstateThu Jan 01 00:00:00 1970 +
-  +++ b/.hgsubstateThu Jan 01 00:00:00 1970 +
-  @@ -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?
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel