Patchwork [2,of,2] requires: write requires file on commit

login
register
mail settings
Submitter Drew Gottlieb
Date May 4, 2015, 6:27 p.m.
Message ID <85c369520dd97bad369a.1430764078@waste.org>
Download mbox | patch
Permalink /patch/8869/
State Rejected
Headers show

Comments

Drew Gottlieb - May 4, 2015, 6:27 p.m.
# HG changeset patch
# User Drew Gottlieb <drgott@google.com>
# Date 1429641493 25200
#      Tue Apr 21 11:38:13 2015 -0700
# Node ID 85c369520dd97bad369a0c4b924597f9e720077b
# Parent  3f86e3b2548def52732e1f782e89297db8075e8b
requires: write requires file on commit

Writing the requires file on commit makes it possible for changes to a
repo's config to enable features that affect the repo's requirements. An
example of this would be enabling manifestv2 in the future, which would then
have future commits be written with the new manifest format, making it
necessary for the requires file to indicate this.

This change also modifies test-inherit-mode to expect the requires file's
mode to inherit from the store's mode after a commit. Previously, it only
inherited from the store's mode on repo init, and the test assumed it would
never be touched after this point.
Adrian Buehlmann - May 4, 2015, 7:26 p.m.
On 2015-05-04 20:27, Drew Gottlieb wrote:
> # HG changeset patch
> # User Drew Gottlieb <drgott@google.com>
> # Date 1429641493 25200
> #      Tue Apr 21 11:38:13 2015 -0700
> # Node ID 85c369520dd97bad369a0c4b924597f9e720077b
> # Parent  3f86e3b2548def52732e1f782e89297db8075e8b
> requires: write requires file on commit
> 
> Writing the requires file on commit makes it possible for changes to a
> repo's config to enable features that affect the repo's requirements. An
> example of this would be enabling manifestv2 in the future, which would then
> have future commits be written with the new manifest format, making it
> necessary for the requires file to indicate this.
> 
> This change also modifies test-inherit-mode to expect the requires file's
> mode to inherit from the store's mode after a commit. Previously, it only
> inherited from the store's mode on repo init, and the test assumed it would
> never be touched after this point.
> 
> diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
> --- a/mercurial/localrepo.py
> +++ b/mercurial/localrepo.py
> @@ -1602,6 +1602,7 @@
>                  #
>                  # if minimal phase was 0 we don't need to retract anything
>                  phases.retractboundary(self, tr, targetphase, [n])
> +            self._writerequirements()
>              tr.close()
>              branchmap.updatecache(self.filtered('served'))
>              return n
> diff --git a/tests/test-inherit-mode.t b/tests/test-inherit-mode.t
> --- a/tests/test-inherit-mode.t
> +++ b/tests/test-inherit-mode.t
> @@ -70,7 +70,7 @@
>    00660 ./.hg/cache/rbc-revs-v1
>    00660 ./.hg/dirstate
>    00660 ./.hg/last-message.txt
> -  00600 ./.hg/requires
> +  00660 ./.hg/requires
>    00770 ./.hg/store/
>    00660 ./.hg/store/00changelog.i
>    00660 ./.hg/store/00manifest.i

This sounds pretty scary...

I think repository readers today can safely assume that the requires
file is not changed any more after they have first seen it. Somebody
might be actually doing that already and indeed depend on that assumption.

At least, this might be quite a big surprise.
Martin von Zweigbergk - May 4, 2015, 7:57 p.m.
On Mon, May 4, 2015 at 12:27 PM Adrian Buehlmann <adrian@cadifra.com> wrote:

> On 2015-05-04 20:27, Drew Gottlieb wrote:
> > # HG changeset patch
> > # User Drew Gottlieb <drgott@google.com>
> > # Date 1429641493 25200
> > #      Tue Apr 21 11:38:13 2015 -0700
> > # Node ID 85c369520dd97bad369a0c4b924597f9e720077b
> > # Parent  3f86e3b2548def52732e1f782e89297db8075e8b
> > requires: write requires file on commit
> >
> > Writing the requires file on commit makes it possible for changes to a
> > repo's config to enable features that affect the repo's requirements. An
> > example of this would be enabling manifestv2 in the future, which would
> then
> > have future commits be written with the new manifest format, making it
> > necessary for the requires file to indicate this.
> >
> > This change also modifies test-inherit-mode to expect the requires file's
> > mode to inherit from the store's mode after a commit. Previously, it only
> > inherited from the store's mode on repo init, and the test assumed it
> would
> > never be touched after this point.
> >
> > diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
> > --- a/mercurial/localrepo.py
> > +++ b/mercurial/localrepo.py
> > @@ -1602,6 +1602,7 @@
> >                  #
> >                  # if minimal phase was 0 we don't need to retract
> anything
> >                  phases.retractboundary(self, tr, targetphase, [n])
> > +            self._writerequirements()
> >              tr.close()
> >              branchmap.updatecache(self.filtered('served'))
> >              return n
> > diff --git a/tests/test-inherit-mode.t b/tests/test-inherit-mode.t
> > --- a/tests/test-inherit-mode.t
> > +++ b/tests/test-inherit-mode.t
> > @@ -70,7 +70,7 @@
> >    00660 ./.hg/cache/rbc-revs-v1
> >    00660 ./.hg/dirstate
> >    00660 ./.hg/last-message.txt
> > -  00600 ./.hg/requires
> > +  00660 ./.hg/requires
> >    00770 ./.hg/store/
> >    00660 ./.hg/store/00changelog.i
> >    00660 ./.hg/store/00manifest.i
>
> This sounds pretty scary...
>
> I think repository readers today can safely assume that the requires
> file is not changed any more after they have first seen it. Somebody
> might be actually doing that already and indeed depend on that assumption.
>
> At least, this might be quite a big surprise.
>

What kind of repository readers are you thinking of? A command server?
Tortoise? Or even something the user doesn't know is running? One
alternative is of course to not allow upgrades. Any other alternatives?


> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel
>
Matt Mackall - May 4, 2015, 8:53 p.m.
On Mon, 2015-05-04 at 13:27 -0500, Drew Gottlieb wrote:
> # HG changeset patch
> # User Drew Gottlieb <drgott@google.com>
> # Date 1429641493 25200
> #      Tue Apr 21 11:38:13 2015 -0700
> # Node ID 85c369520dd97bad369a0c4b924597f9e720077b
> # Parent  3f86e3b2548def52732e1f782e89297db8075e8b
> requires: write requires file on commit
> 
> Writing the requires file on commit makes it possible for changes to a
> repo's config to enable features that affect the repo's requirements. An
> example of this would be enabling manifestv2 in the future, which would then
> have future commits be written with the new manifest format, making it
> necessary for the requires file to indicate this.
> 
> This change also modifies test-inherit-mode to expect the requires file's
> mode to inherit from the store's mode after a commit. Previously, it only
> inherited from the store's mode on repo init, and the test assumed it would
> never be touched after this point.

I can't say I'm excited about this. Today, changing the requirements of
a repository is a big deal, and is intentionally done only at creation
time[1]. Among other things, it means people can create a repo with an
old Mercurial and be assured of continuing compatibility with that
version. Very important for repositories on shared disk.

Writing it unconditionally (and non-atomically!) on every commit is just
inviting people/extensions to break this. I think it should instead
continue to be an extension-specific headache.

[1] Largefiles breaks this rule; largefiles breaks everything and should
not be looked to as a role model.
Drew Gottlieb - May 4, 2015, 10:30 p.m.
Alright, let's drop this patch for now. I'd still like to see patch 1 of 2
queued though.

From IRC it seems a possible way forward can be having a command like 'hg
migrate' to handle migrations to new formats such as manifestv2 or
treemanifests. If such a command were created, the requires write could
happen then. Obviously, whether and how we have such a command is a
considerably larger discussion.

On Mon, May 4, 2015 at 1:53 PM Matt Mackall <mpm@selenic.com> wrote:

