Closed
Bug 1213011
Opened 6 years ago
Closed 6 years ago
Add a route for simulator post-build task
Categories
(Firefox Build System :: Task Configuration, task)
Firefox Build System
Task Configuration
Tracking
(Not tracked)
RESOLVED
FIXED
mozilla44
People
(Reporter: ochameau, Assigned: ochameau)
References
Details
Attachments
(3 files, 6 obsolete files)
821 bytes,
patch
|
garndt
:
review+
|
Details | Diff | Splinter Review |
1.03 KB,
patch
|
ochameau
:
review+
|
Details | Diff | Splinter Review |
4.71 KB,
patch
|
garndt
:
review+
|
Details | Diff | Splinter Review |
That, to easily refer to lastest nighly build of it.
Assignee | ||
Comment 1•6 years ago
|
||
I matched the route name of mulet task. I wish I could use a variable to describe the platform, like {{platform}} or something alike that would be linux32,linux64,macos,win32,... As this post-build task, may, in future be applied to more than just linux64 mulet build. Also, is there anything I can do locally to test such change? Like ./mach graph or run some test suite?
Attachment #8671551 -
Flags: review?(garndt)
Assignee | ||
Comment 2•6 years ago
|
||
I imagine I also need that to offer a link to "latest" xpi via index.taskcluster.net. Before that patch, the filename contains the build date and so is hard to predict. With this patch the filename will be always the same. Greg, Is that fine, if from now on, I ask review to :jryans for the build-simulator.sh who help me maintaining this simulator?
Attachment #8671556 -
Flags: review?(garndt)
Comment 3•6 years ago
|
||
Comment on attachment 8671551 [details] [diff] [review] patch v1 Review of attachment 8671551 [details] [diff] [review]: ----------------------------------------------------------------- So, routes could be whatever you want them to be, but I think the routes that are within mulet are old style routes....they have been since replaced by those found in tasks/build.yml . You can run the `mach taskcluster-graph` command (need to pass in some arguments to it) and that will spit out some JSON that you can verify. Or you could just push to try and have it kick off and see how it looks for you in the index browser. I would recommend starting off with index routes with like "index.public.garbage.ochameu..." so you don't pollute any other namespaces. If this is the route you actually want, then r+
Attachment #8671551 -
Flags: review?(garndt) → review+
Comment 4•6 years ago
|
||
Comment on attachment 8671556 [details] [diff] [review] Use stable file name for simulator xpi in TC. Review of attachment 8671556 [details] [diff] [review]: ----------------------------------------------------------------- Definitely nice to have a artifact name that remains consistent rather than one with version numbers.
Attachment #8671556 -
Flags: review?(garndt) → review+
Comment on attachment 8671551 [details] [diff] [review] patch v1 Review of attachment 8671551 [details] [diff] [review]: ----------------------------------------------------------------- ::: testing/taskcluster/tasks/post-builds/mulet_simulator.yml @@ +17,5 @@ > provisionerId: aws-provisioner-v1 > schedulerId: task-graph-scheduler > > + routes: > + - 'index.buildbot.branches.{{project}}.linux64-simulator' Why do we want this route when we're not using buildbot here? The Mulet builds are under index.gecko.v2.*, shouldn't this be under the same tree?
Assignee | ||
Comment 6•6 years ago
|
||
index.gecko.v2.* is handled automagically and doesn't support post-build tasks from what I understood. Whereas route specified here are manual thing, this is kind of workaround. TBH I don't really care about the name as soon as we can have a link to latest build. I just copy pasted the route of mulet build, which as you highlight looks wrong by itself. I think taskcluster team is aware of this and just use v2 and automatic routes instead.
Comment 7•6 years ago
|
||
For now it seems that at least conforming to the v1 format would be good. https://dxr.mozilla.org/mozilla-central/source/testing/taskcluster/tasks/build.yml#21
Assignee | ||
Comment 8•6 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=7586c3fd4f48
Assignee | ||
Comment 9•6 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=d61a7a954c06
Assignee | ||
Comment 10•6 years ago
|
||
Now uses `index.gecko.v1.{{project}}.latest.simulator.{{build_type}}` as route. But I need a tweak to mach_command.py to expose build_type to post-build yml files!
Attachment #8671551 -
Attachment is obsolete: true
Attachment #8672604 -
Flags: review+
Assignee | ||
Comment 11•6 years ago
|
||
This is the additional patch I would need for this route, using build_type. Today, we are exposing build_{name,type,product} only to `routes` field. That is misleading as you could easily thing it is also exposed in `command` or other YML fields! With this patch, these variables are now exposed to the whole YML, as well as the post-build task. But I see some differences in the routes with this patch, that I can't explain. For example, without this patch we get this route: queue:route:index.gecko.v1.mozilla-central.latest.linux.linux32.opt whereas we get this with the new patch: queue:route:index.gecko.v1.mozilla-central.latest.linux.linux.opt
Attachment #8672607 -
Flags: review?(garndt)
Assignee | ||
Comment 12•6 years ago
|
||
With all these patches, I can see the latest task easily via url like this: https://tools.taskcluster.net/index/#gecko.v1.try.latest.simulator/gecko.v1.try.latest.simulator.opt And even get the xpi like this \o/ https://index.taskcluster.net/v1/task/gecko.v1.try.latest.simulator.opt/artifacts/public/build/fxos-simulator-2.5.20151012130140-linux64.xpi (I forgot to push the build-simulator.sh patch on try to get a unique xpi name)
Comment 13•6 years ago
|
||
Comment on attachment 8672607 [details] [diff] [review] Allow post-build tasks to use build_{name,type,product} variables Review of attachment 8672607 [details] [diff] [review]: ----------------------------------------------------------------- Definitely need to troubleshoot why the routes changed before and after patch. We can't have any mismatch in the routes that were being generated as these are heavily relied upon to be stable. r- for now until we can verify what is causing the routes to be changed. Routes before and after should be compared and resolved, and also probably should just link to a try push running everything just to make sure nothing is obviously broken. ::: testing/taskcluster/mach_commands.py @@ +114,5 @@ > :param dict task: task definition. > :param json_routes: the list of routes to use from routes.json > :param parameters: dictionary of parameters to use in route templates > """ > fmt = parameters.copy() Do we need to copy the parameters anymore if we are not updating them?
Attachment #8672607 -
Flags: review?(garndt) → review-
Assignee | ||
Comment 14•6 years ago
|
||
So I figured out why the routes were different. I think there was something wrong with decorate_task_json_routes. It used to pull build_name/build_type from `build` (itself comes from job_flags.yml). From the jobs flags 32bits linux is labelled "linux" (see testing/taskcluster/tasks/branches/base_jobs.yml), whereas the build_name used in yml files is "linux32" (see testing/taskcluster/tasks/builds/linux32_clobber.yml). I think we should have only one source of truth and use the variables set in build ymls. (And may be ensure that job flags yml uses the same names?) In order to do that, we have to somehow pull these build_name/build_type attributes out of build ymls. This patch allows to do that. The only side effect I see is that build_name/build_type/build_product attributes now appear in tasckcluster-graph. On build objects that now have taskId, task, build_name, build_product and build_type attributes.
Attachment #8673105 -
Flags: review?(garndt)
Assignee | ||
Comment 15•6 years ago
|
||
With the previous patch, this one now doesn't introduce any different behavior in tasckcluster-graph, except for post-build tasks, that now have access to all these variables.
Attachment #8672607 -
Attachment is obsolete: true
Attachment #8673106 -
Flags: review?(garndt)
Comment 16•6 years ago
|
||
so with these two patches I am unable to generate a graph like I would have done before (and like how the decision task does it). Also, the tests for test_templates.py are now failing. $ mach taskcluster-graph --project try --message "try: -b do -p all -u all" --head-repository http://hg.mozilla.org/integration/b2g-inbound --head-rev tip --owner garndt@mozilla.com Error running mach: ['taskcluster-graph', '--project', 'try', '--message', 'try: -b do -p all -u all', '--head-repository', 'http://hg.mozilla.org/integration/b2g-inbound', '--head-rev', 'tip', '--owner', 'garndt@mozilla.com'] The error occurred in the implementation of the invoked mach command. This should never occur and is likely a bug in the implementation of that command. Consider filing a bug for this issue. If filing a bug, please include the full output of mach, including this error message. The details of the failure are as follows: KeyError: 'build_name' File "/Users/mozilla/work/test_gecko/testing/taskcluster/mach_commands.py", line 371, in create_graph build_parameters['build_name'] = build_task['build_name'] -------------------------------------------- $ mach python-test tests/test_templates.py 0:00.37 /Users/mozilla/work/test_gecko/obj-x86_64-apple-darwin13.4.0/_virtualenv/bin/python ./tests/test_templates.py 0:00.61 TEST-PASS | ./tests/test_templates.py | test_deep_inheritance 0:00.62 TEST-UNEXPECTED-FAIL | ./tests/test_templates.py | line 44, test_inheritance: {'content': 'content', 'variable': 'inherit', 'woot': 'inherit'} != {'content': 'content', 'variable': 'inherit'} 0:00.62 - {'content': 'content', 'variable': 'inherit', 'woot': 'inherit'} 0:00.62 ? ------------------- 0:00.62 0:00.62 + {'content': 'content', 'variable': 'inherit'} 0:00.62 TEST-PASS | ./tests/test_templates.py | test_inheritance_circular 0:00.63 TEST-UNEXPECTED-FAIL | ./tests/test_templates.py | line 55, test_inheritance_implicat_pass: {'a': 'a', 'c': 'c', 'b': 'b', 'values': ['overriden', 'b', 'c']} != {'values': ['overriden', 'b', 'c']} 0:00.63 - {'a': 'a', 'b': 'b', 'c': 'c', 'values': ['overriden', 'b', 'c']} 0:00.63 + {'values': ['overriden', 'b', 'c']} 0:00.63 TEST-PASS | ./tests/test_templates.py | test_inheritance_with_simple_extensions 0:00.63 TEST-PASS | ./tests/test_templates.py | test_invalid_path 0:00.63 TEST-PASS | ./tests/test_templates.py | test_no_templates 0:00.63 TEST-PASS | ./tests/test_templates.py | test_with_templates
Comment 17•6 years ago
|
||
Comment on attachment 8673106 [details] [diff] [review] Allow post-build tasks to use build_{name,type,product} variables v2 Review of attachment 8673106 [details] [diff] [review]: ----------------------------------------------------------------- seems that a graph can't be generated now and tests fail
Attachment #8673106 -
Flags: review?(garndt) → review-
Assignee | ||
Comment 18•6 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=0a5dccbf2f50
Assignee | ||
Comment 19•6 years ago
|
||
New patch, still good regarding just the simulators: https://tools.taskcluster.net/task-graph-inspector/#NgVnjyusSnGshHOopkVrlQ/W-UL5_bLRimtrCkGIiNg6g/0 Is there any specific try run I should spawn or do I need to do all/all ?!
Assignee | ||
Updated•6 years ago
|
Assignee: nobody → poirot.alex
Assignee | ||
Comment 20•6 years ago
|
||
I haven't been able to run testing/taskcluster/tests/test_templates.py. mach python-test fails with python exception, not being able to find TC modules... I don't know if that's the right behavior... One alternative could be to set build_name/build_type from the inherited variables in build.yml and build_phone.yml.
Attachment #8673105 -
Attachment is obsolete: true
Attachment #8673105 -
Flags: review?(garndt)
Attachment #8676921 -
Flags: review?(garndt)
Assignee | ||
Comment 21•6 years ago
|
||
Dolphin builds ends up with undefined build_name/build_type, so the routes set by phone_build.yml don't work. Isn't that an issue? In any case I made it so that we would accept a build stack without these attributes.
Attachment #8673106 -
Attachment is obsolete: true
Attachment #8676923 -
Flags: review?(garndt)
Comment 22•6 years ago
|
||
I was looking over this and it seems that now we have things in the graph we didn't have before and are not valid properties for the graph... build_name, build_product, and build_type now show up in the final graph when these are just variables used for rendering the templates.
Comment 23•6 years ago
|
||
> Dolphin builds ends up with undefined build_name/build_type,
> so the routes set by phone_build.yml don't work.
> Isn't that an issue?
That does seem like an issue, perhaps :wcosta knows more.
Wander, it appears in the graph as:
"index.gecko.v1.try.revision.linux.tip.."
Flags: needinfo?(wcosta)
Comment 24•6 years ago
|
||
(In reply to Greg Arndt [:garndt] from comment #23) > > Dolphin builds ends up with undefined build_name/build_type, > > so the routes set by phone_build.yml don't work. > > Isn't that an issue? > > That does seem like an issue, perhaps :wcosta knows more. > > Wander, it appears in the graph as: > "index.gecko.v1.try.revision.linux.tip.." dolphin tasks don't setup these variables, because of that they are undefined. This is problem a mistake made by the original task author that passed by the reviewer's eyes.
Flags: needinfo?(wcosta)
Assignee | ||
Comment 25•6 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/b842ef4420f1b247ab9706ce8d5028c8bca52ecd Bug 1213011 - Use stable file name for simulator xpi in TC. r=garndt
Assignee | ||
Updated•6 years ago
|
Keywords: leave-open
Assignee | ||
Comment 26•6 years ago
|
||
Ok, then here is another approach, already suggested in comment 20. Instead of hacking into template behavior, I define build_type/build_name like build_product, from build.yml and phone_build.yml. Then I can easily fetch that from mach_commands.py. I keep throwing patches at you, but it could be more efficient if you provide more feedback and sometimes highlights what I should or could be implementing instead of only highlighting what artifacts cause my patches.
Attachment #8679003 -
Flags: review?(garndt)
Assignee | ||
Updated•6 years ago
|
Attachment #8676921 -
Attachment is obsolete: true
Attachment #8676923 -
Attachment is obsolete: true
Attachment #8676921 -
Flags: review?(garndt)
Attachment #8676923 -
Flags: review?(garndt)
Comment 28•6 years ago
|
||
(In reply to Alexandre Poirot [:ochameau] from comment #26) > Created attachment 8679003 [details] [diff] [review] > Allow post-build tasks to use build_{name,type,product} variables v4 > > Ok, then here is another approach, already suggested in comment 20. > Instead of hacking into template behavior, > I define build_type/build_name like build_product, from build.yml and > phone_build.yml. > Then I can easily fetch that from mach_commands.py. > > I keep throwing patches at you, but it could be more efficient > if you provide more feedback and sometimes highlights what > I should or could be implementing instead of only highlighting what artifacts > cause my patches. That's definitely fair feedback, and I certainly didn't mean to cause an issue so I apologize. I will be taking a look at this patch in the morning.
Comment 29•6 years ago
|
||
Comment on attachment 8679003 [details] [diff] [review] Allow post-build tasks to use build_{name,type,product} variables v4 Reviewed the before and after output and verified only the route for simulator was changed (based on the previous patch that added the route to it). Route now contains the 'opt' build_type
Attachment #8679003 -
Flags: review?(garndt) → review+
Assignee | ||
Comment 30•6 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/778030b1b88b7857d296eff8d63850c701c35139 Bug 1213011 - Allow post-build tasks to use build_{name,type,product} variables. r=garndt https://hg.mozilla.org/integration/fx-team/rev/d41522b5aed48941893f47d57c54e9cab6e5b5d1 Bug 1213011 - Add a route to post-build simulator task to easily download it. r=garndt
Assignee | ||
Updated•6 years ago
|
Keywords: leave-open
Comment 31•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/778030b1b88b https://hg.mozilla.org/mozilla-central/rev/d41522b5aed4
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla44
Updated•3 years ago
|
Product: TaskCluster → Firefox Build System
You need to log in
before you can comment on or make changes to this bug.
Description
•