1130343 - Invent a convention for noting third-party source in the Mozilla tree

Status: NEW

(Firefox Build System :: General, defect)

Description

[(not currently active) Ted Mielczarek]

We have a lot of third-party source in our tree. It would be nice if we had a consistent convention to indicate that a directory contained third-party source, and provide metadata about it like the upstream repository it was pulled from, the latest revision we're using, any local patches we have, etc.

For people doing tree-wide changes this would be useful since they'd be able to tell which changes need to get upstreamed.

If we put this in a machine-readable format we could add a mach command to consistently update third-party source from upstream, something which right now is left to ad-hoc scripts scattered around the tree (or sometimes a fully manual process).

Comment 1

[Gervase Markham [:gerv]]

No objections here.

Gerv

Comment 2

[Mitchell Field [:Mitch]]

Attachment: Prototype

This is a rough concept with a real, example third-party module.
Included:
* moz.build file with metadata.
* A small, executable script, replacing the manual steps of updating. Not unlike the many, scattered, ad-hoc scripts mentioned.
* A base class for said script, to handle reading/writing the moz.build metadata.

The idea is to make this as declarative as possible (like moz.build), only falling back to executed scripts for the not-so-common, ugly stuff.

I haven't made use of the moz.build reading/writing infrastructure here.

The feedback I'm trying to draw from this is where to place things, what parts we can use (bug 1132771?), etc.

Bikeshedding about variable names is encouraged.

Comment 3

[Gregory Szorc [:gps]]

Comment on attachment 8564388 [details] [diff] [review]
Prototype

Review of attachment 8564388 [details] [diff] [review]:
-----------------------------------------------------------------

Attempting to define a generic schema for handling all third party update cases will likely be an exercise in futility.

I think the first goal should be to introduce a sub-context (see work in bug 1132771) for third-party sources. We should only grow in metadata that is absolutely necessary.

I think moz.build should declare a script (or python module) to execute and its arguments. These arguments could differ for each project. That's fine. If we do things this way and allow variables to be passed from moz.build into the script, module_updater.py can drop its hacky moz.build "parsing" and we can just use moz.build evaluation results themselves. Bug 1132771 is making it so executing any moz.build in isolation will work (well, technically you need to evaluate each moz.build file chained to the root one, but same difference - there is an API that will make this easy). We'll just need to build a mode that finds the "third party sources" sub-context and converts it into a process or function call. Again, bug 1132771 will have some code for you to base your work on. Make sense?

FWIW, bug 1132771 is still in a bit of flux. I'd hold off doing more work on this until it has stabilized. Hopefully I can get things landed by the middle of next week.

Comment 4

[Nick Alexander :nalexander [he/him]]

(In reply to Gregory Szorc [:gps] from comment #3)
> Comment on attachment 8564388 [details] [diff] [review]
> Prototype
> 
> Review of attachment 8564388 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Attempting to define a generic schema for handling all third party update
> cases will likely be an exercise in futility.
> 
> I think the first goal should be to introduce a sub-context (see work in bug
> 1132771) for third-party sources. We should only grow in metadata that is
> absolutely necessary.
> 
> I think moz.build should declare a script (or python module) to execute and
> its arguments. These arguments could differ for each project. That's fine.
> If we do things this way and allow variables to be passed from moz.build
> into the script, module_updater.py can drop its hacky moz.build "parsing"
> and we can just use moz.build evaluation results themselves. Bug 1132771 is
> making it so executing any moz.build in isolation will work (well,
> technically you need to evaluate each moz.build file chained to the root
> one, but same difference - there is an API that will make this easy). We'll
> just need to build a mode that finds the "third party sources" sub-context
> and converts it into a process or function call.

Ooh, this is a nice idea.  IDE specific declarations feel more like sub-contexts than like separate build backends.

Comment 5

[(not currently active) Ted Mielczarek]

Comment on attachment 8564388 [details] [diff] [review]
Prototype

Review of attachment 8564388 [details] [diff] [review]:
-----------------------------------------------------------------

I like the concept a lot, thanks for the patch! I think I'd rather see us try to centralize the update scripts into the mozbuild backend rather than having everyone roll their own (which isn't a whole lot better than the current situation). I think you should put something under python/mozbuild that we can invoke via mach that reads the metadata here and knows how to pull a repo (if you only handle git at first that's fine) into a specified directory, maybe with some extra metadata like you have here, and then write out the revision it pulled and the date into...something (clearly need to think about that more).

If you want to reference an existing update script, the Breakpad one I use covers a lot of ground:
https://dxr.mozilla.org/mozilla-central/source/toolkit/crashreporter/update-breakpad.sh

I think having support for "pull upstream repo", "files to exclude from upstream", "files in the local repo that we should make sure don't get overwritten by upstream", "local patch set" at a minimum would make this useful for a lot of the projects we have in-tree. Of the projects with more complicated needs we could look at adding hooks for moz.build generation or things like that.

::: other-licenses/snappy/module_updater.py
@@ +12,5 @@
> +            ['VERSION', None],
> +            ['DATE', None],
> +            ['ISSUE_TRACKER', None],
> +        ])
> +        self.moz_build = open('moz.build', 'r+b')

This doesn't thrill me. If we're going to pull data out of a moz.build file we should use the existing moz.build reading code.

::: other-licenses/snappy/moz.build
@@ +6,5 @@
>  
> +NAME = 'snappy'
> +WEBSITE = 'https://code.google.com/p/snappy/'
> +LICENSE += [
> +    'BSD 3-Clause License'

I'd just have a stock set of licenses keyed by short names here, I don't expect we have too much goofiness with upstream licenses. Maybe use the names from the URLs on opensource.org, like http://opensource.org/licenses/BSD-3-Clause ? That'd make this "BSD-3-Clause" which isn't that much shorter, but I think restricting this to a specific set of licenses makes sense.

@@ +10,5 @@
> +    'BSD 3-Clause License'
> +]
> +# for non-mainstream/one-off licences
> +LICENSE_TEXT = '''
> +'''

Not sure we'll need this, but even if we do clearly you don't need it here! (Also it'd probably be better off referencing a license file in-tree.)

@@ +15,5 @@
> +DIR = 'src'
> +SOURCE = 'https://github.com/google/snappy.git'
> +REVISION = 'd9068ee301bdf893a4d8cb7c6518eacc44c4c1f2'
> +VERSION = '1.0.4'
> +DATE = '2012-01-05'

I wonder if maybe we should split this into two parts? The description of the project and upstream repo and everything in one place, and the revision/version/date in another, maybe in the generated moz.build?

@@ +16,5 @@
> +SOURCE = 'https://github.com/google/snappy.git'
> +REVISION = 'd9068ee301bdf893a4d8cb7c6518eacc44c4c1f2'
> +VERSION = '1.0.4'
> +DATE = '2012-01-05'
> +ISSUE_TRACKER = 'https://code.google.com/p/snappy/issues/list'

This is pretty great, but as gps says I think it probably wants a sub-context or something.