> On Mon, 2015-05-04 at 13:27 -0500, Drew Gottlieb wrote:
> > # HG changeset patch
> > # User Drew Gottlieb <drgott@google.com>
> > # Date 1429641493 25200
> > #      Tue Apr 21 11:38:13 2015 -0700
> > # Node ID 85c369520dd97bad369a0c4b924597f9e720077b
> > # Parent  3f86e3b2548def52732e1f782e89297db8075e8b
> > requires: write requires file on commit
> >
> > Writing the requires file on commit makes it possible for changes to a
> > repo's config to enable features that affect the repo's requirements. An
> > example of this would be enabling manifestv2 in the future, which would
> then
> > have future commits be written with the new manifest format, making it
> > necessary for the requires file to indicate this.
> >
> > This change also modifies test-inherit-mode to expect the requires file's
> > mode to inherit from the store's mode after a commit. Previously, it only
> > inherited from the store's mode on repo init, and the test assumed it
> would
> > never be touched after this point.
>
> I can't say I'm excited about this. Today, changing the requirements of
> a repository is a big deal, and is intentionally done only at creation
> time[1]. Among other things, it means people can create a repo with an
> old Mercurial and be assured of continuing compatibility with that
> version. Very important for repositories on shared disk.
>
> Writing it unconditionally (and non-atomically!) on every commit is just
> inviting people/extensions to break this. I think it should instead
> continue to be an extension-specific headache.
>
> [1] Largefiles breaks this rule; largefiles breaks everything and should
> not be looked to as a role model.
>
> --
> Mathematics is the supreme nostalgia of our time.
>
>
>
Adrian Buehlmann - May 4, 2015, 11:09 p.m.
On 2015-05-05 00:30, Drew Gottlieb wrote:
> From IRC it seems a possible way forward can be having a command like
> 'hg migrate' to handle migrations to new formats such as manifestv2 or
> treemanifests. If such a command were created, the requires write could
> happen then. Obviously, whether and how we have such a command is a
> considerably larger discussion.

I assume you are aware of

  hg --config experimental.manifestv2=1 clone --pull repoA repoB

See also http://mercurial.selenic.com/wiki/RequiresFile
Drew Gottlieb - May 4, 2015, 11:12 p.m.
If I'm not mistaken, that rewrites the hashes of old commits as it clones
them to manifestv2 format, correct? What we're seeking is a way to have a
repository with manifestv1 to switch to using manifestv2 for future
commits, without modifying old hashes.

On Mon, May 4, 2015 at 4:09 PM Adrian Buehlmann <adrian@cadifra.com> wrote:

> On 2015-05-05 00:30, Drew Gottlieb wrote:
> > From IRC it seems a possible way forward can be having a command like
> > 'hg migrate' to handle migrations to new formats such as manifestv2 or
> > treemanifests. If such a command were created, the requires write could
> > happen then. Obviously, whether and how we have such a command is a
> > considerably larger discussion.
>
> I assume you are aware of
>
>   hg --config experimental.manifestv2=1 clone --pull repoA repoB
>
> See also http://mercurial.selenic.com/wiki/RequiresFile
>
Adrian Buehlmann - May 5, 2015, 6:15 a.m.
On 2015-05-05 01:12, Drew Gottlieb wrote:
> If I'm not mistaken, that rewrites the hashes of old commits as it
> clones them to manifestv2 format, correct?

Really? Why?

But I'm surprised you're asking this, since I am assuming you are the
manifestv2 expert...

I didn't try it myself, but as a mercurial-devel lurker, I just had a
look at

  http://mercurial.selenic.com/wiki/ManifestV2Plan

and got the impression that repositories having manifestv2 in requires
mean, that they may contain manifest revisions in the new version 2
format, right?

So, perhaps doing the well known, canonical, repository "conversion"

  hg --config experimental.manifestv2=1 clone --pull repoA repoB

should just leave repoB as it is (all manifest revisions in version 1),
but add the manifestv2 marker to the requires file in repoB, which in
turn would mean that the next commit in repoB would write a v2 manifest
revision, right?

> What we're seeking is a way
> to have a repository with manifestv1 to switch to using manifestv2 for
> future commits, without modifying old hashes.

I suspect this is what probably should (and perhaps already does) happen
with the command line I mentioned.

> On Mon, May 4, 2015 at 4:09 PM Adrian Buehlmann <adrian@cadifra.com
> <mailto:adrian@cadifra.com>> wrote:
> 
>     On 2015-05-05 00:30, Drew Gottlieb wrote:
>     > From IRC it seems a possible way forward can be having a command like
>     > 'hg migrate' to handle migrations to new formats such as manifestv2 or
>     > treemanifests. If such a command were created, the requires write
>     could
>     > happen then. Obviously, whether and how we have such a command is a
>     > considerably larger discussion.
> 
>     I assume you are aware of
> 
>       hg --config experimental.manifestv2=1 clone --pull repoA repoB
> 
>     See also http://mercurial.selenic.com/wiki/RequiresFile
>
Martin von Zweigbergk - May 5, 2015, 3:04 p.m.
I think you're right. Somehow it never hit me that cloning just to update
.hg/requires would make sense. Thanks for the suggestion!

On Mon, May 4, 2015 at 11:16 PM Adrian Buehlmann <adrian@cadifra.com> wrote:

> On 2015-05-05 01:12, Drew Gottlieb wrote:
> > If I'm not mistaken, that rewrites the hashes of old commits as it
> > clones them to manifestv2 format, correct?
>
> Really? Why?
>
> But I'm surprised you're asking this, since I am assuming you are the
> manifestv2 expert...
>
> I didn't try it myself, but as a mercurial-devel lurker, I just had a
> look at
>
>   http://mercurial.selenic.com/wiki/ManifestV2Plan
>
> and got the impression that repositories having manifestv2 in requires
> mean, that they may contain manifest revisions in the new version 2
> format, right?
>
> So, perhaps doing the well known, canonical, repository "conversion"
>
>   hg --config experimental.manifestv2=1 clone --pull repoA repoB
>
> should just leave repoB as it is (all manifest revisions in version 1),
> but add the manifestv2 marker to the requires file in repoB, which in
> turn would mean that the next commit in repoB would write a v2 manifest
> revision, right?
>
> > What we're seeking is a way
> > to have a repository with manifestv1 to switch to using manifestv2 for
> > future commits, without modifying old hashes.
>
> I suspect this is what probably should (and perhaps already does) happen
> with the command line I mentioned.
>
> > On Mon, May 4, 2015 at 4:09 PM Adrian Buehlmann <adrian@cadifra.com
> > <mailto:adrian@cadifra.com>> wrote:
> >
> >     On 2015-05-05 00:30, Drew Gottlieb wrote:
> >     > From IRC it seems a possible way forward can be having a command
> like
> >     > 'hg migrate' to handle migrations to new formats such as
> manifestv2 or
> >     > treemanifests. If such a command were created, the requires write
> >     could
> >     > happen then. Obviously, whether and how we have such a command is a
> >     > considerably larger discussion.
> >
> >     I assume you are aware of
> >
> >       hg --config experimental.manifestv2=1 clone --pull repoA repoB
> >
> >     See also http://mercurial.selenic.com/wiki/RequiresFile
> >
>
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel
>

Patch

diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
--- a/mercurial/localrepo.py
+++ b/mercurial/localrepo.py
@@ -1602,6 +1602,7 @@ 
                 #
                 # if minimal phase was 0 we don't need to retract anything
                 phases.retractboundary(self, tr, targetphase, [n])
+            self._writerequirements()
             tr.close()
             branchmap.updatecache(self.filtered('served'))
             return n
diff --git a/tests/test-inherit-mode.t b/tests/test-inherit-mode.t
--- a/tests/test-inherit-mode.t
+++ b/tests/test-inherit-mode.t
@@ -70,7 +70,7 @@ 
   00660 ./.hg/cache/rbc-revs-v1
   00660 ./.hg/dirstate
   00660 ./.hg/last-message.txt
-  00600 ./.hg/requires
+  00660 ./.hg/requires
   00770 ./.hg/store/
   00660 ./.hg/store/00changelog.i
   00660 ./.hg/store/00manifest.i