716555 - MPL 2 upgrade: Venkman

Status: RESOLVED FIXED

(mozilla.org :: Licensing, task)

Description

[Gervase Markham [:gerv]]

This bug tracks the MPL 2 upgrade for the project named in the subject line.
The repo is here: http://hg.mozilla.org/venkman/

Gerv

Comment 1

[:Gijs (he/him)]

(In reply to Gervase Markham [:gerv] from comment #0)
> This bug tracks the MPL 2 upgrade for the project named in the subject line.
> The repo is here: http://hg.mozilla.org/venkman/
> 
> Gerv

Is there anything expected from the module owners/peers for this bug? Venkman is basically on bare essentials life support until JSD2 gets released (practically this means: not much is happening on the repo or in terms of releases) so if you just need to run a script, do so at your leisure, as far as I'm concerned. :-)

Comment 2

[Gervase Markham [:gerv]]

Ideally, you don't have to do anything, although you could ask for a patch to review first, and/or you could fix up anything I break afterwards :-))

Gerv

Comment 3

[Gervase Markham [:gerv]]

Attachment: Patch v.1

Try this for size :-) Does it look OK to you?

Gerv

Comment 4

[:Gijs (he/him)]

Comment on attachment 587701 [details] [diff] [review]
Patch v.1

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

r- because of the template files, venkman.pkg and the #filter thing in install.rdf (the RDF parser won't like those comments if they stay).

If the license is necessary then ideally the processors for those template files would get fixed to remove the comments in the resulting file, possibly in a followup bug. TBH, I'm not completely sure when I'd find the time to fix the templating code to deal with them, though (as said, Venkman's on life support...).

In general it looks pretty good, it'd be perfect if the script ate the last bits of empty comment at the top of the files, too. Thanks!

::: locales/en-US/chrome/profile.html.tpl
@@ +1,3 @@
> +<!-- This Source Code Form is subject to the terms of the Mozilla Public
> +   - License, v. 2.0. If a copy of the MPL was not distributed with this
> +   - file, You can obtain one at http://mozilla.org/MPL/2.0/. -->

This, and the other profiling templates (profile.txt.tpl, profile.xml.tpl) worry me slightly because they get reused without much further processing, so these licenses will persist in profile reports generated this way. The files didn't yet have licenses; are we required to add them?

::: locales/generic/install.rdf
@@ +1,4 @@
>  <?xml version="1.0"?>
> +# This Source Code Form is subject to the terms of the Mozilla Public
> +# License, v. 2.0. If a copy of the MPL was not distributed with this
> +# file, You can obtain one at http://mozilla.org/MPL/2.0/.

Shouldn't this go below the #filter substitution, or does it not matter? (I'm not too familiar with the generic Mozilla build tools used to do this processing step, I admit...)

::: resources/content/venkman-bindings.xml
@@ +1,4 @@
>  <?xml version="1.0"?>
>  
>  <!--
>     -

Nit: can we make your script eat comment lines with no alphanumeric characters above the license, like this one? There's a fair amount in this codebase, it seems...

::: venkman.pkg
@@ +1,3 @@
> +# This Source Code Form is subject to the terms of the Mozilla Public
> +# License, v. 2.0. If a copy of the MPL was not distributed with this
> +# file, You can obtain one at http://mozilla.org/MPL/2.0/.

I am (a) not sure this file is ever used anymore (it's a suite thing if it is, possibly mac-only) and (b) not sure it does comments in this manner. Are you?

Comment 5

[Gervase Markham [:gerv]]

> This, and the other profiling templates (profile.txt.tpl, profile.xml.tpl)
> worry me slightly because they get reused without much further processing,
> so these licenses will persist in profile reports generated this way. The
> files didn't yet have licenses; are we required to add them?

I'd like to have licenses in almost every file, but I can let it go, for 
Venkman, as the project is nearing EOL :-)

> ::: locales/generic/install.rdf
> @@ +1,4 @@
> >  <?xml version="1.0"?>
> > +# This Source Code Form is subject to the terms of the Mozilla Public
> > +# License, v. 2.0. If a copy of the MPL was not distributed with this
> > +# file, You can obtain one at http://mozilla.org/MPL/2.0/.
> 
> Shouldn't this go below the #filter substitution, or does it not matter?
> (I'm not too familiar with the generic Mozilla build tools used to do this
> processing step, I admit...)

I suspect it doesn't matter; anything starting with a # will get removed by
the preprocessor. Here's an example of a file with the licence before the 
filter statement:
http://mxr.mozilla.org/mozilla-central/source/b2g/locales/en-US/b2g-l10n.js

> Nit: can we make your script eat comment lines with no alphanumeric
> characters above the license, like this one? There's a fair amount in this
> codebase, it seems...

Not easily; the script works hard to preserve whatever comment formatting is 
currently being used, on the basis that I charitably assume that it was
intentional, and don't want to upset people :-) If there are other fixes you
want, I'm afraid you'll need to write your own scripts.

> I am (a) not sure this file is ever used anymore (it's a suite thing if it
> is, possibly mac-only) and (b) not sure it does comments in this manner. Are
> you?

Looks like these files are rarer than hen's teeth. This mapping from .pkg to 
"#" was left over from 8 years ago; I've removed it.

If that's all OK with you, I'll upload a new patch.

Gerv

Comment 6

[:Gijs (he/him)]

(In reply to Gervase Markham [:gerv] from comment #5)
> > This, and the other profiling templates (profile.txt.tpl, profile.xml.tpl)
> > worry me slightly because they get reused without much further processing,
> > so these licenses will persist in profile reports generated this way. The
> > files didn't yet have licenses; are we required to add them?
> 
> I'd like to have licenses in almost every file, but I can let it go, for 
> Venkman, as the project is nearing EOL :-)

Great. :-)

> <snip>
> > Nit: can we make your script eat comment lines with no alphanumeric
> > characters above the license, like this one? There's a fair amount in this
> > codebase, it seems...
> 
> Not easily; the script works hard to preserve whatever comment formatting is 
> currently being used, on the basis that I charitably assume that it was
> intentional, and don't want to upset people :-) If there are other fixes you
> want, I'm afraid you'll need to write your own scripts.

Fair enough!

> <snip>

> If that's all OK with you, I'll upload a new patch.

Sounds good, thanks a lot for all the work you're putting into this. :-)

Comment 7

[Gervase Markham [:gerv]]

Attachment: Patch v.2

How about this?

Gerv

Comment 8

[:Gijs (he/him)]

Comment on attachment 587732 [details] [diff] [review]
Patch v.2

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

r+, thanks a lot! :-)

(This is when I'm happy for bugzilla's ability to show the diff between two patches...)

Comment 9

[Gervase Markham [:gerv]]

committed changeset 793:60cf34496f76

Gerv