Skip to content

Fix dependencies for iog-full flavor #175

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

Merged
merged 1 commit into from
Dec 18, 2024

Conversation

sgillespie
Copy link
Contributor

@sgillespie sgillespie commented Dec 17, 2024

  • Use correct variant of optional/optionals
  • Clean up formatting

Copy link
Collaborator

@angerman angerman left a comment

Choose a reason for hiding this comment

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

That's weird.

@andreabedini
Copy link
Member

andreabedini commented Dec 18, 2024

optional :: bool -> a -> [a] so isn't there an extra list in both the new and the old version?

++ map pkgs.lib.getDev (with pkgs; [ libblst libsodium-vrf secp256k1 icu ]) # :: [Derivation]
++ lib.optional withIOGFull
   (map pkgs.lib.getDev
     (with pkgs; [ (if stdenv.hostPlatform.isAarch64 then null else R) postgresql ]) # :: [Derivation]
   ) # :: [[Derivation]

Maybe optionals is what should use?

@andreabedini
Copy link
Member

Also, I might be wrong but I don't think we need to use getDev for buildInputs, as they are implied.

@andreabedini
Copy link
Member

andreabedini commented Dec 18, 2024

Right above there is

with pkgs; [ (pkgs.pkg-config or pkgconfig) ... ]

I find it hard to read. Is that supposed to mean pkgs.pkg-config or pkgs.pkgconfig?
Edit: It looks like pkg-config is in nixpkgs but pkgconfig is not. Why do we need this?

@andreabedini
Copy link
Member

This is my take:

  buildInputs =
    let
      inherit (pkgs) lib stdenv;
      inherit (lib) attrValues optional optionals;
    in
    [
      wrapped-cabal
      fixup-nix-deps
      compiler
      # for libstdc++; ghc not being able to find this properly is bad,
      # it _should_ probably call out to a g++ or clang++ but doesn't.
      stdenv.cc.cc.lib
    ]
    ++ (with pkgs; [
      openssl
      pcre
      pkg-config
      zlib
    ])
    ++ optional stdenv.hostPlatform.isLinux pkgs.systemd
    ++ optionals withIOG (
      with pkgs;
      [
        cddl
        cbor-diag
        gh
        jq
        yq-go
        libblst
        libsodium-vrf
        secp256k1
        icu
      ]
      ++ optionals withIOGFull (
        [ postgresql ] # for db-sync
        ++ optional stdenv.hostPlatform.isAarch64 R
      )
    )
    ++ attrValues haskell-tools;

@sgillespie
Copy link
Contributor Author

optional :: bool -> a -> [a] so isn't there an extra list in both the new and the old version?

++ map pkgs.lib.getDev (with pkgs; [ libblst libsodium-vrf secp256k1 icu ]) # :: [Derivation]
++ lib.optional withIOGFull
   (map pkgs.lib.getDev
     (with pkgs; [ (if stdenv.hostPlatform.isAarch64 then null else R) postgresql ]) # :: [Derivation]
   ) # :: [[Derivation]

Maybe optionals is what should use?

Ah, you're right. I was mistakenly looking at the docs for optionals

@sgillespie
Copy link
Contributor Author

Right above there is

with pkgs; [ (pkgs.pkg-config or pkgconfig) ... ]

I find it hard to read. Is that supposed to mean pkgs.pkg-config or pkgs.pkgconfig? Edit: It looks like pkg-config is in nixpkgs but pkgconfig is not. Why do we need this?

IIRC pkgconfig was removed recently-ish. I don't see why we still need pkgconfig in here.

 * Use correct variant of optional/optionals
 * Clean up formatting
@sgillespie sgillespie changed the title fix: Update getDev references Fix dependencies for iog-full flavor Dec 18, 2024
@sgillespie sgillespie added this pull request to the merge queue Dec 18, 2024
Merged via the queue into input-output-hk:main with commit 3493951 Dec 18, 2024
1 check passed
@hamishmack hamishmack mentioned this pull request Apr 7, 2025
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.

3 participants