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 semver checks #32

Closed
wants to merge 41 commits into from
Closed

Cargo semver checks #32

wants to merge 41 commits into from

Conversation

weihanglo
Copy link
Owner

No description provided.

Workspaces by default use the resolver in version 1. It's been
some time already since the 2021 edition which made version 2
a default for the alone packages, but yet most of the projects
that make a use of the workspaces still depends on an old resolver.

By being explicit about the resolver tag inside the workspace
we can lower the usage of older resolver for the new projects.
@weihanglo weihanglo force-pushed the cargo-semver-checks branch 3 times, most recently from 8756a05 to c9d1524 Compare July 24, 2023 13:55
weihanglo and others added 16 commits July 24, 2023 15:18
fix(cargo-credential): should enable feature `serde/derive`
Currently, the UI tests are
- `cargo add`
- `cargo new`
- `cargo remove`
- `init`

One of these is not like the others. This change renames `init` to
`cargo_init` to suggest it is the UI tests for the `cargo init` command,
rather than `init` functionality.
refactor(tests): Name init ui tests more consistently

Currently, the UI tests are
- `cargo add`
- `cargo new`
- `cargo remove`
- `init`

One of these is not like the others. This change renames `init` to `cargo_init` to suggest it is the UI tests for the `cargo init` command, rather than `init` functionality.
This is split out of rust-lang#11912 and is prep for adding more UI tests.

Generally our UI tests are in a directory named after the full cargo
command (`cargo config`).  These tend to use `snapbox`.

Here we are tests for the `cargo config` command not written by
`snapbox` in a `cargo_config.rs` file.  This conflicts with adding
snapbox UI tests later in a `cargo_config/` folder.  Upon looking at this
file, it appears to be UI tests, so I think it would make sense to move
them into the `cargo_config/` folder.  Definitely wouldn't make sense to
move them into `config.rs` since that is general config testing.
refactor(test): Move cargo-config into a dir

This is split out of rust-lang#11912 and is prep for adding more UI tests.

Generally our UI tests are in a directory named after the full cargo command (`cargo config`).  These tend to use `snapbox`.

Here we are tests for the `cargo config` command not written by `snapbox` in a `cargo_config.rs` file.  This conflicts with adding snapbox UI tests later in a `cargo_config/` folder.  Upon looking at this file, it appears to be UI tests, so I think it would make sense to move them into the `cargo_config/` folder.  Definitely wouldn't make sense to move them into `config.rs` since that is general config testing.
Being a bit cautious about not turning this into an error since this is
most likely because of case insensitive filesystems.
This makes it easier to evaluate the usability of PRs, like rust-lang#11905
test(cli): Track --help output

### What does this PR try to resolve?

This makes it easier to evaluate the usability of PRs, like rust-lang#11905

This follows the pattern of `cargo add` and `cargo remove` of putting these ui tests in `cargo_<cmd>/` directories.  `init` didn't follow this pattern, so it was renamed to `cargo_init/`.  `cargo_config.rs` was going to conflict with this, it was merged in.

We can evaluate other `<cmd>.rs` files at a later point and consolidate.

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

