Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

lib.systems: accept *-unknown-elf triples #230160

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

alyssais
Copy link
Member

@alyssais alyssais commented May 5, 2023

Description of changes

These are the canonical GNU triples for various embedded targets. Previously, we only accepted the LLVM-style *-none-elf, which was problematic, because a Matrix user reported needing riscv64-unknown-elf-gcc for some software, and it makes sense for us to be able to produce that. Nixpkgs used to accept such triples, but it was broken by 3b32c92.

With this change, both forms are accepted, and parse equally. Tested that riscv64-unknown-elf is now parsed correctly, and all examples still evaluate.

Link: #165836
Fixes: 3b32c92 ("systems/parse.nix: support eabihf")

Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 23.05 Release Notes (or backporting 22.11 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

These are the canonical GNU triples for various embedded targets.
Previously, we only accepted the LLVM-style *-none-elf, which was
problematic, because a Matrix user reported needing
riscv64-unknown-elf-gcc for some software, and it makes sense for us
to be able to produce that.  Nixpkgs used to accept such triples, but
it was broken by 3b32c92.

With this change, both forms are accepted, and parse equally.
Tested that riscv64-unknown-elf is now parsed correctly, and all
examples still evaluate.

Link: NixOS#165836
Fixes: 3b32c92 ("systems/parse.nix: support eabihf")
@alyssais alyssais added the 6.topic: cross-compilation Building packages on a different platform than they will be used on label May 5, 2023
@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10 labels May 5, 2023
@GenericNerdyUsername
Copy link
Contributor

works for me, but I am confused by line 438

@alyssais
Copy link
Member Author

alyssais commented May 5, 2023

works for me, but I am confused by line 438

That just defaults the "kernel" attribute to "none" if it doesn't end up being set by the last field of the triple (because it encodes ABI instead).

@GenericNerdyUsername
Copy link
Contributor

ok, got it

@alyssais alyssais added the 12.approvals: 1 This PR was reviewed and approved by one reputable person label May 6, 2023
@ghost
Copy link

ghost commented May 7, 2023

Hey thank you for noticing this -- it definitely needs to be fixed, we should be accepting *-unknown-elf.

However this PR has other effects that I'm still trying to understand. For example, it changes the canonicalization of certain triples, like riscv64-none-wasi:

$ /nix/store/b1ch29sg88jggcgs9j1cbb5j2bj62vf7-source/config.sub riscv64-none-wasi
riscv64-none-wasi
$ nix eval --json --impure --expr 'with import ./lib; (systems.elaborate "riscv64-none-wasi").config'
"riscv64-unknown-wasi"

I would like to add a regression test suite to lib/tests/ that verifies that we're parsing triples the same way gnu-config does, except in those cases where we've consciously decided not to (for example ghcjs). I made one when I wrote #180964 except that it checked against the previous behavior of the function rather than against gnu-config-plus-deliberate-exceptions.

Can this PR wait until after I have a chance to add that?

If not, a safe temporary fix might be to just kludge a special case if .. then .. else in at the top of the 3= attrval that checks for this one condition (*-unknown-elf) so we know that nothing else is affected.

Once I have the regression tests working it will be a lot easier to finish cleaning up the mess here (thank you for starting that process!) without having to worry about unexpected effects.

@ghost
Copy link

ghost commented May 31, 2023

I would like to add a regression test suite to lib/tests/ that verifies that we're parsing triples the same way gnu-config does, except in those cases where we've consciously decided not to

WIP is at #235230

@ghost
Copy link

ghost commented Jun 20, 2023

Thank you again for being so patient.

I cherry-picked this on top of #235230 at 66b29b1800083975fda75c7cc46ea6f1655ce6bc and got the following disagreements with gnu-config's handling.

I'll take a closer look at these in the morning. Dealing with missing/unknown vendor fields is the most convoluted part of gnu-config... :(

[
  {
    "expected": "aarch64-none-eabi",
    "name": "testTripleFromString_aarch64-none-eabi",
    "result": "aarch64-unknown-none-eabi"
  },
  {
    "expected": "aarch64-none-eabihf",
    "name": "testTripleFromString_aarch64-none-eabihf",
    "result": "aarch64-unknown-none-eabihf"
  },
  {
    "expected": "aarch64_be-none-eabi",
    "name": "testTripleFromString_aarch64_be-none-eabi",
    "result": "aarch64_be-unknown-none-eabi"
  },
  {
    "expected": "aarch64_be-none-eabihf",
    "name": "testTripleFromString_aarch64_be-none-eabihf",
    "result": "aarch64_be-unknown-none-eabihf"
  },
  {
    "expected": "alpha-none-eabi",
    "name": "testTripleFromString_alpha-none-eabi",
    "result": "alpha-unknown-none-eabi"
  },
  {
    "expected": "alpha-none-eabihf",
    "name": "testTripleFromString_alpha-none-eabihf",
    "result": "alpha-unknown-none-eabihf"
  },
  {
    "expected": "arm-none-eabi",
    "name": "testTripleFromString_arm-none-eabi",
    "result": "arm-unknown-none-eabi"
  },
  {
    "expected": "arm-none-eabihf",
    "name": "testTripleFromString_arm-none-eabihf",
    "result": "arm-unknown-none-eabihf"
  },
  {
    "expected": "armv5tel-none-eabi",
    "name": "testTripleFromString_armv5tel-none-eabi",
    "result": "armv5tel-unknown-none-eabi"
  },
  {
    "expected": "armv5tel-none-eabihf",
    "name": "testTripleFromString_armv5tel-none-eabihf",
    "result": "armv5tel-unknown-none-eabihf"
  },
  {
    "expected": "armv6l-none-eabi",
    "name": "testTripleFromString_armv6l-none-eabi",
    "result": "armv6l-unknown-none-eabi"
  },
  {
    "expected": "armv6l-none-eabihf",
    "name": "testTripleFromString_armv6l-none-eabihf",
    "result": "armv6l-unknown-none-eabihf"
  },
  {
    "expected": "armv6m-none-eabi",
    "name": "testTripleFromString_armv6m-none-eabi",
    "result": "armv6m-unknown-none-eabi"
  },
  {
    "expected": "armv6m-none-eabihf",
    "name": "testTripleFromString_armv6m-none-eabihf",
    "result": "armv6m-unknown-none-eabihf"
  },
  {
    "expected": "armv7a-none-eabi",
    "name": "testTripleFromString_armv7a-none-eabi",
    "result": "armv7a-unknown-none-eabi"
  },
  {
    "expected": "armv7a-none-eabihf",
    "name": "testTripleFromString_armv7a-none-eabihf",
    "result": "armv7a-unknown-none-eabihf"
  },
  {
    "expected": "armv7l-none-eabi",
    "name": "testTripleFromString_armv7l-none-eabi",
    "result": "armv7l-unknown-none-eabi"
  },
  {
    "expected": "armv7l-none-eabihf",
    "name": "testTripleFromString_armv7l-none-eabihf",
    "result": "armv7l-unknown-none-eabihf"
  },
  {
    "expected": "armv7m-none-eabi",
    "name": "testTripleFromString_armv7m-none-eabi",
    "result": "armv7m-unknown-none-eabi"
  },
  {
    "expected": "armv7m-none-eabihf",
    "name": "testTripleFromString_armv7m-none-eabihf",
    "result": "armv7m-unknown-none-eabihf"
  },
  {
    "expected": "armv7r-none-eabi",
    "name": "testTripleFromString_armv7r-none-eabi",
    "result": "armv7r-unknown-none-eabi"
  },
  {
    "expected": "armv7r-none-eabihf",
    "name": "testTripleFromString_armv7r-none-eabihf",
    "result": "armv7r-unknown-none-eabihf"
  },
  {
    "expected": "armv8a-none-eabi",
    "name": "testTripleFromString_armv8a-none-eabi",
    "result": "armv8a-unknown-none-eabi"
  },
  {
    "expected": "armv8a-none-eabihf",
    "name": "testTripleFromString_armv8a-none-eabihf",
    "result": "armv8a-unknown-none-eabihf"
  },
  {
    "expected": "armv8m-none-eabi",
    "name": "testTripleFromString_armv8m-none-eabi",
    "result": "armv8m-unknown-none-eabi"
  },
  {
    "expected": "armv8m-none-eabihf",
    "name": "testTripleFromString_armv8m-none-eabihf",
    "result": "armv8m-unknown-none-eabihf"
  },
  {
    "expected": "armv8r-none-eabi",
    "name": "testTripleFromString_armv8r-none-eabi",
    "result": "armv8r-unknown-none-eabi"
  },
  {
    "expected": "armv8r-none-eabihf",
    "name": "testTripleFromString_armv8r-none-eabihf",
    "result": "armv8r-unknown-none-eabihf"
  },
  {
    "expected": "avr-none-eabi",
    "name": "testTripleFromString_avr-none-eabi",
    "result": "avr-unknown-none-eabi"
  },
  {
    "expected": "avr-none-eabihf",
    "name": "testTripleFromString_avr-none-eabihf",
    "result": "avr-unknown-none-eabihf"
  },
  {
    "expected": "i386-none-eabi",
    "name": "testTripleFromString_i386-none-eabi",
    "result": "i386-unknown-none-eabi"
  },
  {
    "expected": "i386-none-eabihf",
    "name": "testTripleFromString_i386-none-eabihf",
    "result": "i386-unknown-none-eabihf"
  },
  {
    "expected": "i486-none-eabi",
    "name": "testTripleFromString_i486-none-eabi",
    "result": "i486-unknown-none-eabi"
  },
  {
    "expected": "i486-none-eabihf",
    "name": "testTripleFromString_i486-none-eabihf",
    "result": "i486-unknown-none-eabihf"
  },
  {
    "expected": "i586-none-eabi",
    "name": "testTripleFromString_i586-none-eabi",
    "result": "i586-unknown-none-eabi"
  },
  {
    "expected": "i586-none-eabihf",
    "name": "testTripleFromString_i586-none-eabihf",
    "result": "i586-unknown-none-eabihf"
  },
  {
    "expected": "i686-none-eabi",
    "name": "testTripleFromString_i686-none-eabi",
    "result": "i686-unknown-none-eabi"
  },
  {
    "expected": "i686-none-eabihf",
    "name": "testTripleFromString_i686-none-eabihf",
    "result": "i686-unknown-none-eabihf"
  },
  {
    "expected": "loongarch64-none-eabi",
    "name": "testTripleFromString_loongarch64-none-eabi",
    "result": "loongarch64-unknown-none-eabi"
  },
  {
    "expected": "loongarch64-none-eabihf",
    "name": "testTripleFromString_loongarch64-none-eabihf",
    "result": "loongarch64-unknown-none-eabihf"
  },
  {
    "expected": "m68k-none-eabi",
    "name": "testTripleFromString_m68k-none-eabi",
    "result": "m68k-unknown-none-eabi"
  },
  {
    "expected": "m68k-none-eabihf",
    "name": "testTripleFromString_m68k-none-eabihf",
    "result": "m68k-unknown-none-eabihf"
  },
  {
    "expected": "microblaze-none-eabi",
    "name": "testTripleFromString_microblaze-none-eabi",
    "result": "microblaze-xilinx-none-eabi"
  },
  {
    "expected": "microblaze-none-eabihf",
    "name": "testTripleFromString_microblaze-none-eabihf",
    "result": "microblaze-xilinx-none-eabihf"
  },
  {
    "expected": "microblazeel-none-eabi",
    "name": "testTripleFromString_microblazeel-none-eabi",
    "result": "microblazeel-xilinx-none-eabi"
  },
  {
    "expected": "microblazeel-none-eabihf",
    "name": "testTripleFromString_microblazeel-none-eabihf",
    "result": "microblazeel-xilinx-none-eabihf"
  },
  {
    "expected": "mips-none-eabi",
    "name": "testTripleFromString_mips-none-eabi",
    "result": "mips-unknown-none-eabi"
  },
  {
    "expected": "mips-none-eabihf",
    "name": "testTripleFromString_mips-none-eabihf",
    "result": "mips-unknown-none-eabihf"
  },
  {
    "expected": "mips64-none-eabi",
    "name": "testTripleFromString_mips64-none-eabi",
    "result": "mips64-unknown-none-eabi"
  },
  {
    "expected": "mips64-none-eabihf",
    "name": "testTripleFromString_mips64-none-eabihf",
    "result": "mips64-unknown-none-eabihf"
  },
  {
    "expected": "mips64el-none-eabi",
    "name": "testTripleFromString_mips64el-none-eabi",
    "result": "mips64el-unknown-none-eabi"
  },
  {
    "expected": "mips64el-none-eabihf",
    "name": "testTripleFromString_mips64el-none-eabihf",
    "result": "mips64el-unknown-none-eabihf"
  },
  {
    "expected": "mipsel-none-eabi",
    "name": "testTripleFromString_mipsel-none-eabi",
    "result": "mipsel-unknown-none-eabi"
  },
  {
    "expected": "mipsel-none-eabihf",
    "name": "testTripleFromString_mipsel-none-eabihf",
    "result": "mipsel-unknown-none-eabihf"
  },
  {
    "expected": "mmix-none-eabi",
    "name": "testTripleFromString_mmix-none-eabi",
    "result": "mmix-knuth-none-eabi"
  },
  {
    "expected": "mmix-none-eabihf",
    "name": "testTripleFromString_mmix-none-eabihf",
    "result": "mmix-knuth-none-eabihf"
  },
  {
    "expected": "msp430-none-eabi",
    "name": "testTripleFromString_msp430-none-eabi",
    "result": "msp430-unknown-none-eabi"
  },
  {
    "expected": "msp430-none-eabihf",
    "name": "testTripleFromString_msp430-none-eabihf",
    "result": "msp430-unknown-none-eabihf"
  },
  {
    "expected": "or1k-none-eabi",
    "name": "testTripleFromString_or1k-none-eabi",
    "result": "or1k-unknown-none-eabi"
  },
  {
    "expected": "or1k-none-eabihf",
    "name": "testTripleFromString_or1k-none-eabihf",
    "result": "or1k-unknown-none-eabihf"
  },
  {
    "expected": "powerpc-none-eabi",
    "name": "testTripleFromString_powerpc-none-eabi",
    "result": "powerpc-unknown-none-eabi"
  },
  {
    "expected": "powerpc-none-eabihf",
    "name": "testTripleFromString_powerpc-none-eabihf",
    "result": "powerpc-unknown-none-eabihf"
  },
  {
    "expected": "powerpc64-none-eabi",
    "name": "testTripleFromString_powerpc64-none-eabi",
    "result": "powerpc64-unknown-none-eabi"
  },
  {
    "expected": "powerpc64-none-eabihf",
    "name": "testTripleFromString_powerpc64-none-eabihf",
    "result": "powerpc64-unknown-none-eabihf"
  },
  {
    "expected": "powerpc64le-none-eabi",
    "name": "testTripleFromString_powerpc64le-none-eabi",
    "result": "powerpc64le-unknown-none-eabi"
  },
  {
    "expected": "powerpc64le-none-eabihf",
    "name": "testTripleFromString_powerpc64le-none-eabihf",
    "result": "powerpc64le-unknown-none-eabihf"
  },
  {
    "expected": "powerpcle-none-eabi",
    "name": "testTripleFromString_powerpcle-none-eabi",
    "result": "powerpcle-unknown-none-eabi"
  },
  {
    "expected": "powerpcle-none-eabihf",
    "name": "testTripleFromString_powerpcle-none-eabihf",
    "result": "powerpcle-unknown-none-eabihf"
  },
  {
    "expected": "riscv32-none-eabi",
    "name": "testTripleFromString_riscv32-none-eabi",
    "result": "riscv32-unknown-none-eabi"
  },
  {
    "expected": "riscv32-none-eabihf",
    "name": "testTripleFromString_riscv32-none-eabihf",
    "result": "riscv32-unknown-none-eabihf"
  },
  {
    "expected": "riscv64-none-eabi",
    "name": "testTripleFromString_riscv64-none-eabi",
    "result": "riscv64-unknown-none-eabi"
  },
  {
    "expected": "riscv64-none-eabihf",
    "name": "testTripleFromString_riscv64-none-eabihf",
    "result": "riscv64-unknown-none-eabihf"
  },
  {
    "expected": "rx-none-eabi",
    "name": "testTripleFromString_rx-none-eabi",
    "result": "rx-unknown-none-eabi"
  },
  {
    "expected": "rx-none-eabihf",
    "name": "testTripleFromString_rx-none-eabihf",
    "result": "rx-unknown-none-eabihf"
  },
  {
    "expected": "s390-none-eabi",
    "name": "testTripleFromString_s390-none-eabi",
    "result": "s390-ibm-none-eabi"
  },
  {
    "expected": "s390-none-eabihf",
    "name": "testTripleFromString_s390-none-eabihf",
    "result": "s390-ibm-none-eabihf"
  },
  {
    "expected": "s390x-none-eabi",
    "name": "testTripleFromString_s390x-none-eabi",
    "result": "s390x-ibm-none-eabi"
  },
  {
    "expected": "s390x-none-eabihf",
    "name": "testTripleFromString_s390x-none-eabihf",
    "result": "s390x-ibm-none-eabihf"
  },
  {
    "expected": "sparc-none-eabi",
    "name": "testTripleFromString_sparc-none-eabi",
    "result": "sparc-unknown-none-eabi"
  },
  {
    "expected": "sparc-none-eabihf",
    "name": "testTripleFromString_sparc-none-eabihf",
    "result": "sparc-unknown-none-eabihf"
  },
  {
    "expected": "sparc64-none-eabi",
    "name": "testTripleFromString_sparc64-none-eabi",
    "result": "sparc64-unknown-none-eabi"
  },
  {
    "expected": "sparc64-none-eabihf",
    "name": "testTripleFromString_sparc64-none-eabihf",
    "result": "sparc64-unknown-none-eabihf"
  },
  {
    "expected": "wasm32-none-eabi",
    "name": "testTripleFromString_wasm32-none-eabi",
    "result": "wasm32-unknown-none-eabi"
  },
  {
    "expected": "wasm32-none-eabihf",
    "name": "testTripleFromString_wasm32-none-eabihf",
    "result": "wasm32-unknown-none-eabihf"
  },
  {
    "expected": "wasm64-none-eabi",
    "name": "testTripleFromString_wasm64-none-eabi",
    "result": "wasm64-unknown-none-eabi"
  },
  {
    "expected": "wasm64-none-eabihf",
    "name": "testTripleFromString_wasm64-none-eabihf",
    "result": "wasm64-unknown-none-eabihf"
  },
  {
    "expected": "x86_64-none-eabi",
    "name": "testTripleFromString_x86_64-none-eabi",
    "result": "x86_64-unknown-none-eabi"
  },
  {
    "expected": "x86_64-none-eabihf",
    "name": "testTripleFromString_x86_64-none-eabihf",
    "result": "x86_64-unknown-none-eabihf"
  }
]

@ghost
Copy link

ghost commented Jun 20, 2023

I believe that 456619f3a645d9db49848cdf4242de12711cd86a solves this; would you mind checking to see if it works on all the cases that matter?

@roberth roberth added the 6.topic: lib The Nixpkgs function library label Jul 8, 2023
@GenericNerdyUsername
Copy link
Contributor

whats happening now that #235230 is closed?

@wegank wegank added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Mar 19, 2024
@wegank wegank added the 2.status: merge conflict This PR has merge conflicts with the target branch label Jul 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.status: merge conflict This PR has merge conflicts with the target branch 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md 6.topic: cross-compilation Building packages on a different platform than they will be used on 6.topic: lib The Nixpkgs function library 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10 12.approvals: 1 This PR was reviewed and approved by one reputable person
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants