Closed Bug 1359328 Opened 7 years ago Closed 7 years ago

Create --enable-fuzzing TC job for firefox

Categories

(Firefox Build System :: Task Configuration, task)

task
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
mozilla55

People

(Reporter: rforbes, Assigned: aobreja)

Details

Attachments

(5 files, 3 obsolete files)

We need a taskcluster job for a ASAN build with --enable-fuzzing set for fuzzing. Ultimately, this should be a tier 2 build.

Here is the bug for the jsshell as a reference.

https://bugzilla.mozilla.org/show_bug.cgi?id=1346016
Assignee: nobody → aselagea
Assignee: aselagea → aobreja
Attached patch bug1359328.patch (obsolete) — Splinter Review
Created taskcluster job "sm-fuzzing_firefox".Not sure if this is the right way to do it or some aspects need to be changed.
Attachment #8862806 - Flags: feedback?(rforbes)
Attachment #8862806 - Flags: feedback?(rforbes) → feedback?(kmoir)
Comment on attachment 8862806 [details] [diff] [review]
bug1359328.patch

I got this error
name sm-fuzzing_firefox/opt is not in the whitelist in gecko_v2_whitelist.py.

To test this patch

First, download a recent parameters.yml file from a recent m-i push such as this one
https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&selectedJob=95942099

(click under Gecko Decision Task opt)

i.e.
https://queue.taskcluster.net/v1/task/R9PfCwQqR1OS-2y17C6WCg/runs/0/artifacts/public/parameters.yml
 
From my local clone of mozilla-inbound with your patch applied, I ran this to test the patch

./mach taskgraph target -p ~/Downloads/parameters-m-i.yml

Once you get this error sorted, you want to see that the new build is included in the list that is output by the mach command

Once you have it in the list, you can see the optimized graph that is produced via this command

./mach taskgraph optimized --json -p ~/Downloads/parameters-m-i.yml &> task.json

look at the task.json file to ensure the new build has all the parameters you are looking for.

Then you can push to try for further testing
Attachment #8862806 - Flags: feedback?(kmoir) → feedback+
this seems like a pretty straight forward fix of adding the task to the gecko_v2_whitelist.py file. Should I make a patch for that?
actually, looking at the patch it looks like the job name is different than what is put in the whitelist file.
Attached patch bug1359328_v2.patch (obsolete) — Splinter Review
Change and test the new patch.

By comparing the old "./mach taskgraph target-graph -p path_to_parameters.yml" with the new one I got only:

< spidermonkey-sm-fuzzing_firefox/opt
Attachment #8862806 - Attachment is obsolete: true
Attachment #8864866 - Flags: review?(kmoir)
It failed because maybe the file "/js/src/devtools/automation/variants/fuzzing_firefox" was not added or is not recognised.
Kim can you also test with another push on try? thank you
Flags: needinfo?(kmoir)
Your try push didn't include it that file

https://hg.mozilla.org/try/file/67fb16004fa146e3cfb20fb47bc7e2509fa3cfae/js/src/devtools/automation/variants

If you run
hg status

the file should be there but not added. Use hg add to add the file and run your push again.
Flags: needinfo?(kmoir)
so, sorry I finally caught this but the patch wasn't right. The patch was just making a jsshell build and I wanted a whole firefox build. I have submitted a patch to mozreview that I think covers what I want. I am hoping people can look at it.

Thanks!
Comment on attachment 8864866 [details] [diff] [review]
bug1359328_v2.patch

removing the review flag until you have a good try run
Attachment #8864866 - Flags: review?(kmoir)
Comment on attachment 8864866 [details] [diff] [review]
bug1359328_v2.patch

It looks good, I have two questions

Should the name be sm-fuzzing-firefox instead of sm-fuzzing_firefox? The other names seem to follow that pattern.  

Also, should this build be limited to run on certain platforms?  It looks like other spidermonkey builds are limited by  
run-on-projects:

