1386588 - Miscellaneous toolchain tweaks, changes and cleanups

Status: RESOLVED FIXED

Description

[Mike Hommey [:glandium]]

Comment 1

[Mike Hommey [:glandium]]

Created attachment 8892886 [details]
Bug 1386588 - Rename the clang build manifests.

The clang toolchains have not been static-analysis-specific for a while.

Review commit: https://reviewboard.mozilla.org/r/163896/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/163896/

Comment 2

[Mike Hommey [:glandium]]

Created attachment 8892887 [details]
Bug 1386588 - Error out when a resource pattern for toolchains matches nothing.

Review commit: https://reviewboard.mozilla.org/r/163898/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/163898/

Comment 3

[Mike Hommey [:glandium]]

Created attachment 8892888 [details]
Bug 1386588 - Avoid wildcards in toolchain resources lists.

Those resources are used to compute a unique identifier for the
toolchain, and changes to those files will change the unique identifier,
and lead to the toolchain being rebuilt.

Using wildcards, especially in the build-clang directory, makes all the
files from there used for the unique identifier, even files irrelevant.
The side effect is that any change to any json file for clang toolchains
currently triggers *all* clang toolchains to be rebuilt, which is a
waste of resources and time.

But while it is tempting to list all the files involved, it is also
tedious and error-prone. Specifically, listing the relevant patch files
for clang toolchain builds is bound to end up outdated. OTOH, we're not
trying to mitigate bad actors here, but just to avoid shooting ourselves
in the foot. And patch files are, in practice, not changed. The jsons
are changed to reference them or not, but the patches themselves don't
change in relevant ways. They may be updated for new versions of clang,
which require a json change anyways. So we ignore the patch files.

Review commit: https://reviewboard.mozilla.org/r/163900/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/163900/

Comment 4

[Mike Hommey [:glandium]]

Created attachment 8892889 [details]
Bug 1386588 - Rename the linux64-clang and linux64-gcc jobs to include a version number.

We're soon going to build multiple versions of clang and gcc for linux,
and we need to differentiate them. Furthermore, there is a need for the
base-toolchains builds to use a fixed version of clang and gcc. So
rename the clang and gcc toolchain jobs to include their version, add
aliases to satisfy all existing jobs, and adjust the base-toolchains
jobs to use the explicit version.

Review commit: https://reviewboard.mozilla.org/r/163902/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/163902/

Comment 5

[Mike Hommey [:glandium]]

Created attachment 8892890 [details]
Bug 1386588 - Change the GCC build script to be future-proof.

It becomes a library of some sort, so that multiple scripts can benefit
from it to build different versions of GCC.

The GPG key associated with GCC is also refreshed from keys.gnupg.net,
adding a new subkey, used to sign newer versions of GCC (and
postprocessed with pgpstrip to make it smaller).

Review commit: https://reviewboard.mozilla.org/r/163904/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/163904/

Comment 6

[Mike Hommey [:glandium]]

Created attachment 8892891 [details]
Bug 1386588 - Add a toolchain job for GCC 4.8, and build clang against it.

While this looks like going backwards, it is desirable to build clang
against GCC 4.8, such that it contains its libgcc. This, in turn, will
solve problems using clang 3.9 with static-analysis builds (details in
bug 1356926). Another way to fix those problems would be to build clang
3.8 but that too would require GCC 4.8. Upgrading those builds to clang
3.9 will also allow to enable stylo on them.

Review commit: https://reviewboard.mozilla.org/r/163906/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/163906/

Comment 7

[Mike Hommey [:glandium]]

Created attachment 8892892 [details]
Bug 1386588 - Add a toolchain job for GCC 6.

Review commit: https://reviewboard.mozilla.org/r/163908/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/163908/

Comment 8

[Mike Hommey [:glandium]]

Created attachment 8892893 [details]
Bug 1386588 - Add a toolchain job for Clang 4.

Review commit: https://reviewboard.mozilla.org/r/163910/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/163910/

Comment 9

[Mike Hommey [:glandium]]

Created attachment 8892894 [details]
Bug 1386588 - Always print out the commands run by the clang build job.

Not doing so makes debugging harder, for no added value.

Review commit: https://reviewboard.mozilla.org/r/163912/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/163912/

Comment 10

[Dustin J. Mitchell [:dustin]]

Comment on attachment 8892887 [details]
Bug 1386588 - Error out when a resource pattern for toolchains matches nothing.

https://reviewboard.mozilla.org/r/163898/#review169236

This will apply to docker images, too - consider adjusting the commit message?

Comment 11

[Gregory Szorc [:gps]]

Comment on attachment 8892886 [details]
Bug 1386588 - Rename the clang build manifests.

https://reviewboard.mozilla.org/r/163896/#review169378

Comment 12

[Gregory Szorc [:gps]]

Comment on attachment 8892888 [details]
Bug 1386588 - Avoid wildcards in toolchain resources lists.

https://reviewboard.mozilla.org/r/163900/#review169380

Comment 13

[Gregory Szorc [:gps]]

Comment on attachment 8892889 [details]
Bug 1386588 - Rename the linux64-clang and linux64-gcc jobs to include a version number.

https://reviewboard.mozilla.org/r/163902/#review169382

Comment 14

[Gregory Szorc [:gps]]

Comment on attachment 8892890 [details]
Bug 1386588 - Change the GCC build script to be future-proof.

https://reviewboard.mozilla.org/r/163904/#review169384

Nice.

Regarding verifying downloaded files, I still prefer to embed the hash in the repo instead of using GPG. This has stronger guarantees than GPG signature verification because a compromised GPG key can sign anything and GPG signature verification would pass on any properly signed file. However, a SHA-512 hash will only ever match one file.

Comment 15

[Gregory Szorc [:gps]]

Comment on attachment 8892891 [details]
Bug 1386588 - Add a toolchain job for GCC 4.8, and build clang against it.

https://reviewboard.mozilla.org/r/163906/#review169388

Comment 16

[Gregory Szorc [:gps]]

Comment on attachment 8892892 [details]
Bug 1386588 - Add a toolchain job for GCC 6.

https://reviewboard.mozilla.org/r/163908/#review169390

\o/

Comment 17

[Gregory Szorc [:gps]]

Comment on attachment 8892893 [details]
Bug 1386588 - Add a toolchain job for Clang 4.

https://reviewboard.mozilla.org/r/163910/#review169392

Comment 18

[Gregory Szorc [:gps]]

Comment on attachment 8892894 [details]
Bug 1386588 - Always print out the commands run by the clang build job.

https://reviewboard.mozilla.org/r/163912/#review169394

Comment 19

[Mike Hommey [:glandium]]

(In reply to Gregory Szorc [:gps] from comment #14)
> Comment on attachment 8892890 [details]
> Bug 1386588 - Change the GCC build script to be future-proof.
> 
> https://reviewboard.mozilla.org/r/163904/#review169384
> 
> Nice.
> 
> Regarding verifying downloaded files, I still prefer to embed the hash in
> the repo instead of using GPG. This has stronger guarantees than GPG
> signature verification because a compromised GPG key can sign anything and
> GPG signature verification would pass on any properly signed file. However,
> a SHA-512 hash will only ever match one file.

We're doing both, and we should keep doing both.

Comment 20

[Gregory Szorc [:gps]]

OK. Both is definitely better than just 1.

Comment 21

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/mozilla-central/rev/6090ee834fb3
Rename the clang build manifests. r=gps
https://hg.mozilla.org/mozilla-central/rev/69190404d80b
Error out when a resource pattern for toolchains matches nothing. r=dustin
https://hg.mozilla.org/mozilla-central/rev/9a82fd89dff9
Avoid wildcards in toolchain resources lists. r=gps
https://hg.mozilla.org/mozilla-central/rev/75d8fbca1a05
Rename the linux64-clang and linux64-gcc jobs to include a version number. r=gps
https://hg.mozilla.org/mozilla-central/rev/f204aac76f9f
Change the GCC build script to be future-proof. r=gps
https://hg.mozilla.org/mozilla-central/rev/25420b520b68
Add a toolchain job for GCC 4.8, and build clang against it. r=gps
https://hg.mozilla.org/mozilla-central/rev/7e43e6ed3afc
Add a toolchain job for GCC 6. r=gps
https://hg.mozilla.org/mozilla-central/rev/f87b29d9cf03
Add a toolchain job for Clang 4. r=gps
https://hg.mozilla.org/mozilla-central/rev/792d0a2e04d7
Always print out the commands run by the clang build job. r=gps,a=KWierso

Comment 22

[Mike Hommey [:glandium]]

I had to backout the part about building clang with gcc 4.8, because of asan bustage. Can't have nice things, right?

https://hg.mozilla.org/mozilla-central/rev/550c67a208b4

Fortunately, I found another way around the clang 3.9 problem in bug 1356926.

However, what this shows is that we can't necessarily blindly change the gcc version that builds clang, because of the libstdc++/libgcc changes that means. Which means we shouldn't use the toolchain alias, but an explicit toolchain name ; I'll file a new bug for that.

Comment 23

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/mozilla-central/rev/d942ef54fdf7
Fixup after backout of 25420b520b68 (part of bug 1386588). a=bustage

Comment 24

[Aki Sasaki [:aki]]

https://hg.mozilla.org/projects/date/rev/6090ee834fb345c2069c06147ec04deb2b287261
Bug 1386588 - Rename the clang build manifests. r=gps

https://hg.mozilla.org/projects/date/rev/69190404d80b855fa788845d5a9b81cd6df34ed4
Bug 1386588 - Error out when a resource pattern for toolchains matches nothing. r=dustin

https://hg.mozilla.org/projects/date/rev/9a82fd89dff95e2a87f7d3818179b9218e1feb1b
Bug 1386588 - Avoid wildcards in toolchain resources lists. r=gps

https://hg.mozilla.org/projects/date/rev/75d8fbca1a0579e4f9df8f370244379c2afbeac6
Bug 1386588 - Rename the linux64-clang and linux64-gcc jobs to include a version number. r=gps

https://hg.mozilla.org/projects/date/rev/f204aac76f9f395232ef111cb3fe9086ef1fee76
Bug 1386588 - Change the GCC build script to be future-proof. r=gps

https://hg.mozilla.org/projects/date/rev/25420b520b68e30a93025aee6407b38d76a9b755
Bug 1386588 - Add a toolchain job for GCC 4.8, and build clang against it. r=gps

https://hg.mozilla.org/projects/date/rev/7e43e6ed3afc703d76b718b2e128c9b923d13500
Bug 1386588 - Add a toolchain job for GCC 6. r=gps

https://hg.mozilla.org/projects/date/rev/f87b29d9cf03b4f9cfea4907ec6c841e57d185df
Bug 1386588 - Add a toolchain job for Clang 4. r=gps

https://hg.mozilla.org/projects/date/rev/792d0a2e04d7bc337c77f4777848337ac6ba6dd6
Bug 1386588 - Always print out the commands run by the clang build job. r=gps,a=KWierso

https://hg.mozilla.org/projects/date/rev/550c67a208b4b686ed66440562a93ce9e12d94ca
Backout changeset 25420b520b68 (part of bug 1386588) for asan bustage. a=bustage.

https://hg.mozilla.org/projects/date/rev/d942ef54fdf726840a698d2ddcaf989d2c00edbc
Fixup after backout of 25420b520b68 (part of bug 1386588). a=bustage