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

Cannot detect local path dependencies #133

Open
Rizary opened this issue Nov 30, 2020 · 21 comments
Open

Cannot detect local path dependencies #133

Rizary opened this issue Nov 30, 2020 · 21 comments

Comments

@Rizary
Copy link

Rizary commented Nov 30, 2020

I have this dependency in my Cargo.toml:

[package]
name = "rust-backend"
version = "0.1.0"
authors = ["Andika Demas Riyandi <[email protected]>"]
edition = "2018"
build = "build.rs"

# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

[dependencies]
...
common = { path = "../common/", optional=false }
...

[build-dependencies]
ructe = { version = "0.12", features = ["sass", "tide013"] }

Since my common package is a local library, when I build my rust-backend I got the following error:

error: --- Error --------------------------------------------------------------------------------------------------- nix
builder for '/nix/store/mqg5569yzxxdxw8nfvq920lv41gpzwi3-rust-backend-deps-0.1.0.drv' failed with exit code 101; last 10 log lines:

  Caused by:
    Unable to update /tmp/nix-build-rust-backend-deps-0.1.0.drv-0/common

  Caused by:
    failed to read `/tmp/nix-build-rust-backend-deps-0.1.0.drv-0/common/Cargo.toml`

  Caused by:
    No such file or directory (os error 2)
  [naersk] cargo returned with exit code 101, exiting
error: --- Error --------------------------------------------------------------------------------------------------- nix

I tried with rustPlatform.buildRustPackage and the behavior is the same. is there something wrong with my setup?

You can find the project here: https://github.com/numtide/todomvc-nix/tree/flake

@goertzenator
Copy link
Contributor

I also ran into a failing path dependency:

[dependencies]
clap = "2.32"
libc = "0.2.81"
...
usbfs = {path = "usbfs-rs", version = "0.1"}

I see that there are multi-crate workspace tests for naersk but the crates are not depending on each other. Adding such a dependency would be a good way to observe this issue.

@Ekleog
Copy link
Contributor

Ekleog commented Dec 31, 2020

FWIW, within a workspace local path dependencies work without problems for me. What doesn't work is local path dependencies outside of workspaces.

In my case, I hit the issue while trying to build cargo itself with naersk: cargo is architectured as one root crate and then sub-crates in the same repo, without a top-level workspace crate, and thus naersk fails building it due to this reason.

While I see the general problem (especially ../ path dependencies) as (very?) hard to solve, as it requires fetching additional paths, IMO this problem could be solved for the “usual” case where the other crates are already inside $src.

@Rizary
Copy link
Author

Rizary commented Feb 15, 2021

@Ekleog I just got into this error (again), which in my case, workspaces is not a choice. Do you have any example that I can take a look? Especially for my other #150 issue. How do copySourcesFrom and copySource actually work?

@Ekleog
Copy link
Contributor

Ekleog commented Feb 17, 2021

