-
-
Notifications
You must be signed in to change notification settings - Fork 311
Semantic versioning update #5944
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
Conversation
versioning with link to semantic versioning wiki page. Update H5.c and version tests for move of major and minor versions to 1st and 2nd version numbers.
…tic-versioning-update
…x/hdf5 into semantic-versioning-update
test/CMakeTests.cmake
Outdated
| if ("H5TEST-tcheck_version-major" MATCHES "${HDF5_DISABLE_TESTS_REGEX}") | ||
| set_tests_properties (H5TEST-tcheck_version-major PROPERTIES DISABLED true) | ||
| endif () | ||
| # minor + 1 should pass on non-develop branches |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But minor test KEEPS: WILL_FAIL "true"
Release test REMOVES: WILL_FAIL "true"
so
# minor + 1 should FAIL on develop branch (minor=0 in exceptions)
# minor + 1 should PASS on release branches (minor!=0 not in exceptions)
# This test expects failure when run on develop
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't think we can make AI follow this, so we propose to put 999 in the VERS_MINOR_EXCEPTIONS list that will trigger the warning and abort for incompatible minor versions, and test 999 in the tcheck_versions -tm test. Any future incompatible minor versions can be added to that list (there should never be any). The release branches can run the same test, and because WILL_FAIL is set to true, the test will pass on both develop and the release branches. If a truly incompatible branch is added to the list of exceptions and using the incompatible branch is attempted the warning and abort will be triggered as WILL_FAIL is applied only to the test.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good, are you working on this? I don't see the changes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See comments
branches - removed that instruction fromRELEASE_PROCESS. Change release version instructions to use x.y.z.1 for pre-release instead of x.y.z-1 as for develop snapshots.
…x/hdf5 into semantic-versioning-update
|
Release process document refers this link but I don't see it updated for v200 yet as of 2025-10-30. [u13] https://support.hdfgroup.org/documentation/hdf5/latest/api-compat-macros.html Will it be updated after the release automatically? |
@byrnHDF, I think this is a page created with doxygen? I'm thinking about adding a RELEASE_PROCESS.md entry to update it if needed, but it should probably be done before that. Maybe the RELEASE_PROCESS.md task should be to check that it's updated? Also, this is one of 25 technical notes on https://support.hdfgroup.org/documentation/hdf5/latest/_t_n.html. I wonder if any of the others might go out of date. I see one named HDF5 Library Release Version Numbers that I intend to check today. Working on updates now - almost done. #5968 |
…cumentation is updated for the version to be released.
…x/hdf5 into semantic-versioning-update
release_docs/RELEASE_PROCESS.md
Outdated
| 7. Update the .so version numbers in the [config/lt_vers.am][u9] file in the support branch according to [libtool's library interface version](https://www.gnu.org/software/libtool/manual/libtool.html#Versioning) scheme. | ||
| - See [Updating version info (Libtool)](https://www.gnu.org/software/libtool/manual/html_node/Updating-version-info.html#Updating-version-info) for rules to help update library version numbers. | ||
| 8. After the release branch has been created, run `./autogen.sh` to regenerate build system files on the release branch and commit the changes. | ||
| 8. After the release branch has been created, run bin/process_source.sh to regenerate the H5E header files on the release branch, and commit the changes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are H5E header files enough for committing changes?
I can see other modifications:
# Run make_vers
# make_vers automatically generates the public headers that define the API version
# macros for HDF5.
echo "Running API version generation script:"
bin/make_vers src/H5vers.txt || exit 1
echo
# Run make_overflow
# make_overflow automatically generates macros for detecting overflows for type
# conversion.
echo "Running overflow macro generation script:"
bin/make_overflow src/H5overflow.txt || exit 1
echo
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
process_source.sh runs trace, make_err, make_vers, make_overflow to update H5E headers and macros for the release branch. This is everything except autoreconf that the autogen.sh script ran before. I've added the macro types to be updated to the sentence about running bin/process.sh.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
999 seems too ad-hoc.
I'll approve it with a note for future reference.
| break; | ||
| case 'm': | ||
| minor++; | ||
| minor = 999; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Credit: Claude.ai
Critical Flaws with Setting Minor Version to 999
1. Violates Semantic Versioning Principles
The PR proposes adding 999 to a VERS_MINOR_EXCEPTIONS list to trigger warnings and abort for incompatible minor versions. This fundamentally breaks semantic versioning because:
- Semantic versioning uses minor versions to indicate backwards-compatible features, not incompatibility
- Using 999 as a "special incompatible marker" is a hack that contradicts the entire purpose of semver
- In proper semver (X.Y.Z), minor version changes should be safe upgrades - applications should be able to use any version 2.Y.Z without breaking
2. Confusion Between Development and Release Branches
The comments in the PR reveal confusion:
- The test expects minor + 1 to FAIL on develop branch (where minor=999) but PASS on release branches (where minor≠999)
- This creates a situation where version 2.999.0 on develop is somehow "compatible" with the exception list, but this is semantically meaningless
- Version 2.999.0 doesn't convey any meaningful information to users about API stability
3. Arbitrary Magic Number
Using 999 as a sentinel value is problematic:
- No upper bound validation: What happens if someone legitimately needs version 2.999.X in the future (unlikely but theoretically possible)?
- Implicit rather than explicit: The value 999 has no inherent meaning; it requires developers to remember it's a "magic number"
- Better alternatives exist: Proper semver would use major version bumps (3.0.0) for incompatible changes
4. Testing Logic Issues
The solution proposes setting WILL_FAIL to true for the minor version test so it passes on both develop and release branches when testing incompatible versions. This creates:
- Tests that expect failure as success (confusing)
- Reduced ability to catch actual version compatibility bugs
- Maintenance burden as developers must remember this inverted logic
5. Doesn't Address Root Problem
The HDF5 project has a long history of API breakage between what were labeled "minor" versions (1.8, 1.10, 1.12), which violated semantic versioning principles. Using 999 as a workaround for the develop branch doesn't fix the underlying issue - it just adds another layer of complexity.
Better Alternatives
- Use 0 for develop: Set minor version to 0 on develop branch (2.0.0-dev) and increment on each release
- Use pre-release identifiers: Use semver pre-release syntax like 2.0.0-alpha, 2.0.0-beta.1
- Major version bumps: For truly incompatible changes, use 3.0.0, 4.0.0, etc.
- Development builds: Use metadata like 2.0.0+dev.20251103 to indicate development versions
The 999 approach is a workaround that adds technical debt rather than following established versioning conventions.
Important
Update semantic versioning process and adjust version checking logic for minor version compatibility in HDF5.
RELEASE_PROCESS.md.VERS_RELEASE_EXCEPTIONStoVERS_MINOR_EXCEPTIONSinH5.c.H5_check_version()to focus on minor version compatibility instead of release version.WILL_FAIL "true"for minor version check inCMakeTests.cmaketo allow minor version +1 to pass on non-develop branches.This description was created by
for de3e556. You can customize this summary. It will automatically update as commits are pushed.