-
-
Notifications
You must be signed in to change notification settings - Fork 14k
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
Update cargo builder to fetch registry dynamically. #27964
Closed
Closed
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,33 @@ | ||
{ fetchurl, stdenv }: | ||
|
||
let | ||
version = "0.1.4"; | ||
platform = | ||
if stdenv.system == "x86_64-linux" | ||
then "x86_64-unknown-linux-musl" | ||
else if stdenv.system == "x86_64-darwin" | ||
then "x86_64-apple-darwin" | ||
else throw "missing bootstrap url for platform ${stdenv.system}"; | ||
|
||
# fetch hashes by patching print-hashes.sh to not use the "$DATE" variable | ||
# then running `print-hashes.sh 1.16.0` | ||
hash = | ||
if stdenv.system == "x86_64-linux" | ||
then "7716524846297abc6e8a7b26c57f56b1e0c2b261a6d0583e78f2a8fd60b33695" | ||
else if stdenv.system == "x86_64-darwin" | ||
then "439ff0303bc17fd8587196f0658ae4430850a906e67648f9fde047c9c7c75998" | ||
else throw "missing bootstrap hash for platform ${stdenv.system}"; | ||
in stdenv.mkDerivation { | ||
name = "cargo-local-registry-${version}"; | ||
|
||
src = fetchurl { | ||
url = "https://github.com/alexcrichton/cargo-local-registry/releases/download/${version}/cargo-local-registry-${version}-${platform}.tar.gz"; | ||
sha256 = hash; | ||
}; | ||
|
||
phases = "unpackPhase installPhase"; | ||
|
||
installPhase = '' | ||
install -Dm755 cargo-local-registry $out/bin/cargo-local-registry | ||
''; | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file was deleted.
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We will also need
aarch64
. Maybe we can work-around getting cargo build in bootstrap phase without this package and buildcargo-local-registry
from source.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are a lot of dependencies and other stuff to manage. I don't think I know enough about the rust setup to do it myself but if someone can take it on that would be great. I was focused here on showing that cargo-local-registry is probably the way forward.
However building might not actually be necessary if we can rely on the cached deps pacakge. This package is archetecture independant so once we have them we can just build cargo-local-registry from source. I don't know if there is a policy about relying on the cache for things like this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we can use this one for bootstrapping: #24991
I have not found yet find to the time to inspect it closer.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The reason I prefer this approach is it doesn't require any preprocessing or extra files. It simply reads the Cargo.toml file. I do think that those are fixable but this is also a nice quick improvement on the current state.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But a solution for aarch64 must be found first. If upstream adds support for this architecture as well, I am ok with that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or we could just build our own boostrap binary 🤷. Once the bootstrap is in place we can just grab a couple of versions back.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Mic92 As far as i can tell there is currently no support for rust bootstrap for aarch64 either? There are aarch64 builds available on the official rust installer page. Adding a binaries for aarch64 to upstream
cargo-local-registry
would be easy tough.@kevincox I was working on something similar but based on cargo-vendor. Not sure yet which one is better suited but according to the authore (which is the same for both)
cargo-vendor
is getting more attention at the moment.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kevincox I forget to mention that
cargo-vendor
might be easier to bootstrap soon as it will soon have a tarball with vendored dependencies available. See: alexcrichton/cargo-vendor#51There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So I'm not an expert on the differences but it seems like cargo-local-registry is designed to "mirror" the required dependencies whereas cargo-vendor is supposed to the upstream project to vendor dependencies themselves. Either will probably work in practice but cargo-local-registry looks like a better fit for our use case.
And it seems like the bootstrapping issue shouldn't be a major concern as cargo can cross-compile easily. Someone just needs to but in some effort to bootstrap the first version.
That being said I'm happy for whatever solution works.