Skip -march guessing for external cross toolchains#1934
Conversation
Don't add -march flag when CC/CROSS_COMPILE names an external cross toolchain (Yocto/OpenEmbedded, Buildroot, ...). These already inject the correct target. Avoid issues such as an unknown value 'x86-64-v2' for '-march' when cross compiling for ARM machine. Signed-off-by: Leon Anavi <leon.anavi@konsulko.com>
Add an AI assistance disclosure file. Signed-off-by: Leon Anavi <leon.anavi@konsulko.com>
|
Thanks for this! To be clear up front: the problem you hit is real, and the direction you took (don't second-guess an externally managed cross toolchain, which already injects its own I'm closing this PR, but not because the report (implied in the PR) is wrong — because reviewing it showed we can't yet say what it fixes. Let me explain, since I'd very much like to see this land properly eventually. On the referenced issue. The audit file references #1930, which is the 26.7.1 release issue — already closed, and not a report of this problem. Our policy is that every change starts from an open issue describing the behaviour we consider wrong, so there's something concrete to agree on before code is written. (We're currently tightening this: the audit-file format and the open-issue requirement will be enforced in CI, and the contribution guide is being standardized across the WAMP Python projects. Our fault that this wasn't clearer — the guidance is genuinely inconsistent across our repos right now.) On what we actually promise. Reviewing this raised a question I couldn't answer: Does autobahn promise anything about cross-compilation? It turns out the only statements are in a changelog entry and a docstring in So we've filed #1935 to write the promise down properly — in How we'd want the fix to land, once the promise exists — we follow a two-phase test-driven workflow:
That way the PR proves both that the behaviour was wrong and that it's now right, and the test stays in the suite so we don't regress. One thing that would really help, and which only you can provide. I don't think the root cause is where this patch sits. If you got Could you open an issue with:
There's also a design question your patch answers implicitly and which we should settle explicitly: what is authoritative about the target architecture — Two smaller notes on the patch itself, for whenever it's revived:
Thanks again for taking the time — this genuinely improved things on our side, just not in the shape of a merge. If you'd like to pick it back up after #1935 lands, that would be very welcome. |
Description
Don't add -march flag when CC/CROSS_COMPILE names an external cross toolchain (Yocto/OpenEmbedded, Buildroot, ...). These already inject the correct target. Avoid issues such as an unknown value 'x86-64-v2' for '-march' when cross compiling for ARM machine.
Related Issue(s)
Closes or relates to #
Checklist
the style guidelines of this project
is effective or that my feature works
updated the changelog
in this PR
AI Assistance Disclosure File
file at
.audit/<branch-name>.mdright in this PR. Thedisclosure file must follow the exact format and content as
described below. Your PR will not be accepted without a
disclosure file.
Example 1 file contents of your disclosure file
.audit/<branch-name>.md:OR
Example 2 file contents of your disclosure file
.audit/<branch-name>.md:Submitting code generated primarily by AI, or for which you
cannot claim human authorship, is not permitted. See
AI Policy
for details.
Example 1 OR Example 2 show the only valid two variants. You
cannot have both or none of the first two marks checked, and you
must always have the last tick checked.
Well, "must" if you want your PR to be accepted and ultimately
merged that is. Of course you are always free to
Go ahead! Fork my Day. (TM);) This is Open-source.