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

Update package versions and local path dependencies #2247

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

hallipr
Copy link
Member

@hallipr hallipr commented Feb 26, 2025

  • root/workspace Cargo.toml should only reference released versions for dependencies
    • this allows packages in the repo to take workspaces references and release with stable dependencies
  • all /Cargo.toml versions should normally reflect the next unreleased prerelease version
    • the repo is in a continuous state of preparing the next release of any given package.
    • a package shouldn't have changes in its source while keeping the same version number as a released version
  • any package that needs to depend on another unreleased package should take a path dependency to the unreleased package, not a workspace dependency

@github-actions github-actions bot added Azure.Core The azure_core crate Azure.Identity The azure_identity crate Storage Storage Service (Queues, Blobs, Files) labels Feb 26, 2025
@@ -29,41 +29,23 @@ rust-version = "1.80"

[workspace.dependencies.typespec]
default-features = false
path = "sdk/typespec"
Copy link
Member

Choose a reason for hiding this comment

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

Is this what we decided to do? I thought we were going to leave both. This really needs to be written up somewhere and not locked in our heads. We've waffled about this too much.

That said, this is maybe premature because:

  1. We should test package-workspace and help stabilize that. Seems it'll solve most of our problems and we won't have to have this difficult solution.
  2. Until GA, we'll be rev'ing everything anyway. beta.2 is going to include a massive number of changes because I'm changing azure_core significantly.

Copy link
Member

Choose a reason for hiding this comment

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

If anything, maybe we should be keeping the path dependency for the foreseeable future.

Copy link
Member Author

Choose a reason for hiding this comment

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

What will we do about late shippers? If workspace is a path dependency and Cosmos takes a workspace dependency, then we can't change core/typespec packages until after cosmos ships. This could block changes in core or typespec for weeks at a time

@@ -28,8 +28,11 @@ serde_json.workspace = true
sha2 = { workspace = true, optional = true }
tokio = { workspace = true, optional = true }
tracing.workspace = true
typespec = { workspace = true, features = ["http", "json"] }
typespec_client_core = { workspace = true, features = ["http", "json"] }
typespec = { path = "../../typespec", features = ["http", "json"] }
Copy link
Member

Choose a reason for hiding this comment

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

I think all crates should be using path dependencies until GA (or close). See previous comments.

@hallipr hallipr requested a review from weshaggard as a code owner February 27, 2025 22:52
@hallipr hallipr force-pushed the users/pahallis/versions branch from 8a3040e to 285ed44 Compare February 27, 2025 23:02
@hallipr hallipr force-pushed the users/pahallis/versions branch from 285ed44 to 6aacee0 Compare February 27, 2025 23:12
@hallipr hallipr force-pushed the users/pahallis/versions branch from 6aacee0 to f60c6ae Compare February 27, 2025 23:14
@@ -18,7 +18,7 @@ categories = ["api-bindings"]
edition = "2021"

[dependencies]
azure_core = { path = "../azure_core", default-features = false }
azure_core = { path = "../azure_core", version = "0.23.0", default-features = false }
Copy link
Member

Choose a reason for hiding this comment

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

Versions should only be in the workspace Cargo.toml. I don't want to manager versions across a swatch of files.

Copy link
Member Author

Choose a reason for hiding this comment

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

anything with a path dependency is unshippable without a version req. We have to choose between:

  1. Path and version in workspace toml
    • Dependency packages will be frozen until dependents ship against workspace version
  2. Path and version in dependents
    • Dependents will be responsible for keeping their version in sync
  3. Path only in dependents
    • Version can be fixed up at release time
    • Will have to solve for tomlyn mangling of toml (either use rust or regex?)

Copy link
Member

Choose a reason for hiding this comment

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

But I thought we can put both the path and version in the workspace Cargo.toml. I do not want to keep track of all the versions in the crates' Cargo.tomls. Most other languages use centralized versions/dependencies as well. It's far easier to keep track of and resolve CG alerts, etc.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Azure.Core The azure_core crate Azure.Identity The azure_identity crate Storage Storage Service (Queues, Blobs, Files)
Projects
Status: In Progress
Development

Successfully merging this pull request may close these issues.

2 participants