so they don't run on every branch if they are not needed there
Flags: needinfo?(aobreja)
Attachment #8864866 - Flags: review?(kmoir)
Comment on attachment 8864866 [details] [diff] [review]
bug1359328_v2.patch

Thought of another issue

I think the treeherder symbol
 symbol: SM-tc(en-US)

should be 
treeherder:
            symbol: SM-tc(asan)
or similar so it shows up correctly in treeherder
to be clear, this is not correct. This was never supposed to be a spidermonkey build Please reference the patch I submitted to mozreview.
We have another patch added by Raymond on mozreview.

https://reviewboard.mozilla.org/r/136682/diff/1#index_header
Flags: needinfo?(aobreja) → needinfo?(jlund)
:rforbes if you want the mozreview patch to be reviewed, please choose someone else other than jlund as he is on pto
Flags: needinfo?(rforbes)
Attachment #8865024 - Flags: review?(jlund) → review?(aobreja)
Comment on attachment 8865024 [details]
Bug 1359328 - Add a fuzzing build to taskcluster

Checked the patch, added the bellow lines to get rid of some errors:

-Added   'linux64-fuzzing-asan-opt' in taskcluster/taskgraph/transforms/gecko_v2_whitelist.py
-changed the symbol to :"symbol: tc(Bof)" on taskcluster/ci/build/linux.yml"  because there was a conflict symbol with "linux64-asan/opt"

Tested the patch and the build "build-linux64-asan-fuzzing/opt" was added but when pushed to try I did not get this job visible in treeherder:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=f4442990994c80519fb726eca308903bfbabf69b&filter-searchStr=tc
https://hg.mozilla.org/try/rev/f4442990994c80519fb726eca308903bfbabf69b

I don't know what else need to be changed, I'd ask someone with more experience in Taskcluster, maybe Mihai.
Flags: needinfo?(mtabara)
Attachment #8865024 - Flags: review?(aobreja)
Comment on attachment 8865024 [details]
Bug 1359328 - Add a fuzzing build to taskcluster

https://reviewboard.mozilla.org/r/136682/#review141538

So I dug a bit:

1. The revised patch (which is basically :rforbes's patch + that whitelist adding) lies here - https://hg.mozilla.org/try/rev/a0beacde8238e65fcdaa6aaa4d00594892e6e893

2. I initially used `try: -b o -p linux64-asan -u none -t none` thinking that this would trigger the other ASAN related jobs as well but I was wrong. This created an actual asan-opt build + some related source testing. The try push lies here - https://treeherder.mozilla.org/#/jobs?repo=try&revision=982c283503eefc39035c3b20b28c55a77d1c3423

3. I then amended the actual try command to reflect the newly added plaftorm, like this: `try: -b o -p linux64-asan-fuzzing -u none -t none`. This worked smoothly and the fuzzy job got picked-up. Please note that it's listed as `tc(B)` as I left it in my patch. If, supposedebly this was an all-platforms build, it would have been difficult to spot which build is which if all of them were `tc(B)`. So again, symbols don't necessarily need to be different but it's good practice. This way you avoid the need of clicking each `tc(B)` and reading that label to identify which build is which. Things get easier when you have, say `tc(Bo Bd Bof)` for this ASAN build(s).

4. I then changed the symbol to `tc(Bof)` as :aobreja initially suggested and tried another push that lies https://treeherder.mozilla.org/#/jobs?repo=try&revision=e11a68171536c28c938abbba3dc273e38de3e329. You can see it is now depicted as `tc(Bof)`. 

To conclude:
* the patch is good, just need to adjust the comment with whitelisting + change the platform in the try commig messagen when you push, to enforce that platform to be picked-up.
* the actual fuzzy job is failing (see example from 4) above - https://tools.taskcluster.net/task-inspector/#YE3A4FCMTSC5JgdeF9psuQ/0 but this is another story) 
* will stamp this patch with r- in order for you to pick up that whitelist change too. The rest are just nits, patch looks good ;-)

