Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix build metadata in workflow version causing semver parsing to fail #1786

Merged
merged 1 commit into from
Jun 27, 2024

Conversation

PathogenDavid
Copy link
Member

Not important for 2.8.3. The official release builds don't have build meadata so this doesn't affect them.


Bonsai has randomly been telling me that it has "upgraded" workflows I literally just saved. I finally got annoyed enough to poke around to figure out why it thought my workflow was upgraded.

Turns out it was "upgrading" them because the version was null here, causing the check to always succeed:

if (workflowElement is ExpressionBuilder builderElement && version < RemoveMemberSelectorPrefixVersion)
{
upgraded = true;

It was null because the parsing in SemanticVersion was not handling the build metadata. This change properly ignores it per the semver spec.

I considered adding a BuildMetadata property to SemanticVersion, but I felt it wasn't super valuable as this type is internal and is built around handling comparisons for the semver spec and the build metadata is completely ignored for those purposes.

This type currently has symmetry between equality operators and Equals/GetHashCode as well as comparison operators and CompareTo, but introducing this member properly should probably involve refactoring everything to break that symmetry. (Similar to how float.Equals is bitwise equality but float.operator== is IEEE-754 equality.)

@PathogenDavid PathogenDavid marked this pull request as draft June 18, 2024 15:39
@PathogenDavid
Copy link
Member Author

PathogenDavid commented Jun 18, 2024

I keep forgetting to note, but this was happening to me specifically because I have the .NET 8 SDK installed, which enables Source Link by default. Source Link adds the Git revision to the version strings by default (which is usually very useful.)

It also breaks the version display in the about box (because the version string is so long it wraps into an invisible region.)

I'll plan to just include this and similar changes in the release automation PR. Leaving this open as a draft as a reminder. (Gonçalo wanted this sooner rather than later so it got reopened and merged.)

@PathogenDavid PathogenDavid modified the milestone: 2.8.4 Jun 24, 2024
@PathogenDavid PathogenDavid marked this pull request as ready for review June 26, 2024 11:57
@glopesdev glopesdev added the fix Pull request that fixes an issue label Jun 27, 2024
@glopesdev glopesdev self-requested a review June 27, 2024 11:09
@glopesdev glopesdev merged commit 82448b9 into bonsai-rx:main Jun 27, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fix Pull request that fixes an issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants