1171736 - Migrate Linux l10n desktop nightly repacks to TaskCluster

Status: RESOLVED FIXED

(Release Engineering :: General, defect)

Description

[Chris Cooper [:coop] (he/him)]

These repacks use the desktop_l10n.py mozharness script:

https://hg.mozilla.org/build/mozharness/file/13239bfb8b61/scripts/desktop_l10n.py

/tools/buildbot/bin/python scripts/scripts/desktop_l10n.py --branch-config single_locale/mozilla-central.py --platform-config single_locale/linux.py --environment-config single_locale/production.py --balrog-config balrog/production.py --total-chunks 3 --this-chunk 2

As seen in: http://buildbot-master72.bb.releng.usw2.mozilla.com:8001/builders/Firefox%20mozilla-central%20linux%20l10n%20nightly-2/builds/3

Comment 1

[Justin Wood (:Callek)]

This has been being worked on by me:

A few notes, we can't use the build.sh script directly yet, (no --scm-level and such on the desktop_l10n script), we need to pass --revision in atm (not clear yet if that will be able to stay)

I don't have a task-graph definition in tree yet for this, so the actual run is locally using a builder image at present.

Last try push of my day today, (where I'm also scheduling the newer l10n-on-try stuff):

https://treeherder.mozilla.org/#/jobs?repo=try&revision=9acd3f695790077505b4399c636a06d5392c94f4

Comment 2

[Justin Wood (:Callek)]

Forgot to mention, this is also switching from cloning compare-locales (on tc anyway) to using compare-locales from in-tree.

The latter happens everywhere, so when this is ready to land I'll likely stop cloning compare-locales to verify nothing breaks. And to use in-tree version we're doing `mach compare-locales`

Comment 3

[Justin Wood (:Callek)]

Attachment: l10n_on_try_mh

This is a feedback request (since I think I'll be off tomorrow, prior to having the patch ready for the in-tree tc scheduler...

There is a lot that can be cleaned up once we have this as a tier1, but since these same files can potentially break android l10n, I'm leaning toward caution on the changes I make...

fwiw

[root@taskcluster-worker ~]# env | grep MOZHARN
MOZHARNESS_ACTIONS=clone-locales list-locales setup repack summary
MOZHARNESS_SCRIPT=mozharness/scripts/desktop_l10n.py
MOZHARNESS_OPTIONS=environment-config=single_locale/production.py branch-config=single_locale/try.py platform-config=single_locale/linux64.py total-chunks=1 this-chunk=1
MOZHARNESS_CONFIG=single_locale/tc_linux64.py

Comment 4

[Nick Thomas [:nthomas] (UTC+12)]

Comment on attachment 8758463 [details] [diff] [review]
l10n_on_try_mh

It's a little hard to follow your intentions here with all the commented out and duplicated code. It's nice and short at least, but I don't know enough about the l10n build system to be sure this is right from code inspection - how do the logs and resulting binaries look compared to unchanged code ? What is the plan for uploading, just the tc index ?

>diff --git a/testing/mozharness/mozharness/mozilla/l10n/locales.py b/testing/mozharness/mozharness/mozilla/l10n/locales.py
...
>     def run_compare_locales(self, locale, halt_on_failure=False):
>         dirs = self.query_abs_dirs()
>+        env = self.query_bootstrap_env()
>+        #env = self.query_env(partial_env={'PYTHONPATH': dirs['abs_compare_locales_dir']}
>+        target = ["compare-locales"]
>+        #return self._mach(target=target, env=env)
>+        python = self.query_exe('python2.7')
>+        compare_locales_error_list = list(PythonErrorList)
>+        self.rmtree(dirs['abs_merge_dir'])
>+        self.mkdir_p(dirs['abs_merge_dir'])
>+        command = [python, 'mach', 'compare-locales',

Does this work with |['./mach', 'compare-locales',| ? I don't understand our 'pick a random python to call mach with, which might differ from the virtualenv which configure creates' idiom. Lets just use mach directly if we can, eg with your commented out call to self._mach.

>diff --git a/testing/mozharness/scripts/desktop_l10n.py b/testing/mozharness/scripts/desktop_l10n.py
>--- a/testing/mozharness/scripts/desktop_l10n.py
>+++ b/testing/mozharness/scripts/desktop_l10n.py
>@@ -173,6 +173,7 @@
>             'all_actions': [
>                 "clobber",
>                 "pull",
>+                "clone-locales",

I see you're doing this for consistency with other l10n scripts, or to avoid using pull because tc takes care of that. Please watch out for needing merge time changes to default_actions on m-b and higher if the code gets there without being in production.

>diff --git a/testing/taskcluster/scripts/builder/build-linux.sh b/testing/taskcluster/scripts/builder/build-l10n.sh

I won't pretend to know the ins and outs here.

>+python2.7 $WORKSPACE/build/src/testing/${MOZHARNESS_SCRIPT} \
>   --disable-mock \
>+  --revision ${GECKO_HEAD_REV} \
>   $actions \
>   $options \
>+  ${config_cmds} \    # Must come after options due to the config ordering

Could you elaborate on this comment ? Something to do with desktop_l10n taking configs in the order given, vs a proper hierarchy like en-US does ?

Comment 5

[Justin Wood (:Callek)]

Attachment: mozharness - take 2

Answers to your questions in following

Comment 6

[Justin Wood (:Callek)]

(In reply to Nick Thomas [:nthomas] from comment #4)
> Comment on attachment 8758463 [details] [diff] [review]
> l10n_on_try_mh
> 
> It's a little hard to follow your intentions here with all the commented out
> and duplicated code.

I removed most commented out code... 

> about the l10n build system to be sure this is right from code inspection -
> how do the logs and resulting binaries look compared to unchanged code ?

https://tools.taskcluster.net/task-inspector/#QlmiVHppRt6GfHv03CMbDw/0 has more details

> What is the plan for uploading, just the tc index ?

Yea, atm just the tc job artifacts, eventually the tc index (I'm doing this as tier 2 for now)

> >diff --git a/testing/mozharness/mozharness/mozilla/l10n/locales.py b/testing/mozharness/mozharness/mozilla/l10n/locales.py
> ...
> >     def run_compare_locales(self, locale, halt_on_failure=False):
> >         dirs = self.query_abs_dirs()
...
> >+        command = [python, 'mach', 'compare-locales',
> 
> Does this work with |['./mach', 'compare-locales',| ? I don't understand our
> 'pick a random python to call mach with, which might differ from the
> virtualenv which configure creates' idiom. Lets just use mach directly if we
> can, eg with your commented out call to self._mach.

I copied the 'find python' logic from how desktop_l10n calls mach to run configure, so I think this is safer than not getting the specific python.  (I didn't use desktop_l10n's helper methods for mach due to the need for error_list and the fact that the file that is doing this is imported in a few places (like android scripts).

> >diff --git a/testing/mozharness/scripts/desktop_l10n.py b/testing/mozharness/scripts/desktop_l10n.py
> >--- a/testing/mozharness/scripts/desktop_l10n.py
> >+++ b/testing/mozharness/scripts/desktop_l10n.py
> >@@ -173,6 +173,7 @@
> >             'all_actions': [
> >                 "clobber",
> >                 "pull",
> >+                "clone-locales",
> 
> I see you're doing this for consistency with other l10n scripts, or to avoid
> using pull because tc takes care of that. Please watch out for needing merge
> time changes to default_actions on m-b and higher if the code gets there
> without being in production.

I found that most desktop_l10n jobs use the full list of actions by default, nothing specifies actions as a command line elsewhere, though the configs for esr/release/beta (which I've now patched) do specify a default_actions which I've added clone-locales to.

> >diff --git a/testing/taskcluster/scripts/builder/build-linux.sh b/testing/taskcluster/scripts/builder/build-l10n.sh
> 
> I won't pretend to know the ins and outs here.

Its mainly a similar invoke to build-linux.sh (but since build-linux.sh passes args that desktop_l10n doesn't understand yet, such as scm-level, I wanted to use a new script).

The invoke path is ./bin/build.sh for the taskcluster image which is:
https://dxr.mozilla.org/mozilla-central/source/testing/docker/desktop-build/bin/build.sh
Which checks out gecko, at the rev we want, and then calls the JOB_SCRIPT which we will pass as the build-l10n.sh instead.

This then goes through some env var setup and calls the desktop_l10n.py script for us.

> 
> >+python2.7 $WORKSPACE/build/src/testing/${MOZHARNESS_SCRIPT} \
> >   --disable-mock \
> >+  --revision ${GECKO_HEAD_REV} \
> >   $actions \
> >   $options \
> >+  ${config_cmds} \    # Must come after options due to the config ordering
> 
> Could you elaborate on this comment ? Something to do with desktop_l10n
> taking configs in the order given, vs a proper hierarchy like en-US does ?

Sadly I broke my local run with that comment (since the \ was no longer escaping the line return) anyway, the issue is that options like:
   --branch-config, --balrog-config, --platform-config
All "extend" config_files on us, but since config files take order of specified precedence, and tc_linux64 is explicitly just overriding things in linux64.py's config we need that to come last.

Its mostly a hack until we have enough coverage/tier_1 of this script that it can be refactored without as much pain in terms of guessing if we broke anything.

This also implements some parts of the "use in-tree compare-locales" but I'm not yet clear on if we need the in-tree pieces to support this patch.

Comment 7

[Justin Wood (:Callek)]

(Note to nick: I noticed my latest try-push failed mh tests even after adjusting stuff, I'll work on that and provide a new diff [+ interdiff if this gets an r+] to help here.)

Pike, can you check the artifacts I have manually generated to help discern if all-is-well.

Full logs exist, and I have not yet done any verification myself (but it is late and I'd love to know where mistakes lie before weekend if possible)

https://tools.taskcluster.net/task-inspector/#QlmiVHppRt6GfHv03CMbDw/0

Comment 8

[Axel Hecht [:Pike]]

Reading through the log, it seems to be pulling the wrong build?

01:55:37     INFO -  2016-06-03 01:55:37 URL:http://archive.mozilla.org/pub/firefox/nightly/latest-mozilla-central/firefox-49.0a1.en-US.linux-x86_64.tar.bz2 [59055430/59055430] -> "firefox-49.0a1.en-US.linux-x86_64.tar.bz2" [1]

There's a call to `make ident`, which I don't see the result of?

The rest of the log doesn't show anything I can spot, I'll check the artifacts later today.

Comment 9

[Axel Hecht [:Pike]]

I inspected the builds, and they look good, also the langpacks. Didn't actually run them, as I don't have a vm up, but I inspected the content in omni.ja to match what the dashboard says, and also cross-checked that the merge dir isn't leaking from locale to locale.

Comment 10

[Justin Wood (:Callek)]

Comment on attachment 8759445 [details] [diff] [review]
mozharness - take 2

apparently nick is PTO next week, so jlund your this weeks lucky winner.

Comment 11

[Jordan Lund (:jlund)]

(In reply to Justin Wood (:Callek) from comment #10)
> Comment on attachment 8759445 [details] [diff] [review]
> mozharness - take 2
> 
> apparently nick is PTO next week, so jlund your this weeks lucky winner.

\o/

Comment 12

[Jordan Lund (:jlund)]

Comment on attachment 8759445 [details] [diff] [review]
mozharness - take 2

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

Looks great. couple nits but should be near ready to go. have you tested this in buildbot world? my main regression concern is the new mach invocation.

clearing review until nits are commented on or addressed

::: testing/mozharness/configs/single_locale/tc_linux64.py
@@ +1,5 @@
> +import os
> +
> +config = {
> +    "locales_file": "src/browser/locales/all-locales",
> +    "tools_repo": "https://hg.mozilla.org/build/tools",

is this used anywhere?

@@ +2,5 @@
> +
> +config = {
> +    "locales_file": "src/browser/locales/all-locales",
> +    "tools_repo": "https://hg.mozilla.org/build/tools",
> +    "mozconfig": "src/browser/config/mozconfigs/linux64/l10n-mozconfig",

oh, ha, so in bbot world this is "%(branch)s/browser/config/mozconfigs/linux64/l10n-mozconfig" which we never interpolate[0] so it just 'worked' before?

[0] https://dxr.mozilla.org/mozilla-central/rev/3e8ee3599a67edd971770af4982ad4b0fe77f073/testing/mozharness/scripts/desktop_l10n.py#697

::: testing/mozharness/mozharness/mozilla/l10n/locales.py
@@ +141,5 @@
>  
>      def run_compare_locales(self, locale, halt_on_failure=False):
>          dirs = self.query_abs_dirs()
> +        env = self.query_bootstrap_env()
> +        target = ["compare-locales"]

I don't think `target` is used

@@ +154,3 @@
>          self.info("*** BEGIN compare-locales %s" % locale)
> +        status = self.run_command(command,
> +                                  halt_on_failure=True,

we should either use the param here (and make the default True) or delete it if want to leave this as always True

@@ -175,4 @@
>                                                     c['mozilla_dir'])
>              dirs['abs_locales_src_dir'] = os.path.join(dirs['abs_mozilla_dir'],
>                                                         c['locales_dir'])
> -            dirs['abs_l10n_dir'] = os.path.join(dirs['abs_work_dir'],

weird. was this just duplicate code?

@@ +187,4 @@
>                                                   'merged')
>              dirs['abs_locales_dir'] = os.path.join(dirs['abs_objdir'],
>                                                     c['locales_dir'])
> +        dirs['abs_compare_locales_dir'] = os.path.join(dirs['abs_mozilla_dir'],

hm, where/how is this used now? do we need it?

Comment 13

[Justin Wood (:Callek)]

(In reply to Jordan Lund (:jlund) from comment #12)
> Comment on attachment 8759445 [details] [diff] [review]
> mozharness - take 2
> 
> Review of attachment 8759445 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Looks great. couple nits but should be near ready to go. have you tested
> this in buildbot world? my main regression concern is the new mach
> invocation.

It was tested on nick's "l10n on try" bb world, I didn't test on android-l10n repacks due to no (known) good way to test that out of production atm. I've been as careful as I can be mentally to not break that either though. (mach invoke is my one concern there too)

> clearing review until nits are commented on or addressed
> 
> ::: testing/mozharness/configs/single_locale/tc_linux64.py
> @@ +1,5 @@
> > +import os
> > +
> > +config = {
> > +    "locales_file": "src/browser/locales/all-locales",
> > +    "tools_repo": "https://hg.mozilla.org/build/tools",
> 
> is this used anywhere?
> 

Looks like not, (I'll push a new run on try today to verify though)

> @@ +2,5 @@
> > +
> > +config = {
> > +    "locales_file": "src/browser/locales/all-locales",
> > +    "tools_repo": "https://hg.mozilla.org/build/tools",
> > +    "mozconfig": "src/browser/config/mozconfigs/linux64/l10n-mozconfig",
> 
> oh, ha, so in bbot world this is
> "%(branch)s/browser/config/mozconfigs/linux64/l10n-mozconfig" which we never
> interpolate[0] so it just 'worked' before?
> 
> [0]
> https://dxr.mozilla.org/mozilla-central/rev/
> 3e8ee3599a67edd971770af4982ad4b0fe77f073/testing/mozharness/scripts/
> desktop_l10n.py#697

Well, https://dxr.mozilla.org/mozilla-central/rev/3e8ee3599a67edd971770af4982ad4b0fe77f073/testing/mozharness/scripts/desktop_l10n.py#249 has us interpolate in buildbot world.

BUT since we're using tc-vcs and the build.sh script from the Desktop Build docker image, we checkout gecko to `src` regardless of what branch, and thus being explicit here is best.


> ::: testing/mozharness/mozharness/mozilla/l10n/locales.py
> @@ +141,5 @@
> >  
> >      def run_compare_locales(self, locale, halt_on_failure=False):
> >          dirs = self.query_abs_dirs()
> > +        env = self.query_bootstrap_env()
> > +        target = ["compare-locales"]
> 
> I don't think `target` is used

indeed.

> @@ +154,3 @@
> >          self.info("*** BEGIN compare-locales %s" % locale)
> > +        status = self.run_command(command,
> > +                                  halt_on_failure=True,
> 
> we should either use the param here (and make the default True) or delete it
> if want to leave this as always True

Good catch.

> @@ -175,4 @@
> >                                                     c['mozilla_dir'])
> >              dirs['abs_locales_src_dir'] = os.path.join(dirs['abs_mozilla_dir'],
> >                                                         c['locales_dir'])
> > -            dirs['abs_l10n_dir'] = os.path.join(dirs['abs_work_dir'],
> 
> weird. was this just duplicate code?

Yea, and it actually depends on 'l10n_dir' in config which is behind an if-check above anyway.

> @@ +187,4 @@
> >                                                   'merged')
> >              dirs['abs_locales_dir'] = os.path.join(dirs['abs_objdir'],
> >                                                     c['locales_dir'])
> > +        dirs['abs_compare_locales_dir'] = os.path.join(dirs['abs_mozilla_dir'],
> 
> hm, where/how is this used now? do we need it?

I'm actually redoing this piece slightly to attend to some test failures (rather than fight with them) -- Basically its used in clobber calls, so we need a config value.

https://dxr.mozilla.org/mozilla-central/source/testing/mozharness/scripts/desktop_l10n.py#605

Alternatively I can scrub `abs_compare_locales_dir` from clobber and change tests to not expect it in cases where mozilla_dir is not set. (Which might be cleaner)

Comment 14

[Justin Wood (:Callek)]

Attachment: mozharness - take 3

Comment 15

[Justin Wood (:Callek)]

(In reply to Justin Wood (:Callek) from comment #14)
> Created attachment 8760248 [details] [diff] [review]
> mozharness - take 3

https://treeherder.mozilla.org/#/jobs?repo=try&revision=ede856efae4b4b263582da63fe6077c38c794622

(I still have to post a patch for the taskcluster graph though, but if this can land even before that is reviewed, it gives us confidence that I didn't break android/etc.)

Comment 16

[Jordan Lund (:jlund)]

Comment on attachment 8760248 [details] [diff] [review]
mozharness - take 3

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

if it passes your push, wfm :)

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/mozilla-central/rev/396b577b98e6
Migrate Linux l10n desktop nightly repacks to TaskCluster, mozharness part and some docker support. r=jlund a=KWierso

Comment 18

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

There was at least one busted android nightly build with this: https://treeherder.mozilla.org/logviewer.html#?job_id=4023559&repo=mozilla-central

Backed out https://hg.mozilla.org/mozilla-central/rev/d54e29ad6279

Comment 19

[Justin Wood (:Callek)]

Attachment: [m-c] add task defs and let it run on try!

sfink said he was willing to review.

I note the parent patch (take3) was backed out of m-c for busting android, so I need to fix that before this can land, but this was pushed to try (minus some additions to file patterns) with https://treeherder.mozilla.org/#/jobs?repo=try&revision=861714faa56e34110f7f87b3a141987892140d8f

The tests I touched can be run with ./mach taskgraph python-tests and this helps to to match up the extra-builds stuff a bit too.

I note that my discussed-on-irc issue of not being able to see these scheduled was due to no `build_type: opt`  being specified, routes.json uses it as well for the index.gecko.v2 stuff on those jobs.

Comment 20

[Steve Fink [:sfink] [:s:]]

Comment on attachment 8760537 [details] [diff] [review]
[m-c] add task defs and let it run on try!

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

This looks fine to me, but I'd rather see it factored better.

::: taskcluster/taskgraph/test/test_try_option_syntax.py
@@ +122,4 @@
>      def test_p_linux(self):
>          "-p linux sets platforms=['linux']"
>          tos = TryOptionSyntax('try: -p linux', empty_graph)
> +        self.assertEqual(tos.platforms, ['linux', 'linux-l10n'])

Update the docstring

@@ +126,5 @@
>  
>      def test_p_linux_win32(self):
>          "-p linux,win32 sets platforms=['linux', 'win32']"
>          tos = TryOptionSyntax('try: -p linux,win32', empty_graph)
> +        self.assertEqual(sorted(tos.platforms), ['linux', 'linux-l10n', 'win32'])

Update the docstring

@@ +140,5 @@
>              'sm-arm-sim',
>              'sm-arm64-sim',
>              'sm-compacting',
> +            'sm-nonunified',
> +            'sm-package',

Thanks!

::: testing/mozharness/configs/single_locale/linux32.py
@@ +1,1 @@
> +linux.py
\ No newline at end of file

Uh, what? I thought this directory contained config files. What is this?

::: testing/taskcluster/tasks/builds/firefox_l10n_base.yml
@@ +1,4 @@
> +$inherits:
> +  from: 'tasks/l10n.yml'
> +  variables:
> +    platform: 'linux64'

I don't see where l10n.yml uses {{platform}}, and it's a lie for firefox_l10n_linux32.yml anyway. Kill it.

@@ +1,5 @@
> +$inherits:
> +  from: 'tasks/l10n.yml'
> +  variables:
> +    platform: 'linux64'
> +    build_name: 'linux64-l10n'

Another lie. Omit it here.

@@ +5,5 @@
> +    build_name: 'linux64-l10n'
> +    build_product: 'firefox'
> +    build_type: 'opt'
> +task:
> +

The blank line missed. Should be before task:, not after.

@@ +46,5 @@
> +      - staging
> +    treeherder:
> +      machine:
> +        # see https://github.com/mozilla/treeherder/blob/master/ui/js/values.js
> +        platform: {{platform}}

Put task.extra.treeherder.symbol: L10n here.

::: testing/taskcluster/tasks/builds/firefox_l10n_linux32.yml
@@ +14,5 @@
> +    treeherder:
> +      groupSymbol: tc
> +      groupName: Submitted by taskcluster
> +      tier: 2
> +      symbol: L10n

This whole 'extra' section seems redundant and could be dropped.

::: testing/taskcluster/tasks/builds/firefox_l10n_linux64.yml
@@ +14,5 @@
> +    treeherder:
> +      groupSymbol: tc
> +      groupName: Submitted by taskcluster
> +      tier: 2
> +      symbol: L10n

Drop the whole extra:

::: testing/taskcluster/tasks/l10n.yml
@@ +1,4 @@
> +# This is the "base" task which contains the common values all builds must
> +# provide.
> +---
> +taskId: {{build_slugid}}

Wait... why does this file even exit? AFAICT, the only difference between this and build.yml are (1) build.yml creates some unnecessary index.gecko.v1 routes, and (2) you set the task.extra.treeherder.symbol. So why can't you just $inherit build.yml and change task.extra.treeherder.symbol? And you might as well do that in firefox_l10n_base.yml and eliminate this file entirely.

@@ +12,5 @@
> +
> +  tags:
> +    createdForUser: {{owner}}
> +
> +  workerType: b2gbuild

This is cut & paste from build.yml, where it is also bogus. It shouldn't be in either place.

Comment 21

[Justin Wood (:Callek)]

Attachment: mozharness - take 4 (and rebased)

Only substantive change is in locales.py and this fixes android l10n (afaict)

Comment 22

[Jordan Lund (:jlund)]

Comment on attachment 8761441 [details] [diff] [review]
mozharness - take 4 (and rebased)

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

::: testing/mozharness/mozharness/mozilla/l10n/locales.py
@@ +141,5 @@
>  
>      def run_compare_locales(self, locale, halt_on_failure=False):
>          dirs = self.query_abs_dirs()
> +        env = self.query_env()
> +        if hasattr(self, 'query_bootstrap_env'):

would prefer to solve this with the inheritance that is already set up than an introspection condition: `if hasattr(self, 'query_bootstrap_env')`.

can we not just replace lines 144-147 with:
`env = self.query_l10n_env()`

and define a `query_l10n_env` in mobile_1l0n that simply returns `self.query_env()` ?

Comment 23

[Justin Wood (:Callek)]

Attachment: mozharness - take 5

Comment 24

[Jordan Lund (:jlund)]

Comment on attachment 8761547 [details] [diff] [review]
mozharness - take 5

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

lgtm

::: testing/mozharness/mozharness/mozilla/l10n/locales.py
@@ +141,4 @@
>  
>      def run_compare_locales(self, locale, halt_on_failure=False):
>          dirs = self.query_abs_dirs()
> +        env = self.query_l10n_env()

I'm only reviewing this line and multi_locale_build.py lines

Comment 25

[Justin Wood (:Callek)]

Attachment: [mozharness] don't clobber compare-locales

Pike pointed this out on IRC as a potential issue, and tracing the code I noticed "always_clobber_dirs" actually forces a rmtree there, and is not what we want.

Comment 26

[Axel Hecht [:Pike]]

Comment on attachment 8761565 [details] [diff] [review]
[mozharness] don't clobber compare-locales

My suspicion is that this will be funky as long as we're still cloning compare-locales in the pull step?

https://dxr.mozilla.org/mozilla-central/search?q=compare-locales+path%3Aconfigs&redirect=false shows that in tons of places :-(

Don't understand this enough to r- or r+ either way, sadly.

Comment 27

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/mozilla-central/rev/d470904ea638
Migrate Linux l10n desktop nightly repacks to TaskCluster, mozharness part and some docker support. r=jlund a=Tomcat
https://hg.mozilla.org/mozilla-central/rev/c9564b7204d4
Stop clobbering the compare-locales dir, since we're using in-tree now. r=rail a=Tomcat

Comment 28

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/mozilla-central/rev/3ccccf8e5036
fix single locale l10n nightly bustage. r=rail over IRC. a=KWierso

Comment 29

[Justin Wood (:Callek)]

(In reply to Steve Fink [:sfink] [:s:] from comment #20)
> Comment on attachment 8760537 [details] [diff] [review]
> [m-c] add task defs and let it run on try!
> 
> Review of attachment 8760537 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> This looks fine to me, but I'd rather see it factored better.
> 
> 
> ::: testing/mozharness/configs/single_locale/linux32.py
> @@ +1,1 @@
> > +linux.py
> \ No newline at end of file
> 
> Uh, what? I thought this directory contained config files. What is this?

To articulate in bug (I said in IRC) this is hg's way to designate a symlink. While it won't work [as a symlink] on windows it does work just fine on linux and the windows *checkout* is fine


I couldn't find a good way (within good use-of-time) to cleanup the other nits and validate they didn't break things.... so that is all I changed so far (other than accounting for rebase-needs)

Try push at: 
https://treeherder.mozilla.org/#/jobs?repo=try&revision=9933d6080bce

Comment 30

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/c7ddd550008c
Add taskcluster scheduling logic (on try) for linux l10n. r=sfink

Comment 31

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Backed out because it broke gecko-decision opt:

https://hg.mozilla.org/integration/mozilla-inbound/rev/fbc8f897e016
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=30062192&repo=mozilla-inbound

[34m 0:00.48(B[m Generating full task set
[34m 0:00.48(B[m loading kind `docker-image` from `taskcluster/ci/docker-image`
[34m 0:00.60(B[m loading kind `legacy` from `taskcluster/ci/legacy`
Traceback (most recent call last):
  File "/workspace/gecko/taskcluster/mach_commands.py", line 151, in taskgraph_decision
    return taskgraph.decision.taskgraph_decision(options)
  File "/workspace/gecko/taskcluster/taskgraph/decision.py", line 68, in taskgraph_decision
    write_artifact('full-task-graph.json', tgg.full_task_graph.to_json())
  File "/workspace/gecko/taskcluster/taskgraph/generator.py", line 73, in full_task_graph
    return self._run_until('full_task_graph')
  File "/workspace/gecko/taskcluster/taskgraph/generator.py", line 191, in _run_until
    k, v = self._run.next()
  File "/workspace/gecko/taskcluster/taskgraph/generator.py", line 148, in _run
    for task in kind.load_tasks(self.parameters):
  File "/workspace/gecko/taskcluster/taskgraph/kind/legacy.py", line 307, in load_tasks
    job_graph, trigger_tests = parse_commit(message, jobs)
  File "/workspace/gecko/taskcluster/taskgraph/util/legacy_commit_parser.py", line 350, in parse_commit
    platform_builds = jobs['builds'][platform]
KeyError: 'linux-l10n'

Comment 32

[Justin Wood (:Callek)]

Attachment: [m-c] Sanity check extra-build inclusions.

Due to c#31 I needed to attempt this.

It passes `./mach taskgraph python-tests` (at least same state before/after this patch)

This is built ontop of attachment 8760537 [details] [diff] [review] from this bug.

Comment 33

[Dustin J. Mitchell [:dustin] (he/him)]

Comment on attachment 8763582 [details] [diff] [review]
[m-c] Sanity check extra-build inclusions.

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

I'm not sure what "builder" refers to in the commit message, but the patch looks fine.

Comment 34

[Justin Wood (:Callek)]

(In reply to Dustin J. Mitchell [:dustin] from comment #33)
> Comment on attachment 8763582 [details] [diff] [review]
> [m-c] Sanity check extra-build inclusions.
> 
> Review of attachment 8763582 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> I'm not sure what "builder" refers to in the commit message, but the patch
> looks fine.

using "job generation" instead of "builder generation" in commit message.

Comment 35

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/7111016caa7d
Add taskcluster scheduling logic (on try) for linux l10n. r=sfink
https://hg.mozilla.org/integration/mozilla-inbound/rev/68e9726e41eb
Don't force extra-builds to attempt job generation, if no matching job is defined for this branch. r=dustin

Comment 36

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/7111016caa7d
https://hg.mozilla.org/mozilla-central/rev/68e9726e41eb

Comment 37

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/6b6a8f7e0846
Followup to Bug 1171736 - Make linux-l10n actually Platform Linux not Linux64. r=wcosta over IRC

Comment 38

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/6b6a8f7e0846

Comment 39

[Chris Cooper [:coop] (he/him)]

Have we replaced the Linux nightlies in buildbot with those from Taskcluster then? 

This bug shouldn't be closed if that's not being tracked elsewhere.

Comment 40

[Justin Wood (:Callek)]

(In reply to Chris Cooper [:coop] from comment #39)
> Have we replaced the Linux nightlies in buildbot with those from Taskcluster
> then? 
> 
> This bug shouldn't be closed if that's not being tracked elsewhere.

I say we use Bug 1265595 for that