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

Implement support for base paths (RFC 3529) #12974

Closed
wants to merge 1 commit into from

Conversation

dpaoliello
Copy link
Contributor

@dpaoliello dpaoliello commented Nov 14, 2023

RFC: rust-lang/rfcs#3529
Tracking Issue: #14355

Introduce a table of path "bases" in Cargo configuration files that can be used
to prefix the path of path dependencies and patch entries.

This feature will not support declaring path bases in manifest files to avoid
additional design complexity, though this may be added in the future.

Motivation

As a project grows in size, it becomes necessary to split it into smaller
sub-projects, architected into layers with well-defined boundaries.

One way to enforce these boundaries is to use different Git repos (aka
"multi-repo"). Cargo has good support for multi-repo projects using either git
dependencies, or developers can use private registries if they want to
explicitly publish code or need to preprocess their sub-projects (e.g.,
generating code) before they can be consumed.

If all of the code is kept in a single Git repo (aka "mono-repo"), then these
boundaries must be enforced a different way: either leveraging tooling during
the build to check layering, or requiring that sub-projects explicitly publish
and consume from some intermediate directory. Cargo has poor support for
mono-repos: the only viable mechanism is path dependencies, but these require
relative paths (which makes refactoring and moving sub-projects very difficult)
and don't work at all if the mono-repo requires publishing and consuming from an
intermediate directory (as this may very per host, or per target being built).

This RFC proposes a mechanism to specify path bases in config.toml files which
can be used to prepend path dependencies. This allows mono-repos to specify
dependencies relative to their root directory, which allows the consuming
project to be moved freely (no relative paths to update) and a simple
find-and-replace to handle a producing project being moved. Additionally, a
host-specific or target-specific intermediate directory may be specified as a
base, allowing code to be consumed from there using path dependencies.

Implementation Details

  • Adds support in path dependencies and patch table.
  • Validates base path names conform to RFC.
  • Adds workspace built-in path base.
  • Adds support for cargo add (via --base option).
  • Adds documentation to the reference.

@rustbot
Copy link
Collaborator

rustbot commented Nov 14, 2023

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.

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-manifest Area: Cargo.toml issues A-unstable Area: nightly unstable support S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 14, 2023
Copy link
Contributor

@epage epage left a comment

Choose a reason for hiding this comment

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

Blocked on rust-lang/rfcs#3529

(doing this just to make it obvious in tandem with the draft status)

@dpaoliello
Copy link
Contributor Author

FYI, there appears to be a bug where, if a workspace has a path dependency with a base, then the member using that workspace dependency still sees the path relative to the workspace Cargo.toml rather than as-written - I'm currently looking into a fix.

@bors
Copy link
Contributor

bors commented Dec 1, 2023

☔ The latest upstream changes (presumably #13080) made this pull request unmergeable. Please resolve the merge conflicts.

@bors
Copy link
Contributor

bors commented Jan 15, 2024

☔ The latest upstream changes (presumably #13296) made this pull request unmergeable. Please resolve the merge conflicts.

@dpaoliello dpaoliello force-pushed the basepath branch 5 times, most recently from 897f38e to 96c9c05 Compare August 5, 2024 18:55
@dpaoliello dpaoliello mentioned this pull request Aug 5, 2024
10 tasks
@dpaoliello dpaoliello force-pushed the basepath branch 2 times, most recently from b75dd6b to 5d87b9f Compare August 5, 2024 22:40
@rustbot rustbot added A-cli Area: Command-line interface, option parsing, etc. A-documenting-cargo-itself Area: Cargo's documentation Command-add labels Aug 5, 2024
@dpaoliello dpaoliello force-pushed the basepath branch 3 times, most recently from 61b3354 to 1ca5eb2 Compare August 5, 2024 23:22
@rustbot rustbot added the A-cli-help Area: built-in command-line help label Aug 5, 2024
@dpaoliello dpaoliello changed the title Implement support for base paths Implement support for base paths (RFC 3529) Aug 5, 2024
@dpaoliello dpaoliello marked this pull request as ready for review August 6, 2024 16:22
@dpaoliello
Copy link
Contributor Author

Question for reviewers: should I remove the documentation (either for the feature itself or for cargo add), or just mention that it is currently unstable?

@epage
Copy link
Contributor

epage commented Aug 6, 2024

CLI flags should have (unstable). All other documentation should live on https://doc.rust-lang.org/cargo/reference/unstable.html until stabilized.

@epage
Copy link
Contributor

epage commented Aug 6, 2024

I might not get to this for a few days. Something that might be helpful is to split the core feature PR from cargo add. Keeping the PRs smaller will make them easier to review (there are times where I have a PR that is just adding the unstable feature flag and a placeholder location in the docs) and cargo add support for things tends to be a feature in of itself.

@dpaoliello dpaoliello marked this pull request as draft August 6, 2024 20:14
@dpaoliello
Copy link
Contributor Author

I might not get to this for a few days. Something that might be helpful is to split the core feature PR from cargo add. Keeping the PRs smaller will make them easier to review (there are times where I have a PR that is just adding the unstable feature flag and a placeholder location in the docs) and cargo add support for things tends to be a feature in of itself.

Fair enough: I'll convert this to a draft and then split it into chunks.

@dpaoliello
Copy link
Contributor Author

Splitting this PR into a series of smaller PRs

@dpaoliello dpaoliello closed this Aug 6, 2024
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. A-cli-help Area: built-in command-line help A-documenting-cargo-itself Area: Cargo's documentation A-manifest Area: Cargo.toml issues A-unstable Area: nightly unstable support Command-add S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants