1374534 - write a mercurial extension that allows for manually setting of overlay's extra data

Status: RESOLVED FIXED

(Developer Services :: Servo VCS Sync, enhancement)

Description

[:glob ✱]

in order to allow for stylo developers to recover should github/servo and autoland/servo go out of sync they need to push changes to the autoland repository with the overlay extension's extradata set.

as there doesn't appear to be a way to do this from the hg client we'll need a simple hgext to handle setting the data:

REVISION_KEY = 'subtree_revision' (The revision that was landed from linearize)
SOURCE_KEY = 'subtree_source' (The url of the source repository)

Comment 1

[Steven MacLeod [:smacleod]]

(In reply to Byron Jones ‹:glob› from comment #0)
> REVISION_KEY = 'subtree_revision' (The revision that was landed from
> linearize)
> SOURCE_KEY = 'subtree_source' (The url of the source repository)

Do you think we should just mimic exactly what the sync service does? or do something slightly different so that user created commits of this form are obvious? or will we just use pushlog for that?

I was also thinking, do we want to deploy a push hook to restrict pushing commits like this except under certain circumstances? certain users? etc.

Comment 2

[Steven MacLeod [:smacleod]]

(In reply to Steven MacLeod [:smacleod] from comment #1)
> (In reply to Byron Jones ‹:glob› from comment #0)
> > REVISION_KEY = 'subtree_revision' (The revision that was landed from
> > linearize)
> > SOURCE_KEY = 'subtree_source' (The url of the source repository)
> 
> Do you think we should just mimic exactly what the sync service does? or do
> something slightly different so that user created commits of this form are
> obvious? or will we just use pushlog for that?

Alright so after discussing this with glob over vidyo we decided on the following:

The developer is responsible for applying the rogue commit patch locally so that the file contents will match on the next sync. They will create a commit which makes these modifications. They will then run the command:
> $ hg prepare-servo-commit <github-pr-url>

This command will amend the working directories parent commit, setting the proper extra data fields ('subtree_revision', 'subtree_source') and rewriting the commit message to take the same format as the linearized commit. This will fail if the rogue commit has not yet been linearized, as we cannot know the linearized commit's subtree_revision value.


> I was also thinking, do we want to deploy a push hook to restrict pushing
> commits like this except under certain circumstances? certain users? etc.

We will change the current push hook to allow any L3 user to touch the servo directory if their commit takes the proper form (it has the correct commit message, and extra data set).

Comment 3

[Steven MacLeod [:smacleod]]

Attachment: manualoverlay: add extension for manually creating overlay commits (Bug 1374534).

This extension can be used to resync when the overlay repo
becomes out of sync due to the race condition between backout
pushes and homu landing other changes. After this rogue commit
has been landed and linearized, it will fail to overlay due to
file difference in the overlayed directory.

Using the pull request id of the rogue change, the corresponding
linearized commit is found and used for the commit's message and
extra data fields. The user must provide file modifications that
will resync the overlayed sub directory manually.

The created commit can then be pushed up to the overlay target
and overlay will assume it was the problematic changeset but
containing files that will match going forward.

This change has been tested manually for the most part as the
current code relies on network access to hg.mozilla.org. A
future change can add automated tests for this without
much trouble using something like betamax to record this
interaction.

Review commit: https://reviewboard.mozilla.org/r/151854/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/151854/

Comment 4

[Gregory Szorc [:gps]]

Comment on attachment 8880520 [details]
manualoverlay: add extension for manually creating overlay commits (Bug 1374534).

https://reviewboard.mozilla.org/r/151854/#review156962

This is a great start!

::: hgext/manualoverlay/__init__.py:24
(Diff revision 1)
> +demandimport.disable()
> +import requests  # noqa
> +demandimport.enable()

Please use the context manager: `with demandimport.deactivated():`.

::: hgext/manualoverlay/__init__.py:29
(Diff revision 1)
> +demandimport.disable()
> +import requests  # noqa
> +demandimport.enable()
> +
> +
> +testedwith = '4.1'

Please add 4.2 (assuming it works).

::: hgext/manualoverlay/__init__.py:58
(Diff revision 1)
> +
> +    pr = m.group('pr')
> +
> +    revset = urllib.quote('keyword("servo: Merge #%s")' % pr)
> +    url = '%sjson-log?rev=%s' % (LINEAR_REPO_URL, revset)
> +    r = requests.get(url)

Mercurial has its own mechanism for doing HTTP requests (mercurial.url.open()). But using requests is fine. (requests is a superior interface.)

::: hgext/manualoverlay/__init__.py:67
(Diff revision 1)
> +        raise error.Abort(
> +            _('could not find linearized commit corresponding to %s' % pr_url),
> +            hint=_('If this pull requests has recently been merged it '
> +                   'may not be linearized yet, please try again soon'))
> +
> +    repo.manualsync_commit = commits[0]

It would be preferable to normalize this to bytes from unicode *before* it is assigned. Dealing with bytes internally is the Mercurial way. So removing unicode instances ASAP is preferred.

::: hgext/manualoverlay/__init__.py:87
(Diff revision 1)
> +            for key in ('text', 'user'):
> +                if args:
> +                    args = args[1:]
> +
> +                if key in kwargs:
> +                    del kwargs[key]

It is worth commenting this.

::: hgext/manualoverlay/__init__.py:95
(Diff revision 1)
> +
> +                if key in kwargs:
> +                    del kwargs[key]
> +
> +            kwargs['extra'] = kwargs['extra'] if 'extra' in kwargs else {}
> +            kwargs['extra'][SOURCE_KEY] = LINEAR_REPO_URL

Nit: this extension will add a trailing slash whereas the values currently in the repo don't have the trailing slash. Consistency would be nice.

::: hgext/manualoverlay/__init__.py:95
(Diff revision 1)
> +            kwargs['extra'][SOURCE_KEY] = LINEAR_REPO_URL
> +            kwargs['extra'][REVISION_KEY] = encoding.tolocal(
> +                self.manualsync_commit['node'].encode('utf-8'))

It is weird that only 1 of these is using `encoding.tolocal()`.

Since these are both valid UTF-8 byte sequences, `encoding.tolocal()` shouldn't be needed. Just be sure you assign a str/bytes and not a unicode.

::: hgext/manualoverlay/__init__.py:99
(Diff revision 1)
> +            # TODO: Verify that the file changes being committed are only
> +            # under the servo/ directory.

I would hook the creation of the "commit context" instance by creating your own derived class that implements a custom __init__ and replacing the class symbol in mercurial.context as part of extsetup(). Hacky, but it should work.

Comment 5

[Steven MacLeod [:smacleod]]

Comment on attachment 8880520 [details]
manualoverlay: add extension for manually creating overlay commits (Bug 1374534).

https://reviewboard.mozilla.org/r/151854/#review156962

> I would hook the creation of the "commit context" instance by creating your own derived class that implements a custom __init__ and replacing the class symbol in mercurial.context as part of extsetup(). Hacky, but it should work.

After talking with :glob we decided to just warn in this case, rather than reject creating the commit.

Comment 6

[Steven MacLeod [:smacleod]]

Comment on attachment 8880520 [details]
manualoverlay: add extension for manually creating overlay commits (Bug 1374534).

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/151854/diff/1-2/

Comment 7

[Gregory Szorc [:gps]]

Comment on attachment 8880520 [details]
manualoverlay: add extension for manually creating overlay commits (Bug 1374534).

https://reviewboard.mozilla.org/r/151854/#review160824

This is simpler than I thought it would be!

Obviously we need a bunch of docs to understand how this works. But those can be done in a follow-up.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/hgcustom/version-control-tools/rev/bc908c109008
manualoverlay: add extension for manually creating overlay commits . r=gps