Closed Bug 1333167 Opened 3 years ago Closed 3 years ago

--spsProfile doesn't work from try syntax

Categories

(Testing :: Talos, defect)

defect
Not set

Tracking

(firefox54 fixed)

RESOLVED FIXED
mozilla54
Tracking Status
firefox54 --- fixed

People

(Reporter: kmag, Assigned: wcosta)

References

(Depends on 1 open bug)

Details

Attachments

(2 files)

+++ This bug was initially created as a clone of Bug #1257570 +++

This seems to be broken again:

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

On Linux, we don't appear to be trying to collect sps profiling data at all. On Windows, we do appear to be trying to collect it, but the tests fail to run.
Could this be related to running Talos through Buildbot Bridge?
In case it gives you any clues, it also seems that --rebuild-talos no longer has any effect on Linux, but does on Windows.
interesting, I suspect :wlach is correct that we don't extend the task graph with --rebuild when using buildbotbridge.

:wcosta, can you look into this from the taskcluster side?
Flags: needinfo?(wcosta)
It feels like TC in tree doesn't support any of these options.
:jmaher, could we summarize what try options we miss?
Flags: needinfo?(wcosta) → needinfo?(jmaher)
we have:
* --rebuild[-talos] <x>
* --spsProfile
* --setenv var=<value>
* --tag <value>

those are listed in order of frequency, so we use --rebuild often, and I don't know if anyone uses --tag anymore.
Flags: needinfo?(jmaher)
Assignee: nobody → wcosta
Status: NEW → ASSIGNED
:jmaher, I ended up not implementing --no-try.
Flags: needinfo?(jmaher)
ok, the --no-retry is handled elsewhere by a pulse bot, we need to decide how we do handle auto-[no]retry and possibly there is no need for it.

thanks for implementing the rest!
Flags: needinfo?(jmaher)
Comment on attachment 8831462 [details]
Bug 1333167: Add extra try options to taskcluster.  a=jmaher

https://reviewboard.mozilla.org/r/108016/#review109492

This is awesome!

It would be great to have some documentation of `parse-commit` and the resulting `config['args']` in `taskcluster/docs/loading.rst`.  Enough that someone seeing the parameter in a `kind.yml` file can get some idea what it is, and similarly it's clear where `self.config.config['args']` comes from.