The main risks are
- Are all files linked together (`main.rs` -> `<cmd>/mod.rs` -> `<cmd>/<help>.rs`
- Are all `help/mod.rs`s pointing to the right command
Update curl-sys to pull in curl 8.2.0

This is a routine update to curl-sys 0.4.64, updating curl from 8.1.2 to 8.2.0.
Changelog: https://curl.se/changes.html#8_2_0
@weihanglo weihanglo force-pushed the cargo-semver-checks branch 4 times, most recently from 01298e1 to 0beb3d8 Compare July 26, 2023 10:24
weihanglo and others added 6 commits July 26, 2023 11:26
docs: raise awareness of resolver used inside workspace

Workspaces by default use the resolver in version 1. It's been some time already since the 2021 edition which made version 2 a default for alone packages, but yet most of the projects that make a use of the workspaces still depends on  an old resolver.

By being explicit about the resolver tag inside the workspace we can lower the usage of older resolver for the new projects.

This can raise the awareness of this behavior and prevent issues like rust-lang#12387. I also wasn't aware of this behavior before while not being so new to Rust, and we have the resolver 2 for good reasons, so I think we should be more explicit about it in the documentation.
When someone looks for the 'how to make cargo workspace' answers, he's unlikely to get to the `Dependency Resolution` section at the same time, he'll likely just copy paste the workspace example from the `Workspaces` and call it a day, yet extending the usage of an old resolver and not benefiting from the new one.
To keep things simple, especially in getting a `Hash` implementation
correct, I'm leveraging `unicase` for case-insensitive
comparisons which is an existing dependency and I've been using for
years on other projects.

This also opens the door for us to add cross-platform compatibility
hazard warnings about multiple paths that would write to the same
location on a case insensitive file system.  I held off on that because
I assume we would want rust-lang#12235 first.

This does mean we can't test the "no manifest" case anymore because the
one case (no pun intended) I knew of for hitting it is now gone.
ehuss and others added 15 commits July 27, 2023 10:55
Update curl-sys to pull in curl 8.2.1

There were several regressions in 8.2.0. I'm not certain how much they affect cargo, but some of them look concerning.
Changelog: https://curl.se/changes.html#8_2_1
Summary: https://daniel.haxx.se/blog/2023/07/26/curl-8-2-1/
Previously this document implied that the `lto` setting only controlled
`-C lto` but in some cases it can also pass `-C linker-plugin-lto`

https://github.com/rust-lang/cargo/blob/1b3eb297f946779b1ede257074afff62d854ff79/src/cargo/core/compiler/mod.rs#L1234
…ment variables docs

The doc for the env var was added in b6d1979 but was forgotten in env vars
…-doc, r=weihanglo

doc: add missing reference to `CARGO_PROFILE_<name>_STRIP` in environment variables docs

The doc for the env var was added in b6d1979 but was forgotten in env vars
…clinker-plugin-lto, r=ehuss

Clarify `lto` setting passing `-Clinker-plugin-lto`

Previously this document implied that the `lto` setting only controlled `-C lto` but in some cases it can also pass `-C linker-plugin-lto`

https://github.com/rust-lang/cargo/blob/1b3eb297f946779b1ede257074afff62d854ff79/src/cargo/core/compiler/mod.rs#L1234
fix(package): Recognize and normalize `cargo.toml`

### What does this PR try to resolve?

This solution is a blend of conservative and easy
- Normalizes `cargo.toml` to `Cargo.toml` on publish
  - Ensuring we publish the `prepare_for_publish` version and include `Cargo.toml.orig`
  - Avoids dealing with trying to push existing users to `Cargo.toml`
- All other cases of `Cargo.toml` are warnings
  - We could either normalize or turn this into an error in the future
  - When helping users with case previously, we've only handle the `cargo.toml` case
  - We already should have a fallback in case a manifest isn't detected
  - I didn't want to put in too much effort to make the code more complicated to handle this

As a side effect, if a Linux user has `cargo.toml` and `Cargo.toml`, we'll only put one of them in the `.crate` file.  We can extend this out to also include a warning for portability for case insensitive filesystems but I left that for after rust-lang#12235.

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

A PR at a time will show how the behavior changed as the source was edited

This does add a direct dependency on `unicase` to help keep case-insensitive comparisons easy / clear and to avoid riskier areas for bugs like writing an appropriate `Hash` implementation.  `unicase` is an existing transitive dependency of cargo.

### Additional information

Fixes rust-lang#12384

[Discussion on Zulip](https://rust-lang.zulipchat.com/#narrow/stream/246057-t-cargo/topic/.60cargo.2Etoml.60.20on.20case.20insensitive.20filesystems)
test: relax assertions of panic handler message format
When running test in rust-lang/rust via `./x test src/tool/cargo`,
there is no rustup integrated.
Git only assumes a submodule URL is a relative path if it starts with `./`
or `../` [^1]. To fetch the correct repo, we need to construct an aboslute
submodule URL.

At this moment it comes with some limitations:

* GitHub doesn't accept non-normalized URLs wth relative paths.
  (`ssh://[email protected]/rust-lang/cargo.git/relative/..` is invalid)
* `url` crate cannot parse SCP-like URLs.
  (`[email protected]:rust-lang/cargo.git` is not a valid WHATWG URL)

To overcome these, this patch always tries `Url::parse` first to normalize
the path. If it couldn't, append the relative path as the last resort and
pray the remote git service supports non-normalized URLs.

See also rust-lang#12404 and rust-lang#12295.

[^1]: <https://git-scm.com/docs/git-submodule>
fix: normalize relative git submodule urls with `ssh://`
@weihanglo weihanglo force-pushed the cargo-semver-checks branch 4 times, most recently from 7b6585f to a95e615 Compare August 1, 2023 21:31
@weihanglo weihanglo closed this Aug 3, 2023
@weihanglo weihanglo deleted the cargo-semver-checks branch August 3, 2023 13:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants