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

Multi-cargoArtifacts - inheriting artifacts from multiple previous steps #416

Open
dpc opened this issue Oct 10, 2023 · 10 comments
Open

Multi-cargoArtifacts - inheriting artifacts from multiple previous steps #416

dpc opened this issue Oct 10, 2023 · 10 comments

Comments

@dpc
Copy link
Contributor

dpc commented Oct 10, 2023

We started complex builds with cross-compiled projects (think: Android app built for different targets, or multi-arch (lipo) MacOS binaries, or wasm + a native web server), it seemed like it would make sense to sometimes inherit from multiple things to merge the content in side ./target.

@ipetkov
Copy link
Owner

ipetkov commented Oct 12, 2023

One reason I've avoided implementing general artifact merging is dealing with potential ambiguities on which file should "win" in the case of a collision (plus we might not have timestamps to help us out). When working with different targets it might be simpler since they go in their own subdirectories, but that also begs the question where the utility comes from having the completely separate targets built as one output to begin with?

@dpc
Copy link
Contributor Author

dpc commented Oct 12, 2023

but that also begs the question where the utility comes from having the completely separate targets built as one output to begin with?

Mostly that it allows the same state as a one would see locally when working in a dev shell.

But yes, this is a delicate feature that users could misuse. Possibly it can be done as an extension to crane as well.

@ipetkov
Copy link
Owner

ipetkov commented Oct 12, 2023

FWIW there shouldn't be anything stopping one from merging artifacts, and individual projects might have a better understanding of where it is safe to do merging (it's hard to assume anything relating to a a build system because everyone has special build rules for stuff 😄 )

@dpc
Copy link
Contributor Author

dpc commented Oct 19, 2023

How would I merge artifacts exacly?

Here is my use case: I need to run a wasm test (wasm32-uknown-unknown build) in the environment with other stuff from native build.

Ideally I'd like to do:

  wasmTest = craneLibTests.mkCargoDerivation {
    pname = "wasm-test";

    cargoArtifacts = [ craneMultiBuild.ci.workspaceBuild workspaceBuild ];
    buildPhaseCargoCommand = "patchShebangs ./scripts; SKIP_CARGO_BUILD=1 ./scripts/tests/wasm-test.sh";
  };

and have crane extra incremental artifacts from both workspaceBuild (which in this case is going to be wasm), and from craneMultiBuild.ci.workspaceBuild (which in this case will be native build using ci build profile).

I'm not sure if crane exposes anything right now that I could use to somehow do it "manually". But I really think that crane could just detect an array and map over it in order.

@ipetkov
Copy link
Owner

ipetkov commented Oct 19, 2023

How would I merge artifacts exacly?

A super naive way to do it "manually" would be to

let
  cargoArtifacts = pkgs.runCommand "merge-artifacts" { } ''
    mkdir -p $out
    # Handling compression and file collisions left as exercise to the reader
    ${pkgs.rsync}/bin/rsync ${craneMultiBuild.ci.workspaceBuild} $out/target
    ${pkgs.rsync}/bin/rsync ${workspaceBuild} $out/target
  '';
in
# ...

Another option could be to lift the restriction where buildDepsOnly ignores cargoArtifacts and one set of artifacts from building from a previous one (e.g. have workspaceBuild set cargoArtifacts = craneMultiBuild.ci.workspaceBuild)

@dpc
Copy link
Contributor Author

dpc commented Oct 19, 2023

A super naive way to do it "manually" would be to

This is not going to work with use-zstd, especially incremental one, no? If the "take this cargoArtifact and extract here" would be exposed, that might work.

But then again ... it seems much easier to handle in crane, than even maintain, document and debug people breaking it in ad-hoc ways, IMO.

@ipetkov
Copy link
Owner

ipetkov commented Oct 19, 2023

This is not going to work with use-zstd, especially incremental one, no? If the "take this cargoArtifact and extract here" would be exposed, that might work.

I mean I think the following would work the way we extract things today if you wanted to just blindly extract it all

let
  cargoArtifacts = pkgs.runCommand "merge-artifacts" { } ''
    mkdir -p $out
    ln -s ${craneMultiBuild.ci.workspaceBuild}/target.tar.zst $out/target.tar.zst.prev
    ln -s ${workspaceBuild}/target.tar.zst $out/target.tar.zst
  '';
in
# ...

But then again ... it seems much easier to handle in crane, than even maintain, document and debug people breaking it in ad-hoc ways, IMO.

I'm not intimately familiar with how exactly cargo manages all the files in the target directory so I've been holding off on building some kind of "general artifact merging" solution

@dpc
Copy link
Contributor Author

dpc commented Oct 19, 2023

I mean I think the following would work the way we extract things today if you wanted to just blindly extract it all

This will work. I guess I can with it for now.

I'm not intimately familiar with how exactly cargo manages all the files in the target directory so I've been holding off on building some kind of "general artifact merging" solution

How about just implementing it and documenting as "experimental" (or keep hidden), and just see how it goes. Let's face it - users doing non-trivial things need to be aware of lower level details of how cargo works anyway and routinely are debugging issues anyway. And if they find problem, oftentimes it's helpful to refine crane. Sometimes they even come up with a PR.

@dpc
Copy link
Contributor Author

dpc commented Oct 19, 2023

BTW. Does the build workspace (subset) submission also needs some multi-cargoArtifacts? https://github.com/TheNeikos/crane-workspaces/blob/main/lib/mergeTargets.nix

@ipetkov
Copy link
Owner

ipetkov commented Oct 20, 2023

I don't know, I haven't had the time to really dig in and understand those internals yet

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

2 participants