-
Notifications
You must be signed in to change notification settings - Fork 10
Add missing libblst to GHC options of wrapped-cabal in static mode. #125
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
Conversation
|
@yvan-sraka can you test this? @KtorZ mentioned this did not work 😰 I have this on my list to test tomorrow, but if you can before me that would be great! |
|
I got this issue (is this similar to what you got @KtorZ?): I "fixed" it with this patch: diff --git a/flake.nix b/flake.nix
index 95a8722..993df4e 100644
--- a/flake.nix
+++ b/flake.nix
@@ -23,7 +23,7 @@
static-openssl = (final.openssl.override { static = true; });
static-zlib = final.zlib.override { shared = false; };
static-pcre = final.pcre.override { shared = false; };
- static-libblst = (final.libblst.override { enableShared = false; }).overrideDerivation (old: {
+ static-libblst = (final.libblst.override { }).overrideDerivation (old: {
postFixup = "";
});
});Maybe we should rather update the |
That's exactly what I ran into. |
|
Then, I believe that what you want to do is something more like: diff --git a/flake.nix b/flake.nix
index 95a8722..438a181 100644
--- a/flake.nix
+++ b/flake.nix
@@ -23,7 +23,8 @@
static-openssl = (final.openssl.override { static = true; });
static-zlib = final.zlib.override { shared = false; };
static-pcre = final.pcre.override { shared = false; };
- static-libblst = (final.libblst.override { enableShared = false; }).overrideDerivation (old: {
+ static-libblst = final.libblst.overrideDerivation (old: {
+ configureFlags = old.configureFlags ++ [ "--enable-static" "--disable-shared" ];
postFixup = "";
});
});Do you have a minimal example project, so I can test if it builds as expected? |
|
Ahh, so maybe the issue is that we don't pull the blst and friends from IOHK-nix? But rely on nixpkgs upstream? I know that for IOG projects the cryptography folks advised against using nixpkgs releases and default to our vendor release ones. |
Not really minimal but I am experimenting this in kupo. This latest patch seems slightly better but it'll still expect the blst library to be shipped alongside the executable: |
|
Fun note 1 (for some definition of fun): if I compile Kupo on my local dev environment, outside of the devx nix-shell, libblst is nowhere to be seen in the linked dependencies. Which is curious (I double-check and I am using the same compiler version on both, and I'd expect the constraints on the cabal.project to yield a same cardano-ledger and plutus versions that either require blst or not). Fun note 2: when compiling through the devx shell, the linker warns me about "was built for newer macOS version (12.0) than being linked (11.0)" |
|
@yvan-sraka can you please try to figure out with @KtorZ why his local dev env supposedly doesn't have/need libblst? That seems odd to me. plutus-core -> cardano-crypto-class -> libblst I would expect that dependency to be there? Side note: Regarding the linker warning, that's not very problematic, it's just that nix passes some deploymen target. I'm fairly sure that warning can be ignored. But would be nice to clean that up as well. |
I believe it depends on what version of the ledger/plutus gets bundled in. Which points to: https://github.com/IntersectMBO/cardano-base/blob/a4b01d1b990fab63505f2c20b79ccbbed033f3fc/cardano-crypto-class/cardano-crypto-class.cabal#L115-L125. Could it be that it's an old enough plutus-core package with only foundational work on BLS but unused; thus stripped by GHC somehow? Is the compiler smart-enough to do this kind of thing 😅 ? |
|
For dynamic libraries on macOS, we do indeed deadstripping of dynamic libraries. |
|
We use the same code paths in nix as well though. Hence I wouldn't expect that to result in any difference. |
|
I checked and the dependencies selected (cabal freeze) are the same on my local dev env and within the nix shell. The only difference is that I am using: on my local setup. Though I don't see why this would impact the need for libblst 🤔. Anyway, I'll be looking into this tomorrow morning with @yvan-sraka |
|
Are you certain that flag is only used locally? Or should always be used. Relying on the vrf hack is a bad idea. It should at best be used in development. |
The current buildPhase in NixOS/nixpkgs does something like this:
```
./build.sh ${lib.optionalString stdenv.hostPlatform.isWindows "flavour=mingw64"}
./build.sh -shared ${lib.optionalString stdenv.hostPlatform.isWindows "flavour=mingw64"}
```
So we do build both a dynamic and static version, for convenience,
which is then correctly visible in the nix store (both `.a` and
`.dylib` are present).
BUT, it seems that because of the presence of the dynamic lib, cabal
or GHC (?) still chose to bundle the library dynamically despite the
static flags. Removing the `.dylib` from the nix store recover the
expected behavior (libblst being statically linked).
This is odd, but the current fix is to not build the `-shared` version
of the library at all when in the static shell.
Co-authored-by: yvan-sraka <yvan@sraka.xyz>
| static-zlib = final.zlib.override { shared = false; }; | ||
| static-pcre = final.pcre.override { shared = false; }; | ||
| static-libblst = final.libblst.overrideDerivation (old: { | ||
| configureFlags = old.configureFlags ++ [ "--enable-static" "--disable-shared" ]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@hamishmack IIRC, you encountered a similar issue: even when we explicitly tell Cabal to statically link libblst, it links it dynamically if pkgconfig also declares a dynamic library... So, we override the buildPhase and postFixup to prevent the generation of the .dylib file, and then Cabal behaves as expected... This seems to occur only on macOS? Is there a related Cabal issue already opened?
|
@KtorZ could you merge CardanoSolutions#1, and then we could fix the tests of this PR :) |
Enforce the use of static libraries in `-static` devshell
|
@yvan-sraka I guess this one is good to go? Can you have a last look and then merge it? |
No description provided.