717196 - Update MXR to skip MPL 2 licence block

Status: RESOLVED WONTFIX

(Webtools Graveyard :: MXR, defect)

Description

[Gervase Markham [:gerv]]

MXR displays the first comment in a file as the file's description in directory listings.  Examples:

http://mxr.mozilla.org/mozilla-central/source/mfbt/

The code which determines the correct comment to display skips modelines, license blocks, etc.  We should make sure that the license-block switch from tri-licence to MPL 2 doesn't adversely affect this. 

I think this is the code around here:
http://mxr.mozilla.org/webtools-central/source/mxr/Local.pm#92

Gerv

Comment 1

[Steffen Wilberg]

Are you going to drop the license delimiters:
***** BEGIN LICENSE BLOCK *****
***** END LICENSE BLOCK ***** ?

Comment 2

[Gervase Markham [:gerv]]

Yep, those are gone.

Gerv

Comment 3

[Jeff Walden [:Waldo]]

So, it's been pointed out to me that http://www.mozilla.org/MPL/headers/ says the new license *must* be used for new files.  I have nothing against the new license, indeed quite the opposite.  But I really really don't want to lose this description (or so I assume will happen).  Can this bug be prioritized a bit higher than it seems to be now, i.e. not at all?  (I figured the header-rewriting process would sweep up all the old-license-headers that would be added prior to this bug being fixed.)

Here are places where I've noted that I'm not using the new header specifically for this reason, or have requested (seemingly wrongly) that people not use it for this reason:

* bug 728423 comment 1
* bug 715405 comment 7

There might be others I'm forgetting as well.

Comment 4

[Gervase Markham [:gerv]]

bmoss: is your team responsible for MXR coding? If not, can you tell us who is? :-)

timeless: do you have any idea? (I'm not saying it's you :-)

Gerv

Comment 5

[Ed Morley [:emorley]]

CC'ing jakem since he was the last person to work on MXR iirc.

Comment 6

[Jake Maul [:jakem]]

Sorry, this is out of my realm of expertise... I can generally add/remove repos from MXR, and schedule when they get processed, but the underlying code isn't something I can effectively maintain. That does look like the right place, though.

As far as I know, MXR really *doesn't* have a maintainer right now. IT occasionally makes tweaks to the config as far as processing, scheduling, and the like, but I don't know anyone making changes to the app itself. Even IT's involvement is pretty minimal, and I doubt more than a handful of folks (still employed by Mozilla) have touched it.

If anyone does know of a current maintainer, or would like to step up, there are plenty of open bugs in Webtools::MXR that could use some TLC. :)



I am mildly entertained (but not encouraged) by this comment in MXR's source, right above the line you noted:

60 # Yea, though I walk through the valley of the shadow of pattern
61 # matching, I shall fear no regex.

I think this effectively summarizes a lot of MXR. :)

Comment 7

[Gervase Markham [:gerv]]

I'd have thought that we could fix this by changing the two regexps in that block of code from:

/BEGIN.*LICENSE/
/END.*LICENSE/

to

/BEGIN.*LICENSE|This Source Code Form/
/END.*LICENSE|You can obtain one at/

Of course, it would be good to have a testing install of MXR to see whether this was true - do we have one of those? I don't fancy trying to set one up locally...!

Gerv

Comment 8

[Steffen Wilberg]

There's http://mxr-test.mozilla.org/.

Also see bug 418089, which made it so that not only /*-style comments, but also #-style comments containing the license (between # BEGIN LICENSE BLOCK and # END LICENSE BLOCK) don't clutter ident results, e.g.
http://mxr.mozilla.org/mozilla-central/ident?i=copy
Notice the .pl files where this hasn't been fixed.
http://hg.mozilla.org/webtools/mxr/rev/e25247bf901d

Comment 9

[Jake Maul [:jakem]]

Hmm... do we have an example file with the new-style license header, to verify if gerv's proposed regex changes would work? Looking at the link in comment 3 it seems good, but it'd be nice to verify in practice.

This now seems a simple enough change that I would feel comfortable making it, presuming we have a before/after test case I can check. :)

Comment 10

[Gervase Markham [:gerv]]

New-style boilerplate is here:
http://www.mozilla.org/MPL/headers/

(That's what I used to do the regexp.)

Gerv

Comment 11

[Daniel C]

(In reply to Jake Maul [:jakem] from comment #9)
> Hmm... do we have an example file with the new-style license header, to
> verify if gerv's proposed regex changes would work? Looking at the link in
> comment 3 it seems good, but it'd be nice to verify in practice.

I assume you mean in MXR.

http://mxr.mozilla.org/mozilla-central/source/b2g/components/CameraContent.js
http://mxr.mozilla.org/mozilla-central/source/b2g/chrome/content/shell.xul
http://mxr.mozilla.org/mozilla-central/source/netwerk/protocol/device/GonkCaptureProvider.cpp
http://mxr.mozilla.org/mozilla-central/source/testing/mozbase/mozprofile/tests/permissions.py

are 4 files of different types each with the MPL 2 header (JS, XUL, CPP, Python).

Comment 12

[Daniel C]

Although...

http://mxr.mozilla.org/mozilla-central/source/content/base/src/nsAttrValueOrString.h

If you look at http://mxr.mozilla.org/mozilla-central/source/content/base/src/ then you can see a description.

Does that suggest that the existing code will actually work fine?


167     # Strip off license
168     $desc =~ s#(?:/\*.*license.*?\*/)(.*)#$1#is;

I think that line in Local.pm might make this bug WORKSFORME.

Comment 13

[Jake Maul [:jakem]]

Good catch. The 4 examples in comment 11 don't have any descriptive comments in them, and it looks like they all show up with no description (as expected). The example in comment 12 has a descriptive comment and a new-style license header, and the first sentence gets picked up in MXR.

It looks to me like any comment block containing "license" gets ignored. Perhaps the only thing that might need done would be to make this work for '#' or '//' style comments, if there aren't similar lines for those. I suspect that will be somewhat more complicated due to lack of an obvious open/close tag like '/* */' has... maybe comment 7 would work for this.

Do we have any examples of that to check?

Comment 14

[Steffen Wilberg]

Why don't we replace the "#" style license comments by the proper comment style, i.e. "/*" for js "<!--" for xul/xml files? The new headers are short enough that it doesn't make much sense to let the preprocessor strip them.

Comment 15

[Gervase Markham [:gerv]]

Steffen: if we were to do that, it would be a separate bug. When changing the license terms on thousands of files, it is hard enough to avoid breaking anything without changing the comment characters too. That would be a separate step.

It's hard to find examples to test with until I've done the relicensing! Assuming implementing comment #7 wouldn't take too long, perhaps I should just go ahead and we can fix this if it becomes a problem.

Gerv

Comment 16

[Jeff Walden [:Waldo]]

Hm.  Does MXR actually do the right thing here, somehow, despite its not understanding the MPL2 block?  (At least for some kinds of code.)  This might be WORKSFORME.

http://mxr.mozilla.org/mozilla-central/source/mfbt/

Of course, maybe it's only getting it right by dumb luck, and some rigorizing might be useful.  If so perhaps this should stay open.

Comment 17

[Sylvestre Ledru [:Sylvestre]]

mxr is gone, mass closing.
https://searchfox.org/ is a much better alternative.