Closed Bug 343146 Opened 15 years ago Closed 14 years ago

pull and build l10n dictionaries

Categories

(Toolkit Graveyard :: Build Config, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
mozilla1.8.1beta2

People

(Reporter: Pike, Assigned: benjamin)

References

Details

(Keywords: fixed1.8.1)

Attachments

(2 files)

We currently don't pull extensions/spellcheck/myspell for either
LOCALES_mail or LOCALES_browser. Which is a halfway good thing, as those directories are optional, and we can't pull non-existing dirs.
We could probably add some empty dir with just a .cvsignore in it or so, but 
that sounds suboptimal to me, too.
We have time to fix this until our licensing story is a little bit more solid, which is bug 343142. But that's not that much time, hopefully.
Attached patch Checkout, rev. 1Splinter Review
Assignee: nobody → benjamin
Status: NEW → ASSIGNED
Attachment #227582 - Flags: first-review?(l10n)
Flags: blocking-thunderbird2?
Flags: blocking-firefox2?
Flags: blocking-firefox2? → blocking-firefox2+
Flags: blocking-thunderbird2? → blocking-thunderbird2+
Benjamin, I dropped the idea that the l10n cvs repository only contains appropriatly licensed dictionaries. That algorithm is just too brittle, thus I would like to create an all-locales file for dicts, too, so that someone who cares about licenses has to explicitly review the licensing situation and switch the dicts on on purpose.
This patch adds en-US, pl, and sk to the list. All other are just silently dropped.
Attachment #229669 - Flags: first-review?(benjamin)
Comment on attachment 229669 [details] [diff] [review]
firewall, only build dedicated subset of dictionaries

As discussed on IRC, I don't think that central control of dictionaries is a good idea. I think we should remove incompatibly licensed dictionaries, warn the committers that they violated the terms of the CVS checkin privileges, and add this to the licensing checklist for release if necessary.
Attachment #229669 - Flags: first-review?(benjamin) → first-review-
(In reply to comment #3)
> (From update of attachment 229669 [details] [diff] [review] [edit])
> As discussed on IRC, I don't think that central control of dictionaries is a
> good idea. I think we should remove incompatibly licensed dictionaries, warn
> the committers that they violated the terms of the CVS checkin privileges, and
> add this to the licensing checklist for release if necessary.

We don't do checklists without result lists in CVS anymore, and make the release process use those lists. That's what the all-locales file does. Maybe it should be called shipped-locales, like we do in the rest of the release process.

Assuming that all localizers will understand the licensing issues involved with incorporating other peoples works into CVS is naive, and won't help us. We need to review dictionary licenses, and do so on request. There is no place in the release schedule to verify that the licensing information somewhere is good for all the dictionaries in the tree.
Whiteboard: [at risk]
> We don't do checklists without result lists in CVS anymore, and make the
> release process use those lists. That's what the all-locales file does. Maybe

The "result" should be removing badly-licensed dictionaries. There is no need for an additional file to maintain.

> Assuming that all localizers will understand the licensing issues involved with
> incorporating other peoples works into CVS is naive, and won't help us. We 

This file is no different than the other 400 localizable files. If there's a licensing question, you (or gerv) should review it before checkin. That's the way all of our files are handled, and we shouldn't be treating dictionaries differently.
Enough of theory, I don't have the cycles to audit dictionary licenses, I doubt that Gerv has. Nor is after-the-fact cvs removal and account-removal-threat the messaging we want to do.

Dictionaries *are* significantly different than the rest of our 400 localizable files, in terms of contributors, in terms of upstream/downstream changes and a whole other aspects. This smells like low-hanging fruit, but it isn't.

Given that the only major locale that would benefit from doing this would be pl, I suggest that we plain WONTFIX this bug and remove l10n build foo from extensions/spellcheck/locales until we have a significant amount of compatibly licensed dictionaries that is worth the trouble.
Axel - I'm not sure I understand why dictionaries are different.  If we can't assume the standard review/approval process can properly catch badly licensed files landing and staying in cvs we have a much bigger problem...
schrep, there is no such thing as a standard review process for l10n, the tree rules are made up by the localization teams itself.
The rules for trademarks relevant files are working as part of the release process, but for nightlies, they don't. That's a problem, but I have less problems with shipping bad bookmarks in a nightly than creating a build with a insufficiently licensed dictionary.
So? How is including GPL dictionaries different from including GPL translations or code? Adding extra layers of build machinery to work around administrative problems is a technique that we have to avoid.
Attachment #227582 - Flags: first-review?(l10n) → first-review?(preed)
Comment on attachment 227582 [details] [diff] [review]
Checkout, rev. 1

Looks good.

r=preed
Attachment #227582 - Flags: first-review?(preed) → first-review+
Fixed on trunk.
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Whiteboard: [at risk] → [fixed on trunk, needs approval]
Comment on attachment 227582 [details] [diff] [review]
Checkout, rev. 1

Almost impossible to verify on trunk because very few l10ns build, but this shouldn't be risky.
Attachment #227582 - Flags: approval1.8.1?
343142 is still open, and nothing is resolved.
Blocks: 346133
I'd like to land this on branch today because 1) it's a build-config change so that bustage would show up immediately and 2) because the trunk locales don't build, it's impossible to verify that it works
Attachment #227582 - Flags: approval1.8.1? → approval1.8.1+
Nobody is signed up to take care about dictionary licensing on any timeframe, nor has schrep got back on whether that's required or not.

To clarify, I'm not worried about cvs remove here, but cvs add. And dictionaries are different in terms of licensing, as they are not modifications of the en-US sources and as such offer many more varieties in licensing problems than any other stuff we have in the l10n repository.
Fixed on MOZILLA_1_8_BRANCH
Keywords: fixed1.8.1
Whiteboard: [fixed on trunk, needs approval]
Why we can't solve it during QA before the release?
> To clarify, I'm not worried about cvs remove here, but cvs add.

Dictionaries are no different to anything else here - new code which goes into the tree has to be licence reviewed. If people don't get that done, we take it out again.

Gerv
(In reply to comment #18)
> > To clarify, I'm not worried about cvs remove here, but cvs add.
> 
> Dictionaries are no different to anything else here - new code which goes into
> the tree has to be licence reviewed. If people don't get that done, we take it
> out again.

Who does that, and when?
(In reply to comment #19)
> Who does that, and when?

I can gather the list of dictionaries and list of proves that the license is ok. If gerv can iterate through that list and decide which one is ok and which is not, we're ready, right?
> Who does that, and when?

I guess I do, at checkin time.

If we have a problem with already-checked-in dictionaries being potentially dodgy, then Gandalf's plan is the right one.

Gerv
If you test-land a patch, check the results?
This broke the build for missing extensions/spellcheck directories for be nso pt-PT xh zu. I added those now, but this is suboptimal at best. Do we really have to break all locales for a missing optional component?
Axel, I don't understand... I checked to make sure the main l10n tinderboxen were green the entire time. I just looked through the logs for Mozilla1.8-l10n-pt-PT and didn't see any red there, either.
See the build log at http://tinderbox.mozilla.org/showlog.cgi?log=Mozilla1.8-l10n/1154077344.28059.gz&fulltext=1

Bustage doesn't mean that builds go red, these are l10n tinderboxens, which don't worship error conditions. In this particular case, they just refused to check-out l10n and lived happily ever after. That's why for example the finish builds went orange.
Axel, can you file a bug on this? This should definitely turn the tree red. I remember something about "cvs" not returning an error code in this instance, but perhaps we can grep for the error message in the output.
Product: Toolkit → Toolkit Graveyard
You need to log in before you can comment on or make changes to this bug.