(I hate the `self.config.config` thing, but that's my fault, and not new here!)

::: taskcluster/taskgraph/generator.py:44
(Diff revision 6)
>  
>      def load_tasks(self, parameters, loaded_tasks):
>          impl_class = self._get_impl_class()
> -        return impl_class.load_tasks(self.name, self.path, self.config,
> +        config = copy.deepcopy(self.config)
> +
> +        if 'parse_commit' in self.config:

Let's use `parse-commit` (dash)
Attachment #8831462 - Flags: review?(dustin) → review+
Pushed by wcosta@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/50cf83b69046
Add extra try options to taskcluster. r=dustin a=jmaher
Backed out for breaking gecko decision task:

Push with failure: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=50cf83b690462913540effa87a27e8b5becd84a3
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=73319039&repo=autoland

[task 2017-01-31T17:13:55.130564Z] Loading tasks for kind build
[task 2017-01-31T17:13:55.130613Z] Traceback (most recent call last):
[task 2017-01-31T17:13:55.130665Z]   File "/home/worker/checkouts/gecko/taskcluster/mach_commands.py", line 165, in taskgraph_decision
[task 2017-01-31T17:13:55.130711Z]     return taskgraph.decision.taskgraph_decision(options)
[task 2017-01-31T17:13:55.131050Z]   File "/home/worker/checkouts/gecko/taskcluster/taskgraph/decision.py", line 91, in taskgraph_decision
[task 2017-01-31T17:13:55.131544Z]     full_task_json = tgg.full_task_graph.to_json()
[task 2017-01-31T17:13:55.131605Z]   File "/home/worker/checkouts/gecko/taskcluster/taskgraph/generator.py", line 113, in full_task_graph
[task 2017-01-31T17:13:55.131645Z]     return self._run_until('full_task_graph')
[task 2017-01-31T17:13:55.131905Z]   File "/home/worker/checkouts/gecko/taskcluster/taskgraph/generator.py", line 251, in _run_until
[task 2017-01-31T17:13:55.131947Z]     k, v = self._run.next()
[task 2017-01-31T17:13:55.132485Z]   File "/home/worker/checkouts/gecko/taskcluster/taskgraph/generator.py", line 189, in _run
[task 2017-01-31T17:13:55.132545Z]     new_tasks = kind.load_tasks(self.parameters, list(all_tasks.values()))
[task 2017-01-31T17:13:55.132606Z]   File "/home/worker/checkouts/gecko/taskcluster/taskgraph/generator.py", line 46, in load_tasks
[task 2017-01-31T17:13:55.132650Z]     config['args'] = parse_commit(parameters['message'])
[task 2017-01-31T17:13:55.132708Z]   File "/home/worker/checkouts/gecko/taskcluster/taskgraph/try_option_syntax.py", line 205, in parse_message
[task 2017-01-31T17:13:55.132972Z]     parts = shlex.split(escape_whitespace_in_brackets(message))
[task 2017-01-31T17:13:55.133287Z]   File "/usr/lib/python2.7/shlex.py", line 279, in split
[task 2017-01-31T17:13:55.133325Z]     return list(lex)
[task 2017-01-31T17:13:55.133373Z]   File "/usr/lib/python2.7/shlex.py", line 269, in next
[task 2017-01-31T17:13:55.133414Z]     token = self.get_token()
[task 2017-01-31T17:13:55.133457Z]   File "/usr/lib/python2.7/shlex.py", line 96, in get_token
[task 2017-01-31T17:13:55.133489Z]     raw = self.read_token()
[task 2017-01-31T17:13:55.133745Z]   File "/usr/lib/python2.7/shlex.py", line 172, in read_token
[task 2017-01-31T17:13:55.133795Z]     raise ValueError, "No closing quotation"
Flags: needinfo?(wcosta)
Backout by archaeopteryx@coole-files.de:
https://hg.mozilla.org/integration/autoland/rev/fdac8d6dee09
Backed out changeset 50cf83b69046 for breaking gecko decision task. r=backout
<facepalm>this is funny, but the line "--no-retry:" in commit message is the reason for the bustage.</facepalm>
Flags: needinfo?(wcosta)
Pushed by wcosta@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/d6648b8f36ed
Add extra try options to taskcluster. r=dustin a=jmaher
Changed commit message, hopefully this will fix the bustage.
Backed out for breaking gecko decision task:

https://hg.mozilla.org/integration/autoland/rev/c79907ca664d5f0830584b3d6a251ddc5eed83d3

Push with failure: https://hg.mozilla.org/integration/autoland/rev/d6648b8f36ed95f4f28c3697b1ba60363cc83a5e
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=73373523&repo=autoland

[task 2017-01-31T20:20:54.439800Z] Loading tasks for kind build
[task 2017-01-31T20:20:54.468044Z] Generating tasks for build android-api-15/opt
[task 2017-01-31T20:20:54.468117Z] Traceback (most recent call last):
[task 2017-01-31T20:20:54.468202Z]   File "/home/worker/checkouts/gecko/taskcluster/mach_commands.py", line 165, in taskgraph_decision
[task 2017-01-31T20:20:54.468305Z]     return taskgraph.decision.taskgraph_decision(options)
[task 2017-01-31T20:20:54.468374Z]   File "/home/worker/checkouts/gecko/taskcluster/taskgraph/decision.py", line 91, in taskgraph_decision
[task 2017-01-31T20:20:54.468424Z]     full_task_json = tgg.full_task_graph.to_json()
[task 2017-01-31T20:20:54.468486Z]   File "/home/worker/checkouts/gecko/taskcluster/taskgraph/generator.py", line 113, in full_task_graph
[task 2017-01-31T20:20:54.468534Z]     return self._run_until('full_task_graph')
[task 2017-01-31T20:20:54.468595Z]   File "/home/worker/checkouts/gecko/taskcluster/taskgraph/generator.py", line 251, in _run_until
[task 2017-01-31T20:20:54.468820Z]     k, v = self._run.next()
[task 2017-01-31T20:20:54.468870Z]   File "/home/worker/checkouts/gecko/taskcluster/taskgraph/generator.py", line 189, in _run
[task 2017-01-31T20:20:54.468943Z]     new_tasks = kind.load_tasks(self.parameters, list(all_tasks.values()))
[task 2017-01-31T20:20:54.469003Z]   File "/home/worker/checkouts/gecko/taskcluster/taskgraph/generator.py", line 51, in load_tasks
[task 2017-01-31T20:20:54.469045Z]     parameters, loaded_tasks)
[task 2017-01-31T20:20:54.469098Z]   File "/home/worker/checkouts/gecko/taskcluster/taskgraph/task/transform.py", line 76, in load_tasks
[task 2017-01-31T20:20:54.469146Z]     tasks = [cls(kind, t) for t in transforms(trans_config, inputs)]
[task 2017-01-31T20:20:54.469211Z]   File "/home/worker/checkouts/gecko/taskcluster/taskgraph/transforms/task.py", line 772, in build_task
[task 2017-01-31T20:20:54.469252Z]     for task in tasks:
[task 2017-01-31T20:20:54.469318Z]   File "/home/worker/checkouts/gecko/taskcluster/taskgraph/transforms/task.py", line 753, in add_files_changed
[task 2017-01-31T20:20:54.469373Z]     for task in tasks:
[task 2017-01-31T20:20:54.469442Z]   File "/home/worker/checkouts/gecko/taskcluster/taskgraph/transforms/task.py", line 723, in add_index_routes
[task 2017-01-31T20:20:54.469475Z]     for task in tasks:
[task 2017-01-31T20:20:54.469538Z]   File "/home/worker/checkouts/gecko/taskcluster/taskgraph/transforms/task.py", line 624, in validate
[task 2017-01-31T20:20:54.469571Z]     for task in tasks:
[task 2017-01-31T20:20:54.469630Z]   File "/home/worker/checkouts/gecko/taskcluster/taskgraph/transforms/job/__init__.py", line 124, in make_task_description
[task 2017-01-31T20:20:54.469664Z]     for job in jobs:
[task 2017-01-31T20:20:54.469729Z]   File "/home/worker/checkouts/gecko/taskcluster/taskgraph/transforms/job/__init__.py", line 113, in handle_keyed_by
[task 2017-01-31T20:20:54.469771Z]     for job in jobs:
[task 2017-01-31T20:20:54.469841Z]   File "/home/worker/checkouts/gecko/taskcluster/taskgraph/transforms/job/__init__.py", line 91, in expand_platforms
[task 2017-01-31T20:20:54.469963Z]     for job in jobs:
[task 2017-01-31T20:20:54.470016Z]   File "/home/worker/checkouts/gecko/taskcluster/taskgraph/transforms/job/__init__.py", line 84, in validate
[task 2017-01-31T20:20:54.470048Z]     for job in jobs:
[task 2017-01-31T20:20:54.470175Z]   File "/home/worker/checkouts/gecko/taskcluster/taskgraph/transforms/build_attrs.py", line 19, in set_build_attributes
[task 2017-01-31T20:20:54.470229Z]     for job in jobs:
[task 2017-01-31T20:20:54.470305Z]   File "/home/worker/checkouts/gecko/taskcluster/taskgraph/transforms/build.py", line 40, in set_env
[task 2017-01-31T20:20:54.470359Z]     env = config.config['args'].env
[task 2017-01-31T20:20:54.470398Z] AttributeError: 'NoneType' object has no attribute 'env'
Flags: needinfo?(wcosta)
Ok, pushed a fix for it.
Flags: needinfo?(wcosta)
Comment on attachment 8832225 [details]
Bug 1333167: Handle empty try message.

https://reviewboard.mozilla.org/r/108570/#review110032

::: taskcluster/taskgraph/try_option_syntax.py:218
(Diff revision 3)
> +
> +
> +def parse_message(message):
> +    try_idx, parts = find_try_idx(message)
>      if try_idx is None:
> -        return None
> +        try_idx = 0

This doesn't seem right -- if "try:" doesn't appear anywhere, we shouldn't parse the comment it at all, rather than starting at the beginning of the string.  The existing implementation creates a TryOptionSyntax with jobs = [], platforms = [], etc., which will end up running nothing.

The patch hunk in TryHoptionSyntax will make sure those values get set correctly in this situation.  So the issue here is that you need a parsed `args` with the right attributes for all of the other command-line uses.  Perhaps just setting `try_idx, parts = 0, []` in this case would do the trick?
Attachment #8832225 - Flags: review?(dustin) → review-
Comment on attachment 8832225 [details]
Bug 1333167: Handle empty try message.

https://reviewboard.mozilla.org/r/108570/#review110104

::: taskcluster/taskgraph/try_option_syntax.py:235
(Diff revision 4)
> +
> +
> +def parse_message(message):
> +    try_idx, parts = find_try_idx(message)
>      if try_idx is None:
> -        return None
> +        return FakeArgs()

This seems like a lot of work, and could potentially fall out of sync.  Does `parseer.parse_known_args([])` not give a result similar to `FakeArgs()`?
Attachment #8832225 - Flags: review?(dustin) → review+
Comment on attachment 8831462 [details]
Bug 1333167: Add extra try options to taskcluster.  a=jmaher

https://reviewboard.mozilla.org/r/108016/#review110106
Comment on attachment 8832225 [details]
Bug 1333167: Handle empty try message.

https://reviewboard.mozilla.org/r/108570/#review110104

> This seems like a lot of work, and could potentially fall out of sync.  Does `parseer.parse_known_args([])` not give a result similar to `FakeArgs()`?

<goofy>you are right, fixing it!</goofy>
Pushed by wcosta@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/ffd3ef23726e
Add extra try options to taskcluster. r=dustin a=jmaher
https://hg.mozilla.org/integration/autoland/rev/c34193caeeda
Handle empty try message. r=dustin
https://hg.mozilla.org/mozilla-central/rev/ffd3ef23726e
https://hg.mozilla.org/mozilla-central/rev/c34193caeeda
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
Blocks: 1354964
You need to log in before you can comment on or make changes to this bug.