PX4 / PX4-Autopilot

PX4 Autopilot Software
https://px4.io
BSD 3-Clause "New" or "Revised" License
8.22k stars 13.39k forks source link

Git tag regex does not match error message #19886

Open jorgenman opened 2 years ago

jorgenman commented 2 years ago

Description

px_update_git_header.py checks the latest git tag to make sure it matches the approved format, and provides an error message if if doesn't:

The expected format is 'v<PX4 version>[-<custom version>]'
  <PX4 version>: v<major>.<minor>.<patch>[-rc<rc>|-beta<beta>|-alpha<alpha>|-dev]
  <custom version>: <major>.<minor>.<patch>[-rc<rc>|-beta<beta>|-alpha<alpha>|-dev]
Examples:
  v1.9.0-rc3 (preferred)
  v1.9.0-beta1
  v1.9.0-1.0.0
  v1.9.0-1.0.0-alpha2
See also https://dev.px4.io/master/en/setup/building_px4.html#firmware_version

From this message, it sounds like <PX4 version> and <custom version> can each have a suffix ([-rc<rc>|-beta<beta>|-alpha<alpha>|-dev]), but the actual regex does not allow a suffix on the <PX4 version> if there's a <custom version>.

Examples

The following git tags currently fail the check: v1.9.0-alpha2-1.0.0 v1.9.0-alpha2-1.0.0-beta1

Suggested fix

I'm happy to submit a PR, but I need to know what the desired behavior is. Is the current behavior correct (and the error message needs to be fixed)? Or, should the above examples fail (and the regex needs to be fixed)?

I feel like my above examples should be valid tags, and the regex should be fixed, but I'm not intimately familiar with exactly how the tag is used after this script.

bkueng commented 2 years ago

The parser currently only supports a rc/alpha/beta for the custom version if I remember correctly. Which means the regex needs to be fixed, but preferrably we extend the implementation. Here are the tests: https://github.com/PX4/PX4-Autopilot/blob/main/src/systemcmds/tests/test_versioning.cpp#L77=. And the implementation: https://github.com/PX4/PX4-Autopilot/blob/main/src/lib/version/version.c

jorgenman commented 2 years ago

In the existing tests, it looks like the FIRMWARE_TYPE for the version number and vendor version number are expected to be the same. For example, with the string 0.45.99-1.2.3beta4, both the PX version number and the version number get an 80 at the end.

Based on this, I'm inclined to say that my requested example (v1.9.0-alpha2-1.0.0-beta1) shouldn't be supported. To support it, I think one of the following would have to happen, both of which seem problematic:

bkueng commented 2 years ago

Option 2 is to me the expected behavior and the current implementation is broken for that case. The existing behavior should be achieved with v1.6.2-rc2-1.0.0