Skip to content

Conversation

@Artturin
Copy link
Member

Description of changes
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.

@Artturin Artturin force-pushed the bintoolswrappermold branch from 2e2624c to 28e766c Compare February 14, 2023 22:23
@Artturin Artturin mentioned this pull request Feb 14, 2023
15 tasks
@ofborg ofborg bot added 10.rebuild-darwin-stdenv This PR causes stdenv to rebuild on Darwin and must target a staging branch. 10.rebuild-linux-stdenv This PR causes stdenv to rebuild on Linux and must target a staging branch. labels Feb 15, 2023
@ofborg ofborg bot added 10.rebuild-darwin: 501+ This PR causes many rebuilds on Darwin and should normally target the staging branches. 10.rebuild-darwin: 5001+ This PR causes many rebuilds on Darwin and must target the staging branches. 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches. 10.rebuild-linux: 5001+ This PR causes many rebuilds on Linux and must target the staging branches. labels Feb 15, 2023
@Ericson2314 Ericson2314 changed the title wrapBintoolsWith: wrap ld.mold bintools-wrapper: wrap ld.mold Feb 15, 2023
@Artturin Artturin force-pushed the bintoolswrappermold branch 3 times, most recently from e4872a4 to 9fba3af Compare February 15, 2023 17:33
allows using wrapBintoolsWith with all linkers

```
$ nix build ".#binutils"
$ ls ./result/bin/ld*
./result/bin/ld*  ./result/bin/ld.bfd*  ./result/bin/ld.gold*

$ nix build "nixpkgs#binutils"
$ ls ./result/bin/ld*
./result/bin/ld*  ./result/bin/ld.bfd* ./result/bin/ld.gold*
```
@Artturin Artturin force-pushed the bintoolswrappermold branch from e5385bc to 3697dde Compare February 15, 2023 18:51
@Artturin Artturin changed the title bintools-wrapper: wrap ld.mold bintools-wrapper: wral all 'ld.*' Feb 15, 2023
@Artturin Artturin changed the title bintools-wrapper: wral all 'ld.*' bintools-wrapper: wrap all 'ld.*' Feb 15, 2023
@Artturin
Copy link
Member Author

Artturin commented Feb 15, 2023

binutils works

$ nix build ".#binutils"
$ ls ./result/bin/ld*
./result/bin/ld*  ./result/bin/ld.bfd*  ./result/bin/ld.gold*

$ nix build "nixpkgs#binutils"
$ ls ./result/bin/ld*
./result/bin/ld*  ./result/bin/ld.bfd* ./result/bin/ld.gold*

$ ./result/bin/ld
/nix/store/sc0h1nnb9rxbiknq692dq9n3wwsrhbbl-binutils-2.40/bin/ld: no input files
$ ./result/bin/ld.bfd
/nix/store/sc0h1nnb9rxbiknq692dq9n3wwsrhbbl-binutils-2.40/bin/ld.bfd: no input files
$ ./result/bin/ld.gold
/nix/store/sc0h1nnb9rxbiknq692dq9n3wwsrhbbl-binutils-2.40/bin/ld.gold: fatal error: no input files

bin/ld in wrapped mold fails

$ nix build --impure --expr "with import ./. {}; wrapBintoolsWith { bintools = mold; }"
$ ls ./result/bin
ld*  ld.mold*
$ ./result/bin/ld.mold
mold: fatal: -m option is missing
$ ./result/bin/ld
./result/bin/ld: line 254: /nix/store/nkxfnbmkpv53y7fibkxq4c9aq21ik606-mold-1.10.1/bin/ld: No such file or directory

wrap ${targetPrefix}ld ${./ld-wrapper.sh} ''${ld:-$ldPath/${targetPrefix}ld}

@Ericson2314
Copy link
Member

So I guess there are two issues:

  1. Mold doesn't install an ld
  2. Mold doesn't support all the flags our wrapper uses yet

Right?

@Artturin
Copy link
Member Author

Artturin commented Feb 15, 2023

They install a ld in libexec

https://github.com/rui314/mold/blob/main/CMakeLists.txt#L453

$ tree result
result
├── bin
│   ├── ld.mold -> mold
│   └── mold
├── lib
│   └── mold
│       └── mold-wrapper.so
├── libexec
│   └── mold
│       └── ld -> ../../bin/mold
└── share
    ├── doc
    │   └── mold
    │       └── LICENSE
    └── man
        └── man1
            ├── ld.mold.1.gz -> mold.1.gz
            └── mold.1.gz

