Skip to content
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

Version information in ANTs build #1231

Closed
cookpa opened this issue Aug 26, 2021 · 11 comments
Closed

Version information in ANTs build #1231

cookpa opened this issue Aug 26, 2021 · 11 comments
Labels
enhancement Enhancements under development - feel free to join discussion if you'd like to contribute help needed Issues that require feedback or expertise from the community. Discussion and PRs welcome

Comments

@cookpa
Copy link
Member

cookpa commented Aug 26, 2021

Is your feature request related to a problem?

Version information in ANTs builds is only available when building from a cloned git repo. This is a problem for people trying to build from a tarball.

Also, the version information is not checked for local modifications. This could be fixed by the use of the --dirty option to git describe.

Proposed new features

A system that provides accurate version information however the source was obtained.

I don't think we can help people who hit the download snapshot version to get the current HEAD revision. But we can attach our own tarballs to releases, so we can include version information there.

  • If a git repo, output version information from git describe and use --dirty to catch local modifications
  • If building from a release tarball, have version information embedded in the tarball somehow (maybe CircleCI can do this?)
  • Some kind of checksum built into release tarballs to catch local modifications? Too much?

If a tagged release is built, the version information should say "vX.Y.Z" whether built from Git or a tarball, and this information should be propagated to the libraries (no more 0.0.0).

Alternatives you've considered

  • Adapting existing code to allow version information to be set manually at build time, such that the user has full control over the version info. But this could lead to confusion, as there's no guarantee the user-supplied info is correct.

Additional context

The versioning behavior affects all of ANTsX, and builds in other packages.

Related:

#1059
#1183
#1199

and others.

@cookpa cookpa added enhancement Enhancements under development - feel free to join discussion if you'd like to contribute help needed Issues that require feedback or expertise from the community. Discussion and PRs welcome labels Aug 26, 2021
@ghisvail
Copy link
Contributor

TL;DR a version module with git describe parsing + fallback on hardcoded version metadata if git is not present should be more than enough.

If a git repo, output version information from git describe and use --dirty to catch local modifications

👍

If building from a release tarball, have version information embedded in the tarball somehow (maybe CircleCI can do this?)

Looks like bigger CMake projects are doing just fine with a separate version module which is bumped before tagging a new release or daily by a bot.

Some kind of checksum built into release tarballs to catch local modifications? Too much?

Likely too much. Are you aware of many users downloading a release tarball, applying local modifications and then request support? Those advanced users who want to hack on the ANTs codebase would likely use a git clone instead, imo.

@cookpa
Copy link
Member Author

cookpa commented Aug 27, 2021

Looks like bigger CMake projects are doing just fine with a separate version module which is bumped before tagging a new release or daily by a bot.

@ghisvail we used to have the hard-coded method, but it was not updated correctly. Part of that is just on us developers to be more careful. But it's also a problem if people think they have version 2.3.5 when they are actually multiple commits forward of that point. Changing the version file in a finer grained way leads to confusion with branches and PRs.

Perhaps though a bot could do it. Maybe we could have CircleCI bundle a nightly build of master with full git version info, and make that the recommended way to get ANTs. And the same for releases.

Likely too much. Are you aware of many users downloading a release tarball, applying local modifications and then request support? Those advanced users who want to hack on the ANTs codebase would likely use a git clone instead, imo.

I agree it's not a very likely use case. But I am mindful that many users do not install the software themselves (eg, a cluster sysadmin does it), and only have the output of antsRegistration --version to report.

But it's probably not a priority for us. If people have bugs that can't be reproduced, there's little we can do to help anyway.

@cookpa
Copy link
Member Author

cookpa commented Aug 27, 2021

Speaking of modifying the source tarball, here's where I think you could enter your own version information:

ANTs/ANTS.cmake

Lines 36 to 42 in d11d416

if( ${PROJECT_NAME}_VERSION_HASH STREQUAL "GITDIR-NOTFOUND" AND NOT "${ANTS_SNAPSHOT_VERSION}" STREQUAL "" )
set(${PROJECT_NAME}_VERSION_MAJOR 0)
set(${PROJECT_NAME}_VERSION_MINOR 0)
set(${PROJECT_NAME}_VERSION_PATCH 0)
set(${PROJECT_NAME}_VERSION_TWEAK 0)
set(${PROJECT_NAME}_VERSION "snapshot-${ANTS_SNAPSHOT_VERSION}")
endif()

Though I'm not sure if this would be allowed by conda rules, because it involves modifying the source code after download.

@ghisvail
Copy link
Contributor

Conda packages do support declaration of additional patches to apply on top of the source release. However, patching a source release to add appropriate versioning information really is not great from a downstream maintenance perspective. That means updating the patch every new release and discarding fully-automated rebuilds by the conda-forge infrastructure. I would rather avoid that.

@cookpa
Copy link
Member Author

cookpa commented Aug 27, 2021

OK. So that's motivation for us to fix it on the CI end, and provide source distributions that will work automatically. Thanks for this context.

@cookpa cookpa linked a pull request Sep 23, 2021 that will close this issue
@cookpa
Copy link
Member Author

cookpa commented Sep 23, 2021

I tried and failed to get the -dirty tag included in locally modified repos, but my main goal is to have a means to put version information into a release tarball, so that ANTs can be built with version information.

@cookpa
Copy link
Member Author

cookpa commented Sep 23, 2021

Ideally I'd like an automated way for releases to be published with the correct version information, without having it hard-coded into the repository. Looking at Github Packages

https://docs.github.com/en/packages/learn-github-packages/introduction-to-github-packages

Edit: actually it looks like this works for Docker, but not source.

@oesteban
Copy link

@cookpa just to understand the problem. You basically want an automated way to stick the version information into the tarball when you create a new release, so that the tarballs attached to each release here have the version information.

At the moment, that is handled by the git-archive built-in into GitHub, and therefore, once you create the tag the archive is generated automatically without the possibility of modification - correct?

@oesteban
Copy link

I say this because perhaps you could create releases automatically triggered by the creation of a git tag (there's lots of granularity here to filter in and out tag patterns) with something like https://github.com/actions/upload-release-asset

In this case, you want to build the tarball and the zipball with some textfile you write the version in. Then, (in theory) if you assign the same names (Sources blah blah), the release page would distribute the sources with the version information within the package.

@cookpa
Copy link
Member Author

cookpa commented Oct 15, 2021

Hi @oesteban, you are correct. Originally, we had version numbers hard-coded into the release, which solved the problem. Some time ago, these version numbers were replaced with variables that came from git describe. This created the problem of missing version information in tarballs.

In #1246, I am putting back the explicit version numbers, with some additional changes. I think this will solve the problem, as you explained, and then the Git tarball release will have a version number automatically.

I'm working on getting a new release together. I'm trying to close out some outstanding issues like in #1062 first.

@cookpa
Copy link
Member Author

cookpa commented Oct 15, 2021

I say this because perhaps you could create releases automatically triggered by the creation of a git tag (there's lots of granularity here to filter in and out tag patterns) with something like https://github.com/actions/upload-release-asset

In this case, you want to build the tarball and the zipball with some textfile you write the version in. Then, (in theory) if you assign the same names (Sources blah blah), the release page would distribute the sources with the version information within the package.

This could be quite useful, thanks. Ultimately I would like to have binaries and containers built with releases too.

@cookpa cookpa closed this as completed Oct 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Enhancements under development - feel free to join discussion if you'd like to contribute help needed Issues that require feedback or expertise from the community. Discussion and PRs welcome
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants