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

Add "--workspace" to update command #8725

Merged
merged 7 commits into from Dec 2, 2020
Merged

Add "--workspace" to update command #8725

merged 7 commits into from Dec 2, 2020

Conversation

chaaz
Copy link
Contributor

@chaaz chaaz commented Sep 22, 2020

My --bin project has CI which updates the version number in Cargo.toml, which it then commits. However, this means that any further cargo command (build, test, etc) will update the existing Cargo.lock file (updating the root version), causing some frustration for users. Furthermore, it breaks the publish command, which requires the repo to be current.

I've added a sync-lockfile command to simply update the root version in the Cargo.lock file to match the Cargo.toml in the same way that simple commands like fetch do. If no Cargo.lock file is present, and new one is generated based on the index.

This is a demo PR for Pre-RFC conversation at https://internals.rust-lang.org/t/pre-rfc-cargo-command-to-just-sync-lockfile/13119, but may become a real PR if it gets approval.

@rust-highfive
Copy link

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

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 22, 2020
@ehuss
Copy link
Contributor

ehuss commented Sep 28, 2020

Thanks for the PR! The Cargo team talked about this, and we feel that this would probably be better to be part of cargo update. There is a pretty high bar to add a new top-level command, and this doesn't feel like it really needs a new command. Perhaps a --workspace flag to mirror other commands? I'm not certain that it is 100% equivalent to pass workspace members to the avoid list, but I believe it should be roughly the same.

@chaaz
Copy link
Contributor Author

chaaz commented Sep 28, 2020

Thanks for the PR! The Cargo team talked about this, and we feel that this would probably be better to be part of cargo update. There is a pretty high bar to add a new top-level command, and this doesn't feel like it really needs a new command. Perhaps a --workspace flag to mirror other commands? I'm not certain that it is 100% equivalent to pass workspace members to the avoid list, but I believe it should be roughly the same.

Thanks, @ehuss. Discussion on the forums came to roughly the same conclusion. I'll change the PR so that update --workspace does what I want, instead of an entirely new command.

Are the Cargo Book descriptions auto-pulled from the source, or should I be updating that separately?

@ehuss
Copy link
Contributor

ehuss commented Sep 29, 2020

Are the Cargo Book descriptions auto-pulled from the source, or should I be updating that separately?

The documentation is written in markdown. The cargo-update documentation is here: https://github.com/rust-lang/cargo/blob/master/src/doc/man/cargo-update.md. After updating the docs, you need to run build-man.sh to rebuild the man pages. There's more documentation on how this works at https://github.com/rust-lang/cargo/tree/master/src/doc.

@ehuss ehuss 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 Oct 7, 2020
@chaaz
Copy link
Contributor Author

chaaz commented Nov 6, 2020

@ehuss Sorry for the slow response: real life intervened. Anywho, I have moved the functionality to update --workspace as discussed, and have added the flag to the docs as well. Please let me know how that looks.

@chaaz chaaz changed the title Add "sync-lockfile" command Add "--workspace" to update command Nov 7, 2020
Copy link
Contributor

@ehuss ehuss left a comment

Choose a reason for hiding this comment

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

Thanks!

From an implementation standpoint, I was thinking that the --workspace flag would just add the set of member PackageIds to the to_avoid list. That would avoid needing a special code path. This current implementation seems to ignore additional -p flags, and doesn't report what was updated. Do you think there would be any particular problems with that approach, or were there other reasons to go with the current design?

@@ -7,6 +7,7 @@ pub fn cli() -> App {
subcommand("update")
.about("Update dependencies as recorded in the local lock file")
.arg(opt("quiet", "No output printed to stdout").short("q"))
.arg(opt("workspace", "Only update the workspace pakcages").short("w"))
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
.arg(opt("workspace", "Only update the workspace pakcages").short("w"))
.arg(opt("workspace", "Only update the workspace packages").short("w"))

@chaaz
Copy link
Contributor Author

chaaz commented Nov 11, 2020

Thanks!

From an implementation standpoint, I was thinking that the --workspace flag would just add the set of member PackageIds to the to_avoid list. That would avoid needing a special code path. This current implementation seems to ignore additional -p flags, and doesn't report what was updated. Do you think there would be any particular problems with that approach, or were there other reasons to go with the current design?

Thanks, that does seem like a better approach. Will put that in.

@chaaz
Copy link
Contributor Author

chaaz commented Nov 12, 2020

@ehuss Just checked in the suggested changes: looks like the best way is just to keep the to_avoid array empty (-ish) if there's a --workspaces flag. That is much easier approach, and integrates better with existing behavior. Let me know what you think.

@ehuss
Copy link
Contributor

ehuss commented Nov 12, 2020

@rfcbot fcp merge

This adds a new stable flag --workspace (or -w) to cargo update which will cause it to update only the workspace members (or more specifically, it does an update equivalent to cargo build which updates any out-of-date path sources). This can be useful when bumping the version in Cargo.toml, and you want to update Cargo.lock, but don't want to run some other command (like build or metadata), or don't want to explicitly list each workspace member like cargo update -p foo.

@ehuss ehuss added T-cargo Team: Cargo and removed S-waiting-on-author Status: The marked PR is awaiting some action (such as code changes) from the PR author. labels Nov 12, 2020
@ehuss
Copy link
Contributor

ehuss commented Nov 12, 2020

@rfcbot fcp merge

@rfcbot
Copy link
Collaborator

rfcbot commented Nov 12, 2020

Team member @ehuss has proposed to merge this. The next step is review by the rest of the tagged team members:

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

@rfcbot rfcbot added proposed-final-comment-period An FCP proposal has started, but not yet signed off. disposition-merge FCP with intent to merge labels Nov 12, 2020
@rfcbot rfcbot added the final-comment-period FCP — a period for last comments before action is taken label Dec 1, 2020
@rfcbot
Copy link
Collaborator

rfcbot commented Dec 1, 2020

🔔 This is now entering its final comment period, as per the review above. 🔔

@rfcbot rfcbot removed the proposed-final-comment-period An FCP proposal has started, but not yet signed off. label Dec 1, 2020
@ehuss
Copy link
Contributor

ehuss commented Dec 2, 2020

Thanks again @chaaz!

@bors r+

@bors
Copy link
Collaborator

bors commented Dec 2, 2020

📌 Commit 42f3469 has been approved by ehuss

@bors bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Dec 2, 2020
@bors
Copy link
Collaborator

bors commented Dec 2, 2020

⌛ Testing commit 42f3469 with merge 63d0fe4...

@bors
Copy link
Collaborator

bors commented Dec 2, 2020

☀️ Test successful - checks-actions
Approved by: ehuss
Pushing 63d0fe4 to master...

@bors bors merged commit 63d0fe4 into rust-lang:master Dec 2, 2020
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Dec 4, 2020
Update cargo

12 commits in bfca1cd22bf514d5f2b6c1089b0ded0ba7dfaa6e..63d0fe43449adcb316d34d98a982b597faca4178
2020-11-24 16:33:21 +0000 to 2020-12-02 01:44:30 +0000
- Add "--workspace" to update command (rust-lang/cargo#8725)
- Add an FAQ for "Why is my crate rebuilt?" (rust-lang/cargo#8927)
- Set docs.rs as the default extern-map for crates.io (rust-lang/cargo#8877)
- remove extra whitespace when running cargo -Z help (rust-lang/cargo#8924)
- Remove version from dev-dependencies to make it easier to publish. (rust-lang/cargo#8920)
- update dependency queue to consider cost for each node (rust-lang/cargo#8908)
- Fix some rustdoc warnings. (rust-lang/cargo#8911)
- Bump miow dependency to not invalidly assume memory layout (rust-lang/cargo#8909)
- Remove unnecessary trailing semicolons (rust-lang/cargo#8906)
- Fix custom_target_dependency test. (rust-lang/cargo#8907)
- fix: we don't ignore `version` for `git`/`path` deps now (rust-lang/cargo#8900)
- doc (book): add "Getting Started" subsection: "Essential Terminology" (rust-lang/cargo#8855)
@rfcbot rfcbot added finished-final-comment-period FCP complete to-announce and removed final-comment-period FCP — a period for last comments before action is taken labels Dec 11, 2020
netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this pull request Apr 19, 2021
Pkgsrc changes:
 * Adjust patches, re-compute line offsets, fix capitalization.
 * Remove i686/FreeBSD support, no longer provided upstream.
 * Bump bootstraps to 1.49.0.
 * Change USE_TOOLS from bsdtar to gtar.
 * Reduce diffs to pkgsrc-wip package patches.
 * Allow rust.BUILD_TARGET to override automatic choice of target.
 * Add an i586/NetBSD (pentium) bootstrap variant (needs testing),
   not yet added as bootstrap since 1.49 doesn't have that variant.

Upstream changes:

Version 1.50.0 (2021-02-11)
============================

Language
-----------------------
- [You can now use `const` values for `x` in `[x; N]` array
  expressions.][79270] This has been technically possible since
  1.38.0, as it was unintentionally stabilized.
- [Assignments to `ManuallyDrop<T>` union fields are now considered
  safe.][78068]

Compiler
-----------------------
- [Added tier 3\* support for the `armv5te-unknown-linux-uclibceabi`
  target.][78142]
- [Added tier 3 support for the `aarch64-apple-ios-macabi` target.][77484]
- [The `x86_64-unknown-freebsd` is now built with the full toolset.][79484]

\* Refer to Rust's [platform support page][forge-platform-support] for more
information on Rust's tiered platform support.

Libraries
-----------------------

- [`proc_macro::Punct` now implements `PartialEq<char>`.][78636]
- [`ops::{Index, IndexMut}` are now implemented for fixed sized
  arrays of any length.][74989]
- [On Unix platforms, the `std::fs::File` type now has a "niche"
  of `-1`.][74699] This value cannot be a valid file descriptor,
  and now means `Option<File>` takes up the same amount of space
  as `File`.

Stabilized APIs
---------------

- [`bool::then`]
- [`btree_map::Entry::or_insert_with_key`]
- [`f32::clamp`]
- [`f64::clamp`]
- [`hash_map::Entry::or_insert_with_key`]
- [`Ord::clamp`]
- [`RefCell::take`]
- [`slice::fill`]
- [`UnsafeCell::get_mut`]

The following previously stable methods are now `const`.

- [`IpAddr::is_ipv4`]
- [`IpAddr::is_ipv6`]
- [`Layout::size`]
- [`Layout::align`]
- [`Layout::from_size_align`]
- `pow` for all integer types.
- `checked_pow` for all integer types.
- `saturating_pow` for all integer types.
- `wrapping_pow` for all integer types.
- `next_power_of_two` for all unsigned integer types.
- `checked_power_of_two` for all unsigned integer types.

Cargo
-----------------------

- [Added the `[build.rustc-workspace-wrapper]` option.][cargo/8976]
  This option sets a wrapper to execute instead of `rustc`, for
  workspace members only.
- [`cargo:rerun-if-changed` will now, if provided a directory, scan the entire
  contents of that directory for changes.][cargo/8973]
- [Added the `--workspace` flag to the `cargo update` command.][cargo/8725]

Misc
----

- [The search results tab and the help button are focusable with
  keyboard in rustdoc.][79896]
- [Running tests will now print the total time taken to execute.][75752]

Compatibility Notes
-------------------

- [The `compare_and_swap` method on atomics has been deprecated.][79261]
  It's recommended to use the `compare_exchange` and
  `compare_exchange_weak` methods instead.
- [Changes in how `TokenStream`s are checked have fixed some cases
  where you could write unhygenic `macro_rules!` macros.][79472]
- [`#![test]` as an inner attribute is now considered unstable like
  other inner macro attributes, and reports an error by default
  through the `soft_unstable` lint.][79003]
- [Overriding a `forbid` lint at the same level that it was set is
  now a hard error.][78864]
- [Dropped support for all cloudabi targets.][78439]
- [You can no longer intercept `panic!` calls by supplying your
  own macro.][78343] It's recommended to use the `#[panic_handler]`
  attribute to provide your own implementation.
- [Semi-colons after item statements (e.g. `struct Foo {};`) now
  produce a warning.][78296]

[74989]: rust-lang/rust#74989
[79261]: rust-lang/rust#79261
[79896]: rust-lang/rust#79896
[79484]: rust-lang/rust#79484
[79472]: rust-lang/rust#79472
[79270]: rust-lang/rust#79270
[79003]: rust-lang/rust#79003
[78864]: rust-lang/rust#78864
[78636]: rust-lang/rust#78636
[78439]: rust-lang/rust#78439
[78343]: rust-lang/rust#78343
[78296]: rust-lang/rust#78296
[78068]: rust-lang/rust#78068
[75752]: rust-lang/rust#75752
[74699]: rust-lang/rust#74699
[78142]: rust-lang/rust#78142
[77484]: rust-lang/rust#77484
[cargo/8976]: rust-lang/cargo#8976
[cargo/8973]: rust-lang/cargo#8973
[cargo/8725]: rust-lang/cargo#8725
[`IpAddr::is_ipv4`]: https://doc.rust-lang.org/stable/std/net/enum.IpAddr.html#method.is_ipv4
[`IpAddr::is_ipv6`]: https://doc.rust-lang.org/stable/std/net/enum.IpAddr.html#method.is_ipv6
[`Layout::align`]: https://doc.rust-lang.org/stable/std/alloc/struct.Layout.html#method.align
[`Layout::from_size_align`]: https://doc.rust-lang.org/stable/std/alloc/struct.Layout.html#method.from_size_align
[`Layout::size`]: https://doc.rust-lang.org/stable/std/alloc/struct.Layout.html#method.size
[`Ord::clamp`]: https://doc.rust-lang.org/stable/std/cmp/trait.Ord.html#method.clamp
[`RefCell::take`]: https://doc.rust-lang.org/stable/std/cell/struct.RefCell.html#method.take
[`UnsafeCell::get_mut`]: https://doc.rust-lang.org/stable/std/cell/struct.UnsafeCell.html#method.get_mut
[`bool::then`]: https://doc.rust-lang.org/stable/std/primitive.bool.html#method.then
[`btree_map::Entry::or_insert_with_key`]: https://doc.rust-lang.org/stable/std/collections/btree_map/enum.Entry.html#method.or_insert_with_key
[`f32::clamp`]: https://doc.rust-lang.org/stable/std/primitive.f32.html#method.clamp
[`f64::clamp`]: https://doc.rust-lang.org/stable/std/primitive.f64.html#method.clamp
[`hash_map::Entry::or_insert_with_key`]: https://doc.rust-lang.org/stable/std/collections/hash_map/enum.Entry.html#method.or_insert_with_key
[`slice::fill`]: https://doc.rust-lang.org/stable/std/primitive.slice.html#method.fill
@ehuss ehuss added this to the 1.50.0 milestone Feb 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
disposition-merge FCP with intent to merge finished-final-comment-period FCP complete S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-cargo Team: Cargo to-announce
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants