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

buildRustCrate support editions #55972

Merged
merged 2 commits into from Feb 24, 2019
Merged

buildRustCrate support editions #55972

merged 2 commits into from Feb 24, 2019

Conversation

andir
Copy link
Member

@andir andir commented Feb 18, 2019

Motivation for this change

This is my initial version of editions support. I have a local change to carnix that also exposed the edition field (if availabe). I can finally build my private projects again that have dependencies with an edition = 2018 field in the Cargo.toml. \o/

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option 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/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Assured whether relevant documentation is up to date
  • Fits CONTRIBUTING.md.

I am currently unable to apply / upload / propose the changes to carnix since my account is never created (tried months ago, tried now, …). One of the patches I wrote is below (the one for this PR; the other one crashes pijul :/). (cc @P-E-Meunier)


$ pijul patch 8pcBAtmobyqgYGqkvTieAvQKnxshTwQL2XXPhxbASRPdUAuhUaSNASMjxpqUcbmQnKPkLrYZFYLKPrRWsbfRiaZK
In "["src", "krate.rs"]":
From line 39:
+     pub edition: Option<String>,
In "["src", "output.rs"]":
From line 1013:
+
+         if let Some(ref edition) = meta.edition {
+             writeln!(w, "{}  edition = {};", indent, edition)?;
+         }
In "["src", "prefetch.rs"]":
From line 101:
+         let edition = v.get("package").and_then(|p| p.get("edition").map(|s| s.to_string()));
In "["src", "prefetch.rs"]":
From line 134:
+             edition

Previously build flags would not be available during the configure phase
while they might be required to build the `build.rs` file.
In combination with carnix we can now build crates that require a
specific edition of rust features. A few crates started requiring that
already and having this in nixpkgs is just logical.
@andir andir changed the title WIP: buildRustCrate support editions buildRustCrate support editions Feb 18, 2019
@andir
Copy link
Member Author

andir commented Feb 18, 2019

@GrahamcOfBorg build carnix buildRustCrateTests toml2nix cargo-download cargo-vendor cargo-update

@andir
Copy link
Member Author

andir commented Feb 18, 2019

I published the patch in pijul in the meantime: https://nest.pijul.com/pmeunier/carnix/discussions/27

@andir
Copy link
Member Author

andir commented Feb 24, 2019

@GrahamcOfBorg build carnix buildRustCrateTests toml2nix cargo-download cargo-vendor cargo-update

@tazjin
Copy link
Member

tazjin commented Feb 24, 2019

The Nix changes look good to me, haven't looked at the carnix patch itself yet.

The upstream carnix repo hasn't seen any changes in a while. After I made my Github mirror of it, it was forked to nix-community. Should we maybe update that repository and submit patches such as the edition one there?

@P-E-Meunier
Copy link
Contributor

P-E-Meunier commented Feb 24, 2019

@tazjin: the README in your fork states that the webpage https://nest.pijul.com/pmeunier/nix-rust is "offline". It indeed is, but the actual repository https://nest.pijul.com/pmeunier/carnix has changed recently.

GitHub, just like nest.pijul.com, allows people to delete repositories, and you can get a 404 from GitHub and from the Pijul Nest if the authors move their repositories, which is what happened here.

Also, I just accepted the edition patch, about a week after it's been submitted to that repository, which is not at all an unreasonable delay.

Obviously, the license of Carnix doesn't prevent you from forking it and maintaining a fork on GitHub, but I wrote about 95% of this tool, and while I agree that writing and maintaining an almost accurate README is a useful contribution, these few lines of text are still your main contribution to this project so far. Not only have you not cared to submit that contribution to the original repository, but you're also falsely implying in that README that this project has been abandoned, and that your GitHub repository is the new project home.

You could at least have asked me directly before doing this.

@tazjin
Copy link
Member

tazjin commented Feb 24, 2019

@P-E-Meunier It hasn't been easy to get a hold of you on IRC, the Nest, Github or elsewhere - and not for a lack of trying. What is your preferred method of communication?

The second sentence of the README points people at the current repository on Pijul and has done so for a while. The README also makes it abundantly clear that the fork is not the development repository of carnix, and is merely attempting to track the source for the various versions of carnix on crates.io.

As I've just pushed the sources of Carnix 0.8.6 - 0.9.6 (extracted from crates.io, as it is not easily possible to get those from the patch list) I can take care of updating the README and making this even clearer.

Occasionally people will go looking for the source of carnix and either fail to find it entirely, or not be able to find the version they're looking for. The fork at nix-community is supposed to help with that.

these few lines of text are still your main contribution to this project so far

I'm not claiming to be a contributor to carnix itself anywhere, the repository is a mirror. But just to make it clear: My "contribution" is painstakingly extracting sources from crates.io, mirroring them into git with appropriate commits and tags and attempting to figure out what changed between versions to provide a changelog of sorts. These things are necessary due to your source repository frequently not matching what is released on crates.io or not being available at all.

I am grateful for your work on carnix, but that is separate from the structure that has to exist around such a project as it gains more traction.

and you can get a 404 from GitHub and from the Pijul Nest if the authors move their repositories

This isn't really relevant to this discussion, but Github establishes redirects if repositories are moved. (See for example the redirect on google/pulldown-cmark). This may be a useful feature to add to the Nest.

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

4 participants