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

rust: add hasLibraries and hasIncludes #119

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 42 additions & 2 deletions extra/language/rust.nix
Original file line number Diff line number Diff line change
@@ -1,10 +1,26 @@
{ lib, config, pkgs, ... }:
let
cfg = config.language.rust;
strOrPackage = import ../../nix/strOrPackage.nix { inherit lib pkgs; };

hasLibraries = lib.length cfg.libraries > 0;
hasIncludes = lib.length cfg.includes > 0;
in
with lib;
{
options.language.rust = {
libraries = mkOption {
type = types.listOf strOrPackage;
default = [ ];
description = "Use this when another language dependens on a dynamic library";
Copy link
Member

Choose a reason for hiding this comment

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

Typo, but I still don't understand what this means.

Suggested change
description = "Use this when another language dependens on a dynamic library";
description = "Use this when another language depends on a dynamic library";

What would be another language? Can you make an example.

Choose a reason for hiding this comment

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

I think "another language" isn't really necessary (and a holdover from the equivalent C option), this should really be for any non-crate library dependencies (i.e. anything that would need pkg-config).

};

includes = mkOption {
type = types.listOf strOrPackage;
default = [ ];
description = "Rust dependencies from nixpkgs";
Copy link
Member

@Mic92 Mic92 Jun 3, 2021

Choose a reason for hiding this comment

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

Suggested change
description = "Rust dependencies from nixpkgs";
description = "Dependencies on C headers to provided by nixpkgs";

};

packageSet = mkOption {
# FIXME: how to make the selection possible in TOML?
type = types.attrs;
Expand All @@ -30,8 +46,32 @@ with lib;
# Used by tools like rust-analyzer
name = "RUST_SRC_PATH";
value = toString cfg.packageSet.rustPlatform.rustLibSrc;
}];
}]
++
(lib.optionals hasLibraries [
Copy link
Member

Choose a reason for hiding this comment

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

Do we actually want to make the separation into libraries/includes? Would it not be easier to have just one package options that adds both?

Choose a reason for hiding this comment

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

Since includes were for C and specifically C_INCLUDE_PATH, I don't see how it's necessary to port that to the rust options.

{
name = "LD_LIBRARY_PATH";
prefix = "$DEVSHELL_DIR/lib";
}
])
++ lib.optionals hasIncludes [
{
name = "LD_INCLUDE_PATH";
bbigras marked this conversation as resolved.
Show resolved Hide resolved
prefix = "$DEVSHELL_DIR/include";
}
{
name = "PKG_CONFIG_PATH";
Copy link
Member

Choose a reason for hiding this comment

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

Don't libraries also depend on PKG_CONFIG_PATH some time?

prefix = "$DEVSHELL_DIR/lib/pkgconfig";
}
];

devshell.packages = map (tool: cfg.packageSet.${tool}) cfg.tools;
devshell.packages =
(lib.optionals hasLibraries (map lib.getLib cfg.libraries))
++
# Assume we want pkg-config, because it's good
(lib.optionals hasIncludes ([ pkgs.pkg-config ] ++ (map lib.getDev cfg.includes)))
++
(map (tool: cfg.packageSet.${tool}) cfg.tools)
;
};
}