Closed Bug 717196 Opened 8 years ago Closed 2 months ago
Update MXR to skip MPL 2 licence block
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
Are you going to drop the license delimiters: ***** BEGIN LICENSE BLOCK ***** ***** END LICENSE BLOCK ***** ?
Yep, those are gone. Gerv
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.
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
CC'ing jakem since he was the last person to work on MXR iirc.
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. :)
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
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
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. :)
New-style boilerplate is here: http://www.mozilla.org/MPL/headers/ (That's what I used to do the regexp.) Gerv
(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).
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.
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?
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.
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
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.
Status: NEW → RESOLVED
Closed: 2 months ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.