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

Explain how version works for git dependencies #12270

Merged
merged 3 commits into from
Aug 28, 2023
Merged

Conversation

BatmanAoD
Copy link
Member

What does this PR try to resolve?

Explains how version works for git dependencies. This confused me, and I couldn't find any mention of it e.g. on the user forums.

How should we test and review this PR?

Documentation-only change

@rustbot
Copy link
Collaborator

rustbot commented Jun 14, 2023

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @epage (or someone else) soon.

Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

  • @rustbot author: the review is finished, PR author should check the comments and take action accordingly
  • @rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue

@rustbot rustbot added A-documenting-cargo-itself Area: Cargo's documentation S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 14, 2023
Copy link
Member

@weihanglo weihanglo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR. Just wonder if Multiple locations section has already covered this, no?

@BatmanAoD
Copy link
Member Author

@weihanglo It actually does make things a lot clearer; I'll take another look and revise my suggestion. I didn't actually look at that section before because I didn't realize that the version key would still cause an attempt to look up the crate in a registry, so I didn't think that "multiple locations" even applied to this situation.

@BatmanAoD
Copy link
Member Author

@weihanglo Sorry for not getting back to this sooner. How does it look now?

Copy link
Member

@weihanglo weihanglo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

path dependencies also have the exact behavior. Is it possible to merge this into the notice below, and update for both path and git dependencies to share the similar text?

> **Note**: [crates.io] does not allow packages to be published with `git`
> dependencies (`git` [dev-dependencies] are ignored). See the [Multiple
> locations](#multiple-locations) section for a fallback alternative.

So maybe something like:

> **Note**: A dependency can have both a `git` and `version` keys, these are
> considered separate locations for resolving the dependency, as [crates.io]
> does not allow packages to be published with `git` dependencies (`git`
> [dev-dependencies] are ignored). See the [Multiple locations](#multiple-locations)
> section for a fallback alternative.

What do you think?

(I tried to make it concise but I believe you could do this better that me)

@weihanglo
Copy link
Member

@rustbot author

@rustbot rustbot added S-waiting-on-author Status: The marked PR is awaiting some action (such as code changes) from the PR author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 10, 2023
@weihanglo
Copy link
Member

let me take over the review

r? @weihanglo

@rustbot rustbot assigned weihanglo and unassigned epage Aug 10, 2023
* Explain how `version` works for `git` dependencies
* Link to multiple-locations, since this is already explained there
* more extensive rewrite
@BatmanAoD
Copy link
Member Author

I think the note about crates.io not permitting packages with git dependencies is saying something fairly different, namely that users are not permitted to publish packages on crates.io that import code from somewhere other than crates.io itself. (This note doesn't explicitly mention third-party registries, but the one a couple sections above does.)

I've made some more extensive changes to reflect that; please let me know what you think.

(The introduction of the "requirements syntax" header, and demoting of the different version-requirement syntax headers to go under that header, is technically unrelated, so let me know if I need to undo that change.)

Copy link
Member

@weihanglo weihanglo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work! Thanks!

@weihanglo
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Aug 28, 2023

📌 Commit 6bdbe0a has been approved by weihanglo

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: The marked PR is awaiting some action (such as code changes) from the PR author. labels Aug 28, 2023
@bors
Copy link
Contributor

bors commented Aug 28, 2023

⌛ Testing commit 6bdbe0a with merge 3119bd7...

bors added a commit that referenced this pull request Aug 28, 2023
Explain how `version` works for `git` dependencies
@BatmanAoD BatmanAoD closed this Aug 28, 2023
@BatmanAoD BatmanAoD deleted the patch-1 branch August 28, 2023 03:28
@weihanglo
Copy link
Member

Hmm… I guess bors was stuck, so it was never merged successfully. @BatmanAoD, could you reopen so that we can merge it again?

@BatmanAoD BatmanAoD restored the patch-1 branch August 28, 2023 15:55
@BatmanAoD BatmanAoD reopened this Aug 28, 2023
@BatmanAoD
Copy link
Member Author

@weihanglo Oops, for some reason I thought it was safe to delete the branch. Restored and re-opened.

@weihanglo
Copy link
Member

No worries!

@bors r+

@bors
Copy link
Contributor

bors commented Aug 28, 2023

💡 This pull request was already approved, no need to approve it again.

@bors
Copy link
Contributor

bors commented Aug 28, 2023

📌 Commit 6bdbe0a has been approved by weihanglo

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Aug 28, 2023

⌛ Testing commit 6bdbe0a with merge 25050d9...

@bors
Copy link
Contributor

bors commented Aug 28, 2023

☀️ Test successful - checks-actions
Approved by: weihanglo
Pushing 25050d9 to master...

@bors bors merged commit 25050d9 into rust-lang:master Aug 28, 2023
36 checks passed
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 29, 2023
Update cargo

18 commits in 925280f028db3a322935e040719a0754703947cf..96fe1c9e1aecd8f57063e3753969bb6418fd2fd5
2023-08-25 21:16:44 +0000 to 2023-08-29 20:10:34 +0000
- fix(lints): Fail when overriding inherited lints (rust-lang/cargo#12584)
- cargo install: suggest --git when package name is url (rust-lang/cargo#12575)
- chore: remove unstable-options for logout (rust-lang/cargo#12588)
- Improve logout message for asymmetric tokens (rust-lang/cargo#12587)
- fix(update): Remove references to -p in help (rust-lang/cargo#12586)
- fix(update): Make `-p` more convenient by being positional (rust-lang/cargo#12545)
- Set tracing target for networking messages. (rust-lang/cargo#12582)
- Retry docs (rust-lang/cargo#12583)
- feat(resolver): **Very** preliminary MSRV resolver support (rust-lang/cargo#12560)
- Update git2 (rust-lang/cargo#12580)
- Explain how `version` works for `git` dependencies (rust-lang/cargo#12270)
- Improve deserialization errors of untagged enums (rust-lang/cargo#12574)
- Add support for `target.'cfg(..)'.linker` (rust-lang/cargo#12535)
- Improve resolver version mismatch warning (rust-lang/cargo#12573)
- Stabilize `--keep-going` (rust-lang/cargo#12568)
- Define {{command}} for use in src/doc/man/includes (rust-lang/cargo#12570)
- Update serde (rust-lang/cargo#12569)
- chore: add missing `windows-sys` features back (rust-lang/cargo#12563)

r? ghost
@ehuss ehuss added this to the 1.74.0 milestone Sep 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-documenting-cargo-itself Area: Cargo's documentation S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants