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

Option to not bump versions of already released crates #4

Open
ascjones opened this issue Nov 16, 2022 · 5 comments
Open

Option to not bump versions of already released crates #4

ascjones opened this issue Nov 16, 2022 · 5 comments

Comments

@ascjones
Copy link

Recently we released new versions of sp-core and sp-runtime using subpub paritytech/substrate#12599. A follow up was required to release a couple of higher level crates: paritytech/substrate#12716.

However using subpub tried to bump all those already released crates again:

image

I did it manually in the end which was fairly simple, however it would be good to be able to have an option to not bump all those already released crates. Either explicitly exclude some crates by name e.g. sp-core or specify to only bump the crate to be released.

@dvdplm
Copy link

dvdplm commented Nov 16, 2022

I'd even say not bumping and re-relasing already released crates should be the default? It seems like the most common behaviour.

/cc @joao-paulo-parity

@niklasad1
Copy link
Member

niklasad1 commented Nov 16, 2022

I'd even say not bumping and re-relasing already released crates should be the default? It seems like the most common behaviour.

That's implies that the tool has to check "the crate's dependency graph" and compare each dep's version to previous release on crates.io if one exist otherwise do nothing. So that should be possible but requires some work.

Alternatively, the tool could also initially bump the versions of all crates which depends on the updated crates "not only its dependencies". To elaborate if I want to bump sp-core with the tool then sp-core is bumped and its dependencies but it may be x other crates that depend on sp-core with the new version and those crates are not bumped. Thus, it may end up in situation where a crate such as sp-keyring where the dependencies are already updated just not version of that crate.

I'm still in favor what you are suggesting @dvdplm but --exclude-crate foo could also be an option.

@dvdplm
Copy link

dvdplm commented Nov 16, 2022

That's implies that the tool has to check "the crate's dependency graph" and compare each dep's version to previous release on crates.io if one exist otherwise do nothing. So that should be possible but requires some work.

Right, I somehow thought it already did this through the crates.io API. We are probably already using crates_io_api yeah? It looks like the Crate stores the version as a u64 so there'll be some conversion to do as well.

@joao-paulo-parity
Copy link

I think this code section is the most pertaining to the scenario (note the comment)

subpub/src/crates.rs

Lines 259 to 263 in 4871f8d

// If the crate itself needs publishing, mark it and anything
// depending on it as needing publishing.
if details.details.needs_publishing()? {
set_needs_publishing(&mut tree, name);
}

There's a function to decide on if a crate needs publishing by comparing its contents against crates.io

subpub/src/crate_details.rs

Lines 186 to 193 in 4871f8d

/// This checks whether we actually need to publish a new version of the crate. It'll return `false`
/// only if, as far as we can see, the current version is published to crates.io, and there have been
/// no changes to it since.
pub fn needs_publishing(&self) -> anyhow::Result<bool> {
let name = &self.name;
let crate_bytes = external::crates_io::try_download_crate(&self.name, &self.version)
.with_context(|| format!("Could not download crate {name}"))?;

But as far as I can tell that check only matters for the crates you've selected on the CLI, not their dependents. Any dependents of the input crates are automatically set for publishing without first comparing them against crates.io, as per the start of this post.

@jsdw
Copy link
Collaborator

jsdw commented Jan 6, 2023

FWIW I thought that it did check all dependents and only bumped versions when it detected some change in them (or some change in their dependencies which mean they need bumping too).

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

No branches or pull requests

5 participants