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

Fix extraction of version number if sources are inside another Git repository #1362

Merged
merged 4 commits into from
Dec 12, 2019

Conversation

steffengraber
Copy link
Contributor

@steffengraber steffengraber commented Dec 10, 2019

The problem:
If you are inside the folder structure of a git repository and you install NEST within this repository from the source download files (e.g. v2.18.0.tar.gz), the hash of the last commit of the git repository will be displayed as the NEST version number instead of the version of the download package .
This also happens within the conda forge packages.

The solution:
It is checked if there is a .git folder inside the source folder. Only then it tries to create the version from the git information. If the folder does not exist, the given version information (UNKNOWN, nest-2.18 ...) is used.

This pull request fixes #1347 .

Copy link
Contributor

@jougs jougs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First of all, I consider the act of downloading tarballs into git-managed source directories and building them therein, as well as in-source-builds in general to be totally evil.

That said, I acknowledge that there are valid (but probably nonetheless evil) use cases where this might happen in automatic builds and we should report the right version numbers also in these cases.

Many thanks for addressing this!

@heplesser heplesser added ZC: Installation DO NOT USE THIS LABEL I: No breaking change Previously written code will work as before, no one should note anything changing (aside the fix) ZP: PR Created DO NOT USE THIS LABEL S: Normal Handle this with default priority T: Maintenance Work to keep up the quality of the code and documentation. labels Dec 11, 2019
Copy link
Contributor

@heplesser heplesser left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@steffengraber Looks good to me, but the indentation on lines 43-56 seems off. Shouldn't the execute_process on line 43 align with the one on line 36, and correspondingly the ifs on lines 50 and 54? It seems the file contains Tabs which may cause the trouble.

@heplesser heplesser removed the request for review from terhorstd December 12, 2019 07:28
@jougs jougs merged commit bbde13c into nest:master Dec 12, 2019
@jougs jougs changed the title Fix show version issue Fix extraction of version number if sources are inside another Git repository Jan 30, 2020
@steffengraber steffengraber deleted the fix-issue-1347 branch September 6, 2023 05:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
I: No breaking change Previously written code will work as before, no one should note anything changing (aside the fix) S: Normal Handle this with default priority T: Maintenance Work to keep up the quality of the code and documentation. ZC: Installation DO NOT USE THIS LABEL ZP: PR Created DO NOT USE THIS LABEL
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Version is returned as a hash instead of a human readable string in pynest and sli
3 participants