Add a docker container for the l10n repack process

REOPENED
Assigned to

Status

Release Engineering
General Automation
REOPENED
a year ago
a year ago

People

(Reporter: vjoshi, Assigned: vjoshi)

Tracking

(Blocks: 1 bug)

unspecified
Dependency tree / graph

Firefox Tracking Flags

(firefox47 fixed)

Details

Attachments

(2 attachments, 5 obsolete attachments)

(Assignee)

Description

a year ago
User Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/48.0.2564.97 Safari/537.36

Steps to reproduce:

Containerize the l10n repack process carried out by mozharness/scripts/desktop_l10n.py to make it easier to contribute to. This image will be similar to the desktop-build image.
(Assignee)

Comment 1

a year ago
Created attachment 8722593 [details] [diff] [review]
Desktop l10n repack docker image

Adds an l10n repack process image under testing/docker.

This image expects /home/worker/.tc-vcs, /home/worker/tooltool-cache and /home/worker/workspace to be mounted as volumes (like the desktop-build taskcluster image), so that reuse of downloaded repositories is possible. There might be issues with permissions on the cache after interrupted builds that need to be investigated.
Attachment #8722593 - Flags: review?(rail)
Attachment #8722593 - Flags: review?(catlee)
Blocks: 1250626
Comment on attachment 8722593 [details] [diff] [review]
Desktop l10n repack docker image

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

Overall this looks really great, thanks for working on this!

Let's wait and see if Rail catches anything and then figure out next steps.

::: testing/docker/desktop-l10n/Dockerfile
@@ +1,4 @@
> +FROM		taskcluster/centos6-build-upd:0.1.3.20160122142300
> +MAINTAINER	Varun Joshi <varunj.1011@gmail.com>
> +
> +ADD		l10n.sh /home/worker/l10n.sh

should l10n.sh go into bin/ as well?

@@ +5,5 @@
> +ADD		remove_mock.diff /home/worker/remove_mock.diff
> +ADD             bin /home/worker/bin
> +RUN             chmod +x /home/worker/bin/* l10n.sh
> +
> +RUN dbus-uuidgen --ensure=/var/lib/dbus/machine-id

what does this do?

::: testing/docker/desktop-l10n/bin/build.sh
@@ +27,5 @@
> +ln -s /home/worker/workspace/build/src/ /home/worker/workspace/build/mozilla-central
> +
> +pushd workspace/build/src
> +hg import --no-commit ~/remove_mock.diff
> +popd

ideally we can figure out a better way of disabling mock via the commandline instead of patching it out of the configs here.

::: testing/docker/desktop-l10n/bin/checkout-script.sh
@@ +13,5 @@
> +
> +# download script from the gecko repository
> +url=${GECKO_HEAD_REPOSITORY}/raw-file/${GECKO_HEAD_REV}/${SCRIPT_PATH}
> +wget --directory-prefix=${SCRIPT_DOWNLOAD_PATH} $url
> +chmod +x `basename ${SCRIPT_PATH}`

is this file used?

::: testing/docker/desktop-l10n/bin/checkout-sources.sh
@@ +4,5 @@
> +
> +# Inputs, with defaults
> +
> +# mozharness builds use three repositories: gecko (source), mozharness (build
> +# scripts) and tools (miscellaneous) for each, specify *_REPOSITORY.  If the

the mozharness build scripts are actually part of gecko itself, so we should only require two repositories.

::: testing/docker/desktop-l10n/l10n.sh
@@ +23,5 @@
> +if ! [ -f /builds/tooltool.py ]
> +then
> +	git clone https://github.com/mozilla/build-tooltool
> +	cp build-tooltool/tooltool.py /builds
> +fi 

should this be moved up into checkout-sources perhaps?
Attachment #8722593 - Flags: review?(catlee)
Assignee: nobody → varunj.1011
(Assignee)

Comment 3

a year ago
Created attachment 8723755 [details] [diff] [review]
Desktop l10n repack docker image

Moved things around a bit, made tc-vcs use the destination workspace/build/mozilla-central instead of workspace/build/src. Also added a .hgrc to fix permissions issues that cropped up sometimes.
Attachment #8722593 - Attachment is obsolete: true
Attachment #8722593 - Flags: review?(rail)
Attachment #8723755 - Flags: review?(rail)
Attachment #8723755 - Flags: review?(catlee)
(Assignee)

Comment 4

a year ago
(In reply to Chris AtLee [:catlee] from comment #2)
> 
> @@ +5,5 @@
> > +ADD		remove_mock.diff /home/worker/remove_mock.diff
> > +ADD             bin /home/worker/bin
> > +RUN             chmod +x /home/worker/bin/* l10n.sh
> > +
> > +RUN dbus-uuidgen --ensure=/var/lib/dbus/machine-id
> 
> what does this do?
> 

this was done in the desktop-build taskcluster image to generate a unique machine id for each worker (I think?), so I thought it might be needed here too.

> ::: testing/docker/desktop-l10n/bin/build.sh
> @@ +27,5 @@
> > +ln -s /home/worker/workspace/build/src/ /home/worker/workspace/build/mozilla-central
> > +
> > +pushd workspace/build/src
> > +hg import --no-commit ~/remove_mock.diff
> > +popd
> 
> ideally we can figure out a better way of disabling mock via the commandline
> instead of patching it out of the configs here.
> 

I have patched desktop_l10n.py to support the '--disable-mock' argument, but we still need to patch m-c. What else could we do until the patch with the changes lands?

> ::: testing/docker/desktop-l10n/bin/checkout-script.sh
> @@ +13,5 @@
> > +
> > +# download script from the gecko repository
> > +url=${GECKO_HEAD_REPOSITORY}/raw-file/${GECKO_HEAD_REV}/${SCRIPT_PATH}
> > +wget --directory-prefix=${SCRIPT_DOWNLOAD_PATH} $url
> > +chmod +x `basename ${SCRIPT_PATH}`
> 
> is this file used?
> 
> ::: testing/docker/desktop-l10n/bin/checkout-sources.sh
> @@ +4,5 @@
> > +
> > +# Inputs, with defaults
> > +
> > +# mozharness builds use three repositories: gecko (source), mozharness (build
> > +# scripts) and tools (miscellaneous) for each, specify *_REPOSITORY.  If the
> 
> the mozharness build scripts are actually part of gecko itself, so we should
> only require two repositories.
> 

I had mostly reused the code from the desktop-build image, and I didn't change it for fear of breaking something. Do you think I should try and remove the mozharness build script repository?
 
I have made the other changes you requested in my latest patch.
(Assignee)

Comment 5

a year ago
Created attachment 8725398 [details] [diff] [review]
Desktop l10n repack docker image

Removes some discrepancies in the comments in checkout-sources.sh for both desktop-l10n and desktop-build
Attachment #8723755 - Attachment is obsolete: true
Attachment #8723755 - Flags: review?(rail)
Attachment #8723755 - Flags: review?(catlee)
(Assignee)

Comment 6

a year ago
Created attachment 8725399 [details] [diff] [review]
Desktop l10n repack docker image

Includes changes to desktop-build too
Attachment #8725399 - Flags: review?(rail)
Attachment #8725399 - Flags: review?(catlee)
(Assignee)

Updated

a year ago
Attachment #8725398 - Attachment is obsolete: true
Comment on attachment 8725399 [details] [diff] [review]
Desktop l10n repack docker image

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

awesome, thanks!
Attachment #8725399 - Flags: review?(catlee) → review+
Comment on attachment 8725399 [details] [diff] [review]
Desktop l10n repack docker image

I trust catlee! :)

Also, LGTM.
Attachment #8725399 - Flags: review?(rail) → review+
(Assignee)

Comment 9

a year ago
Won't we need to change just patching mozilla-central to disable mock? Should I file another bug adding --disable-mock to desktop-l10n.py?
(In reply to Varun Joshi from comment #9)
> Won't we need to change just patching mozilla-central to disable mock?
> Should I file another bug adding --disable-mock to desktop-l10n.py?

Sure thing. I can review that patch.
(Assignee)

Updated

a year ago
Depends on: 1252744
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
(Assignee)

Comment 11

a year ago
Created attachment 8726344 [details] [diff] [review]
Desktop l10n repack docker image

Removed patching of mozilla-central to disable mock
Attachment #8725399 - Attachment is obsolete: true
Comment on attachment 8726344 [details] [diff] [review]
Desktop l10n repack docker image

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

thanks again! I'll land this shortly
Attachment #8726344 - Flags: review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/26fe286d51cd1282d310d0452c177373c9a439eb
Bug 1248713 - add docker container for l10n repacks r=catlee DONTBUILD

Comment 14

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/26fe286d51cd
Status: ASSIGNED → RESOLVED
Last Resolved: a year ago
status-firefox47: --- → fixed
Resolution: --- → FIXED
Was a l10n.sh supposed to land as part of that ? It's referenced at https://hg.mozilla.org/mozilla-central/rev/26fe286d51cd#l3.30. Might have just missed doing 'hg add' for it ?
(Assignee)

Comment 16

a year ago
(In reply to Nick Thomas [:nthomas] from comment #15)
> Was a l10n.sh supposed to land as part of that ? It's referenced at
> https://hg.mozilla.org/mozilla-central/rev/26fe286d51cd#l3.30. Might have
> just missed doing 'hg add' for it ?

Oh yes! I must have forgotten to add it. I'll need to recreate the file since I lost all my patches due to an issue with my hard drive.
D'oh! Perhaps attachement 8722593 gets you most of the way. Lets reopen this bug for the recreation.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Assignee)

Comment 18

a year ago
Created attachment 8749381 [details] [diff] [review]
Adds docker image for desktop l10n

So I tried to recreate the files I lost due to my bust hard drive, but I won't be able to do it quickly enough. Here's the status for now: 
https://pastebin.mozilla.org/8869750
This is the error that I'm getting, it's something I am not familiar with, maybe it is because of changes to desktop_l10n.py or one of the configs. The entrypoint to the process is bin/build.sh, that does some cleaning and calls checkout-sources.sh, which  checks out the sources or loads them from the cache. l10.sh then sets up some of the environment and calls desktop_l10n.py.
(Assignee)

Comment 19

a year ago
Created attachment 8749383 [details] [diff] [review]
Adds docker image for desktop l10n

Sorry! Forgot to add files again!
Attachment #8749381 - Attachment is obsolete: true
You need to log in before you can comment on or make changes to this bug.