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

swtpm: fix build on darwin #163569

Merged
merged 2 commits into from
Mar 13, 2022
Merged

Conversation

willcohen
Copy link
Contributor

@willcohen willcohen commented Mar 10, 2022

Description of changes

Fix build on darwin. Note that because netstat still uses openssl-1.0.2u on darwin, building this requires NIXPKGS_ALLOW_INSECURE=1. See #101229 and #150880.

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.

@ofborg ofborg bot added 6.topic: darwin Running or building packages on Darwin 10.rebuild-darwin: 0 10.rebuild-linux: 1-10 labels Mar 10, 2022
@willcohen
Copy link
Contributor Author

@baloo

@willcohen willcohen requested a review from Luflosi March 10, 2022 15:41
@Luflosi
Copy link
Contributor

Luflosi commented Mar 10, 2022

I fixed all failing tests. For some reason I didn't need to touch python at all. Here is the full diff, which could replace this PR:

index 648165d8262..fae491d00db 100644
--- a/pkgs/tools/security/swtpm/default.nix
+++ b/pkgs/tools/security/swtpm/default.nix
@@ -37,14 +37,18 @@ stdenv.mkDerivation rec {
 
   buildInputs = [
     libtpms
-    openssl libtasn1 libseccomp
-    fuse glib json-glib
+    openssl libtasn1
+    glib json-glib
     gnutls
+  ] ++ lib.optionals stdenv.isLinux [
+    fuse
+    libseccomp
   ];
 
   configureFlags = [
-    "--with-cuse"
     "--localstatedir=/var"
+  ] ++ lib.optionals stdenv.isLinux [
+    "--with-cuse"
   ];
 
   postPatch = ''
@@ -56,9 +60,31 @@ stdenv.mkDerivation rec {
 
     # Use the correct path to the certtool binary
     # instead of relying on it being in the environment
-    substituteInPlace src/swtpm_localca/swtpm_localca.c --replace \
+    substituteInPlace src/swtpm_localca/swtpm_localca.c \
+      --replace \
+        '# define CERTTOOL_NAME "gnutls-certtool"' \
+        '# define CERTTOOL_NAME "${gnutls}/bin/certtool"' \
+      --replace \
         '# define CERTTOOL_NAME "certtool"' \
         '# define CERTTOOL_NAME "${gnutls}/bin/certtool"'
+
+    substituteInPlace tests/common --replace \
+        'CERTTOOL=gnutls-certtool;;' \
+        'CERTTOOL=certtool;;'
+
+    # Fix error on macOS:
+    # stat: invalid option -- '%'
+    # This is caused by the stat program not being the BSD version,
+    # as is expected by the test
+    substituteInPlace tests/common --replace \
+        'if [[ "$(uname -s)" =~ (Linux|CYGWIN_NT-) ]]; then' \
+        'if [[ "$(uname -s)" =~ (Linux|Darwin|CYGWIN_NT-) ]]; then'
+
+    # Otherwise certtool seems to pick up the system language on macOS,
+    # which might cause a test to fail
+    substituteInPlace tests/test_swtpm_setup_create_cert --replace \
+        '$CERTTOOL' \
+        'LC_ALL=C.UTF-8 $CERTTOOL'
   '';
 
   doCheck = true;

I did not test on Linux, please do.
Any idea why you need to change the python dependency but I don't? Are you running on Apple silicon? Do you have the sandbox enabled?

@baloo
Copy link
Member

baloo commented Mar 10, 2022

swtpm> ============================================================================
swtpm> Testsuite summary for swtpm 0.7.1
swtpm> ============================================================================
swtpm> # TOTAL: 68
swtpm> # PASS:  56
swtpm> # SKIP:  12
swtpm> # XFAIL: 0
swtpm> # FAIL:  0
swtpm> # XPASS: 0
swtpm> # ERROR: 0
swtpm> ============================================================================

👍

@baloo
Copy link
Member

baloo commented Mar 10, 2022

@Luflosi

@willcohen needed python because it's only provided if you run tests (which he disabled on macos). Since you're running tests, python is provided.

I still believe it makes sense to bring python in nativeBuildInputs as it fixes cross compilation too (this is checked by autoconf, as will showed).

@Luflosi
Copy link
Contributor

Luflosi commented Mar 10, 2022

Ok, then 👍 on moving python to nativeBuildInputs.

@willcohen
Copy link
Contributor Author

Lovely. Will revise. Many thanks!

@willcohen
Copy link
Contributor Author

willcohen commented Mar 10, 2022

Result of nixpkgs-review pr 163569 run on x86_64-darwin 1

Edit: right, because of openssl.

@willcohen
Copy link
Contributor Author

Result of nixpkgs-review pr 163569 run on x86_64-linux 1

2 packages built:
  • quickemu
  • swtpm

@willcohen
Copy link
Contributor Author

@alyssais does this look good to merge? Not directly related to 9p for Darwin, but still useful for QEMU

@baloo
Copy link
Member

baloo commented Mar 11, 2022

thanks to both of you!

Copy link
Contributor

@Luflosi Luflosi left a comment

Choose a reason for hiding this comment

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

LGTM

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-already-reviewed/2617/461

@SuperSandro2000 SuperSandro2000 merged commit 218f367 into NixOS:master Mar 13, 2022
@willcohen
Copy link
Contributor Author

Thanks @SuperSandro2000!

@willcohen willcohen deleted the swtpm-darwin branch March 14, 2022 13:05
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.

None yet

5 participants