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

Build with Nix #29

Open
wants to merge 27 commits into
base: master
Choose a base branch
from
Open

Build with Nix #29

wants to merge 27 commits into from

Conversation

quapka
Copy link
Collaborator

@quapka quapka commented Jun 21, 2024

Building with Nix is one way of allowing multiple versions of tested libs to be running at the same time. Currently, few versions of OpenSSL are possible, check the commit message for more details.

More work is needed before merging this.

Jan Kvapil and others added 5 commits March 26, 2024 09:42
Currently, only running different versions of OpenSSL is implemented as
PoC to see if it would work. You can either build it or run it directly
with:
```
$ nix run '.?submodules=1#openssl_316' list-libs # uses OpenSSL 3.1.6
$ nix run '.?submodules=1#openssl_322' list-libs # uses OpenSSL 3.2.2
$ nix run '.?submodules=1' list-libs # uses OpenSSL 3.3.1
```

Adding new versions is possible, check `packages` part of `flake.nix`.

To see the supported versions run:
```
$ nix flake show
```
Copy link
Member

@J08nY J08nY left a comment

Choose a reason for hiding this comment

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

Cool stuf, I will try it out. Could you also add a github workflow that does the build using nix? Basically take the test workflow, strip out the reader and applet stuff.

flake.nix Outdated Show resolved Hide resolved
flake.nix Outdated Show resolved Hide resolved
@J08nY
Copy link
Member

J08nY commented Jun 21, 2024

Oh and one more thing. Can you make the version of the library propagate as an environment variable to the build and run environment? Then in the Makefile we can pass it on as a define to the compiler and thus get it in the shim code so that we can do ifdefs based on it.

@quapka
Copy link
Collaborator Author

quapka commented Jun 21, 2024

Oh and one more thing. Can you make the version of the library propagate as an environment variable to the build and run environment? Then in the Makefile we can pass it on as a define to the compiler and thus get it in the shim code so that we can do ifdefs based on it.

Yes. Both should be somewhat simple and straightforward.

@codecov-commenter
Copy link

codecov-commenter commented Jul 22, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 67.49%. Comparing base (c112d3e) to head (bddeacb).

Additional details and impacted files
@@             Coverage Diff              @@
##             master      #29      +/-   ##
============================================
- Coverage     69.74%   67.49%   -2.26%     
- Complexity      430      436       +6     
============================================
  Files            55       55              
  Lines          3064     3064              
  Branches        377      377              
============================================
- Hits           2137     2068      -69     
- Misses          753      827      +74     
+ Partials        174      169       -5     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

In order to prevent problems with dynamic linking of OpenSSL, LibreSSL
and BoringSSL -- that all have libcrypto.so -- we link statically into
the providers.
''
cp $dev/lib/pkgconfig/libcrypto.pc $dev/lib/pkgconfig/libresslcrypto.pc
sed --in-place --expression 's/-lcrypto/-lresslcrypto/' $dev/lib/pkgconfig/libresslcrypto.pc
ln -s $out/lib/libcrypto.so $out/lib/libresslcrypto.so
Copy link
Member

Choose a reason for hiding this comment

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

patchelf necessary here. The library knows its soname, see:

readelf -d  libresslcrypto.so | grep SONAME

Will print out libcrypto.so which is wrong.

flake.nix Outdated
Comment on lines 113 to 118
# devLibPath = pkgs.lib.makeLibraryPath [ pkgs.libressl.dev ];
# libressl = pkgs.libressl.overrideAttrs (_old: {
# fixupPhase = ''
# cp ${devLibPath}/openssl.pc ${devLibPath}/libressl.pc
# '';
# });
Copy link
Member

Choose a reason for hiding this comment

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

This can be removed right?

Suggested change
# devLibPath = pkgs.lib.makeLibraryPath [ pkgs.libressl.dev ];
# libressl = pkgs.libressl.overrideAttrs (_old: {
# fixupPhase = ''
# cp ${devLibPath}/openssl.pc ${devLibPath}/libressl.pc
# '';
# });

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

As said below, there is still work to be done. Cleaning up and polishing is still down the pipeline.

makeWrapper \
${jdk17_headless}/bin/java $out/bin/${pname} \
--add-flags "-jar $out/build/libs/${pname}.jar" \
--set LD_LIBRARY_PATH ${LD_LIBRARY_PATH}:$LD_LIBRARY_PATH
Copy link
Member

Choose a reason for hiding this comment

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

Messing with LD_LIBRARY_PATH should not be necessary if statically linked shims are to be used.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, this is still a WIP.


boringssl.o: boringssl.c
$(CC) -I$(PROJECT_ROOT_PATH)/ext/boringssl/include/ $(CFLAGS) -c $<
NIX_CFLAGS_COMPILE= $(CC) -I$(BORINGSSL_CFLAGS) $(CFLAGS) -c $<
Copy link
Member

Choose a reason for hiding this comment

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

Ideally, this Makefile will still remain useful to a user that does not use nix. Can we keep that? Or back it up somewhere? Essentially I would like to keep a simple Makefile somewhere for someone that has the libraries installed system-wide (or locally) and just wants to build them.

The use case is also testing custom versions of the libraries and not having to fuff with nix.

Copy link
Member

Choose a reason for hiding this comment

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

By custom I mean when someone is actively working on them/patching them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants