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 astacus/version issues #193

Closed
wants to merge 6 commits into from
Closed

Fix astacus/version issues #193

wants to merge 6 commits into from

Conversation

alanfranz
Copy link
Contributor

@alanfranz alanfranz commented Mar 26, 2024

Astacus relied on autogenerated versioning from git which could happen or change without real knowledge of the developer, and could cause issues if an accidental git directory was found in a parent when using astacus from a non-git tarball.

This PR:

  • fixes accidental version setting
  • Tries to untangle automagic version setting in multiple parts of the codebase
  • Supports external setting for the desired package version

It seems that we're using all possible packaging files and approaches - multiple requirements files, pyproject, setup.py AND setup.cfg - I didn't want to enter that packaging-related discussion because I couldn't see the end of it, so I just took the existing versioning code and made it predictable & error-proof, I didn't change the "current way of doing things".

In some situations (.e.g. when building packages from aiven-core) it
could happen that the wrong .git directory is found, causing a total
version mismatch. This target should work if and only if we've got an
actual .git directory for the current project, and fail otherwise.
We don't want to have version automatically generated, as it can cause
mistakes. We let users generate the version file if it's really needed.
@alanfranz alanfranz force-pushed the alanfranz-versioning branch 3 times, most recently from bf4e3f6 to 47d7ef1 Compare March 26, 2024 13:43
Some issues where found with version generation; version didn't follow
pep-440 standards, and it relied on git for actual fetching, while we
may want to override the version from outside to match the actual
version from the rpm package we build.

Also, we don't want generation to happen automagically, but
intentionally, so we had clear targets for what should be happening.
We don't want setup.py to do anything magic; it should just use the
already-set version, which should be previously generated.
Target is unused, remove it.
@alanfranz alanfranz force-pushed the alanfranz-versioning branch 2 times, most recently from 0080cdc to 29b8340 Compare March 26, 2024 13:48
We need to generate the version at the right moment, before full
requirements need to be installed.
@codecov-commenter
Copy link

codecov-commenter commented Mar 26, 2024

Codecov Report

Attention: Patch coverage is 0% with 42 lines in your changes are missing coverage. Please review.

Project coverage is 89.07%. Comparing base (7a4b1ae) to head (b507269).

Files Patch % Lines
version_setter.py 0.00% 41 Missing ⚠️
setup.py 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #193      +/-   ##
==========================================
- Coverage   89.10%   89.07%   -0.04%     
==========================================
  Files         144      144              
  Lines       10216    10220       +4     
==========================================
  Hits         9103     9103              
- Misses       1113     1117       +4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@alanfranz alanfranz changed the title Fix astacus versioning Fix astacus/version issues Mar 26, 2024
@alanfranz alanfranz requested a review from a team March 26, 2024 15:42
@alanfranz alanfranz marked this pull request as ready for review March 26, 2024 15:42
Copy link
Contributor

@aris-aiven aris-aiven left a comment

Choose a reason for hiding this comment

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

Conflicts need to be fixed since we're now packaging using pyproject.toml

@aris-aiven aris-aiven requested a review from a team March 28, 2024 11:28
@alanfranz
Copy link
Contributor Author

I'm not sure what should I do anymore. It seems that a #194 was merged before this and touched the same files. But I don't understand whether it addresses the same concerns.

@alanfranz
Copy link
Contributor Author

it seems this was already solved independently.

@alanfranz alanfranz closed this Apr 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants