Version management update #1696

bennibbelink · 2024-03-13T19:19:59Z

This PR automates the process of setting the version number throughout the code.

src/version.h is now generated so that CMake can set the version numbers during the build
setup.py fixed the regex so it replaces the version string in __init__.py. (also maybe we want to modify the hardcoded string in __init__.py so that it doesn't always say 1.5.5)
added core_version as a build arg in the Dockerfiles
modified publish_release.yml to pass in the core_version build arg so that when we publish packages they use the correct version

github-actions · 2024-03-13T19:34:29Z

Downstream Build Status Report

Build `FROM cyclus_20.04_apt/cyclus`

Cycamore: Success ✅
Cymetric: Success ✅

Build `FROM cyclus_20.04_conda/cyclus`

Cycamore: Success ✅
Cymetric: Success ✅

Build `FROM cyclus_22.04_apt/cyclus`

Cycamore: Success ✅
Cymetric: Success ✅

Build `FROM cyclus_22.04_conda/cyclus`

Cycamore: Success ✅
Cymetric: Success ✅

coveralls · 2024-03-13T19:43:03Z

Pull Request Test Coverage Report for Build 8380326460

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

For more information on this, see Tracking coverage changes with pull request builds.
To avoid this issue with future PRs, see these Recommended CI Configurations.
For a quick fix, rebase this PR at GitHub. Your next report should be accurate.

Details

0 of 0 changed or added relevant lines in 0 files are covered.
15 unchanged lines in 4 files lost coverage.
Overall coverage increased (+0.005%) to 32.594%

Files with Coverage Reduction	New Missed Lines	%
src/toolkit/res_buf.h	1	58.53%
src/resource.h	3	41.27%
tests/toolkit/res_buf_tests.h	5	55.0%
src/context.cc	6	54.52%

Totals
Change from base Build 8255130199:	0.005%
Covered Lines:	53047
Relevant Lines:	129057

💛 - Coveralls

gonuke · 2024-03-13T20:35:16Z

I don't think these version strings all mean the same thing... we probably need to discuss

gonuke · 2024-03-14T15:05:29Z

I'll have to look into it, but there is a release version that is set in a file and there is the version number we provide at build time.

gonuke · 2024-03-14T15:06:57Z

I think (??) the version at build time specifies which version of the data model to build whereas the one in the version.h file is the code version. The challenge is to remember if/how/why/when these are different.

bennibbelink · 2024-03-14T16:04:06Z

Are you referring to the data model in share/dbtypes.json?

bennibbelink · 2024-03-14T16:33:52Z

Found this PR, trying to get a handle on the process

EDIT: Also this Release Procedure and this Release repo

bennibbelink · 2024-03-16T15:44:31Z

This is my understanding of share/dbtypes.json.in:

It is used to maintain a version history of the data model - so if one wanted to build cyclus with an older data model they could specify --core-version=x.x and cyclus would use the appropriate data model
If cyclus is built with a version greater than what exists in share/dbtypes.json.in it will generate a new data model using the core version passed into the build. This generated data model just copies the latest data model that exists in share/dbtypes.json.in (I think)
with each release share/dbtypes.json.in should be manually updated to reflect the data model for the release candidate version (old PR convo). If there were no changes made, this should be as easy as copying relevant sections from the generated share/dbtypes.json (note no .in) to share/dbtypes.json.in

If these notes are correct (and align with your memory @gonuke), I think the changes in this PR are still appropriate improvements to automate the version propagation throughout the code at build-time.

However... if these notes are correct then it appears that when cyclus v1.5 was released share/dbtypes.json.in was not manually updated to reflect the new data model for v1.5 since the latest model in that file is v1.4 (I'm assuming there were no changes between the two versions otherwise things probably would have broken). What is the best way to move forward? Add the model for v1.5 to main before we make a release candidate, then add the model for our release candidate in the release candidate branch?

P.S. What will the next release be - v1.6.0 or v2.0.0?

gonuke · 2024-03-20T01:10:37Z

I can't contest your interpretation, although I can't recall if there was an expectation of appending/extending dtypes.json.in with a new data model version if it didn't change, or just relying on the latest one that existed in that case??

One thing that still concerns me about this is that the source code will no longer document the release number upon release. It will be embedded in the docker image that publish_release.yml generates via the GitHub release tag, but if someone downloaded the source code and installed it, the release number would exist nowhere.

bennibbelink · 2024-03-20T18:15:39Z

I can't contest your interpretation, although I can't recall if there was an expectation of appending/extending dtypes.json.in with a new data model version if it didn't change, or just relying on the latest one that existed in that case??

Maybe things changed since this comment (and I don't see anything in the doc/ directory about this, is there a different docs repo?) but it sounds like it should be extended on every release. I will dig around to confirm/deny this suspicion.

One thing that still concerns me about this is that the source code will no longer document the release number upon release. It will be embedded in the docker image that publish_release.yml generates via the GitHub release tag, but if someone downloaded the source code and installed it, the release number would exist nowhere.

Fair point. One clarifying question: if a user downloads source code and installs using the hardcoded version number, don't they need to pass the correct build arg for --core-version when building cyclus? I believe this argument is mainly used in the CMake build to generate dbtypes.json. Why pass this in as a parameter? If a user wanted to use an older data model then they would also have to go into version.h and manually change the hardcoded version anyway (I think). We may want to take a step back and rethink the versioning system. (possibly a combination of the current method and this PR.... Have a config file with a hardcoded version and then use CMake to populate the appropriate source code files?)

bennibbelink · 2024-03-21T00:23:17Z

Summary of the different versions and their locations in Cyclus:

src/hdf5_back_gen.py

uses core_version passed into CMake
reads data model from share/dbtypes.json
build fails in this file if I pass in 1.3 or 1.4 as --core-version
lines 2772-2779 imply that this uses the highest data model version it can find (bug? Whats the point of maintaining old data models if they never get used)

share/dbtypes_gen.py

generates data model for version even if it already exists (I passed in 1.3 and it generated a second v1.3 model after the v1.4 model)
generates data model directly from the comments in src/query_backend.h

src/version.cc(.in)

describe() returns the version passed in as --core-version
core() returns the version hardcoded in src/version.h
cyclus --version returns "Cyclus Core <core()> (<describe()>)"

Some general impressions I have:

We can change the version in src/version.h with seemingly no repercussions
The data model version we use doesn't seem to matter?? It will get generated from the comments in src/query_backend.h regardless of what we pass in... passing in older versions just seems to break the build (which might be a result of the bug(?) in src/hdf5_back_gen.py)

We can talk about this in-person tomorrow but I wanted to get these notes down somewhere.

…build

gonuke

A couple of final clarifying comments

CMakeLists.txt

gonuke · 2024-03-21T15:28:04Z

cmake/SetupPyInstall.cmake.in

@@ -2,8 +2,8 @@
 # Runs setup.py install
 #
 set(PYTHON_EXECUTABLE @Python3_EXECUTABLE@)
-set(core_version @core_version@)
-set(ENV{CYCLUS_CORE_VERSION} "${core_version}")
+set(CYCLUS_CORE_VERSION @CMAKE_PROJECT_VERSION@)


If a user specifies core_version during setup, shouldn't we use that still?

and don't we now set CYCLUS_CORE_VERSION reliably at the top level?

Refactored build to differentiate between CMAKE_PROJECT_VERSION(CYCLUS_PROJECT_VERSION in setup.py) and DATA_MODEL_VERSION

bennibbelink · 2024-03-21T18:29:10Z

I had to update tests/test_cycluslib.py and tests/tools.py to correctly set up the tests. Not sure why this started failing now, my best guess is that new versions of pytest removed the functionality we were previously relying on

gonuke

two more naming cleanup suggestions

CMakeLists.txt

gonuke · 2024-03-21T18:43:19Z

cyclus/CMakeLists.txt

@@ -10,7 +10,7 @@ INCLUDE_DIRECTORIES(${CYCLUS_CORE_INCLUDE_DIRS})
 message(STATUS "Generating Type System API for Python")
 EXECUTE_PROCESS(COMMAND python ${CMAKE_CURRENT_SOURCE_DIR}/gentypesystem.py
    "--src-dir=${CMAKE_CURRENT_SOURCE_DIR}"
-    "--cyclus-version=${core_version}"
+    "--cyclus-version=${DATA_MODEL_VERSION}"


Do we want to change this argparse variable for gentypesystem.py?

Suggested change

"--cyclus-version=${DATA_MODEL_VERSION}"

"--data-model-version=${DATA_MODEL_VERSION}"

Addressed this. At some point (maybe not right now) should we rename version::core() or keep it as is? As is, version::core() is the "source code version" and version::describe() is the "data model version"

Let's hold off on this for now

Co-authored-by: Paul Wilson <paul.wilson@wisc.edu> Signed-off-by: bennibbelink <79653949+bennibbelink@users.noreply.github.com>

gonuke

LGTM - will merge following successful tests

bennibbelink · 2024-03-21T19:26:17Z

After merge are we ready to make a release candidate branch? http://fuelcycle.org/cep/cep3.html#release-candidates-tags-branches

Related: CEP 3 dictates we make release candidates for all 3 projects, I will implement a similar versioning strategy in cycamore so it is ready for release as well. Should be much more straightforward than cyclus

bennibbelink and others added 3 commits March 13, 2024 13:42

Pass version to all src from build command

caa02f6

fix version update in python setup.py

1d223f9

update changelog

1e18227

bennibbelink marked this pull request as ready for review March 13, 2024 19:21

bennibbelink requested a review from gonuke March 20, 2024 00:49

gonuke assigned gonuke and bennibbelink Mar 21, 2024

bennibbelink added 2 commits March 21, 2024 10:03

specify version in CMakeLists.txt, populate to appropriate places in …

cc4c501

…build

remove core-version arg in Rocky dockerfile

7f728f1

munkm mentioned this pull request Mar 21, 2024

Updates to cep3 in preparation for releases cyclus/cyclus.github.com#339

Open

gonuke requested changes Mar 21, 2024

View reviewed changes

bennibbelink and others added 2 commits March 21, 2024 11:47

change core_version to data_model_version

6c33868

setup cyclus lib in tets

d8c2d0f

gonuke requested changes Mar 21, 2024

View reviewed changes

bennibbelink and others added 2 commits March 21, 2024 13:54

Change "core version" to "Cyclus version"

e99347a

Co-authored-by: Paul Wilson <paul.wilson@wisc.edu> Signed-off-by: bennibbelink <79653949+bennibbelink@users.noreply.github.com>

change gentypesystem arg to --data-model-version

47a646c

gonuke approved these changes Mar 21, 2024

View reviewed changes

gonuke merged commit 68ef954 into main Mar 21, 2024
23 checks passed

bennibbelink deleted the version-update branch March 21, 2024 23:08

bennibbelink mentioned this pull request Mar 21, 2024

Issue release #1685

Closed

gonuke mentioned this pull request Mar 22, 2024

make package routines virtual nuclearkatie/cyclus#1

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Version management update #1696

Version management update #1696

bennibbelink commented Mar 13, 2024

github-actions bot commented Mar 13, 2024

coveralls commented Mar 13, 2024 •

edited

gonuke commented Mar 13, 2024

gonuke commented Mar 14, 2024

gonuke commented Mar 14, 2024

bennibbelink commented Mar 14, 2024

bennibbelink commented Mar 14, 2024 •

edited

bennibbelink commented Mar 16, 2024

gonuke commented Mar 20, 2024

bennibbelink commented Mar 20, 2024

bennibbelink commented Mar 21, 2024

gonuke left a comment

gonuke Mar 21, 2024

gonuke Mar 21, 2024

bennibbelink Mar 21, 2024

bennibbelink commented Mar 21, 2024

gonuke left a comment

gonuke Mar 21, 2024

bennibbelink Mar 21, 2024

gonuke Mar 21, 2024

gonuke left a comment

bennibbelink commented Mar 21, 2024

	"--cyclus-version=${DATA_MODEL_VERSION}"
	"--data-model-version=${DATA_MODEL_VERSION}"

Version management update #1696

Version management update #1696

Conversation

bennibbelink commented Mar 13, 2024

github-actions bot commented Mar 13, 2024

Downstream Build Status Report

Build FROM cyclus_20.04_apt/cyclus

Build FROM cyclus_20.04_conda/cyclus

Build FROM cyclus_22.04_apt/cyclus

Build FROM cyclus_22.04_conda/cyclus

coveralls commented Mar 13, 2024 • edited

Pull Request Test Coverage Report for Build 8380326460

Warning: This coverage report may be inaccurate.

Details

💛 - Coveralls

gonuke commented Mar 13, 2024

gonuke commented Mar 14, 2024

gonuke commented Mar 14, 2024

bennibbelink commented Mar 14, 2024

bennibbelink commented Mar 14, 2024 • edited

bennibbelink commented Mar 16, 2024

gonuke commented Mar 20, 2024

bennibbelink commented Mar 20, 2024

bennibbelink commented Mar 21, 2024

gonuke left a comment

Choose a reason for hiding this comment

gonuke Mar 21, 2024

Choose a reason for hiding this comment

gonuke Mar 21, 2024

Choose a reason for hiding this comment

bennibbelink Mar 21, 2024

Choose a reason for hiding this comment

bennibbelink commented Mar 21, 2024

gonuke left a comment

Choose a reason for hiding this comment

gonuke Mar 21, 2024

Choose a reason for hiding this comment

bennibbelink Mar 21, 2024

Choose a reason for hiding this comment

gonuke Mar 21, 2024

Choose a reason for hiding this comment

gonuke left a comment

Choose a reason for hiding this comment

bennibbelink commented Mar 21, 2024

Build `FROM cyclus_20.04_apt/cyclus`

Build `FROM cyclus_20.04_conda/cyclus`

Build `FROM cyclus_22.04_apt/cyclus`

Build `FROM cyclus_22.04_conda/cyclus`

coveralls commented Mar 13, 2024 •

edited

bennibbelink commented Mar 14, 2024 •

edited