Skip to content
This repository was archived by the owner on Nov 15, 2023. It is now read-only.

WIP: Enable continuous package release to crates.io #9417

Closed
Xanewok wants to merge 5 commits into from
Closed

WIP: Enable continuous package release to crates.io #9417

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
71e7e22
Run de-dev deps manually in publish-to-crates step
Xanewok Jul 22, 2021
9ef1b1f
Mark packages we don't want to publish as such in the manifest
Xanewok Jul 22, 2021
e05a582
WIP: See if we can unleash packages
Xanewok Jul 22, 2021
26ba28c
oops
Xanewok Jul 22, 2021
48c847d
Update .gitlab-ci.yml
Xanewok Aug 3, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 16 additions & 6 deletions .gitlab-ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ variables: &default-vars
CI_IMAGE: "paritytech/ci-linux:production"
# FIXME set to release
CARGO_UNLEASH_INSTALL_PARAMS: "--version 1.0.0-alpha.12"
CARGO_UNLEASH_PKG_DEF: "--skip node node-* pallet-template pallet-example pallet-example-* subkey chain-spec-builder"

default:
cache: {}
Expand Down Expand Up @@ -337,12 +336,15 @@ unleash-check:
<<: *test-refs-no-trigger
Copy link
Contributor

@TriplEight TriplEight Jul 22, 2021

Choose a reason for hiding this comment

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

do you really want to publish before the tests are green?

Copy link
Contributor Author

@Xanewok Xanewok Jul 22, 2021

Choose a reason for hiding this comment

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

That's the unleash-check - I think it should be run early/at the same time as tests; only publishing should be done after every test is green

Copy link
Contributor

@TriplEight TriplEight Jul 22, 2021

Choose a reason for hiding this comment

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

In my view (I actually do not know what does it check, Check whether crates can be packaged in the docs is probably not enough) it's an overkill to run these checks on every commit. We should run them before the release target, so maybe on master, or even nightly?

Copy link
Contributor Author

@Xanewok Xanewok Jul 22, 2021

Choose a reason for hiding this comment

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

If I understand this correctly, what you propose may lead to the PR checks being green at first but only discovered to be red once merged. If that's the case, then won't be much incentive to fix this once a PR is merged and it'll just hang around being red until someone disables it again - I'd rather we catch that early.

If this is deemed to expensive to run on every commit, we might want to run this as an extra step for bot merge to not merge a given PR until we know for sure that this check is green.

Copy link
Contributor

@TriplEight TriplEight Jul 23, 2021

Choose a reason for hiding this comment

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

It is expensive, the job takes 30 minutes. Any possible way to combine it with something to reuse the cache?

Ideally, it will run on the pre-merge pipeline and a merge-train, but they are not ready yet.

Copy link
Contributor

@TriplEight TriplEight Aug 3, 2021

Choose a reason for hiding this comment

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

Maybe it's better to either check only if certain files are changed or versions got bumped?

Copy link
Contributor

@TriplEight TriplEight Sep 13, 2021

Choose a reason for hiding this comment

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

Suggested change
<<: *test-refs-no-trigger
<<: *build-refs

script:
- cargo install cargo-unleash ${CARGO_UNLEASH_INSTALL_PARAMS}
Copy link
Contributor

@TriplEight TriplEight Jul 22, 2021

Choose a reason for hiding this comment

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

Is it mature enough to be installed into CI image?

Copy link
Contributor Author

@Xanewok Xanewok Jul 22, 2021

Choose a reason for hiding this comment

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

Probably - until it's not v1 I'd prefer not to; the time savings is miniscule I'd say comparing to the actual checking done by the tool later on

Copy link
Contributor

@TriplEight TriplEight Jul 22, 2021

Choose a reason for hiding this comment

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

not on every commit tho

# TODO: Don't specify crates version in dev-dependencies - cargo automatically
# prunes such dependencies from the manifest. Otherwise we form a package
# cycle and cannot proceed with publishing/checking
- cargo unleash de-dev-deps
# Reuse build artifacts when running checks (cuts down check time by 3x)
# TODO: Implement this optimization in cargo-unleash rather than here
- mkdir -p target/unleash
- export CARGO_TARGET_DIR=target/unleash
- cargo unleash check ${CARGO_UNLEASH_PKG_DEF}
- cargo unleash check

test-frame-examples-compile-to-wasm:
# into one job
Expand Down Expand Up @@ -670,14 +672,22 @@ publish-draft-release:
allow_failure: true

publish-to-crates-io:
# `cargo unleash em-dragons` first syncs the latest crates.io versions, then checks the local
# tree against them. Only the crates that locally have a version not found on crates.io (and
# don't have a `publish = false` flag in their `Cargo.toml`) will be attempted to be released.
# If the PR didn't change any version (for the majority of PRs) – no matter what others changes
# have occurred – this does exactly nothing, states that all is fine and quits.
stage: publish
needs:
- job: unleash-check
Copy link
Contributor

@TriplEight TriplEight Jul 22, 2021

Choose a reason for hiding this comment

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

Either you should list the jobs necessary to be green in order to start this job, because this way it starts right after unleash-check without waiting anything else. Or remove this needs and the stage won't start if the previous jobs are not successful. I prefer the latter.

Copy link
Contributor Author

@Xanewok Xanewok Jul 22, 2021

Choose a reason for hiding this comment

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

Can we somehow make it so that it runs at publish but also make sure that unleash-check is never allow_failure: true?

Copy link
Contributor Author

@Xanewok Xanewok Jul 22, 2021

Choose a reason for hiding this comment

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

By publish I mean only after everything else completes from the test stage and so on

Copy link
Contributor

@TriplEight TriplEight Jul 22, 2021

Choose a reason for hiding this comment

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

We can run unleash check on the build stage, so after all the code tests pass, and unleash em-dragons on the publish phase after everything's proven to be buildable.

Copy link
Contributor Author

@Xanewok Xanewok Jul 22, 2021

Choose a reason for hiding this comment

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

I can move this to the build stage if you want me to, running parallel to the tests is also good (at least wrt developer feedback latency, maybe not so much wrt CI builder overhead 😀 )

Copy link
Contributor

@TriplEight TriplEight Jul 23, 2021

Choose a reason for hiding this comment

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

it's worth it, the test stage already brings a lot of concurrent jobs. The longest job in the test stage is test-linux-stable , which takes 36 minutes. This one takes ~32. The longest on the build stage is check-polkadot-companion-build , so unless we make use of caching (or simplify it somehow) it will severely contribute to the pipelines concurrency.

<<: *docker-env
Copy link
Contributor

@TriplEight TriplEight Sep 13, 2021

Choose a reason for hiding this comment

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

Suggested change
<<: *docker-env
<<: *docker-env
<<: *build-refs

rules:
- if: $CI_COMMIT_REF_NAME =~ /^ci-release-.*$/
- if: $CI_COMMIT_REF_NAME =~ /^v[0-9]+\.[0-9]+.*$/ # i.e. v1.0, v2.1rc1
script:
- cargo install cargo-unleash ${CARGO_UNLEASH_INSTALL_PARAMS}
- cargo unleash em-dragons --no-check --owner github:paritytech:core-devs ${CARGO_UNLEASH_PKG_DEF}
# TODO: Don't specify crates version in dev-dependencies - cargo automatically
# prunes such dependencies from the manifest. Otherwise we form a package
# cycle and cannot proceed with publishing/checking
- cargo unleash de-dev-deps
- cargo unleash em-dragons --no-check --owner github:paritytech:core-devs --dry-run
allow_failure: true
Copy link
Contributor

@TriplEight TriplEight Jul 22, 2021

Choose a reason for hiding this comment

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

this may be removed as soon as it will publish stably.


#### stage: deploy
Expand Down
1 change: 1 addition & 0 deletions bin/node/bench/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ authors = ["Parity Technologies <[email protected]>"]
description = "Substrate node integration benchmarks."
edition = "2018"
license = "GPL-3.0-or-later WITH Classpath-exception-2.0"
publish = false

# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

Expand Down
1 change: 1 addition & 0 deletions bin/node/browser-testing/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ authors = ["Parity Technologies <[email protected]>"]
description = "Tests for the in-browser light client."
edition = "2018"
license = "Apache-2.0"
publish = false

[dependencies]
futures-timer = "3.0.2"
Expand Down
1 change: 1 addition & 0 deletions bin/node/cli/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ license = "GPL-3.0-or-later WITH Classpath-exception-2.0"
default-run = "substrate"
homepage = "https://substrate.dev"
repository = "https://github.com/paritytech/substrate/"
publish = false

[package.metadata.wasm-pack.profile.release]
# `wasm-opt` has some problems on linux, see
Expand Down
1 change: 1 addition & 0 deletions bin/node/executor/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ edition = "2018"
license = "Apache-2.0"
homepage = "https://substrate.dev"
repository = "https://github.com/paritytech/substrate/"
publish = false

[package.metadata.docs.rs]
targets = ["x86_64-unknown-linux-gnu"]
Expand Down
1 change: 1 addition & 0 deletions bin/node/inspect/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ edition = "2018"
license = "GPL-3.0-or-later WITH Classpath-exception-2.0"
homepage = "https://substrate.dev"
repository = "https://github.com/paritytech/substrate/"
publish = false

[package.metadata.docs.rs]
targets = ["x86_64-unknown-linux-gnu"]
Expand Down
1 change: 1 addition & 0 deletions bin/node/primitives/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ edition = "2018"
license = "Apache-2.0"
homepage = "https://substrate.dev"
repository = "https://github.com/paritytech/substrate/"
publish = false

[package.metadata.docs.rs]
targets = ["x86_64-unknown-linux-gnu"]
Expand Down
1 change: 1 addition & 0 deletions bin/node/rpc-client/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ edition = "2018"
license = "Apache-2.0"
homepage = "https://substrate.dev"
repository = "https://github.com/paritytech/substrate/"
publish = false

[package.metadata.docs.rs]
targets = ["x86_64-unknown-linux-gnu"]
Expand Down
1 change: 1 addition & 0 deletions bin/node/rpc/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ edition = "2018"
license = "Apache-2.0"
homepage = "https://substrate.dev"
repository = "https://github.com/paritytech/substrate/"
publish = false

[package.metadata.docs.rs]
targets = ["x86_64-unknown-linux-gnu"]
Expand Down
1 change: 1 addition & 0 deletions bin/node/runtime/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ build = "build.rs"
license = "Apache-2.0"
homepage = "https://substrate.dev"
repository = "https://github.com/paritytech/substrate/"
publish = false

[package.metadata.docs.rs]
targets = ["x86_64-unknown-linux-gnu"]
Expand Down
2 changes: 1 addition & 1 deletion bin/node/testing/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ edition = "2018"
license = "GPL-3.0-or-later WITH Classpath-exception-2.0"
homepage = "https://substrate.dev"
repository = "https://github.com/paritytech/substrate/"
publish = true
publish = false

[package.metadata.docs.rs]
targets = ["x86_64-unknown-linux-gnu"]
Expand Down
1 change: 1 addition & 0 deletions frame/example-offchain-worker/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ homepage = "https://substrate.dev"
repository = "https://github.com/paritytech/substrate/"
description = "FRAME example pallet for offchain worker"
readme = "README.md"
publish = false

[package.metadata.docs.rs]
targets = ["x86_64-unknown-linux-gnu"]
Expand Down
1 change: 1 addition & 0 deletions frame/example-parallel/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ license = "Unlicense"
homepage = "https://substrate.dev"
repository = "https://github.com/paritytech/substrate/"
description = "FRAME example pallet using runtime worker threads"
publish = false

[package.metadata.docs.rs]
targets = ["x86_64-unknown-linux-gnu"]
Expand Down
1 change: 1 addition & 0 deletions frame/example/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ homepage = "https://substrate.dev"
repository = "https://github.com/paritytech/substrate/"
description = "FRAME example pallet"
readme = "README.md"
publish = false

[package.metadata.docs.rs]
targets = ["x86_64-unknown-linux-gnu"]
Expand Down