Skip to content

Comments

Improve bootloader version parsing from GIT_VERSION#374

Open
piotrmalek wants to merge 2 commits intoadafruit:masterfrom
piotrmalek:master
Open

Improve bootloader version parsing from GIT_VERSION#374
piotrmalek wants to merge 2 commits intoadafruit:masterfrom
piotrmalek:master

Conversation

@piotrmalek
Copy link

Description of Change

This PR improves the parsing of GIT_VERSION in the Makefile when generating
the bootloader version value.

The previous logic assumed a strict MAJOR.MINOR.PATCH format and could produce
incorrect results for common Git tag formats such as:

  • v1.2.3
  • 1.2.3-

The new implementation extracts semantic version components more robustly
and safely defaults to 0.0.0 if the version string does not match the expected
format.

This change does not affect existing version formats and is backward compatible.

@hathach
Copy link
Member

hathach commented Jan 26, 2026

hmm, I don't know if we need this, since we consistently release this with 1.2.3 format

@piotrmalek
Copy link
Author

Thanks! Totally fair - if tags are always strictly 1.2.3, the old logic usually works.
That said, GIT_VERSION often ends up being not a clean tag in a few common scenarios (building from a non-tagged commit, local builds, shallow clones/CI, or when git describe yields v1.2.3-147-g). In those cases the previous parsing can produce a wrong packed integer (or inconsistent results), while this change keeps the behavior identical for 1.2.3 and v1.2.3, and safely falls back to 0.0.0 when it’s not a semantic version.
It’s a small change, doesn’t impact releases, and makes the version packing more robust/reproducible for non-release builds.

# 1.2.3
# 1.2.3-147-gd71abcd
# If the version string does not match MAJOR.MINOR.PATCH, defaults to 0.0.0.
_VER3 := $(shell echo "$(GIT_VERSION)" | sed -E 's/^v?([0-9]+)\.([0-9]+)\.([0-9]+).*/\1 \2 \3/; t; s/.*/0 0 0/')
Copy link
Member

Choose a reason for hiding this comment

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

I resolved the conflict with master, my question would be whether using sed -E portable, i.e if this work on windows command prompt ? If this isn't the isssue, I think we can merge this right away.

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.

2 participants