paritytech / parity-publish

A tool to manage publishing Parity's crates
Apache License 2.0
5 stars 1 forks source link

Supporting Workspace Dependencies #35

Open Morganamilo opened 2 months ago

Morganamilo commented 2 months ago

Problem

Between polkadot release 1.13 and 1.14 many of the polkadot crates were declared as dependencies in the workspace Cargo.toml file and then referenced as dependencies elsewhere using dep.workspace = true.

Parity publish does not know how to edit these which led to the tooling failing on publish. Adding support for this is a little tricky due to a few edge cases and cargo not providing an API to edit these like it does with normal dependencies.

The naive solution would be to replace the dep.workspace = true with a normal dependency at publish time. However this is still tricky because the package may be renamed in the workspace dependency declaration and there's no way to figure this out based on the declarations in individual packages.

Even without this issue, this would not be a good idea long term. As a backport may adjust the workspace dependency in some way and this would then not propagate to the packages as they no longer reference the workspace dependency.

Solution

The only sensible solution we really have although hacky is for parity publish to open the Cargo.toml file itself and edit the dependencies via adjusting the toml directly instead of going through cargo's API.

This solves the issue but leaves us with some new problems.

Dev dependencies

Normally we strip all versions from crate dev-dependencies as this makes cargo ignore them during publishing. Now that these dependencies are declared in one centralised location. Giving them a version during the regular dependency rewriting also forces them to have a version in the dev-dependency section.

This breaks things because parity publish assumes cargo will ignore dev-dependencies and thus doesn't factor them in when deciding the publish order.

This is not a fixable issue as when factoring in dev-dependencies, the dependency graph becomes cyclic and there becomes no valid way to order the crates for publishing.

Solution

The solution to this is again hacky. For dev-depdependencies we read the workspace dependency declaration from the toml directly then replace the dep.workspace = true declaration and merge the feature fields together.

This is mostly fine, as we don't give dep-dependencies versions it's impossible for this to desync with the declaration in the workspace manifest. However if the dependencies are to move path or have their features adjusted they could then go out of sync. The impact of this is minimal as dev-dependencies are only used for tests so some desync here shouldn't break the release.

A workaround for this could be to remerge the fields together whenever parity-publish is ran. This would have to assume any dev-dependency that matches a dependency in the root manifest was originally a workspace dependency. While this isn't strictly true it should always be true in practise.