Skip to content

Conversation

@ptrus
Copy link
Member

@ptrus ptrus commented Dec 14, 2025

The eq function was used in common.mk but never defined, causing VERSION to always append -git<commit> even when building from an exact tag.

cli/common.mk

Line 55 in 1323206

$(and $(call eq,$(GIT_COMMIT_EXACT_TAG),YES), $(GIT_VERSION)), \

Also fix +dirty suffix to be added in case of uncommitted changes on the exact tag.

@netlify
Copy link

netlify bot commented Dec 14, 2025

Deploy Preview for oasisprotocol-cli canceled.

Name Link
🔨 Latest commit 82d834a
🔍 Latest deploy log https://app.netlify.com/projects/oasisprotocol-cli/deploys/693fce30982ff90008e450de

@ptrus ptrus requested review from kostko and matevz December 14, 2025 23:56
@ptrus ptrus force-pushed the ptrus/fix/version-eq branch from de63e87 to ad06fdf Compare December 15, 2025 00:00
Copy link
Member

@matevz matevz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code is mostly from the oasis-node release process. Can you check if there are any issues there too?

@ptrus
Copy link
Member Author

ptrus commented Dec 15, 2025

This code is mostly from the oasis-node release process. Can you check if there are any issues there too?

No, eq is defined there:
https://github.com/oasisprotocol/oasis-core/blob/5c2b576da712b5d998bdf21b749423a95e8075e8/common.mk#L14

But the +dirty handling might also be wrong there when building from a tag with dirty git state.

@ptrus
Copy link
Member Author

ptrus commented Dec 15, 2025

Updated the comment to be consistent with the one in oasis-core.

@ptrus ptrus force-pushed the ptrus/fix/version-eq branch from ad06fdf to 82d834a Compare December 15, 2025 09:00
@ptrus ptrus merged commit 6177878 into master Dec 15, 2025
5 checks passed
@ptrus ptrus deleted the ptrus/fix/version-eq branch December 15, 2025 09:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants