Use `path = [bala]` for `rust-bitcoin`'s workspace members dependencies. remove `[patch.crates-io.balab]` by eval-exec · Pull Request #4284 · rust-bitcoin/rust-bitcoin

eval-exec · 2025-03-25T10:36:38Z

This PR want to:

make all workspace members use workspace = true syntax to import dependencies.
use path = [balabala] to define dependencies, instead of useing [patch.crates-io.balabala] , fix: Can't use latest git version of rust-bitcoin #4283

coveralls · 2025-03-25T10:39:18Z

Pull Request Test Coverage Report for Build 14099166171

Details

0 of 0 changed or added relevant lines in 0 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage increased (+0.004%) to 83.46%

Totals
Change from base Build 14089914296:	0.004%
Covered Lines:	22202
Relevant Lines:	26602

💛 - Coveralls

apoelstra · 2025-03-25T13:12:47Z

Doesn't work with our MSRV, sorry. Otherwise we would've done this a while ago. I agree that the current situation is pretty annoying.

eval-exec · 2025-03-25T13:18:43Z

Hello @apoelstra , Could you please guide me where is the file: ./maintainer-tools/ci/run_task.sh I can't found it. And I wonder why the CI (MSRV toolchain (minimal) and MSRV toolchain (recent)) failed.

I want to run these failed CI jobs on my local machine.

apoelstra · 2025-03-25T13:30:43Z

Just try to build the project with 1.63.0, like cargo +1.63.0 test.

But the relevant files are in https://github.com/rust-bitcoin/rust-bitcoin-maintainer-tools

eval-exec · 2025-03-25T13:33:21Z

cargo +1.63.0 test

❯ cargo +1.63.0 test
error: failed to load manifest for workspace member `/home/exec/Projects/github.com/rust-bitcoin/rust-bitcoin/base58`

Caused by:
  failed to parse manifest at `/home/exec/Projects/github.com/rust-bitcoin/rust-bitcoin/base58/Cargo.toml`

Caused by:
  feature `workspace-inheritance` is required

  The package requires the Cargo feature called `workspace-inheritance`, but that feature is not stabilized in this version of Cargo (1.63.0 (fd9c4297c 2022-07-01)).
  Consider trying a newer version of Cargo (this may require the nightly release).
  See https://doc.rust-lang.org/nightly/cargo/reference/unstable.html#workspace-inheritance for more information about the status of this feature.

OK, the MSRV 1.63.0 not support Workspace Inheritance, So I'll remove workspace = true related stuff, but I suggest we should still use path = [balabala], and remove [patch.crates-io.balaba]. What do you think?

eval-exec · 2025-03-25T13:37:36Z

I removed workspace = true related stuff, and waiting for the CI's results.

yancyribbens · 2025-03-25T15:26:05Z

I think you need to add a [patch] line telling Cargo to also use the version of bitcoin-internals from this repo. (And same with all the other crates in this repo.)

Right, so in your local bitcoin-test, you can add this to your Cargo.toml:

[patch.crates-io]
bitcoin-internals = { git = "https://github.com/rust-bitcoin/rust-bitcoin.git", rev = "a commit hash, branch or tag" }

That being said, I don't know if there's a reason to not accept this patch to make this sort of thing work out of the box?

Kixunil

Sadly, I don't believe this is correct. The reason is secp256k1 depends on bitcoin-hashes via crates.io so doing this will cause it to pull the version from there rather than here. Since we removed ThirtyTwoByteHash this is not longer as bad as before but it still has the problem that it'll stop us from testing the chagnes to hashes with secp256k1.

However, if I understand it correctly, [patch] for hashes can still be there, so I request that you keep that one.

…review comment

eval-exec · 2025-03-26T01:47:16Z

However, if I understand it correctly, [patch] for hashes can still be there, so I request that you keep that one.

Thank you, I added a commit to keep [patch] for [hashes].

…review comment Signed-off-by: Eval EXEC <execvy@gmail.com>

apoelstra · 2025-03-26T02:48:56Z

