Hook Environment Variables Mirrored in Template Placeholders

[baxterjo]

I appreciate the add for having placeholders resolve in pre-release hooks.

Could we expand this functionality to also include some of the ENV vars that are passed to the hook? I would love the ability to define my hook at the root of my workspace and run it anywhere, regardless of the shape/depth of the workspace. This would require me to expand the WORKSPACE_ROOT env var in the pre-release-hook argument list. Which is not possible since it is not a shell.

Thanks!

[epage]

Looks like the only ones missing are

• WORKSPACE_ROOT
• CRATE_ROOT
• DRY_RUN

Not to say "no" but to further explore this, is there a reason you need access to these on the command-line, rather than using a script to wrap them that can access them?

[baxterjo]

Mainly repository hygiene. I could add a script at the root of each package that accesses the WORKSPACE_ROOT env var to call the main pre-release hook. But that option requires N scripts to be added/changed in the repo when they're just wrappers for the pre-release hook I really want.

To address the issue that I really^2 want is the option to run a pre-release hook only once at the root of the workspace when releasing the workspace. The way I'm versioning/releasing the workspace is by sharing versions with all the packages and releasing the whole workspace at once.

The behavior I expected originally when adding the pre-release-hook to [workspace.metadata.release] was that it would run once in the root of the workspace. Then if I wanted a "per-package" release hook I could add it to the [package.metadata.release] for that package. But that was not the case.

[epage]

To address the issue that I really^2 want is the option to run a pre-release hook only once at the root of the workspace when releasing the workspace. The way I'm versioning/releasing the workspace is by sharing versions with all the packages and releasing the whole workspace at once.

My main concern with supporting this is cargo-release can't tell when the end of a release set is done. We can't infer user intention which is made harder by the fact that releases are non-atomic and doing releases in piece-meal is our recommended recovery step if a release fails.

In the past, we've recommended people pick a representative crate to run a hook. Is there a reason you can't do that here?

It sounds like you want one post-workspace-release hook but instead you run a hook after every package. Is it happenstance that your scripts don't interfere with each other or do you have to take extra steps for that?

[baxterjo]

It sounds like you want one post-workspace-release hook but instead you run a hook after every package.

Running a hook before every package is what is happening, but was not the behavior I was expecting. My current workspace Cargo.toml looks like this:

[workspace.metadata.release]
allow-branch = ["release/*"]
sign-tag = true
sign-commit = true
tag-name = "v{{version}}"
publish = false
push = true
shared-version = true
# This runs once per package in the workspace, with the CWD being the root of the package.
pre-release-hook = ["../hooks/pre-release.sh"]

And the pre-release hook runs once for every package in the workspace. I think the behavior I expected here was that if I put a hook in the [workspace] table rather than the [package] table then it would only run once for the whole workspace.

Is it happenstance that your scripts don't interfere with each other or do you have to take extra steps for that?

The pre-release script is just doing some (read-only) source control checks to ensure the user is releasing something that is in sync with remote. So it is happenstance, but I would venture to guess that if the pre-release did perform a writing action, then the repetition of the script would clobber the last invocation based on the ordering of the packages in pre-release.

[epage]

Sorry, pre- instead of post. The concerns about a "workspace" release are still relevant. It might work in one off cases but it does't apply generally and people can cause themselves problems by using a feature not suited to their workflow but looks like it would.

Is there a reason you can't put the hook in a representative package?

And the pre-release hook runs once for every package in the workspace. I think the behavior I expected here was that if I put a hook in the [workspace] table rather than the [package] table then it would only run once for the whole workspace.

workspace.metadata.release is mostly used for (implicit) workspace inheritance into the packages. If we had a workspace pre-release hook, it would need its own field so people can still inherit the existing one.

[baxterjo]

A representative package could work for my use case. But tbh I think that would come with a bit of a code smell. Hard to explain, but I don't think I would want the successful release of package C to be dependent on the config in package A, even if it is shared-version = true. Since my pre-release hook is read only and the number of members in my workspace is small, I am going to keep this the way it is.

I think the implicit inheritance was the gotcha for me. Since inheritance for other cargo configs must be explicit, and is mutually exclusive from the package having its own config in the package/Cargo.toml, I did not assume that the workspace Cargo.toml was one source in a hierarchical config for each package. I assumed the workspace config was treated as a wrapper around the all of the workspace members. I now see that there is a section in the docs that explains precedence in config.

I think more documentation on workspace releases might be beneficial, in the configuration section of the docs there seems to be an implication that there is a workspace release process. But when I run cargo release --workspace, no actions take place at the workspace level, they all occur at the package level.

[epage]

I think the implicit inheritance was the gotcha for me.

Yes, our inheritance predates Cargo's. We'd also need features that Cargo hasn't setup yet (inheriting and overriding) and would hit a pain point with it (people want [lints] to be auto-inherited)

I think more documentation on workspace releases might be beneficial,

We have several FAQ entries: https://github.com/crate-ci/cargo-release/blob/master/docs/faq.md#how-do-i-apply-a-setting-only-to-one-crate-in-my-workspace

in the configuration section of the docs there seems to be an implication that there is a workspace release process.

I'm assuming this is referring to "Workspace configuration is read from the following (in precedence order)". This referring to multi-package operations, like when sharing a commit. Any thoughts on how to improve this?

[baxterjo]

Any thoughts on how to improve this?

I think the source of confusion is that the [workspace.metadata.release] table is overloaded, and careful review of the docs is required for each config.

e.g.:

[workspace.metadata.release]
allow-branch = ["release/*"] # Workspace only config. But ignored if in package?
sign-tag = true # Workspace config if shared-version is true, but inherited if false
sign-commit = true # Workspace config if sign-commit is true, inherited if false
tag-name = "v{{version}}" # Workspace config if shared-version is true, inherited if false
publish = false # Inherited config
push = true # Workspace config if consolidate-commits is true, inherited if false
shared-version = true # Inherited config?
pre-release-hook = ["../hooks/pre-release.sh"] # Inherited config

A non breaking change could take the form of

## Config Fields section being refactored and renamed into two separate "Package Config Fields" and "Workspace Config Fields"

A breaking change could look like

[workspace.metadata.release]
allow-branch = ["release/*"]
sign-tag = true
sign-commit = true
tag-name = "v{{version}}"
push = true


[workspace.metadata.release.package-defaults]
pre-release-hook = ["../hooks/pre-release.sh"]
publish = false
shared-version = true

And it would come with some warnings or errors if some settings are being ignored or are not compatible.

[epage]

## Config Fields section being refactored and renamed into two separate "Package Config Fields" and "Workspace Config Fields"

How would this look with the conditional fields?

A breaking change could look like

Not too thrilled with that atm