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

cargo install: suggest --git when package name is url #12575

Merged
merged 2 commits into from
Aug 29, 2023

Conversation

cardoso
Copy link
Contributor

@cardoso cardoso commented Aug 28, 2023

What does this PR try to resolve?

Improve the error message when specifying a URL for a package name in cargo install.

Fixes #10485

How should we test and review this PR?

Just cargo test and trying a common case like cargo install https://github.com/rust-lang/cargo

Additional information

I found this PR after finishing this one: #10522
But it seems have a larger scope to refactor some of the related code.

Perhaps this one would be easier to merge and that one could focus on the refactor, otherwise sorry for the noise and feel free to close.

@rustbot
Copy link
Collaborator

rustbot commented Aug 28, 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-cli Area: Command-line interface, option parsing, etc. Command-install S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 28, 2023
@cardoso
Copy link
Contributor Author

cardoso commented Aug 28, 2023

@rustbot review

@arlosi
Copy link
Contributor

arlosi commented Aug 29, 2023

While this doesn't cover all possible git URLs, it catches the most common case and provides a helpful suggestion. Thanks!

@bors r+

@bors
Copy link
Contributor

bors commented Aug 29, 2023

📌 Commit 40dae61 has been approved by arlosi

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-review Status: Awaiting review from the assignee but also interested parties. labels Aug 29, 2023
@bors
Copy link
Contributor

bors commented Aug 29, 2023

⌛ Testing commit 40dae61 with merge 01b8801...

bors added a commit that referenced this pull request Aug 29, 2023
cargo install: suggest --git when package name is url

### What does this PR try to resolve?

Improve the error message when specifying a URL for a package name in `cargo install`.

Fixes #10485

### How should we test and review this PR?

Just cargo test and trying a common case like `cargo install https://github.com/rust-lang/cargo`

### Additional information

I found this PR after finishing this one: #10522
But it seems have a larger scope to refactor some of the related code.

Perhaps this one would be easier to merge and that one could focus on the refactor, otherwise sorry for the noise and feel free to close.
@bors
Copy link
Contributor

bors commented Aug 29, 2023

💔 Test failed - checks-actions

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Aug 29, 2023
@weihanglo
Copy link
Member

I've seen this a couple of times. We should definitely fix this flaky test.

Let's move on today

@bors retry

---- registry_auth::token_not_logged stdout ----
req: Request {
    url: Url {
        scheme: "http",
        cannot_be_a_base: false,
        username: "",
        password: None,
        host: Some(
            Ipv4(
                127.0.0.1,
            ),
        ),
        port: Some(
            43655,
        ),
        path: "/index/config.json",
        query: None,
        fragment: None,
    },
    method: "get",
    authorization: None,
    if_modified_since: None,
    if_none_match: None,
}
req: Request {
    url: Url {
        scheme: "http",
        cannot_be_a_base: false,
        username: "",
        password: None,
        host: Some(
            Ipv4(
                127.0.0.1,
            ),
        ),
        port: Some(
            43655,
        ),
        path: "/index/config.json",
        query: None,
        fragment: None,
    },
    method: "get",
    authorization: Some(
        "a-unique_token",
    ),
    if_modified_since: None,
    if_none_match: None,
}
req: Request {
    url: Url {
        scheme: "http",
        cannot_be_a_base: false,
        username: "",
        password: None,
        host: Some(
            Ipv4(
                127.0.0.1,
            ),
        ),
        port: Some(
            43655,
        ),
        path: "/index/config.json",
        query: None,
        fragment: None,
    },
    method: "get",
    authorization: Some(
        "a-unique_token",
    ),
    if_modified_since: None,
    if_none_match: None,
}
req: Request {
    url: Url {
        scheme: "http",
        cannot_be_a_base: false,
        username: "",
        password: None,
        host: Some(
            Ipv4(
                127.0.0.1,
            ),
        ),
        port: Some(
            43655,
        ),
        path: "/index/3/b/bar",
        query: None,
        fragment: None,
    },
    method: "get",
    authorization: Some(
        "a-unique_token",
    ),
    if_modified_since: None,
    if_none_match: None,
}
req: Request {
    url: Url {
        scheme: "http",
        cannot_be_a_base: false,
        username: "",
        password: None,
        host: Some(
            Ipv4(
                127.0.0.1,
            ),
        ),
        port: Some(
            43655,
        ),
        path: "/dl/bar/1.0.0/download",
        query: None,
        fragment: None,
    },
    method: "get",
    authorization: Some(
        "a-unique_token",
    ),
    if_modified_since: None,
    if_none_match: None,
}
/home/runner/work/cargo/cargo/target/tmp/cit/t2390/dl/bar/1.0.0/download
req: Request {
    url: Url {
        scheme: "http",
        cannot_be_a_base: false,
        username: "",
        password: None,
        host: Some(
            Ipv4(
                127.0.0.1,
            ),
        ),
        port: Some(
            43655,
        ),
        path: "/api/v1/crates/new",
        query: None,
        fragment: None,
    },
    method: "put",
    authorization: Some(
        "a-unique_token",
    ),
    if_modified_since: None,
    if_none_match: None,
}
req: Request {
    url: Url {
        scheme: "http",
        cannot_be_a_base: false,
        username: "",
        password: None,
        host: Some(
            Ipv4(
                127.0.0.1,
            ),
        ),
        port: Some(
            43655,
        ),
        path: "/index/config.json",
        query: None,
        fragment: None,
    },
    method: "get",
    authorization: Some(
        "a-unique_token",
    ),
    if_modified_since: None,
    if_none_match: None,
}
req: Request {
    url: Url {
        scheme: "http",
        cannot_be_a_base: false,
        username: "",
        password: None,
        host: Some(
            Ipv4(
                127.0.0.1,
            ),
        ),
        port: Some(
            43655,
        ),
        path: "/index/3/f/foo",
        query: None,
        fragment: None,
    },
    method: "get",
    authorization: Some(
        "a-unique_token",
    ),
    if_modified_since: None,
    if_none_match: None,
}
thread 'registry_auth::token_not_logged' panicked at tests/testsuite/registry_auth.rs:499:5:
assertion `left == right` failed
  left: 8
 right: 7

@weihanglo
Copy link
Member

@bors r=arlosi

@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-review Status: Awaiting review from the assignee but also interested parties. labels Aug 29, 2023
@bors
Copy link
Contributor

bors commented Aug 29, 2023

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

@bors
Copy link
Contributor

bors commented Aug 29, 2023

📌 Commit 40dae61 has been approved by arlosi

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Aug 29, 2023

⌛ Testing commit 40dae61 with merge 333ca23...

@bors
Copy link
Contributor

bors commented Aug 29, 2023

☀️ Test successful - checks-actions
Approved by: arlosi
Pushing 333ca23 to master...

@bors bors merged commit 333ca23 into rust-lang:master Aug 29, 2023
18 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-cli Area: Command-line interface, option parsing, etc. Command-install 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.

Suggest cargo install url using --git
7 participants