Thanks for your continued work on this. I think it's probably fine to remove the patch section for everything but hashes (or at least, it will become increasingly fine as we remove hashes from our public APIs of downstream crates).

Can you squash all these commits together, and add a comment next to the hashes patch explaining why it's necessary when all the other dependencies use path.

…o.toml Signed-off-by: Eval EXEC <execvy@gmail.com>

eval-exec · 2025-03-27T04:56:07Z

Can you squash all these commits together, and add a comment next to the hashes patch explaining why it's necessary when all the other dependencies use path.

Thank you, I squashed and added comments for [patch.crates-io.hashes]. Friendly invite @apoelstra @yancyribbens @Kixunil review this PR

Kixunil

ACK 9a572da

apoelstra

ACK 9a572da; successfully ran local tests

`cargo publish` does not allow publishing a crate without all dependencies having explicit version numbers. However in rust-bitcoin#4284 we started using `path = ../internals` for the `internals` dependency (as well as the rest of the repo crates). This means we cannot publish `units`. Change the dependency to the explicit version number of the latest `units` release.

tcharding · 2025-06-24T03:05:12Z

This breaks cargo publish. What was the motivation for this PR? I don't see how #4283 led to this PR.

eval-exec · 2025-06-24T03:06:56Z

This breaks cargo publish. What was the motivation for this PR? I don't see how #4283 led to this PR.

What's the error report of cargo publish?

apoelstra · 2025-06-24T03:07:56Z

@tcharding without this PR it's a bit annoying to directly depend on the repo. You basically have to copy/paste all the [patch] lines from this repo into your own repo.

I don't think this is causing cargo publish to fail though -- I think it's the lack of version.

The exact error is

error: all dependencies must have a version specified when publishing.
dependency `bitcoin-internals` does not specify a version
Note: The published dependency will use the version from crates.io,
the `path` specification will be removed from the dependency declaration.

The path thing seems to be a "Note:" while the error is about a missing version.

tcharding · 2025-06-24T03:28:00Z

Oh I get it now, thanks. (I couldn't work out why one would want to do this and not be patching rust-bitcoin itself, but of course if folk want to test out master.)

`cargo publish` does not allow publishing a crate without all dependencies having explicit version numbers. We discovered recently that having patching the root manifest was annoying when downstream testing so we removing the version numbers in rust-bitcoin#4284 in favor of `path`. We should have included both ref: https://doc.rust-lang.org/cargo/reference/specifying-dependencies.html#multiple-locations Props to nyonson for finding this. Add an explicit version number to the `internals` dep in `units`.

`cargo publish` does not allow publishing a crate without all dependencies having explicit version numbers. We discovered recently that having patching the root manifest was annoying when downstream testing so we removing the version numbers in rust-bitcoin#4284 in favor of `path`. We can include both but having to explicitly opt in for any changes to `internals` for `units` seems better than accidentally creating something locally we cannot publish. Use an explicit version number to the `internals` dep in `units`.

`cargo publish` does not allow publishing a crate without all dependencies having explicit version numbers. We discovered recently that having patching the root manifest was annoying when downstream testing so we removing the version numbers in rust-bitcoin#4284 in favor of `path`. It turns out we can include both. Props to nyonson for discovering that. ref: https://doc.rust-lang.org/cargo/reference/specifying-dependencies.html#multiple-locations Use an explicit version number to the `internals` dep in `units`.

tcharding · 2025-10-08T21:35:10Z

Why did we remove the version here then? Seems we should have only swapped patch section of top level manifest with path section in dependency and left the version number alone?

apoelstra · 2025-10-08T21:38:08Z

Probably just an oversight because we were so focused on dealing with the dependency hole.

tcharding · 2025-10-09T00:07:25Z

I've added version number in all the release tracking PRs.

github-actions bot added C-bitcoin PRs modifying the bitcoin crate C-hashes PRs modifying the hashes crate C-units PRs modifying the units crate C-internals PRs modifying the internals crate C-io PRs modifying the io crate test C-primitives C-base58 PRs modifying the base58 crate labels Mar 25, 2025

eval-exec mentioned this pull request Mar 25, 2025

Request Publish bitcoin 0.33.0 to crates.io #4282

Open

eval-exec force-pushed the exec/reorganize-cargo-toml branch from fa85b1e to 0d52160 Compare March 25, 2025 13:36

github-actions bot removed C-internals PRs modifying the internals crate test labels Mar 25, 2025

eval-exec changed the title ~~Use workspace syntax for rust-bitcoin's worspace members~~ Use path = [bala] for rust-bitcoin's workspace members dependencies. remove [patch.crates-io.balab] Mar 25, 2025

Kixunil requested changes Mar 25, 2025

View reviewed changes

eval-exec added a commit to eval-exec/rust-bitcoin that referenced this pull request Mar 26, 2025

chore: Keep [patch] for [hashes], apply rust-bitcoin#4284 (review) …

353e0b0

…review comment

eval-exec added a commit to eval-exec/rust-bitcoin that referenced this pull request Mar 26, 2025

chore: Keep [patch] for [hashes], apply rust-bitcoin#4284 (review) …

da662fd

…review comment Signed-off-by: Eval EXEC <execvy@gmail.com>

eval-exec force-pushed the exec/reorganize-cargo-toml branch from 353e0b0 to da662fd Compare March 26, 2025 01:49

eval-exec added a commit to eval-exec/rust-bitcoin that referenced this pull request Mar 26, 2025

chore: Keep [patch] for [hashes], apply rust-bitcoin#4284 (review) …

bbf14a9

…review comment Signed-off-by: Eval EXEC <execvy@gmail.com>

eval-exec force-pushed the exec/reorganize-cargo-toml branch from da662fd to bbf14a9 Compare March 26, 2025 01:50

refactor: use path dependencies for workspace members in bitcoin/Carg…

9a572da

…o.toml Signed-off-by: Eval EXEC <execvy@gmail.com>

eval-exec force-pushed the exec/reorganize-cargo-toml branch from bbf14a9 to 9a572da Compare March 27, 2025 04:55

eval-exec requested a review from Kixunil March 27, 2025 04:55

Kixunil approved these changes Mar 27, 2025

View reviewed changes

apoelstra approved these changes Mar 27, 2025

View reviewed changes

apoelstra merged commit 158240c into rust-bitcoin:master Mar 27, 2025
24 checks passed

eval-exec deleted the exec/reorganize-cargo-toml branch March 27, 2025 15:23

tcharding mentioned this pull request Jun 24, 2025

units: Bump version to 1.0.0-rc.0 #4204

Merged

apoelstra mentioned this pull request Oct 8, 2025

Addversion specifier for dependencies in this repo #5102

Closed

Conversation

eval-exec commented Mar 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coveralls commented Mar 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request Test Coverage Report for Build 14099166171

Details

💛 - Coveralls

Uh oh!

apoelstra commented Mar 25, 2025

Uh oh!

eval-exec commented Mar 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

apoelstra commented Mar 25, 2025

Uh oh!

eval-exec commented Mar 25, 2025

Uh oh!

eval-exec commented Mar 25, 2025

Uh oh!

yancyribbens commented Mar 25, 2025

Uh oh!

Kixunil left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

eval-exec commented Mar 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

apoelstra commented Mar 26, 2025

Uh oh!

eval-exec commented Mar 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Kixunil left a comment

Choose a reason for hiding this comment

Uh oh!

apoelstra left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

tcharding commented Jun 24, 2025

Uh oh!

eval-exec commented Jun 24, 2025

Uh oh!

apoelstra commented Jun 24, 2025

Uh oh!

tcharding commented Jun 24, 2025

Uh oh!

tcharding commented Oct 8, 2025

Uh oh!

apoelstra commented Oct 8, 2025

Uh oh!

tcharding commented Oct 9, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

eval-exec commented Mar 25, 2025 •

edited

Loading

coveralls commented Mar 25, 2025 •

edited

Loading

eval-exec commented Mar 25, 2025 •

edited

Loading

Kixunil left a comment •

edited

Loading

eval-exec commented Mar 26, 2025 •

edited

Loading

eval-exec commented Mar 27, 2025 •

edited

Loading