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

Pijul: 0.8 -> 0.10 #39317

Closed
wants to merge 1 commit into from
Closed

Pijul: 0.8 -> 0.10 #39317

wants to merge 1 commit into from

Conversation

P-E-Meunier
Copy link
Contributor

Motivation for this change

New release of Pijul, now using Carnix.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option build-use-sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Fits CONTRIBUTING.md.

@infinisil
Copy link
Member

Where can one find the Cargo.lock file used for this?

@yrashk
Copy link
Contributor

yrashk commented Apr 22, 2018

@infinisil it's in the tar.gz and https://nest.pijul.com/pijul_org/pijul

@P-E-Meunier looks like 0.10.1 is almost out of the door?

@P-E-Meunier
Copy link
Contributor Author

It is. There was an important (yet quite subtle) bug introduced by the latest so-called "bugfixes", 0.10.1 fixes it.

@yrashk yrashk mentioned this pull request Apr 22, 2018
8 tasks
@yrashk
Copy link
Contributor

yrashk commented Apr 22, 2018

@P-E-Meunier really looking forward to having pijul 0.10.1 in nixpkgs! 😍

@Mic92
Copy link
Member

Mic92 commented Apr 22, 2018

Would it be technically possible to share the generated carnix files between multiple packages or would feature flags conflict in this case?

@infinisil
Copy link
Member

infinisil commented Apr 22, 2018

@yrashk Which tar.gz? The download page only contains one for version 0.8.3. And https://nest.pijul.com/pijul_org/pijul gives me currently an internal server error, and last time I checked the Cargo.lock file there had a conflict.

Edit: If we're gonna put a 4000 line file in nipkgs I wanna know how I can reproduce it.

@obadz
Copy link
Contributor

obadz commented Apr 28, 2018

Wow. That's a lot of nix code. Are we saying that carnix generated nix is the best way forward for nixpkgs?

@P-E-Meunier
Copy link
Contributor Author

It also has its upsides, such as faster compilation, and factoring out common parts of Rust crates.

One issue is redundancy, as an important part of this code describes packages that are probably used by other crates.

Another option would be to generate Nix expressions for all of crates.io (each would be much smaller), include that in nixpkgs (like what is done for Haskell packages), and then have much smaller package descriptions.

@Mic92
Copy link
Member

Mic92 commented May 1, 2018

@P-E-Meunier we would then need multiple version x features of different crates for this to work, no? Have tried to generating this?

@Mic92
Copy link
Member

Mic92 commented May 1, 2018

@P-E-Meunier
Copy link
Contributor Author

@Mic92: Yes, a change of feature requires a recompilation, but my guess is that if a crate is included in nixkgs, then its dependencies will be compiled too, and might be included in a channel, which would make it much faster to compile Rust code on Nix than on any other platform (except during development, where incremental compilation by rustc, and the fact that Cargo needs to do less than Nix, make it faster).

I just pushed a patch to https://nest.pijul.com/pmeunier/carnix which splits the output into two parts, a common one describing the crates and their dependencies (without specific versions, sort of a relaxed Cargo.toml), and another (pretty small) one that sets the actual versions of everything (doing part of the job of a Cargo.lock).

This is not ready yet, because the generated code contains a giant attrset with all the crates used in the compilation, and they still reference each other.

@P-E-Meunier
Copy link
Contributor Author

I've just opened #40587 to fix this, waiting for it to be accepted.

@xeji
Copy link
Contributor

xeji commented May 17, 2018

#40622 has an alternative approach without carnix, please have a look.

@xeji xeji mentioned this pull request May 17, 2018
8 tasks
@P-E-Meunier
Copy link
Contributor Author

