Comment on attachment 9086314 [details] [diff] [review] 1574835-gdata-repack-1.diff Review of attachment 9086314 [details] [diff] [review]: ----------------------------------------------------------------- Looks good. I think there are going to be some corner cases with the file parsing, but they shouldn't be too bad with this small data set. One thing that's going to happen is network glitches whether its from hg.m.o or the network itself. It looks like any kind of error just winds up skipping whatever language it's working on? There's the "redo" library under third_party/python that has the retry function that get used all over the place. You'll probably have to hack up your sys.path to get it until you can use Python 3 with "mach python". There's also a "download_url" function in "comm-task-env" that supports retries you could copy. Do we need to deal with pinned l10n changesets? I guess these strings really haven't changed in a few years. The only other thing is not writing back into the checkout directory. You can create a directory in workspace for your output file. Same goes for any temp files that get created. I don't see any, but in case I missed something... ::: calendar/providers/gdata/locale-repack.py @@ +1,1 @@ > +#!/usr/bin/python NIT: /usr/bin/python3 if you're insisting on Python 3 @@ +20,5 @@ > +from urllib.request import urlopen > +from zipfile import ZipFile, ZipInfo, ZIP_DEFLATED > + > +source = os.path.join(os.environ['MOZ_FETCHES_DIR'], 'gdata-provider.xpi') > +dest = '/builds/worker/checkouts/gecko/comm/gdata-provider.xpi' VCS checkouts are meant to be "read-only". You can make an artifacts directory in /builds/worker/workspace for output files. @@ +21,5 @@ > +from zipfile import ZipFile, ZipInfo, ZIP_DEFLATED > + > +source = os.path.join(os.environ['MOZ_FETCHES_DIR'], 'gdata-provider.xpi') > +dest = '/builds/worker/checkouts/gecko/comm/gdata-provider.xpi' > +locales_file = os.path.join(os.path.dirname(__file__), '../../../mail/locales/all-locales') os.environ['GECKO_PATH'] is set by run-task. I'd make this a parameter too since nightlies will use all-locales, but release builds should stick to shipped-locales. @@ +103,5 @@ > + try: > + for file in files: > + url = '{repo}/raw-file/tip/{source}'.format(repo=locale_repo, source=file['source']) > + > + with urlopen(url) as r: This will fail at some point. What if hg.m.o throws a 500? That will get caught by URLError and then you continue. The resulting XPI will be missing that locale? ::: taskcluster/ci/addon/kind.yml @@ +24,5 @@ > + max-run-time: 600 > + artifacts: > + - type: file > + name: public/build/gdata-provider.xpi > + path: /builds/worker/checkouts/gecko/comm/gdata-provider.xpi See note in the script; checkout dirs are meant to be read-only. @@ +32,5 @@ > + using: run-task > + comm-checkout: true > + command: > > + cd /builds/worker/checkouts/gecko/comm && > + python3 calendar/providers/gdata/locale-repack.py This is probably okay for now, but checkout directories tend to get moved and then we have bustage.