crate-ci / cargo-release

Cargo subcommand `release`: everything about releasing a rust crate.
Apache License 2.0
1.31k stars 109 forks source link

Maintaining a repo with workspaces #805

Closed blelump closed 1 month ago

blelump commented 1 month ago

Hey,

We're aiming to extend the repo https://github.com/THCLab/oca-rs with the ability to generate the changelog via git cliff. The repo itself is a workspace with members, each of whom is also a crate on crates.io. Assuming release.toml in the repo root dir gets

pre-release-hook = ["git", "cliff", "-o", "CHANGELOG.md", "--tag", "{{version}}" ]

to generate the changelog immediately fails in the workspace member oca-ast with the following error:

ERROR git_cliff > Git error: `could not find the repository at '..../oca-ast'; class=Repository (6); code=NotFound (-3)`

The oca-ast is a .git less dir as a workspace member. I have added pre-release-hook = ["ls"] to oca-ast/release.toml, which solves it but sounds more like a hack than a solution. What's the proper way to solve this?

epage commented 1 month ago

I'm a bit confused.

It sounds like your repo has

/release.toml

pre-release-hook = ["git", "cliff", "-o", "CHANGELOG.md", "--tag", "{{version}}" ]

with that being inherited into each packge.

However, oca-ast is failing somehow and so you are wanting to override the hook and did that with a no-op command by adding /oca-ast/release.toml

pre-release-hook = ["ls"]

We could add a way to override a hook to say "do not run. You can also instead specify the hook in the workspace members where it makes sense.

What isn't too clear to me is why oca-ast is failing. Each workspace member is in a .git-less directory because they aren't in the root. A .../ path is invalid and I'm not seeing how we are providing that to git-cliff, so it seems like this would both be a wider issue and a git-cliff specific issue, unless I'm missing something.

btw you shouldn't need to use pre-release-replacements to bump dependency versions in Cargo.toml as we bump the dependent workspace members when releasing a package. You can see this in the logged output when doing a dry-run with verbose logging enabled.

blelump commented 1 month ago

Thanks for getting deeper into it, @epage .

For clarity (at least this is what I initially thought), I have truncated the path ..../oca-ast with ...., which is, by default, the dir absolute path. It works great, and apologies for the confusion.

I am more interested in how people deal with repos where the repo root is a "root" crate and the workspace members are crates as well. The final outcome I want to get (I think?) is to have CHANGELOG.md in the root dir and omit the members, at least until we don't have proper Conventional Commits scoping for each member.

do not run is a way to solve it, but what about such approach pre-release-hook = []? Such a construct fails now with

thread 'main' panicked at /media/aa/work/.cargo/registry/src/index.crates.io-6f17d22bba15001f/cargo-release-0.25.10/src/ops/cmd.rs:23:32:
called `Option::unwrap()` on a `None` value
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
Aborted (core dumped)
epage commented 1 month ago

Ah, you only want it on the root.

You can define the hook in Cargo.tomls package.metadata table (see our docs for sub table name).

For me, i only use release.toml work workspace config (unsure way Ive not switched to workspace.metadata).

blelump commented 1 month ago

It seems the problem is still there even if I moved all the content from release.toml to Cargo.toml. I am actually working on a different repo to make it work, but the repo organization is exactly the same. I have added [workspace.metadata.release] to the Cargo.toml

When running the release, the error is the same:

 ERROR git_cliff > Git error: `could not find repository at '/media/aa/work/hcf/keriox/keriox_core'; class=Repository (6); code=NotFound (-3)`

I'm not sure if it's relevant, but in the above explanation, I might skip an important detail. That is, the repo "root" dir isn't a library in the Rust sense. It is just the workspace organizational dir. Therefore, I can't add the release under package.metadata but rather workspace.metadata. Now, if it is under workspace, will likely all the members also inherit the configuration?

epage commented 1 month ago

I suggested package.metadata for you to set it in only those packages for which it is needed. Is there a reason you are setting it in the root?

blelump commented 1 month ago

The pre-release-hook I'd like to have precisely in the root :-) . It's kinda odd then to have changelog in the root , however it seems to be the most convenient at least right now. Therefore members don't get changelog

epage commented 1 month ago

I think you are going to need to go into more detail. I worry we are spinning in circles from us each having a limited undestanding of the problem.

blelump commented 1 month ago

Let's try this:

epage commented 1 month ago

cargo-release only knows how to release packages and not any meta concept above that, this includes pre-release hooks. You'll need to pick a representative package to run your collective pre-release hook.

blelump commented 1 month ago

What you say is different from how it behaves. It works for both repos mentioned above in their root dirs (root isn't a lib nor binary), but not for members. The error ERROR git_cliff > Git error: could not find repository at '/media/aa/work/keriox/keriox_core'; class=Repository (6); code=NotFound (-3) occurs in the workspace member keriox_core because it somehow inherits the behavior from the root and understandably fails because git repo is missing as the parent is proper git repo. The pre-release-hook

pre-release-hook = ["git", "cliff", "-o", "CHANGELOG.md", "--tag", "{{version}}" ]

It expects to be run within a git repo. Entering the ./keriox_core dir and doing git cliff from Bash fails with the same error, so it's clear the hook must not run there.

epage commented 1 month ago

No,it is working as described. The other key is that it has implicit workspace inheritance.

blelump commented 1 month ago

I think it's vital information, specifically that it has implicit workspace inheritance.

I solved the issue by applying the concept proposed by you in another issue https://github.com/crate-ci/cargo-release/issues/162 along with extending the member Cargo.toml with https://github.com/THCLab/keriox/blob/3d54663873e949257fc579b2eee0dc52d3cfc11b/keriox_core/Cargo.toml#L64. TBH I don't know what's the right approach in repos with workspaces regarding the integration with git cliff and auto-generating CHANGELOGs, but here is my attempt. Thanks for getting through it.

epage commented 1 month ago

I solved the issue by applying the concept proposed by you in another issue https://github.com/crate-ci/cargo-release/issues/162 along with extending the member Cargo.toml with https://github.com/THCLab/keriox/blob/3d54663873e949257fc579b2eee0dc52d3cfc11b/keriox_core/Cargo.toml#L64.

That sounds like the solution I proposed before. You pick a representative package that is responsible for workspace-wide changes (whether its a tag, changelog generation, etc) and put it in that package's release settings.

I think it's vital information, specifically that it has implicit workspace inheritance.

I'm not sure what you meant by this.

epage commented 1 month ago

At this point, I'm not seeing anything actionable for this issue, so I'm going to close it. If there is something I missed, let us know!