Document the range of compilers the project uses for building, CI and
external toolchain support, and try to assign the proper level of effort
developers should go to for each of them.
Sponsored by: Netflix
Details
Details
- Reviewers
emaste - Commits
- R9:9df88b0b78a7: committers-guide: Add section on range of compilers
Diff Detail
Diff Detail
- Repository
- R9 FreeBSD doc repository
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
| documentation/content/en/articles/committers-guide/_index.adoc | ||
|---|---|---|
| 3126 | also Clang | |
| 3130 | This will quickly become out of date, unless we get @dim to update this every time a new Clang goes in. What about something like "a contemporary Clang version" | |
| 3132 | ||
| 3151 | we don't use an assembler in the tree at all; for the in-tree tolchain it's LLVM's LLD and ELF Tool Chain utilities. | |
| documentation/content/en/articles/committers-guide/_index.adoc | ||
|---|---|---|
| 3136 | OK. My src/Makefile was out of date. | |
| documentation/content/en/articles/committers-guide/_index.adoc | ||
|---|---|---|
| 3130 | I want the versions we're using, and plan to keep it up to date. I need to list the clang 12, 13, and 14 we have in CI on github. This makes it more concrete. I can list them at the end if that helps... | |
| 3151 | Well, yes and no.... We use the built-in assembler for clang builds, but binutils one for gcc builds. And I'm trying to convey you have to work with both. Plus lld vs ld.bdf is an occasional issue. I'm open on how to convey these issues / variety. | |
| documentation/content/en/articles/committers-guide/_index.adoc | ||
|---|---|---|
| 3151 | We never invoke the assembler though is my point, for GCC builds we call gcc to assemble. | |
| documentation/content/en/articles/committers-guide/_index.adoc | ||
|---|---|---|
| 3151 | how do you suggest I capture that one must target both lld and ld.bdf, as well as account for variations between what the building clang assembler swallows and what the binutils assembler swallows? That's the motivation for this whole paragraph, to make it clear to people what the range of environments you need to operate in gcc12 is busted right now because (I think) the last OpenZFS import has assembler with a directive that bintuils doens't grok. | |
Comment Actions
update, per review
Try to address Ed's concerns about assemblers
List all major versions inline.
| documentation/content/en/articles/committers-guide/_index.adoc | ||
|---|---|---|
| 3151 | From a user's perspective really all we need to require is that builds are done with`CROSS_TOOLCHAIN=amd64-gcc12` and without (or equivalently, with CROSS_TOOLCHAIN=llvm15 instead). We probably don't need to be explicit that the user needs to test with lld and ld.bfd, it's implicitly controlled by the selected toolchain. gcc12 was broken by an unused variable warning, should have been fixed with 9a93b6cf3dd97ca7bcb938f87afeb523d023f807. | |
| documentation/content/en/articles/committers-guide/_index.adoc | ||
|---|---|---|
| 3151 | How do you like my rephrasing? | |
Comment Actions
I think the one thing missing is the rationale. The reason we support both toolchains (clang and GNU) is to get better warning coverage (different compilers catch different things), and flexibility for users (e.g. historically GCC has had better debug info, and for some workloads one toolchain might generate more optimal code than another). However, we might also want to have some phrasing that tempers this a bit to explain why we don't support, say, old versions of Intel's compiler. GCC and Clang both support very similar dialects of C and aim to (mostly) be compatible with each other.
Comment Actions
Also add a note about older standards. The project supports building software
that's conforming to these older standards, and that constrains a more agressive
removal of code in places like header files that's necessary for that support.
MAke a note of that.
| documentation/content/en/articles/committers-guide/_index.adoc | ||
|---|---|---|
| 3086–3087 | IMO this is excessively patronizing, something we should revisit. | |
| 3091 | My guess is that most of the time folks won't realize that their change could be compiler-specific, and that we'll usually catch these in CI on some delayed basis; the real eventual solution is to have CI test this on pull requests/phabricator reviews. | |
| 3126 | Let's make it a more explicit policy: FreeBSD builds with both Clang and GCC. | |
| 3144 | supports one or more out-of-tree compilers? | |
| 3146 | ||
| documentation/content/en/articles/committers-guide/_index.adoc | ||
|---|---|---|
| 3086–3087 | I think we can just drop this sentence entirely, and no time like the present, eh? | |
| 3091 | 'make sure it works' gives a lot of rope to do it after the commit, imho, but since that's the intent, I'm open for ways to convey that intent better. Even in a pre-CI commit, there are cases where it breaks something after the build stage, and we'd need to embrace that nuance as well. Maybe the whole thing should just be: | |
| 3126 |
Oh, I like that. It's a bit firmer than I had without being mean about it. | |
| 3144 | yup | |
| 3146 | yup | |
Comment Actions
Oh, add a hint for how to test things. It's brief enough that we should
just include it inline.
Comment Actions
catch up with all suggestions and tick all the boxes. At this point, it's my belief that all the feedback has been addressed. Please comment again if I missed anything.
| documentation/content/en/articles/committers-guide/_index.adoc | ||
|---|---|---|
| 3162 | ||