Closed Bug 1321444 Opened 4 years ago Closed 4 years ago

add win32/win64 clang-cl tier2 build jobs

Categories

(Firefox Build System :: Task Configuration, task)

task
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
mozilla53

People

(Reporter: froydnj, Assigned: froydnj)

References

Details

Attachments

(5 files, 4 obsolete files)

I have patches that add what I think are all the necessary pieces, but I can't seem to get the tasks to trigger on try or even show up in the "Add new jobs".

My current patches are using the semi-defunct windows static analysis mozconfigs, with the clang-plugin bits disabled, as bug 1316545 is working on spinning those up.
I don't think "add new jobs" is a good indication of much..
Component: General → Task Configuration
These are all based off of the win32 debug-static-analysis config.  I
chose to use separate configs because the debug-static-analysis config
is currently being used for other purposes.  We'll need to consolidate
after clang-cl and windows static analysis builds are running on
automation.
Attachment #8816295 - Flags: review?(mshal)
Nothing complex here, just new tooltool packages.
Attachment #8816296 - Flags: review?(mshal)
We need this for the Windows SDK et al.
Attachment #8816297 - Flags: review?(mshal)
This part is the part I'm most unsure about, because there's a lot of
duplicated code.  Feedback welcome!
Attachment #8816298 - Flags: review?(pmoore)
We need to define the jobs and we also need to whitelist them appropriately.

Victory: https://treeherder.mozilla.org/#/jobs?repo=try&revision=de4c71b7d62519e48d269b9ec5aa737679f77644

(Those jobs will be red, but that's a different fix.  I won't commit this
patchset until the jobs are actually green, at the risk of immediately earning
a philor ban-hide.)
Attachment #8816299 - Flags: review?(dustin)
\o/

Thank you for driving this to TreeHerder finally!
Comment on attachment 8816298 [details] [diff] [review]
part 4 - add mozharness configs for clang-cl build jobs

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

::: testing/mozharness/configs/builds/taskcluster_firefox_win32_clang_debug.py
@@ +16,5 @@
> +
> +    'default_actions': [
> +        'clone-tools',
> +        'build',
> +        'check-test',

It seems that these tests are currently failing.  Perhaps we should remove this for now and work on enabling them later?
(In reply to :Ehsan Akhgari from comment #8)
> Comment on attachment 8816298 [details] [diff] [review]
> part 4 - add mozharness configs for clang-cl build jobs
> 
> Review of attachment 8816298 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> :::
> testing/mozharness/configs/builds/taskcluster_firefox_win32_clang_debug.py
> @@ +16,5 @@
> > +
> > +    'default_actions': [
> > +        'clone-tools',
> > +        'build',
> > +        'check-test',
> 
> It seems that these tests are currently failing.  Perhaps we should remove
> this for now and work on enabling them later?

Oh man, I didn't even realize that we could not run the tests here.  Thank you for pointing this out!  Yes, we should disable them so we can get this landed.  (Assuming we can get the NSS fix done in a reasonable amount of time.)
Fix a small typo.  Comments on the previous version of the patch still apply.
Attachment #8816348 - Flags: review?(pmoore)
Attachment #8816298 - Attachment is obsolete: true
Attachment #8816298 - Flags: review?(pmoore)
Attachment #8816299 - Flags: review?(dustin) → review+
Attachment #8816295 - Flags: review?(mshal) → review+
Attachment #8816297 - Flags: review?(mshal) → review+
Comment on attachment 8816296 [details] [diff] [review]
part 2 - update windows clang manifests to r286542


> "filename": "clang32.tar.bz2",

> "filename": "clang64.tar.bz2",


These should probably be called "clang.tar.bz2" since the directory the packages unpack into is "clang". Tooltool uses the first part of the filename to clean up the old directory, so with the filenames as is it will try to do "rm -rf clang32" before unpacking into "clang". It's possible a previous clang package exists there that isn't cleaned up.
Attachment #8816296 - Flags: review?(mshal) → review+
Pushed by nfroyd@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/31df89121834
part 1 - add win32 and win64 clang mozconfigs; r=mshal
https://hg.mozilla.org/integration/mozilla-inbound/rev/13a8f7ddf7f4
part 2 - update windows clang manifests to r286542; r=mshal
https://hg.mozilla.org/integration/mozilla-inbound/rev/ba62b4bebebf
part 3 - add vs2015u3 to clang.manifest files; r=mshal
Landed the mozconfig and manifest changes just to get those out of the way.
Keywords: leave-open
Rebase on recent changes.  Carrying over review.
Attachment #8817261 - Flags: review+
Attachment #8816299 - Attachment is obsolete: true
Rebase over some recent changes.  Note that the comments in the files were not
updated for the recent renaming of the windows configs; I figure that can be
addressed in a followup bug.  I also didn't rename the clang configs to follow
the MSVC config conventions because I'm not sure that it really matters: the
MSVC configs were renamed for the benefit of artifact builds, but we shouldn't
be doing artifact builds with the clang configs.  I guess they could be renamed
for consistency, but I'm not sure what the consistent naming would be...
Attachment #8817264 - Flags: review?(pmoore)
Attachment #8816348 - Attachment is obsolete: true
Attachment #8816348 - Flags: review?(pmoore)
We need to define the jobs and we also need to whitelist them appropriately.
Comment on attachment 8817293 [details] [diff] [review]
part 5 - add taskcluster windows clang jobs; r=dustin

This updates the job-name field to deal with the removal of buildbot routes from taskcluster configs.  Carrying over r+.
Attachment #8817293 - Flags: review+
Attachment #8817261 - Attachment is obsolete: true
Comment on attachment 8817264 [details] [diff] [review]
part 4 - add mozharness configs for clang-cl build jobs

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

r+ based on latest try push https://treeherder.mozilla.org/#/jobs?repo=try&revision=e47907629d5e261d3c659384de265caf63c01a72

We might want to add generate-build-stats mozharness action at some point later - but I think we don't need to do that in this bug.

Thanks!
Attachment #8817264 - Flags: review?(pmoore) → review+
Pushed by nfroyd@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/b8866907ddc0
part 4 - add mozharness configs for clang-cl build jobs; r=pmoore
https://hg.mozilla.org/integration/mozilla-inbound/rev/b277f63a4095
part 5 - add taskcluster windows clang jobs; r=dustin
Status: NEW → RESOLVED
Closed: 4 years ago
Flags: in-testsuite-
Keywords: leave-open
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
Depends on: 1333302
Product: TaskCluster → Firefox Build System
You need to log in before you can comment on or make changes to this bug.