::: taskcluster/ci/build/linux.yml:250
(Diff revision 1)
>          custom-build-variant-cfg: asan-tc
>          tooltool-downloads: public
>          need-xvfb: true
>  
> +
> +linux64-asan-fuzzing/opt:

As :aobreja said, you need to add the job name within the https://hg.mozilla.org/mozilla-central/file/tip/taskcluster/taskgraph/transforms/gecko_v2_whitelist.py#l33 to whitelist the fact that you're adding a new build to the system.

::: taskcluster/ci/build/linux.yml:257
(Diff revision 1)
> +    index:
> +        product: firefox
> +        job-name: linux64-fuzzing-asan-opt
> +    treeherder:
> +        platform: linux64/asan
> +        symbol: tc(Bo)

AFAIK you can leave this tc(B) as well, symbols do not (necessarily) need to be different. It's a matter of convenience. Once you get the patch working, you should be seeing in Treeherder a Linux-based platform related set of build "Linux x64" or alike (not sure if this creates a something-fuzzing or not with tc(B) there. Better double check with :kmoir on this one.

::: taskcluster/ci/build/linux.yml:270
(Diff revision 1)
> +        config:
> +            - builds/releng_base_linux_64_builds.py
> +            - balrog/production.py
> +        script: "mozharness/scripts/fx_desktop_build.py"
> +        secrets: true
> +        custom-build-variant-cfg: fuzzing-asan-tc

Maybe rename this to `asan-tc-and-fuzzing` to be consistent with the others?

::: testing/mozharness/configs/builds/releng_sub_linux_configs/64_fuzzing_asan_tc.py:1
(Diff revision 1)
> +import os

maybe rename this as well to `64_asan_tc_and_fuzzing` to be consistent with the other renames should you go down this path.

::: testing/mozharness/mozharness/mozilla/building/buildbase.py:349
(Diff revision 1)
>      # platform/bits
>      build_variants = {
>          'add-on-devel': 'builds/releng_sub_%s_configs/%s_add-on-devel.py',
>          'asan': 'builds/releng_sub_%s_configs/%s_asan.py',
>          'asan-tc': 'builds/releng_sub_%s_configs/%s_asan_tc.py',
> +        'fuzzing-asan-tc': 'builds/releng_sub_%s_configs/%s_fuzzing_asan_tc.py',

If you change in the taskcluster/ci/build/linux.yml file the custom-build-variant-cfg, you need to change it here too to `asan-tc-and-fuzzing`. See the other comment above.
Attachment #8865024 - Flags: review-
Clearing my NI. Feel free to make comments in the reviewboard, I'm keeping an eye on it.
Flags: needinfo?(mtabara)
:mtabara * the actual fuzzy job is failing (see example from 4) above - https://tools.taskcluster.net/task-inspector/#YE3A4FCMTSC5JgdeF9psuQ/0 but this is another story)from the treeherder job?

How did you find the actual task ID?
Flags: needinfo?(rforbes) → needinfo?(mtabara)
(In reply to Raymond Forbes[:rforbes] from comment #21)
> :mtabara * the actual fuzzy job is failing (see example from 4) above -
> https://tools.taskcluster.net/task-inspector/#YE3A4FCMTSC5JgdeF9psuQ/0 but
> this is another story)from the treeherder job?
> 
> How did you find the actual task ID?

a) you go to TreeHerder job https://treeherder.mozilla.org/#/jobs?repo=try&revision=e11a68171536c28c938abbba3dc273e38de3e329
b) you click on your job. in this case tc(Bof)
c) a window will pop-out at the bottom of the screen with lots of information (Job Details, Failure Summary, Failure Classification, etc
d) you click on "Job Details" and head there to "Inspect Task". This will take you to Taskcluster and implicit to Task Id.
Flags: needinfo?(mtabara)
Attached patch bug1359328_v3.patch (obsolete) — Splinter Review
Recreated the patch with all the sugestions above and push it to try:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=708129b801ba38aefee0966c2793e6689fbe8e2e&selectedJob=99153307

The build is failing and I'm not sure if this is normal to happen on try or we should also change other things to have a working build.
Attachment #8864866 - Attachment is obsolete: true
Attachment #8867714 - Flags: review?(mtabara)
Attachment #8867714 - Flags: feedback?(rforbes)
Missed this initially due to PTO.

I may be getting confused between bz and mozreview but I believe there is no needinfo for me now that Andrei took the reviews and patches.

Please re-request if there is something for me to look at.
Flags: needinfo?(jlund)
I updated everything and fixed a few various issues that were causing build failures. This should work now. Could you rereview?

Thanks!
Flags: needinfo?(aobreja)
I attached the same changes here in this patch
Also tested,it seems to be fine

check below the test:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=5442bd37e1aa07aa284906212119b906e7290c87&selectedJob=102315798
Attachment #8867714 - Attachment is obsolete: true
Attachment #8867714 - Flags: review?(mtabara)
Attachment #8867714 - Flags: feedback?(rforbes)
Flags: needinfo?(aobreja)
Attachment #8871725 - Flags: review+
Comment on attachment 8865024 [details]
Bug 1359328 - Add a fuzzing build to taskcluster

https://reviewboard.mozilla.org/r/136682/#review147514
Attachment #8865024 - Flags: review?(aobreja) → review+
Attachment #8871522 - Flags: review?(aobreja)
Attachment #8871523 - Flags: review?(aobreja)
Attachment #8871524 - Flags: review?(aobreja)
Comment on attachment 8871522 [details]
Bug 1359328 - Updates for fuzzing taskcluster build

https://reviewboard.mozilla.org/r/142988/#review147790
Attachment #8871522 - Flags: review?(aobreja) → review+
Comment on attachment 8871523 [details]
Bug 1359328 - update geck_v2_whitelist.py

https://reviewboard.mozilla.org/r/142990/#review147792
Attachment #8871523 - Flags: review?(aobreja) → review+
Comment on attachment 8871524 [details]
Bug 1359328 - disable libstdc++ compat check for fuzzing tc build

https://reviewboard.mozilla.org/r/142992/#review147794
Attachment #8871524 - Flags: review?(aobreja) → review+
Comment on attachment 8871522 [details]
Bug 1359328 - Updates for fuzzing taskcluster build

https://reviewboard.mozilla.org/r/142988/#review147822

Reviewed by aobreja
Comment on attachment 8871522 [details]
Bug 1359328 - Updates for fuzzing taskcluster build

https://reviewboard.mozilla.org/r/142988/#review147824
Attachment #8871522 - Flags: review+
Comment on attachment 8871523 [details]
Bug 1359328 - update geck_v2_whitelist.py

https://reviewboard.mozilla.org/r/142990/#review147826

Reviewed by aobreja
Attachment #8871523 - Flags: review+
Comment on attachment 8871524 [details]
Bug 1359328 - disable libstdc++ compat check for fuzzing tc build

https://reviewboard.mozilla.org/r/142992/#review147830

Reviewed by aobreja
Attachment #8871524 - Flags: review+
Comment on attachment 8865024 [details]
Bug 1359328 - Add a fuzzing build to taskcluster

https://reviewboard.mozilla.org/r/136682/#review147964

r+ by aobreja
Comment on attachment 8865024 [details]
Bug 1359328 - Add a fuzzing build to taskcluster

https://reviewboard.mozilla.org/r/136682/#review147966
Attachment #8865024 - Flags: review+
Pushed by choller@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/cd192fa5f5de
Add a fuzzing build to taskcluster r=aobreja,decoder
https://hg.mozilla.org/integration/autoland/rev/de7eb82f8b79
Updates for fuzzing taskcluster build r=aobreja,decoder
https://hg.mozilla.org/integration/autoland/rev/0dde8401aa70
update geck_v2_whitelist.py r=aobreja,decoder
https://hg.mozilla.org/integration/autoland/rev/0b0edaa4552b
disable libstdc++ compat check for fuzzing tc build r=aobreja,decoder
Product: TaskCluster → Firefox Build System
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: