benduran / lets-version

A package that reads your conventional commits and git history and recommends (or applies) a SemVer version bump for you
MIT License
1 stars 3 forks source link

`git tag` step of applyRecommendedBumpsByPackage is not robust to git tag already existing #27

Open amybingzhao opened 2 months ago

amybingzhao commented 2 months ago

Our publish job has been failing recently in a way that requires re-runs, but re-runs are failing to publish through turbo-tools with the error: ExecaError: Command failed with exit code 128: git tag '{package}@{version}'.

I suspect this is because the git tag was created by the first run, and now it's throwing an error b/c it already exists. I can repro this by running:

Would it make sense for this line https://github.com/benduran/lets-version/blob/main/src/lets-version.ts#L705 to either be surrounded by a try/catch or to check for local + remote git tags that already exist?

benduran commented 1 month ago

@amybingzhao thanks for the report. I've seen this error typically appear in flaky CI jobs where a prebuild (build before the version bump) succeeds, the bump and the git tags are created and applied, but then the postbuild or publish fails after the tags were created.

What would you expect the "correct" behavior to be in the case where a tag already exists? No-op? Perhaps the behavior lets-version exhibits today can be kept as default, and an additional silenceExistingTags (name TBD) flag could be provided that would simply skip over a package whose version already corresponds to an existing tag?

benduran commented 2 weeks ago

@amybingzhao gentle ping, as my last comment may have fallen through the cracks on your end

amybingzhao commented 2 weeks ago

Thanks for the bump! Yep we were seeing this in flaky CI jobs. I'm not sure what would be the "correct" behavior in the context of just this lets-version package 🤔 We actually ended up moving off of lets-version and b/c folks on the team wanted to remove the conventional commits requirement so there's no urgency on our end. We now only push the git tags if the publish step succeeds, but lets-version wouldn't have access to that info.

I think even just an error message saying that duplicate git tags are a common cause of this exit code would be helpful. A silenceExistingTags or skipTagging escape hatch flag would also have been useful

benduran commented 2 weeks ago

Hi Amy,

Shoot, that’s very disappointing to hear the team wanted to move away from Conventional Commits, despite its popularity in the open source world.

Are you all still leveraging turbo tools for task management, publishing and changelog generation for packages in the monorepo?

On Tue, 27 Aug 2024 at 06:03, amybingzhao @.***> wrote:

Thanks for the bump! Yep we were seeing this in flaky CI jobs. I'm not sure what would be the "correct" behavior in the context of just this lets-version package 🤔 We actually ended up moving off of lets-version and b/c folks on the team wanted to remove the conventional commits requirement so there's no urgency on our end. We now only push the git tags if the publish step succeeds, but lets-version wouldn't have access to that info.

I think even just an error message saying that duplicate git tags are a common cause of this exit code would be helpful. A silenceExistingTags or skipTagging escape hatch flag would also have been useful

— Reply to this email directly, view it on GitHub https://github.com/benduran/lets-version/issues/27#issuecomment-2311578289, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABQYTH2HDDRWF6ZWBPCTB6LZTQCDFAVCNFSM6AAAAABKVV2KI2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGMJRGU3TQMRYHE . You are receiving this because you commented.Message ID: @.***>