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

openssl: fix mips64 abi detection when not cross compiling #165746

Merged
merged 1 commit into from Apr 11, 2022
Merged

openssl: fix mips64 abi detection when not cross compiling #165746

merged 1 commit into from Apr 11, 2022

Conversation

ghost
Copy link

@ghost ghost commented Mar 25, 2022

When not cross-compiling, OpenSSL will not attempt to detect the
host ABI. For mips64, the OpenSSL authors have chosen to assume that
the n32 ABI is used.

Since nixpkgs knows the correct ABI based on stdenv.hostPlatform,
let's pass this information to OpenSSL explicitly.

At the moment (bootstrappable) nixpkgs on mips64 can only be used with
the n64 ABI due to the fact that boost-context (required by nix) does
not support the n32 ABI. Without this commit the openssl expression
can be cross-compiled to a mips64 host, but a mips64 host cannot
self-compile the expression due to OpenSSL's incorrect assumption.

Description of changes
Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
    • mips64el-linux-gnuabi64
  • 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.

@SuperSandro2000
Copy link
Member

Can you please amend to the commit with no changes to trigger CI?

@ghost
Copy link
Author

ghost commented Mar 28, 2022

Done (I added an extra newline to the commit message).

@SuperSandro2000
Copy link
Member

Done (I added an extra newline to the commit message).

Acutally you don't need to change anything. Just amending without any changes generates a new hash and that is the only thing we need.

Copy link
Member

@SuperSandro2000 SuperSandro2000 left a comment

Choose a reason for hiding this comment

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

LGTM but I would like another person to take a look.

@ghost
Copy link
Author

ghost commented Mar 28, 2022

Acutally you don't need to change anything. Just amending without any changes generates a new hash and that is the only thing we need.

Oh right, because of the commit date. Thanks, I hadn't thought of that.

LGTM but I would like another person to take a look.

Cool; since you know the community better than I do could you suggest somebody to ping? The package lists @cstrahan as maintainer, so I guess that person is a good start.

Is there some way for me to add people to the "reviews requested from" list? Or do I need superGithubPowers to do that?

@ghost ghost requested a review from sternenseemann March 29, 2022 21:49
@sternenseemann
Copy link
Member

@ofborg build pkgsCross.mips64el-linux-gnuabi64.openssl pkgsCross.mips64el-linux-gnuabin32.openssl

@ghost
Copy link
Author

ghost commented Mar 29, 2022

@ofborg build pkgsCross.mips64el-linux-gnuabi64.openssl pkgsCross.mips64el-linux-gnuabin32.openssl

By the way, now that quite a bit of stuff in pkgsCross.mips64el-linux-gnuabi64 is able to build (from an x86_64 host), can you tell me which .nix file I should add stuff to in order to instruct ofborg to build these packages regularly? That way I will notice if it stops working, and more importantly I'll know down to a reasonably small range of commits which one caused it to stop working.

Obviously it is a separate decision whether or not to merge those additions to the .nix files, but right now I don't even know where to add stuff in order to submit a PR. How ofborg knows what to do when it wakes up each morning still seems a bit mysterious to me.

@sternenseemann
Copy link
Member

sternenseemann commented Mar 29, 2022

Obviously it is a separate decision whether or not to merge those additions to the .nix files, but right now I don't even know where to add stuff in order to submit a PR. How ofborg knows what to do when it wakes up each morning still seems a bit mysterious to me.

ofborg parses commit messages and builds the attribute path mentioned in them before the colon if it has changed. Whether the attribute path has changed, is determined using nix-env (and ofborg/src/outpaths.nix, I think). This means that cross compilation of packages is never tested as things stand (because ofborg doesn't evaluate pkgsCross, as you've experienced).

Also ofborg doesn't continuously build stuff and notifies ppl about breakages, it exclusively tests PRs.

If you haven't already, you should update pkgs/top-level/release-cross.nix so that Hydra cross builds linuxCommon packages for mipsel platforms.

@ghost
Copy link
Author

ghost commented Mar 30, 2022

If you haven't already, you should update pkgs/top-level/release-cross.nix so that Hydra cross builds linuxCommon packages for mipsel platforms.

Ah, that's exactly what I was looking for. Thanks!

Copy link
Member

@SuperSandro2000 SuperSandro2000 left a comment

Choose a reason for hiding this comment

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

I think we can squash merge this.

@ghost
Copy link
Author

ghost commented Apr 11, 2022

I think we can squash merge this.

Squashed.

When *not* cross-compiling, OpenSSL will not attempt to detect the
host ABI.  For mips64, the OpenSSL authors have chosen to assume that
the n32 ABI is used.

Since nixpkgs knows the correct ABI based on stdenv.hostPlatform,
let's pass this information to OpenSSL explicitly.

At the moment (bootstrappable) nixpkgs on mips64 can only be used with
the n64 ABI due to the fact that boost-context (required by nix) does
not support the n32 ABI.  Without this commit the openssl expression
can be cross-compiled to a mips64 host, but a mips64 host cannot
self-compile the expression due to OpenSSL's incorrect assumption.

#165746 (review)
@sternenseemann sternenseemann merged commit a985b2b into NixOS:master Apr 11, 2022
@ghost ghost deleted the openssl-fix-mips64-abi-detection-when-not-cross-compiling branch April 12, 2022 04:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants