Closed
Bug 1248713
Opened 9 years ago
Closed 2 years ago
Add a docker container for the l10n repack process
Categories
(Release Engineering :: General, defect)
Release Engineering
General
Tracking
(firefox47 fixed)
RESOLVED
FIXED
Tracking | Status | |
---|---|---|
firefox47 | --- | fixed |
People
(Reporter: vjoshi, Assigned: vjoshi)
References
Details
Attachments
(2 files, 5 obsolete files)
6.14 KB,
patch
|
catlee
:
review+
|
Details | Diff | Splinter Review |
2.30 KB,
patch
|
Details | Diff | Splinter Review |
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•9 years ago
|
||
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)
Comment 2•9 years ago
|
||
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)
Updated•9 years ago
|
Assignee: nobody → varunj.1011
Assignee | ||
Comment 3•9 years ago
|
||
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•9 years 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•9 years ago
|
||
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•9 years ago
|
||
Includes changes to desktop-build too
Attachment #8725399 -
Flags: review?(rail)
Attachment #8725399 -
Flags: review?(catlee)
Assignee | ||
Updated•9 years ago
|
Attachment #8725398 -
Attachment is obsolete: true
Comment 7•9 years ago
|
||
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 8•9 years ago
|
||
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•9 years 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?
Comment 10•9 years ago
|
||
(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.
Updated•9 years ago
|
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Assignee | ||
Comment 11•9 years ago
|
||
Removed patching of mozilla-central to disable mock
Attachment #8725399 -
Attachment is obsolete: true
Comment 12•9 years ago
|
||
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+
Comment 13•9 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/26fe286d51cd1282d310d0452c177373c9a439eb
Bug 1248713 - add docker container for l10n repacks r=catlee DONTBUILD
Comment 14•9 years ago
|
||
bugherder |
Comment 15•9 years ago
|
||
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•9 years 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.
Comment 17•9 years ago
|
||
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•9 years ago
|
||
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•9 years ago
|
||
Sorry! Forgot to add files again!
Attachment #8749381 -
Attachment is obsolete: true
Updated•7 years ago
|
Component: General Automation → General
Comment 20•2 years ago
|
||
This is long since done.
Status: REOPENED → RESOLVED
Closed: 9 years ago → 2 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•