openmc icon indicating copy to clipboard operation
openmc copied to clipboard

Add Versioning Support from `version.txt`

Open ahnaf-tahmid-chowdhury opened this issue 1 year ago • 25 comments

Overview

This pull request introduces a versioning mechanism that utilizes a version.txt file located in the tools folder. Both pyproject.toml and CMakeLists.txt have been updated to read version information from this file, ensuring consistency across the project.

Changes

  • Added version.txt: A new file in the tools directory containing the version string in the format X.X.X or X.X.X-any.

  • Updated pyproject.toml:

    • The version field in pyproject.toml is now set to read the version from version.txt. This enables automatic versioning based on the contents of version.txt.
  • Updated CMakeLists.txt:

    • The CMake configuration has been modified to read the version from version.txt and set the OPENMC_VERSION accordingly.
    • Implemented logic to handle versions with optional suffixes (e.g., -dev, -any), ensuring that the header files use only the main version number without the suffix.

Benefits

  • Consistency: Centralizing version information in a single file (version.txt) ensures that both Python and CMake projects remain in sync.
  • Flexibility: The version can be easily updated by modifying just one file, simplifying version management.

Fixs

  • #3136

ahnaf-tahmid-chowdhury avatar Sep 24 '24 06:09 ahnaf-tahmid-chowdhury

Not sure how to resolve this

ERROR: THESE PACKAGES DO NOT MATCH THE HASHES FROM THE REQUIREMENTS FILE. 
If you have updated the package versions, please update the hashes. Otherwise, 
examine the package contents carefully; someone may have tampered with them.
unknown package:
    Expected sha256 f728bb61f43fce850d622ced3b3d51b3116f767685ca4e4e0076f624e2d2307d
         Got        afe0e1873a0a0858a245ccd771066eddc07d20068859f1dd669a002e5dc68a65

ahnaf-tahmid-chowdhury avatar Sep 24 '24 13:09 ahnaf-tahmid-chowdhury

The CI passed when I rerun a single job just now. So I have re triggered the others and I guess it will pass now. I think the previous failure was unrelated to this CI

shimwell avatar Sep 26 '24 16:09 shimwell

CI is all green :tada:

Do you think it is worth adding a version test to the pytests, perhaps something like this https://github.com/fusion-energy/cad_to_dagmc/blob/main/tests/test_version.py

shimwell avatar Sep 26 '24 21:09 shimwell

I have already added something similar to this in the CMakeLists.txt, which will perform a similar task during configuration. I am also using setuptools to get the version number on the Python side, which will cause an error if an improper format is set.

ahnaf-tahmid-chowdhury avatar Sep 27 '24 01:09 ahnaf-tahmid-chowdhury

This looks good to me, does anyone else want to take a peak. Perhaps @MicahGale

shimwell avatar Sep 27 '24 11:09 shimwell

I followed the build from source method cmake .. && make. However build/bin/openmc -v was still showing openmc version 0.15.0.

It seems you have an old version of openmc installed. And you have exported that on your PATH. You can easily review it by calling

which openmc

ahnaf-tahmid-chowdhury avatar Sep 27 '24 14:09 ahnaf-tahmid-chowdhury

I followed the build from source method cmake .. && make. However build/bin/openmc -v was still showing openmc version 0.15.0.

It seems you have an old version of openmc installed. And you have exported that on your PATH. You can easily review it by calling

which openmc

Oh yep that was my issue. I'm used to python where $PATH does not shadow the current directory.

MicahGale avatar Sep 27 '24 14:09 MicahGale

  • Some people argue that you shouldn't store version information in files in the first place. If there's a way we can do this that automatically pulls information from git. If that case can be addressed I really prefer using git tag.

@paulromano: @gonuke brought up some good points, that there are use cases for when the source code may be used while detached from git. I'm not sure right now of a work-around for that case using git based methods, which I generally agree would be the ideal solution.

If we do go with what you have here, there's one more instance of hardcoding of the version in docs/source/conf.py.

I'd recommend that @ahnaf-tahmid-chowdhury go with using a importlib.... call in this file no matter what because that will be agnostic to however python retrieves version information.

MicahGale avatar Sep 27 '24 15:09 MicahGale

I have added the get_version_info function to extract the version info, but I am unsure what working directory Read the Docs is using. For now, I am assuming it is using docs/source/ as the working directory.

Recently, I noticed @MicahGale's comment on this. We can use importlib, but again, I need to understand how Read the Docs processes this. If it runs the conf.py file before installing OpenMC, it will fail.

ahnaf-tahmid-chowdhury avatar Sep 27 '24 16:09 ahnaf-tahmid-chowdhury

It seems docs buils are passing https://readthedocs.org/projects/openmc/builds/25766475/

We can use importlib.metadata.version, but we still need to use the get_version_info function as there are two var.

ahnaf-tahmid-chowdhury avatar Sep 27 '24 16:09 ahnaf-tahmid-chowdhury

We definitely came here, @paulromano , to get broader perspective on this issue. I definitely like the elegance of git being a single source of truth, but worry about folks who build from tarballs (e.g. those generated automatically by GitHub upon release) rather than clones. Do we ignore them? Or do we find a way to make that work, too?

gonuke avatar Oct 08 '24 00:10 gonuke

@gonuke one thing I thought about is that setuptools-scm will create a _version.py file. As part of the GH release process a source tar ball with a version file could be included. I think building from a source tar ball for a specific release would be a lot more common than from a specific branch.

MicahGale avatar Oct 08 '24 01:10 MicahGale

An additional note: If we use setuptools-scm, when someone installs OpenMC from a tarball, it will build a version labeled 0.0.0.

ahnaf-tahmid-chowdhury avatar Oct 08 '24 14:10 ahnaf-tahmid-chowdhury

@ahnaf-tahmid-chowdhury Good point. Do you want to try and see if we could make a CI release that would embed a version file in a release tar ball from setuptools-scm?

MicahGale avatar Oct 08 '24 15:10 MicahGale

For this, we may need to use a workflow to build OpenMC that will create the version file. Once done, it will push the updated commit to the develop branch. We can use any existing workflow, like this one, and it will only run if the PR gets merged. Let me know if I should proceed with that approach.

ahnaf-tahmid-chowdhury avatar Oct 08 '24 15:10 ahnaf-tahmid-chowdhury

No this is a bad idea. _version.py should never be in git. What I was thinking was as part of the release process having python -m build . create the version file. Create a complete source tar ball which include _version.py and then upload that to the GH release as an artifact.

MicahGale avatar Oct 08 '24 16:10 MicahGale

We are currently working on PR #3087, which will create wheel (.whl) artifacts after a release. Your approach is good, but GitHub automatically creates its own tarball with each release. So, in the end, we will be releasing multiple tarballs. And, I am unfamiliar with resolving that.

ahnaf-tahmid-chowdhury avatar Oct 08 '24 17:10 ahnaf-tahmid-chowdhury

It looks like the GH release GHA allows you to overwrite any artifacts, so I think we could just overwrite the default tar ball... hopefully.

MicahGale avatar Oct 08 '24 17:10 MicahGale

I’m still unsure if there is a GHA available that allows modification of the Source code (zip) and Source code (tar.gz) files created by GitHub during a release. If such an action exists, it might involve pushing a commit to the branch and then making the release. I’d be grateful for any guidance on this.

ahnaf-tahmid-chowdhury avatar Oct 09 '24 02:10 ahnaf-tahmid-chowdhury

In the past I've just used the gh cli, which is always available. gh release upload has an option to overwrite whatever is there. So I think that should work for this?

MicahGale avatar Oct 09 '24 13:10 MicahGale

But also it seems like we are really going down a rabbit hole of how to get this to work via git. I think this is approaching a point where perfect is the enemy of good. I personally think it would be good to move forward with a version file for the time being, and open a separate issue to migrate this to git, because making the release process work properly is becoming non-trivial.

MicahGale avatar Oct 09 '24 13:10 MicahGale

I just stumbled across the fact that setuptools-scm might be able to handle a source tar ball: https://setuptools-scm.readthedocs.io/en/latest/usage/#git-archives

MicahGale avatar Oct 16 '24 14:10 MicahGale

Secondly, AFAICT, python isn't getting the version information very effectively; if at all. The ideal situation is for __version__ to be read from _version.py.

Since we are using setuptools_scm and have .git_archival.txt, I think we may not need _version.py. We can get the version info from the metadata.

  1. ensure that cmake always builds with the same version as setuptools_scm provides

Unfortunately, it will never be the same on develop branch, but always work on a specific tag. The Python version has an extra surfix (.dev*) in the development branch, but the version.h file uses numbers only in the format X.X.X. You can see in the CmakeLists.txt file where I have removed the surfix.

ahnaf-tahmid-chowdhury avatar Oct 17 '24 13:10 ahnaf-tahmid-chowdhury

  1. Esnure that a git archive has a version attached to it.

