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.

Sign up for GitHub

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

Jump to bottom

fixes for mips cross compiling toolchains #165020

Closed
wants to merge 3 commits into from
Closed

fixes for mips cross compiling toolchains #165020

wants to merge 3 commits into from

Conversation

Mic92
Copy link
Member

@Mic92 Mic92 commented Mar 20, 2022 •
edited
Loading

Those don't evaluate.

Description of changes

Current status: still building. Testing here: nix-community/cross-toolchains.nix#19

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/)
  • 22.05 Release Notes (or backporting 21.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
    • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
  • Fits CONTRIBUTING.md.

@Mic92
Copy link
Member Author

Mic92 commented Mar 20, 2022

Exception: error: Unknown CPU type: mipsisa32r6; type: nix::ThrownError
see https://hercules-ci.com/github/nix-community/cross-toolchains.nix/jobs/75

@Mic92
Copy link
Member Author

Mic92 commented Mar 20, 2022

cc @a-m-joseph

@Mic92 Mic92 changed the title drop invalid mips platforms fixes for mips platforms Mar 20, 2022
@Mic92 Mic92 changed the title fixes for mips platforms fixes for mips cross compiling toolchains Mar 20, 2022
@ghost
Copy link

ghost commented Mar 21, 2022 •
edited by ghost
Loading

This is my first time using github to do a code review; if I did something stupid I apologize.

All of my review comments have an orange "Pending" on them. I'm not sure what that means. Am I supposed to do something now? Are you able to see them? Github apparently has some weird UI glitch that causes the green "review changes" drop-down button (which has the "submit" dialog) to vanish when I type comments. Weird. Navigating away from the review-in-progress and then going back makes it reappear.

@ghost
Copy link

ghost commented Mar 21, 2022

Hi, I can't figure out how to suggest specific changes.

Please consider the changes found in the topmost commit of this branch:

https://github.com/a-m-joseph/nixpkgs/tree/mic92-fix-platforms

A lot of the messiness here is because nixpkgs treats gnu 5-tuples as 4-tuples, and mixes up the libc with the abi. So we have a lot of code duplication where adding an abi means you have to add it multiple times, once for each libc.

ghost
ghost suggested changes Mar 21, 2022
View reviewed changes
lib/systems/doubles.nix
@@ -28,6 +28,7 @@ let
"aarch64-linux" "armv5tel-linux" "armv6l-linux" "armv7a-linux"
"armv7l-linux" "i686-linux" "m68k-linux" "mipsel-linux" "mips64el-linux"
"powerpc64-linux" "powerpc64le-linux" "riscv32-linux"
"mips-linux" "mips64-linux"
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By adding these two tuples to the list in lib/systems/doubles.nix you are telling nixpkgs that they evaluate. However, as you've noticed, they don't evaluate :) So, you should not probably not add them here until things evaluate.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They evaluate and build stuff, see https://hercules-ci.com/github/nix-community/cross-toolchains.nix/jobs/79

lib/systems/examples.nix Show resolved Hide resolved
lib/systems/examples.nix Show resolved Hide resolved
lib/systems/examples.nix Show resolved Hide resolved
lib/systems/platforms.nix Outdated
gcc_mips32r2_o32 = { gcc = { arch = "mips32r2"; abi = "o32"; }; };
gcc_mips32r6_o32 = { gcc = { arch = "mips32r6"; abi = "o32"; }; };
gcc_mips32r2_o32 = { gcc = { arch = "mips32r2"; }; };
gcc_mips32r6_o32 = { gcc = { arch = "mips32r6"; }; };
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did you remove the abi attribute? Do you believe it is set incorrectly?

See also this, which might at least partially resolve any errors you may have encountered while trying to use abi="o32".

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The commit messages points to an gentoo issue, where they choose a similar ABI: https://bugs.gentoo.org/515694#c2
With the current abi target gcc does not compile.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See #170736

lib/tests/systems.nix Show resolved Hide resolved
lib/tests/systems.nix
@@ -28,7 +28,7 @@ with lib.systems.doubles; lib.runTests {
testredox = mseteq redox [ "x86_64-redox" ];
testgnu = mseteq gnu (linux /* ++ kfreebsd ++ ... */);
testillumos = mseteq illumos [ "x86_64-solaris" ];
testlinux = mseteq linux [ "aarch64-linux" "armv5tel-linux" "armv6l-linux" "armv7a-linux" "armv7l-linux" "i686-linux" "mips64el-linux" "mipsel-linux" "riscv32-linux" "riscv64-linux" "x86_64-linux" "powerpc64-linux" "powerpc64le-linux" "m68k-linux" "s390-linux" "s390x-linux" ];
testlinux = mseteq linux [ "aarch64-linux" "armv5tel-linux" "armv6l-linux" "armv7a-linux" "armv7l-linux" "i686-linux" "mips64el-linux" "mipsel-linux" "riscv32-linux" "riscv64-linux" "x86_64-linux" "powerpc64-linux" "powerpc64le-linux" "m68k-linux" "s390-linux" "s390x-linux" "mips-linux" "mips64-linux" ];
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Likewise.

This is my first time using github for a review; is there some way I can indicate that one of my review comments applies to multiple lines in a given file, or to multiple chunks of code scattered across multiple files?

@ghost
Copy link

ghost commented Mar 21, 2022 •
edited by ghost
Loading

https://hercules-ci.com/github/nix-community/cross-toolchains.nix/jobs/75

Can you provide a nix-env -E expression that I can use to see if I've fixed the problem you're experiencing?

ghost
ghost suggested changes Mar 21, 2022
View reviewed changes
Copy link

@ghost ghost left a comment •
edited by ghost
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still can't figure out how to tell github that I suggest these changes.

$ git push git@github:NixOS/nixpkgs pull/165020/head
ERROR: Permission to NixOS/nixpkgs.git denied to a-m-joseph.
fatal: Could not read from remote repository.

Please make sure you have the correct access rights
and the repository exists.

@Mic92
Copy link
Member Author

Mic92 commented Mar 21, 2022

https://hercules-ci.com/github/nix-community/cross-toolchains.nix/jobs/75

Can you provide a nix-env -E expression that I can use to see if I've fixed the problem you're experiencing?

The code is here: https://github.com/nix-community/cross-toolchains.nix/blob/main/flake.nix

Mic92
Mic92 commented Mar 21, 2022
View reviewed changes
lib/systems/doubles.nix Outdated Show resolved Hide resolved
@Mic92 Mic92 force-pushed the fix-platforms branch 2 times, most recently from 104b9ac to 1471db9 Compare March 21, 2022 08:53
@Mic92
Copy link
Member Author

Mic92 commented Mar 21, 2022

No longer interested in investing time into MIPS.

@Mic92 Mic92 closed this Mar 21, 2022
@ghost ghost mentioned this pull request Mar 21, 2022
Merged
13 tasks
@ghost
Copy link

ghost commented Apr 28, 2022

See NixOS/nix#6456 (comment)

Cross-builds to mips32 are now working, and the nix interpreter seccomp filters have been updated to allow mips32 binaries to execute in a sandbox on a mips64 host.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zowoq zowoq zowoq left review comments

@Ericson2314 Ericson2314 Awaiting requested review from Ericson2314

@matthewbauer matthewbauer Awaiting requested review from matthewbauer

Assignees
No one assigned
Labels
6.topic: golang 10.rebuild-darwin: 0 10.rebuild-linux: 1-10
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Mic92 @zowoq