Closed Bug 1250709 Opened 4 years ago Closed 4 years ago

Implement new taskcluster-based rooting hazard analysis jobs

Categories

(Core :: JavaScript: GC, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla48
Tracking Status
firefox48 --- fixed

People

(Reporter: sfink, Assigned: sfink)

References

(Blocks 2 open bugs)

Details

Attachments

(5 files, 4 obsolete files)

I would like to switch all hazard analysis jobs over taskcluster and shoot the old ones in the head.
This implements the main job, the whole-browser analysis. It depends on the to-be-uploaded patch.
Split out the hazard analysis logic into a separate script to be shared with the already-reviewed mulet analysis.
Attachment #8722741 - Flags: review?(garndt)
Attachment #8722739 - Flags: review?(garndt)
...and a JS shell hazard analysis (corresponding to the current linux64-sh-haz).
Attachment #8722742 - Flags: review?(garndt)
Comment on attachment 8722739 [details] [diff] [review]
Taskcluster-based browser rooting hazard analysis

r? terrence for the stuff that does the stuff.
Attachment #8722739 - Flags: review?(terrence)
Comment on attachment 8722739 [details] [diff] [review]
Taskcluster-based browser rooting hazard analysis

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

::: testing/taskcluster/tasks/builds/haz_linux.yml
@@ +15,5 @@
> +    - 'docker-worker:relengapi-proxy:tooltool.download.public'
> +
> +  payload:
> +    cache:
> +      level-{{level}}-{{project}}-build-haz-linux-workspace: '/home/worker/workspace'

the scope for this cache needs to be added to task.scopes
Attachment #8722741 - Flags: review?(garndt) → review+
Comment on attachment 8722742 [details] [diff] [review]
Implement shell-only hazard analysis job

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

::: testing/taskcluster/tasks/builds/haz_shell_linux.yml
@@ +15,5 @@
> +    - 'docker-worker:relengapi-proxy:tooltool.download.public'
> +
> +  payload:
> +    cache:
> +      level-{{level}}-{{project}}-build-haz-linux-workspace: '/home/worker/workspace'

I believe task.scopes should have the scope for this cache.
Comment on attachment 8722739 [details] [diff] [review]
Taskcluster-based browser rooting hazard analysis

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

rs=me
Attachment #8722739 - Flags: review?(terrence) → review+
Is there anything I can help with on this bug?  I think with the task.scopes updated, these things should be good to go.
Oh, maybe I'll re-upload the fixed versions so I can get the happy r+ stamp. Thanks for asking.

But these are currently blocked on the gcc version. I thought we could just upgrade GCC to 4.9.3 across the board, but apparently we want some gcc 4.8 jobs still, so I'm going to have to use a different tooltool manifest for these jobs so I can get gcc 4.9.3. And there are multiple ways of doing that, and I need to get feedback from glandium.
This is the task definition I've been testing with for a while. I believe it has all scope and other problems fixed.
Attachment #8725020 - Flags: review?(garndt)
Attachment #8722739 - Attachment is obsolete: true
Attachment #8722739 - Flags: review?(garndt)
And here's the shell version. Hopefully bzexport will be smart enough to obsolete the right ones.
Attachment #8725021 - Flags: review?(garndt)
Attachment #8722742 - Attachment is obsolete: true
Attachment #8722742 - Flags: review?(garndt)
glandium - I would like to land the new hazard analysis jobs with gcc 4.9.3. As per 1029245, it seems we cannot upgrade across the board to 4.9.3, so I'd like to just use 4.9.3 for the hazard builds.

One way to do that is to continue sharing the gecko releng.manifest, but then use a hazard-specific manifest file to overwrite the gcc that it installs. This could be a gcc-only manifest file, or I could have one hazard manifest that installs both gcc and a corresponding sixgill. (It is probably safest to keep the two paired, even though sometimes a sixgill binary works across several gcc versions.)

Alternatively, I could duplicate the gecko releng.manifest and either replace its gcc, or remove it entirely (and install it with a hazard-specific manifest).

Similarly, I could make a single browser hazard manifest that installs everything needed in one go (everything from releng.manifest but gcc, plus gcc 4.9.3 and sixgill). But that would also lead to duplication, in that the shell, browser, and mulet hazard builds all require different subsets of the other packages (gtk3, rustc, etc.)

Which would you prefer?
Flags: needinfo?(mh+mozilla)
Attachment #8725020 - Flags: review?(garndt) → review+
Attachment #8725021 - Flags: review?(garndt) → review+
(In reply to Steve Fink [:sfink, :s:] from comment #12)
> glandium - I would like to land the new hazard analysis jobs with gcc 4.9.3.
> As per 1029245, it seems we cannot upgrade across the board to 4.9.3, so I'd
> like to just use 4.9.3 for the hazard builds.
> 
> One way to do that is to continue sharing the gecko releng.manifest, but
> then use a hazard-specific manifest file to overwrite the gcc that it
> installs. This could be a gcc-only manifest file, or I could have one hazard
> manifest that installs both gcc and a corresponding sixgill. (It is probably
> safest to keep the two paired, even though sometimes a sixgill binary works
> across several gcc versions.)
> 
> Alternatively, I could duplicate the gecko releng.manifest and either
> replace its gcc, or remove it entirely (and install it with a
> hazard-specific manifest).
> 
> Similarly, I could make a single browser hazard manifest that installs
> everything needed in one go (everything from releng.manifest but gcc, plus
> gcc 4.9.3 and sixgill). But that would also lead to duplication, in that the
> shell, browser, and mulet hazard builds all require different subsets of the
> other packages (gtk3, rustc, etc.)
> 
> Which would you prefer?

The latter is what looks more like what we do everywhere. If we want more fancy things like includes in tooltool, we should change that in tooltool, not by invoking tooltool manually multiple times, imho.
Flags: needinfo?(mh+mozilla)
Attached patch Upgrade browser to gcc 4.9.3 (obsolete) — Splinter Review
Ok, then I'll switch the b2g tooltool manifest to that too, so I'm requesting review on a patch to eliminate the second tooltool invocation.

Personally, I think tooltool is already a bit complicated, and would rather have multiple invocations be the standard way of using it instead of having *the* manifest that you install. That requires instantiating a separate manifest for every relevant configuration, when varying parts are shared. Seems like the jobs ought to be the repository of that logic, not materialized manifests nor complex include webs (because I'd immediately want substitution to decide which manifests to include, and the yml files already have includes + a substitution mechanism etc.)

But I'm happy to pretend that the problem is simple for now, while it still is. :-)
Attachment #8725512 - Flags: review?(mh+mozilla)
Comment on attachment 8725512 [details] [diff] [review]
Upgrade browser to gcc 4.9.3

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

::: b2g/dev/config/tooltool-manifests/linux64/releng.manifest
@@ +13,5 @@
> +"digest" : "49627d734df52cb9e7319733da5a6be1812b9373355dc300ee5600b431122570e00d380d50c7c5b5003c462c2c2cb022494b42c4ad00f8eba01c2259cbe6e502",
> +"filename" : "sixgill.tar.xz",
> +"size" : 2628868,
> +"unpack" : true
> +},

You don't need sixgill on the normal builds anymore, do you?
Attachment #8725512 - Flags: review?(mh+mozilla) → feedback+
Attached patch Clobber buildsSplinter Review
I was seeing some mysterious problems that I wanted to blame on reusing old analysis results, so it seems safer to nuke them just in case. I'm not convinced it actually mattered, but I guess it's better to be safe.
Attachment #8729176 - Flags: review?(terrence)
Attached patch Upgrade browser to gcc 4.9.3 (obsolete) — Splinter Review
> You don't need sixgill on the normal builds anymore, do you?

I never did, but this is the mulet hazard analysis and I was using the same manifest for mulet browser build and mulet hazard build. It seems you'd really rather duplicate all the manifests so they can be single-purpose, so I've updated the patch to do that.
Attachment #8729182 - Flags: review?(mh+mozilla)
Attachment #8725512 - Attachment is obsolete: true
Comment on attachment 8729182 [details] [diff] [review]
Upgrade browser to gcc 4.9.3

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

::: browser/config/tooltool-manifests/linux64/hazard.manifest
@@ +1,3 @@
> +[
> +{
> +"version": "gcc 4.9.3",

Why is this file not marked as the copy of another?
Attachment #8729182 - Flags: review?(mh+mozilla)
Attachment #8729676 - Flags: review?(mh+mozilla)
Attachment #8729182 - Attachment is obsolete: true
Attachment #8729176 - Flags: review?(terrence) → review+
Comment on attachment 8729676 [details] [diff] [review]
Upgrade browser to gcc 4.9.3

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

Thanks, the copy marker makes it easier to review.
Attachment #8729676 - Flags: review?(mh+mozilla) → review+
You need to log in before you can comment on or make changes to this bug.