I have created a release in my repo and found that the version info is written in the .git_archival.txt.

ahnaf-tahmid-chowdhury avatar Oct 18 '24 12:10 ahnaf-tahmid-chowdhury

Secondly, AFAICT, python isn't getting the version information very effectively; if at all. The ideal situation is for __version__ to be read from _version.py.

Since we are using setuptools_scm and have .git_archival.txt, I think we may not need _version.py. We can get the version info from the metadata.

I'm still looking into this but I'm worried that doing it this way will always make a runtime call to setuptools_scm which is all-advised. More research is needed, but the documentation seems ambiguous.

  1. ensure that cmake always builds with the same version as setuptools_scm provides

Unfortunately, it will never be the same on develop branch, but always work on a specific tag. The Python version has an extra surfix (.dev*) in the development branch, but the version.h file uses numbers only in the format X.X.X. You can see in the CmakeLists.txt file where I have removed the surfix.

Ok that makes sense to drop the dev part, but I'm still hesitant to rely on OpenMC to properly crawl the git tree in the same way that setuptools_scm does, but making setuptools_scm a compile time dependency seems like not a great option.

MicahGale avatar Oct 18 '24 20:10 MicahGale

Let me explain how the metadata works. Whenever we install something with pip, it always comes with a package.dist-info folder that contains a METADATA file, which stores the version number. So, instead of relying on _version.py, we can rely on that METADATA file, as they will be the same. We don't need the setuptools_scm, it is a buildtime dependency.

ahnaf-tahmid-chowdhury avatar Oct 23 '24 05:10 ahnaf-tahmid-chowdhury

Let me explain how the metadata works. Whenever we install something with pip, it always comes with a package.dist-info folder that contains a METADATA file, which stores the version number. So, instead of relying on _version.py, we can rely on that METADATA file, as they will be the same. We don't need the setuptools_scm, it is a buildtime dependency.

I dug into this a little bit more. One clarification is needed: importlib.metadata will only get the version of OpenMC that is installed. So if it hasn't been installed or a dev version is being used that is different from the installed version this could lead to issues. Overall I don't think that it's too important of an edge case.

MicahGale avatar Oct 24 '24 03:10 MicahGale

  1. ensure that cmake always builds with the same version as setuptools_scm provides

As I mentioned, it is quite hard to achieve here, or we may need to change the OpenMC header so that it can work with strings. After that, we need to implement the setuptools_scm format, as git describe does not follow the setuptools_scm format.

setuptools_scm format
0.15.0.dev144+gddb6c96ef

git describe format
0.15.0-144-gddb6c96ef

For now, if the format is not supported, the build will fail. So, the test is actually implemented during the configuration stage.

  1. ensure that the python api gets a version in all circumstances

I think we can rely on setuptools_scm and let it handle this for us. Once the wheel is created, Python's built-in library importlib.metadata can easily read the version info from the METADATA. We may not need to write a dedicated test script for that, as they may have their own tests in their repository. However, for consistency, we can limit the version of setuptools_scm if it is recommended.

  1. Esnure that a git archive has a version attached to it.

I believe this is an initial-time test that I have already run on my branch while releasing a version. Creating a dedicated test for this may consume resources and increase the time required for processing tests. I think there is no need to test this until we modify the .git_archival.txt. However, any suggestions on this are welcome.

ahnaf-tahmid-chowdhury avatar Oct 24 '24 03:10 ahnaf-tahmid-chowdhury

I dug into this a little bit more. One clarification is needed: importlib.metadata will only get the version of OpenMC that is installed. So if it hasn't been installed or a dev version is being used that is different from the installed version this could lead to issues. Overall I don't think that it's too important of an edge case.

It is always recommended to install Python packages in editable mode for developers, and this is also mentioned in the OpenMC developer docs. So, once OpenMC is installed, even in development mode, it comes with the METADATA file.

ahnaf-tahmid-chowdhury avatar Oct 24 '24 04:10 ahnaf-tahmid-chowdhury

Yes, developers may need to periodically update the OpenMC installation; otherwise, the METADATA will always contain the last installed version.

I think we can add logic where, if someone tries to use OpenMC directly from the source, it will prompt them with a message like, 'Hey, please install OpenMC and run it outside the source root directory.' Let me know if this approach is feasible.

ahnaf-tahmid-chowdhury avatar Oct 24 '24 05:10 ahnaf-tahmid-chowdhury