Hmm… Does just adding copySources = ["crates"]; to the naersk call work for you? (If I read correctly #150 I think it probably should)

The way it works is just, that it copies the non-workspaced sources to naersk's first (dependency-only) build.

However, if all your source code is in crates, it means that you'll end up having to rebuild dependencies on each change, and I'm not sure how hard it'd be to fix that as I don't think cargo offers easy tooling around that. So if all or most of your changes affect the crates folder, you may be better off just setting singleStep = true.

Does that make sense?

TL;DR:

  • naersk's issue is that it builds all dependencies in the first step, but also removes all the source code in the first step so that the dep-only derivation doesn't get rebuilt on each code change
  • copySources allows overriding that by forcing some sources to be copied to the dependency-only source tree, thus allowing you to avoid the issue of building a crate emptied by naersk; to the cost of rebuilding all dependencies every time a copied file changes
  • setting singleStep to true removes the split between dep-only and actual-build altogether, which usually makes builds much slower, but could actually make them faster if you always change code in the copied files anyway

@Rizary
Copy link
Author

Rizary commented Feb 17, 2021

@Ekleog I've updated the question to clear some misunderstanding. So in your explanation, we should only use naersk's buildPackage in the root directory? What I'm trying to do is to have each default.nix on my package with buildPackage inside it.

To avoid workspace, I thought I should make that default.nix on each package folder and since there is a package that depends on another package, it can't find the dependencies.

@Ekleog
Copy link
Contributor

Ekleog commented Feb 19, 2021

This is currently correct, to the best of my understanding naersk not being able to find path dependencies outside of its (src =) root is this very issue (#133), which isn't solved yet and probably will be very hard to solve.

You can have a single default.nix that points to the root of the workspace (so long as your main package is in the root of the workspace), and it should work.

If all your packages are in subfolders and there is no Cargo.toml at the lowest common directory of all your path-dependency crates, I'm not sure naersk can currently handle this situation, unfortunately :/

@Rizary
Copy link
Author

Rizary commented Feb 21, 2021

Thank you, that clarifies my current situation.

@exFalso
Copy link
Contributor

exFalso commented Feb 25, 2021

There is a way to do this. In our project we've been using a home-rolled 2-phase build solution until a couple of weeks ago when we switched to naersk, and we had this exact issue. Long story short: naersk as-is currently only supports local crates in a common workspace. This has various disadvantages (the big one for us being that cargo merges conditional features into a single featureset for the whole workspace, so sometimes you simply can't put a crate in a workspace), so there's an alternate way to have non-workspace local dependencies in the form of copySources.

copySources introduces a very large build-invalidation surface as @Ekleog mentioned, to address this i submitted #147 but i'm not sure if/when it's going to be merged.

(Btw we also have "overlapping" crate dependencies for different projects, so we're actually creating workspaces dynamically on the fly which is quite tricky... I really hope both cargo and its nix support will mature over time)

@noonien
Copy link

noonien commented May 16, 2022

Because I also need to build crates that have dependencies specified at paths, I've come to the solution of actually modifying Cargo.toml before passing it as a source to naerks like so:

{ lib, formats, runCommand, naersk }:
let
  replaceDepsSrc = { src, deps, cargoToml ? "Cargo.toml" }:
    let
      inherit (builtins) mapAttrs hasAttr isAttrs removeAttrs;
      inherit (lib) importTOML mapAttrsToList filterAttrs;
      toml = importTOML (src + "/${cargoToml}");
      tdeps = toml.dependencies;
      tdeps' = mapAttrs
        (n: v:
          if hasAttr n deps then
            (if isAttrs v then
              removeAttrs v [ "registry" "git" "branch" "rev" ]
            else
              { version = v; }
            ) // { path = deps."${n}"; }
          else v
        )
        tdeps;
      toml' = toml // { dependencies = tdeps'; };
      newCargoToml = (formats.toml { }).generate "Cargo.toml" toml';
    in
    runCommand "replace-deps-src"
      {
        propagatedBuildInputs = mapAttrsToList (n: p: p) (filterAttrs (n: _: hasAttr n tdeps) deps);
      } ''
      cp -r ${src} $out
      chmod +w "$out/${cargoToml}"
      cat < ${newCargoToml} > "$out/${cargoToml}"
    '';
in
naerks.lib.buildPackage rec {
    pname = "my-project";
    src = replaceDepsSrc {
        src = ./.;
        deps = {
            foo = ./../foo;
            bar = ./../bar;
        };
    };

    buildInputs = src.propagatedBuildInputs;
}

I don't really like the use of propagatedBuildInputs here. But I find this a decent solution, maybe it could be implemented in naerks itself?

@evanrelf
Copy link

I ended up switching to https://github.com/ipetkov/crane to work around this issue.

@noonien
Copy link

noonien commented May 16, 2022

I'm not completely sure how crane solves the issue of building adjacent crates that are not part of the same workspace.

@exFalso
Copy link
Contributor

exFalso commented May 16, 2022

Use copySources for adjacent crates

@noonien
Copy link

noonien commented May 16, 2022

I can't really find any documentation for copySources nor can I really figure out how to call it :(

@exFalso
Copy link
Contributor

exFalso commented May 17, 2022

copySources is a list of relative paths to folders, relative to src (or optionally you can also specify copySourcesFrom). naersk when constructing the base source folder (both for the dependencies derivation and for the top-level build) will copy the indicated paths verbatim (as opposed to linking in crates.io sources for regular dependencies).

So if you have:

repo/
repo/Cargo.toml
repo/my-non-workspace-crate/Cargo.toml
repo/my-non-workspace-crate/src
repo/my-non-workspace-crate/src/lib.rs

then you can do

naerks.buildPackage {
  ...
  src = ./repo;
  copySources = [
    "my-non-workspace-crate"
  ];
}

Because of the way this is used in the naersk code, it destroys the two-step caching. This is mentioned here:

# If `copySourcesFrom` is set, then it looks like the benefits brought by
. The reason is actually very simple, the way it's used here

naersk/lib.nix

Line 187 in 94beb7a

for p in $copySources; do
is by doing $copySourcesFrom/$p which means if any source file in the base src path changes then that whole bash script invalidates, even though the relative path $p may not have changed.

I fixed this in this PR: #147 but this didn't get merged because I also included an IFD indirection for handling other corner cases (when the source itself comes from a /nix/store path).

@noonien
Copy link

noonien commented May 17, 2022

Does replacing dependencies (modifying Cargo.toml) also invalidate two-step caching?

Edit:
I've also tried converting my example to:

naerks.lib.buildPackage rec {
    pname = "my-project";
    root = ./.;
    copyDependenciesFrom = [
       ./../foo
       ./../bar
    ];
}

which resulted in a build error, because Cargo.toml still has a relative path to foo and bar, like so:

foo = { path = "../foo" }
bar = { path = "../bar" }

The example structure of the crates is as follows:

some-project-dir/foo/Cargo.toml
some-project-dir/bar/Cargo.toml
some-project-dir/baz/Cargo.toml

baz is the project I want to build. foo and bar are common dependencies that other crates use.

@exFalso
Copy link
Contributor

exFalso commented May 17, 2022

Generally speaking it's best if you point src to the "top-level" folder and specify relative paths (strings) with copySources. Otherwise if you specify proper nix paths like ./../foo that will actually copy those sources into the nix store and replace those references with absolute /nix/store/... strings so relative references in Cargo.toml won't work. You basically want the full source tree to be under one /nix/store path so that these relative paths keep working.

We've been through this with our stack, and we ended up rolling a "synthetic workspace" design where we create a /nix/store path with all the sources, including generating a top-level workspace Cargo.toml on the fly. This is because we also had cases where two projects A and B both referred to local crate C, but C cannot be present in two cargo workspaces. It's all a bit messy.

@noonien
Copy link

noonien commented May 17, 2022

Yeah, using workspaces introduces the limitations that come with namespaces, which is what I'm trying to avoid.

From what I can tell (did an ls -R and exit 1 on postPatch in buildPackage), naerks creates a simple crate to build all dependencies, for which a Cargo.toml is created. I think this would be a good place to replace dependencies (with nix paths).

I personally don't mind having a nix store path for each dependency I want to replace (or even for every dependency), so replacing paths, in this case, allows me to have relative paths in Cargo.toml, to make development easier, because I can build using cargo just fine, without having to go through nix.

It also has the added benefit that I can more easily patch dependencies and replace them in case I want to test something fast. I know cargo also has this feature, but having it using nix feels a bit better.

I still don't completely understand how copySources[From] works, would this be good descriptions?

  • copySources - takes a list of strings, that are paths relative to src (so can't go up in the tree), to copy to the build directory (which should be populated with files from src). I don't fully understand the point of this, shouldn't the paths already be present if they are children of the paths that src point to?
  • copySourcesFrom - takes a list of nix paths/derivations, that will be copied to the build directory

Are there additional details I'm missing? From what I can tell, these don't really solve the problem of crates having dependencies specified as relative paths outside their src.

The solution would be, as you suggested, to create a structure that conforms to how dependencies are specified in Cargo.toml so that building just works. This results in a single nix store path that contains all the crates, so every time the main program changes, those crates also get recompiled.

Does changing Cargo.toml not avoid this complexity? One downside being that each dependency that has a relative path outside of src will need to be specified manually without IFD.

@exFalso
Copy link
Contributor

exFalso commented May 17, 2022

The logic is described here:

naersk/lib.nix

Line 187 in 94beb7a

for p in $copySources; do

So for each p in copySources, naersk copies the sources from $copySourcesFrom/$p to $out/$p. copySourcesFrom in return may either be specified directly, or it will default to src. It's basically just a way to copy folders relative to a path, and that relative path will be reflected in the resulting source set. So in your case it sounds like you want to do

  src = ./..;
  copySources = ["foo" "bar"];

or something like it.

@noonien
Copy link

noonien commented May 17, 2022

Another version of the function, that recursively translates all Cargo.toml relative dependency paths that contain .. to nix store paths without having to specify them manually. It also does not require IFD as I first suspected.

fixCargoPaths = src:
  let
    inherit (builtins) isAttrs hasAttr mapAttrs;
    inherit (pkgs.lib) importTOML hasInfix mapAttrsToList filterAttrs isString splitString last;
    toml = importTOML (src + "/Cargo.toml");
    tdeps = toml.dependencies;
    isPathDep = v: (isAttrs v) && hasAttr "path" v;
    tdeps' = mapAttrs
      (n: v:
        if (isPathDep v) && (hasInfix ".." v.path) then
          v // { path = fixCargoPaths (src + "/${v.path}"); }
        else
          v
      )
      tdeps;
    toml' = toml // {
      dependencies = tdeps';
    };
    newCargoToml = (pkgs.formats.toml { }).generate "Cargo.toml" toml';
  in
  pkgs.runCommand "${last (splitString "/" src)}-fixed-paths"
    {
      propagatedBuildInputs = mapAttrsToList (n: p: p.path) (filterAttrs (n: v: (isPathDep v) && (! (isString v.path))) tdeps');
    } ''
    cp -r ${src} $out
    chmod +w "$out/Cargo.toml"
    cat < ${newCargoToml} > "$out/Cargo.toml"
  '';

used as such:

naerks.lib.buildPackage rec {
    pname = "my-project";
    src = fixCargoPaths ./.;

    buildInputs = src.propagatedBuildInputs;
}

It is similar in functionality with the previous function, however, it automatically detects path dependencies and does not allow for manual replacements which can be powerful.

It would be cool for naerks to support both scenarios. I don't have a good understanding of how naerks works yet, so I can't implement it myself right now. Maybe somebody more knowledgeable includes it until I understand it better.

@tv42
Copy link

tv42 commented Jul 25, 2022

For anyone else who stumbled here by wild googling, https://github.com/ipetkov/crane managed to build my problem project without issues.

@andrewbaxter
Copy link

I tried crane, substituting naersk.buildPackage for crane.buildPackage and hit the same issue. I second @noonien here, I'm not sure how that is supposed to fix it. @evanrelf @tv42 do you have more info on your approach there?

I'm going to try to summarize my understanding of things (also for my self) and approaches I've tried. My scenario is dead simple, crate_a has dep crate_b = { path = ../crate_b }

Naersk basically makes 2 derivations:

  • dummy-src

    This a directory with a fake build.rs and lib.rs with a synthetic Cargo.toml based on the deps of the original source. This is built and cached.

    I think copySources copies sources into this derivation, so that it's not just a directory with build.rs lib.rs Cargo.toml etc but also the specified sources. (I think this doesn't work for me, Cargo.toml keeps ../crate_b so even with the sources copied the path is wrong and the build fails... but maybe I did it wrong, I tried a bunch of things trying to get this to work)

  • crate_a

    This builds from src, I think with maybe a modified Cargo.toml again and a synthetic target with all the artifacts from dummy-src.

    I made a src that has all the different crates under it and tried to use src and root to build from that, but it tries to do the build in the wrong directory and can't find Cargo.toml Cargo.toml can't be found when setting both src and root #258 . You can do cd ${root} in preBuild but that messes up other things (target dir, synthetic Cargo.toml). Uh, this also doesn't work because preBuild applies to both dummy-src and crate_a and the cd fails in the former... I ended up checking pwd for dummy-scrc to guard the behavior. It still didn't work, the paths are messed up (i.e. wrong target dir, the rewritten Cargo.toml isn't present...). Also I had a dir of symlinks to store dirs which are RO so they couldn't have the rewritten files anyways. I reached my "unmaintainable hack" threshold here and stopped.

@noonien 's solution seems to be working great for me, once I copied and pasted it correctly.

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

No branches or pull requests

8 participants