#1066 Update compiler guidelines for compiler policy change
Merged 8 months ago by churchyard. Opened a year ago by tstellar.
tstellar/packaging-committee compiler-policy  into  master

@@ -738,7 +738,34 @@ 

  [#compiler]

  == Compiler

  

- Fedora uses gcc as the compiler (for all languages that gcc supports). Packages may only build with an alternative compiler to gcc if upstream does not support gcc.

+ Fedora packages should default to using gcc as the compiler (for all languages that gcc supports) or clang if upstream does not support building with gcc. However, if there is a good technical reason, packagers may choose not to use the default compiler. Examples of valid technical reasons to not use the default compiler, include but are not limited to:

+ 

+ * The default compiler cannot build a package correctly.

+ * The packager needs to disable a compiler feature (e.g. LTO) in order for the default compiler to correctly compile their package.

+ * The default compiler takes significantly longer to build a package.

+ * The default compiler is missing a feature that would benefit the package.

+ 

+ Packagers choosing to use a non-default compiler should document the reason for this decision in a comment in the spec file.

+ 

+ == Compiler macros

+ 

+ If clang is being used to build a package, packagers must set the %toolchain macro to clang:

+ 

+ ....

+ %global toolchain clang

+ ....

+ 

+ This ensures that Fedora's clang-specific compiler flags are used when compiling.

+ 

+ If a packager wants to use conditional macros in a spec file to make it easier to switch between two different compilers, then the following macros names should be used:

+ 

+ ....

+ %bcond_with toolchain_clang

+ %bcond_with toolchain_gcc

+ ....

+ 

+ Packagers may also use the %build_cc, %build_cxx, or %build_cpp macros in the spec file in place of hard-coding the compiler name.  The values of these variables are controled by setting the %toolchain macro to either clang or gcc.

+ 

  

  == Compiler flags

  

https://fedoraproject.org/wiki/Changes/CompilerPolicy

This is a proposed change to the compiler policy to go along with the Fedora change proposal: https://fedoraproject.org/wiki/Changes/CompilerPolicy

It should not be merged unless the change proposal is accepted.

There is a missing documentation requirement here. Deviating from our defaults should be documented and tracked. We require this with things like ExcludeArch/ExclusiveArch, so we should require it for this case too.

rebased onto 515dae02fd44354a9aa51e80047323e69c26c43d

a year ago

There is a missing documentation requirement here. Deviating from our defaults should be documented and tracked. We require this with things like ExcludeArch/ExclusiveArch, so we should require it for this case too.

OK, I've added this to the new policy.

Do we really need a package bug and a tracking bug? Taking the concrete example of Firefox, if Firefox maintainer switches the package for Firefox 89 to clang, and for Firefox 92 to gcc, and then maybe back to clang on arm64 at some point, I don't see how the dozen messages in the tracking bug would help with anything. First, the reasons for the switch would be very specific to the package, so "tracking" it together with other packages which switched to clang would do nothing to help other maintainers. Second, there is nothing to "fix", the package may well use the alternative compiler for the next 10 years without any intent to change bug, until the technical situation changes.

And if we want to find packages which require e.g. clang, repoquery --arch=src --disablerepo=\* --enablerepo=fedora-source --whatrequires clang works just as well, without wasting any additional maintainer cycles.

So overall, I'd say that the decision should be documented somewhere (in particular in the the spec file and/or dist-git commit message), and that's all.

OK, I'm fine with either approach.

I will move forward without the tracking bug requirement unless there are major objections.

Metadata Update from @tibbs:
- Pull-request tagged with: meeting

10 months ago

rebased onto 4a2b92b174f27afd6a0ad0b52d0f5725258cdbca

9 months ago

I've discovered that rpm requires macros to be at least 3 characters, so we can't use %cc. Is %ccc a good alternative?

I've discovered that rpm requires macros to be at least 3 characters, so we can't use %cc. Is %ccc a good alternative?

Is this a restriction that could be easily fixed? Maybe @pmatilai knows?

The restriction is strictly in the "user side" of things, to guarantee automatic macros (ie `%*, %1 and the like) cannot be messed with. Removing the limitation would be easy enough but not a good idea, in otherwords: not going to happen.

FWIW %ccc seems weird.

Digging deeper into the archived memories zone, the name length was once used to detect automatic macros. We have more sophisticated methods for that these days, but the limit is still used for preventing automatic macro overrides. If the override prevention was handled by some other means we could probably consider changing the limit as well, but this is not going to happen overnight and would need discussing upstream anyhow.

The macros should be called %build_cc etc., to match %build_cflags. They also need to be documented in buildflags.md. We might need %extension_cc eventually, so the prefix makes sense from that perspective as well.

@fweimer That seems OK, to me. Does anyone object to changing the macros to %build_cc, %build_cxx, %build_cpp ?

Fine with me, go ahead and make the PR for it.

rebased onto f601143eedbd499fff473c5c5e0e083b937ed2c9

9 months ago

I have formatting changes committed locally but I cannot push to @tstellar's fork.

Could you please grant me access?

https://pagure.io/fork/tstellar/packaging-committee/adduser

rebased onto 07b8f25751cc6acdc868d70e9ce7dc8eb09c8672

9 months ago

rebased onto 43865a73814ef7d417a05185cd11e13404104192

9 months ago

Formatting changes pushed.

Consider me +1.

Can we add a SHOULD requirement to include a comment in the spec file documenting the "valid technical reason" for not using the default compiler? This would be similar to our existing SHOULD requirement for upstream patch status.

rebased onto 273c370846db67c7354a0df9fc5cbe24cea02f09

9 months ago

rebased onto 97b3d2a

9 months ago

Can we add a SHOULD requirement to include a comment in the spec file documenting the "valid technical reason" for not using the default compiler? This would be similar to our existing SHOULD requirement for upstream patch status.

OK, I've added this to the documentation.

+1, Looks good to me, thanks!

Pull-Request has been merged by churchyard

8 months ago