On the fallback path, we hardcode the X_ prefix to the info variables.
Since the only prefixed tool in _cc_vars/_ld_vars is XCC/XLD this works
for us at the moment, but potentially breaks for anyone adding another
tool type to _cc_vars/_ld_vars, clobbering the wrong variables and
leaving the correct variables unset. Instead we should be more general
and use the X_ variable's contents as the prefix.
Details
Details
Diff Detail
Diff Detail
- Lint
Lint Passed - Unit
No Test Coverage - Build Status
Buildable 20600 Build 20021: arc lint + arc unit
Event Timeline
| share/mk/bsd.compiler.mk | ||
|---|---|---|
| 212–213 | This makes no sense now. COMPILER_TYPE=${COMPILER_TYPE}? | |
| share/mk/bsd.compiler.mk | ||
|---|---|---|
| 212–213 | If it wasn't clear, that is the effect of your change. It achieves nothing that I can see. | |
| share/mk/bsd.compiler.mk | ||
|---|---|---|
| 212–213 | We only reach this branch if ${cc} != "CC", ie ${X_} != "". | |
| share/mk/bsd.compiler.mk | ||
|---|---|---|
| 212–213 | It is an NFC as far as FreeBSD upstream is concerned. However, downstreams exist that add additional variables to _cc_vars, and in that case ${X_} is neither empty nor "X_". Thus, whilst it's "pointless" upstream, it takes no effort to keep upstream and saves having to maintain it as a downstream patch and resolve conflicts every time someone adds a new COMPILER/LINKER_FOO. | |
| share/mk/bsd.compiler.mk | ||
|---|---|---|
| 212–213 | This is also what the patch summary is trying to convey, in case it isn't clear. | |
| share/mk/bsd.compiler.mk | ||
|---|---|---|
| 212–213 | It's nonsense though and could easily be "fixed" by anyone since it's a bug/pointless to set a variable to itself. Especially without any comments or real reason. Just looking at this file I want to rewrite it since it's such chaos. Your change is easily lost if someone is motivated to do that. _cc_vars is not supposed to be modifiable. An underscore means that. I don't think it's good to commit hacks for private variables. Can you elaborate on what you have done exactly downstream? | |
| share/mk/bsd.compiler.mk | ||
|---|---|---|
| 212–213 |
We're not setting a variable to itself; this case isn't hit when ${X_} is empty, so we will never be trying to execute COMPILER_TYPE=COMPILER_TYPE.
This is in CheriBSD; we've had _cc_vars+= CHERI_CC CHERI_ for years in our bsd.compiler.mk. | |
| share/mk/bsd.compiler.mk | ||
|---|---|---|
| 212–213 | Ok, turns out I'm wrong, _cc_vars+= CHERI_CC CHERI_ was added in September, in the precursor to the revision you mentioned. | |
| share/mk/bsd.compiler.mk | ||
|---|---|---|
| 212–213 | You want CHERI_COMPILER_TYPE to default to COMPILER_TYPE? CheriBSD is compiled using 3 different compilers and 3 different linkers? | |
| share/mk/bsd.compiler.mk | ||
|---|---|---|
| 212–213 | Well, yes, that is a current problem and should be fixed by just changing the if to ${cc} == "CC" || ${${cc}} != ${CC} (which is what the intent of the if *really* is). However, theoretically ${CHERI_CC} can be equal to ${CC} (could just use our built LLVM toolchain for both), which would then hit the else and still trash X_COMPILER_TYPE even if XCC is something different. As for why these exist, it's so pure MIPS stuff can still be built with GCC/binutils. It's quite possible we don't exercise that ability any more, but the flexibility in our build system is there and potentially useful. | |
| share/mk/bsd.compiler.mk | ||
|---|---|---|
| 212–213 | We use /use/bin/cc and /use/bin/LD for bootstrap/build tools. Until fairly recently we were using the bootstrapped GCC 4.2 + bfd as XCC/XLD and our forks of llvm/clang and lld for any code using CHERI capabilities (CHERI_CC/CHERI_LD) Some files are compiled with CHERI_CC instead of XCC during the libraries phase since we need to build e.g. a memcpy that works with CHERI capabilities. We then also have a buildcheri phase similar to lib32 that builds all libraries as using CHERI_CC. (It's actually slightly more complicated but hopefully this explains where we use the different compilers/linkers) | |