Chris Cooper [:coop] (he/him) Reporter
	Description • 9 years ago

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

	Justin Wood (:Callek) Assignee
	Comment 1 • 8 years ago

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

	Justin Wood (:Callek) Assignee
	Comment 2 • 8 years ago

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`

	Justin Wood (:Callek) Assignee
	Comment 3 • 8 years ago

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

	Nick Thomas [:nthomas] (UTC+12)
	Comment 4 • 8 years ago

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 ?

	Justin Wood (:Callek) Assignee
	Comment 5 • 8 years ago

Answers to your questions in following

	Justin Wood (:Callek) Assignee
	Comment 6 • 8 years ago

(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.

	Justin Wood (:Callek) Assignee
	Comment 7 • 8 years ago

(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

	Axel Hecht [:Pike]
	Comment 8 • 8 years ago

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.

	Axel Hecht [:Pike]
	Comment 9 • 8 years ago

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.

	Justin Wood (:Callek) Assignee
	Comment 10 • 8 years ago

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

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

	Jordan Lund (:jlund)
	Comment 11 • 8 years ago

(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/

	Jordan Lund (:jlund)
	Comment 12 • 8 years ago

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?

	Justin Wood (:Callek) Assignee
	Comment 13 • 8 years ago

(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)

	Justin Wood (:Callek) Assignee
	Comment 15 • 8 years ago

(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.)

	Jordan Lund (:jlund)
	Comment 16 • 8 years ago

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

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

if it passes your push, wfm :)

	Pulsebot
	Comment 17 • 8 years ago

Pushed by Callek@gmail.com:
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

	Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)
	Comment 18 • 8 years ago

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

	Justin Wood (:Callek) Assignee
	Comment 19 • 8 years ago

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.

	Steve Fink [:sfink] [:s:]
	Comment 20 • 8 years ago

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.

	Justin Wood (:Callek) Assignee
	Comment 21 • 8 years ago

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

	Jordan Lund (:jlund)
	Comment 22 • 8 years ago

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()` ?

	Jordan Lund (:jlund)
	Comment 24 • 8 years ago

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

	Justin Wood (:Callek) Assignee
	Comment 25 • 8 years ago

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.

	Axel Hecht [:Pike]
	Comment 26 • 8 years ago

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.

	Pulsebot
	Comment 27 • 8 years ago

Pushed by Callek@gmail.com:
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

	Pulsebot
	Comment 28 • 8 years ago

Pushed by Callek@gmail.com:
https://hg.mozilla.org/mozilla-central/rev/3ccccf8e5036
fix single locale l10n nightly bustage. r=rail over IRC. a=KWierso

	Justin Wood (:Callek) Assignee
	Comment 29 • 8 years ago

(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

	Pulsebot
	Comment 30 • 8 years ago

Pushed by Callek@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/c7ddd550008c
Add taskcluster scheduling logic (on try) for linux l10n. r=sfink

	Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)
	Comment 31 • 8 years ago

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'

	Justin Wood (:Callek) Assignee
	Comment 32 • 8 years ago

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.

	Dustin J. Mitchell [:dustin] (he/him)
	Comment 33 • 8 years ago

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.

	Justin Wood (:Callek) Assignee
	Comment 34 • 8 years ago

(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.

	Pulsebot
	Comment 35 • 8 years ago

Pushed by Callek@gmail.com:
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

	Carsten Book [:Tomcat]
	Comment 36 • 8 years ago
bugherder

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

	Pulsebot
	Comment 37 • 8 years ago

Pushed by Callek@gmail.com:
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

	Carsten Book [:Tomcat]
	Comment 38 • 8 years ago
bugherder

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

	Chris Cooper [:coop] (he/him) Reporter
	Comment 39 • 8 years ago

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.

	Justin Wood (:Callek) Assignee
	Comment 40 • 8 years ago

(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