@xeji: yes, this has been proposed before. Honestly, I think this PR:

  • yields faster compile times.
  • shares build products with other Rust software installed on the system.
  • stress-tests Carnix, which will get NixOS an awesome Rust ecosystem in the future (we're not 100% there yet, but thanks to the various crates already using Carnix, Carnix itself has made huge progress).

@sheganinans
Copy link
Contributor

I think that Carnix seems to be the right direction in the future but I'm not sure if such a huge default.nix file is the right way to go in the current moment.

If there was some way to factor out the shared nix expressions out into their own files and then have the Pijul default.nix file reference the default.nix files that it depends on, cleaning up the main Pijul default.nix, that would be the correct solution.

I am very excited about the future of Carnix and what it will accomplish for the nix/rust ecosystem, having all of crates.io automagically built by Carnix, similarly to how Nix currently packages Haskell libraries and binaries is absolutely "The Right Thing (tm)" but this PR seems to be a bit unwieldy for a rather simple version bump.

Either way, I think that waiting for 0.10.1 would be the right thing to do as well, since it includes an important bugfix.

@sheganinans
Copy link
Contributor

sheganinans commented May 18, 2018

After looking at the generated nix expressions a bit more, it seems pretty straight forward to me.
Mostly I just want what is best for the community.
Pijul does seem like a good binary to stress test Carnix, if there isn't much opposition maybe we can give this a try.

So @P-E-Meunier the low hanging fruit in this instance is the duplication of crates that are shared across many other libraries. Maybe instead of using Cargo.toml files directly to generate these default.nix files, you have a set of nix files that has already been generated by scraping crates.io for all versions of all libraries. For example from your generated output you could have an arrayvec.nix file with every single version:

arrayvec_0_4_7 = { features?(arrayvec_0_4_7_features {}) }: arrayvec_0_4_7_ {
  dependencies = mapFeatures features ([ nodrop_0_1_12 ]);
  features = mkFeatures (features.arrayvec_0_4_7 or {});
};
arrayvec_0_4_7_ = { dependencies?[], buildDependencies?[], features?[] }: buildRustCrate {
    crateName = "arrayvec";
    version = "0.4.7";
    authors = [ "bluss" ];
    sha256 = "0fzgv7z1x1qnyd7j32vdcadk4k9wfx897y06mr3bw1yi52iqf4z4";
    inherit dependencies buildDependencies features;
};
arrayvec_0_4_7_features = f: updateFeatures f (rec {
  arrayvec_0_4_7.default = (f.arrayvec_0_4_7.default or true);
  arrayvec_0_4_7.serde =
    (f.arrayvec_0_4_7.serde or false) ||
    (f.arrayvec_0_4_7.serde-1 or false) ||
    (arrayvec_0_4_7.serde-1 or false);
  arrayvec_0_4_7.std =
    (f.arrayvec_0_4_7.std or false) ||
    (f.arrayvec_0_4_7.default or false) ||
    (arrayvec_0_4_7.default or false);
  nodrop_0_1_12.default = (f.nodrop_0_1_12.default or false);
}) [ nodrop_0_1_12_features ];

Then the Pijul default.nix file would then import arrayvec.nix and all the other dependencies, select the proper versions, put them into the right slots, and then the file should be much shorter and there would be no redundancies in the generated derivations.

@P-E-Meunier
Copy link
Contributor Author

As I said in an earlier comment, the new Carnix (0.8) yields very small Nix expressions, and this might even improve in the future.

@sheganinans
Copy link
Contributor

sheganinans commented May 18, 2018 via email

@yrashk
Copy link
Contributor

yrashk commented May 22, 2018

From what I can tell, Pijul 0.10.1 has been on crates.io for quite some time. @P-E-Meunier any idea when/if it is going to be in downloads?

@sheganinans
Copy link
Contributor

I've closed #40622
Please merge this branch of Pijul 0.8 -> 0.10

@nanexcool
Copy link

This failed for me on MacOS:

Building pijul (src/main.rs)
....
error: linking with `/nix/store/1raxjnnz5fzjl216r6w9bffprqqxw3jr-clang-wrapper-5.0.2/bin/cc` failed: exit code: 1
...
 = note: ld: library not found for -lcrypto
          clang-5.0: error: linker command failed with exit code 1 (use -v to see invocation)
          
error: aborting due to previous error
builder for '/nix/store/xxshgdn9s15bqsbzjcgp0c4r29frwzs8-rust_pijul-0.10.0.drv' failed with exit code 101
error: build of '/nix/store/xxshgdn9s15bqsbzjcgp0c4r29frwzs8-rust_pijul-0.10.0.drv' failed

@jasoncarr0 jasoncarr0 mentioned this pull request Oct 18, 2018
8 tasks
@Mic92
Copy link
Member

Mic92 commented Oct 29, 2018

This was merged in #49331

@Mic92 Mic92 closed this Oct 29, 2018
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

9 participants