Closed Bug 1374534 Opened 7 years ago Closed 7 years ago

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

Categories

(Developer Services :: Servo VCS Sync, enhancement)

enhancement
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: glob, Assigned: smacleod)

References

Details

Attachments

(1 file)

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)
Blocks: 1365181
(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.
Flags: needinfo?(glob)
(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).
Flags: needinfo?(glob)
Depends on: 1375092
Blocks: 1375092
No longer depends on: 1375092
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.
Attachment #8880520 - Flags: review?(gps) → review-
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 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.
Attachment #8880520 - Flags: review?(gps) → review+
Pushed by smacleod@mozilla.com: https://hg.mozilla.org/hgcustom/version-control-tools/rev/bc908c109008 manualoverlay: add extension for manually creating overlay commits . r=gps
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: