nrwl / nx

Smart Monorepos · Fast CI
https://nx.dev
MIT License
23.77k stars 2.37k forks source link

Nx release with conventional commit incorrectly reading prerelease version as the most recent version #27577

Open jlaminate opened 3 months ago

jlaminate commented 3 months ago

Current Behavior

When running nx release version with Conventional Commits enabled, if the most recently released version is a prerelease version, Nx will consider the prerelease as the most recent version.

This causes Nx to fail to transition prerelease artifacts to release artifacts

Expected Behavior

When running nx release without a prerelease identifier, nx only reads finalized versions.

Nx is able to transition prerelease versions into release versions

GitHub Repo

https://github.com/jlaminate/nx-update-dependents-bug-min-repro

Steps to Reproduce

  1. create git tag `git tag @scope-1/package-a-1.1.0-alpha.0
  2. run nx release version --dry-run true
  3. Verify that nx resolves the current version to 1.1.0-alpha.0 rather than falling back to disk

For a more advanced test

  1. Create git tag git tag @scope-1/package-a-1.0.0
  2. Create an arbitrary commit modifying a file in scope-1/package-a
  3. Create a tag on that commit `git tag @scope-1/package-a-1.1.0-alpha.0
  4. run nx release version --dry-run true
  5. Verify that nx resolves the version to 1.1.0-alpha.0 rather than 1.0.0

This was also tested on nx 19.6.1 with the same result

Nx Report

Node   : 20.14.0
OS     : linux-x64
yarn   : 4.3.1

nx (global)    : 19.3.0
nx             : 19.5.1
@nx/js         : 19.5.1
@nx/workspace  : 19.5.1
@nx/devkit     : 19.5.1
@nrwl/tao      : 19.5.1
typescript     : 5.5.4

Failure Logs

No response

Package Manager Version

No response

Operating System

Additional Information

Likely related the git utils only verifying that a tag matches the general semantic version regex, not accounting for what preid the tag is for.

brogueady commented 1 month ago

+1 for this. We have had the scenario where different teams are working on the same repo. One team is working in a feature branch creating pre-releases and the other team are releasing to main using conventional commits to drive the versions.

After the team have created/published a pre-release version, then all future changes to the main branch are versioned (incorrectly IMO) off the pre-release versions even though they are not using the prerelease specifier.

mpalliser commented 1 week ago

+1, we have this same problem for months now. I found some people who are removing rc tags locally temporarily to prevent nx from taking them into account, but it would be nice to have a better solution 🤭.

I'll keep an eye on this thread to see if it gets resolved, Thanks James Henry and your team for your work.

I think there is related issue https://github.com/nrwl/nx/issues/22408

Michael-Sammereier commented 3 days ago

We want to try out monorepos for our angular application in my project. So I set up a monorepo using NX 20.0.12 the last couple of days. Everything went relatively smooth until I started to set up our CI pipeline with versioning. I want to publish release candidates first until we can release a new minor or patch version. The Version bumps worked for minor and patch versions as well as for new release candidates but somehow it broke once I merged the release candidate to get a new release. It would continue to create new release candidates, exactly as described here. I couldn't figure out what I did wrong until I found this issue.

I agree with your expected behavior. It should only create new prereleases as long as I specify that using the preid. If that is not specified, it should search for the last non-prerelease tag for the actual correct next version.

Hopefully this will get fixed soon because it surely does not help our confidence trying out monorepos with nx.

Michael-Sammereier commented 3 days ago

I think there is related issue #22408

I actually digged a bit deeper into this now and I do not think that this problem is necessarily related to that issue, even though that problem also needs to be adressed because that would be the next problem down the line.

I tried different configurations and read a bit through the code. In my opinion the problem lies within the getLatestGitTagForPattern method https://github.com/nrwl/nx/blob/c3d53a490083c971e698a563bfa93bbb88b2ea11/packages/nx/src/command-line/release/utils/git.ts#L46

I would argue that it is ok for the function to return a prerelease version in case I actually want to release a new prerelease version. For example when going from beta.0 to beta.1. In that case I only want the difference between those two prerelease versions. So that's fine.

However, in my opinion, prerelease versions should only be returned if we actually want another prerelease version. Otherwise, this current behavior creates problems down the line when we want "normal" releases because at least for conventional commits a previous prerelease always results in another prerelease https://github.com/nrwl/nx/blob/c3d53a490083c971e698a563bfa93bbb88b2ea11/packages/js/src/generators/release-version/release-version.ts?plain=1#L437-L441 @JamesHenry actually added a TODO there to reevaluate if this behavior works. In my opinion it would, if the correct corresponding git tag (which is used for currentVersion) were retrieved. For a new prerelease it would be the previous prerelease, fine. But for a "normal" release, we need the previous "normal" release to get the complete list of changes anyways. So we would not run into problems here either.

In addition to just checking if a prerelease should be retrieved in the getLatestGitTagForPattern method, I think there should also be a filter for the correct preid. I did some very rudimentary tests, switching between preid alpha and preid beta and everytime I switched the preid, the count reset to 0. I don't think that it should reset and instead continue incrementing the respective prerelease. If getLatestGitTagForPattern returned the newest git-tag for the respective preids, this would be the case. However, please take this with a small grain of salt as I just did some very basic and quick tests on the behavior of switching preids. And I am not sure if this might have some other unwanted consequences if different preids were handled separately. This would probably be something to look into though.

Assuming this issue is fixed and we are able to actually release a "normal" release after prereleases, then the other issue #22408 would need to be adressed as well.