perhaps we could just add a ld in bin

@Ericson2314
Copy link
Member

Ericson2314 commented Feb 15, 2023

That sounds good. The wrapper could also "promote" one of the ld.* if there is no ld.

@Artturin Artturin requested review from azahi and nitsky February 15, 2023 20:29
@azahi
Copy link
Member

azahi commented Feb 15, 2023

Mold doesn't install an ld

I guess we can just apply this patch in the scope of this PR to avoid opening more PRs.

diff --git i/pkgs/development/tools/mold/default.nix w/pkgs/development/tools/mold/default.nix
index bb55ba80796..4b2b35fb26f 100644
--- i/pkgs/development/tools/mold/default.nix
+++ w/pkgs/development/tools/mold/default.nix
@@ -1,6 +1,7 @@
 { lib
 , stdenv
 , fetchFromGitHub
+, makeWrapper
 , cmake
 , mimalloc
 , ninja
@@ -45,6 +46,10 @@ stdenv.mkDerivation rec {
     "-faligned-allocation"
   ];
 
+  postInstall = ''
+    makeWrapper "$out/bin/mold" "$out/bin/ld"
+  '';
+
   passthru.tests.version = testers.testVersion { package = mold; };
 
   meta = with lib; {

Mold doesn't support all the flags our wrapper uses yet

I'm not sure about a 100% flag parity to GNU ld, but I've used mold as a drop-in replacement for it in a few relatively big projects for a while now (not on NixOS) and had alsmost no issues.

@Ericson2314
Copy link
Member

The fact that they have an ld.mold indicates to me they at least intend to hit feature parity. That is reassuring ;).

@ofborg ofborg bot requested a review from Ericson2314 February 16, 2023 04:43
not all linkers have a ld binary in bin

also note the '${ld:-}' which allows users to set the ld path with a env
var

> '${foo:-val}' $foo, or val if unset (or null)
@Artturin Artturin force-pushed the bintoolswrappermold branch from eff450b to b5abc3d Compare February 17, 2023 03:00
@github-actions github-actions bot added the 6.topic: stdenv Standard environment label Feb 17, 2023
@Artturin Artturin changed the title bintools-wrapper: wrap all 'ld.*' stdenvAdapters: add useMoldLinker Feb 17, 2023
@Artturin Artturin force-pushed the bintoolswrappermold branch from 2465936 to a159267 Compare February 17, 2023 03:33
@Artturin Artturin force-pushed the bintoolswrappermold branch from a159267 to 299a7bd Compare February 17, 2023 04:35
@Artturin
Copy link
Member Author

i removed mold: symlink ld to ld.mold for compat with bintools-wrapper because its not necessary now

this works and the resulting binaries have mold 1.10.1 (compatible with GNU ld)

diff --git a/pkgs/tools/misc/man-db/default.nix b/pkgs/tools/misc/man-db/default.nix
index 2bdd49f672f..8976d3314cb 100644
--- a/pkgs/tools/misc/man-db/default.nix
+++ b/pkgs/tools/misc/man-db/default.nix
@@ -12,9 +12,14 @@
 , stdenv
 , zstd
 , autoreconfHook
+, stdenvAdapters
 }:
 
-stdenv.mkDerivation rec {
+let
+  stdenv' = stdenvAdapters.useMoldLinker stdenv;
+in
+
+stdenv'.mkDerivation rec {
   pname = "man-db";
   version = "2.11.1";

@Artturin Artturin requested a review from VitalyAnkh February 17, 2023 04:38
@ofborg ofborg bot removed the request for review from VitalyAnkh February 17, 2023 04:46
@ofborg ofborg bot requested a review from Ericson2314 February 17, 2023 05:20
@Artturin Artturin merged commit ee54eb7 into NixOS:staging Feb 17, 2023
@Artturin Artturin deleted the bintoolswrappermold branch February 17, 2023 17:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

6.topic: stdenv Standard environment 10.rebuild-darwin: 501+ This PR causes many rebuilds on Darwin and should normally target the staging branches. 10.rebuild-darwin: 5001+ This PR causes many rebuilds on Darwin and must target the staging branches. 10.rebuild-darwin-stdenv This PR causes stdenv to rebuild on Darwin and must target a staging branch. 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches. 10.rebuild-linux: 5001+ This PR causes many rebuilds on Linux and must target the staging branches. 10.rebuild-linux-stdenv This PR causes stdenv to rebuild on Linux and must target a staging branch.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants