Open snawaz opened 5 years ago
Hey! I'm kind of new to the Rust community. I'm interested in learning more about it and making contributions, so I see this as kind of low-priority low-hanging fruit. Maybe this feature could prove more valuable especially considering how GitHub and other git hosting platforms have recently changed the default branch name from "master" to "main", etc...
As long as the implementing code won't just be a nuisance subject to bitrot, I'd be happy to submit a PR!
Thank you for asking before submitting a PR. As you may have noticed we have a lot of feature-request issues and have not come to decisions about most of them. I personally wouldn't mind seeing a PR for this. Others know that part of the code base better than I so may have stronger opinions and suggestions for how to start.
Hi, @Xeyler did you have any chance to try to work on this? I think the same and maybe I could help somehow?
Hey, synek!
I haven't put a lot of time into this one yet. Although, I have been spending a little more time with Rust. I'm thinking I'll try to find the time to create a PR soon.
Unfortunately, I'm not sure how this sort of job could be segmented into a project which multiple people could work on. If you have suggestions, I'd love to hear them! I'll try to keep you posted as I find time to draft a PR.
Does anyone mind if I start working on this? It seems that it did not receive much track and after exploring a bit, I am now rather confident on how to implement this.
Hey @snawaz! I began implementing it, but I found myself in a tricky spot I had not foreseen:
I wanted to try extending the feature to allow specifying the reference not only with a branch
key, but also with a tag
or rev
key. However, these should not be compatible, only one of them should be present at a time, just like for package git dependencies. Therefore, a method I wanted to add on crate::util::config::Config
in order to retrieve the specified Git reference must be fallible. But the context in which I want to use it, namely crate::sources::registry::remote::RemoteRegistry::new
, is infallible and part of the public API. So unless someone with actual experience like @Eh2406 or @ehuss helps me a bit, I don't see where I should gracefully fail for this sort of validation.
However, all of this was just me thinking it might be a good idea to add the possibility for these tag
and rev
keys to the feature, but it might not be at all. What do you think? Would it make any sense to use a registry index with a defined tag or even a fixed revision? If it does not, then retrieving the registry index branch would become infallible, which would render the implementation much easier for me with my current understanding of this part of the project.
Thanks in advance!
part of the public API.
The API is not stable, the version number goes up every release. So make the changes you need.
@PaulDance
I wanted to try extending the feature to allow specifying the reference not only with a
branch
key, but also with atag
orrev
key.
Nope. tag
, rev
(or for that matter commit hash
) does not make sense in this case, as all of them mean a specific commit but cargo
does not download from or upload to a specific commit, but from or to a branch (and a new commit is pushed every time it publishes a new (version of) crate). So if we implement it at all, we need branch only. Hope that makes sense to you now.
@snawaz Oh yes, I was thinking about the downloading of dependencies which could use immobile references but not the publishing of new versions which does need a branch. Thanks for the clarification.
@snawaz Considering how registries work using a defined Web API, we do agree that the current cargo publish
process may not be done in order to target a specific branch of the index, as publishing means uploading a tarball of selected source files to the /api/v1/crates/new
route, right? or have I missed something? If so, how do you personally go at modifying a non-default branch of the index? through normal publishing? another way?
@PaulDance
@snawaz Considering how registries work using a defined Web API, we do agree that the current
cargo publish
process may not be done in order to target a specific branch of the index, as publishing means uploading a tarball of selected source files to the/api/v1/crates/new
route, right? or have I missed something? If so, how do you personally go at modifying a non-default branch of the index? through normal publishing? another way?
Not sure if I understood your doubts or concerns. Anyway, here are my thoughts:
cargo
must work in a backward-compatible way. Means, if the configuration mentions branch
, then it should checkout the said branch to resolve the dependencies and then download them from the server. If it does not mention branch, it should work with the master
branch by default. Note that cargo publish
does not change in any way, as it does not require any branch info as such. In this case, the server handles this accordingly, as per the test environment setup.
It is also assumed that cargo
and the registry-server should be in sync. Means if a cargo
works with branch=dev
, and the server should also work with branch=dev
. This is how these things will be setup. cargo
will NOT tell the server which branch to use, or vice versa.
You understood them well, thanks! This means that in order to produce an MVP, I currently aim only Cargo's config.toml
file format: no registry-branch
key is being added to the manifest format for now. I have done that yesterday and am currently looking into implementing tests for this specific part before moving on to anything else.
Personally I feel it would be very useful if you could point it to commits as well as branches. In fact, often I clone the registry on purpose so that I can recreate a lost Cargo.lock file. It would be much nicer if you could just point it to a custom commit directly.
Personally I feel it would be very useful if you could point it to commits as well as branches. In fact, often I clone the registry on purpose so that I can recreate a lost Cargo.lock file. It would be much nicer if you could just point it to a custom commit directly.
How is it going to work? Let's imagine it points to a "commit".
How role does the "commit" play here? Every time you publish a crate, the HEAD changes. So what does the "commit" ensure in such cases?
@snawaz the fact that you explicitly point to a commit means that you don't pick up any new crate versions published since then. This is useful in some scenarios, e.g. when you ran cargo update locally and it broke your build but you don't have Cargo.lock in git, but you maybe know when you ran cargo update last time, so you check out the old state.
@snawaz the fact that you explicitly point to a commit means that you don't pick up any new crate versions published since then. This is useful in some scenarios, e.g. when you ran cargo update locally and it broke your build but you don't have Cargo.lock in git, but you maybe know when you ran cargo update last time, so you check out the old state.
I didn't get that. What does "pick up" mean? Are you referring to "resolving to crates versions published after the commit"? If so, then you're talking about an entirely different thing here, and it's not in the line of supporting a different branch for the registry for normal operations like publish and download.
From a technical point of view it's not different at all.
From a technical point of view it's not different at all.
Not different from what? And what technical point of view? You also did not answer the questions I asked, neither did you explain the confusions as to what exactly you mean.
when you ran cargo update locally and it broke your build but you don't have Cargo.lock in git, but you maybe know when you ran cargo update last time, so you check out the old state
How do you check out the old state
By setting the commit revision instead of a branch name.
How do you remember the old state, to begin with, to which you want to revert back?
You might remember a general date or so when you last made cargo update. You can then obtain a commit hash from that date using github or such.
By setting the commit revision instead of a branch name. You might remember a general date or so when you last made cargo update. You can then obtain a commit hash from that date using github or such.
If you have not committed your Cargo.lock
, then it's likely that you would not remember reliably date correctly either. In fact, you would not remember the date reliably irrespective of your mistake to not committing your Cargo.lock
. But assume for a while that you vaguely remember it.
cargo
suppose to work? Currently, cargo does not have any notion of "do not resolve to versions after this reference (which in your case is a commit-hash)".publish
supposed to work with the commit-hash? BTW, if you break your Cargo.lock
, why not simply regenerate it? The fact that it's not committed in git
tells me that it is not a large/serious project yet, and not many people work on it. At such stage, it'd be easier to solve the issue by regenerating the lock file and working from there.
As said, how is publish supposed to work with the commit-hash?
publish is the only command that has issues with hardcoded commits. All other commands don't have any issues.
The fact that it's not committed in git tells me that it is not a large/serious project yet
It's still officially recommended that you don't commit your Cargo.lock in library projects. This has nothing to do with seriousness. This has something to do with control over the process. Right now there is little control for users.
For those of us running internal mirrors of the crates.io registry, being able to specify a branch so that I can easily sync the crates index and add my custom commit to config.json on a branch that doesn't conflict with upstream names would be quite handy.
Now that Cargo alternative registries have been stabilized, they are configured as:
Currently, Cargo uses the
master
branch by default, which makes development & testing difficult as it interferes with the master which is used in the production. So it'd be good if Cargo allows us to usenon-master
branch as well:where
branch
could be an optional field whose default value ismaster
to be backward-compatible.