Skip to content

Conversation

carlopi
Copy link
Contributor

@carlopi carlopi commented Mar 15, 2024

Changes the scripts used by python build OR by amalgamation to use (if available) the environment variable OVERRIDE_GIT_DESCRIBE.

Idea is:

  1. if OVERRIDE_GIT_DESCRIBE is complete, use that
  2. if OVERRIDE_GIT_DESCRIBE has only version, add "-0" and go to point 3
  3. if OVERRIDE_GIT_DESCRIBE has version and dev version (possibly through step 2): add the git hash (or the default deadbbeeff)
  4. if OVERRIDE_GIT_DESCRIBE is empty or not present: use git describe --tags value

Code is a simple but a bit intricate (try / except and all), would love a proper review (ping to @Mause, @Mytherin or @Tishj).

I noticed that a similar role is covered by SETUPTOOLS_SCM_PRETEND_VERSION / SETUPTOOLS_SCM_PRETEND_HASH. Those variables keep having precedence as they had before, but also that might need a review.

@carlopi carlopi marked this pull request as draft March 15, 2024 15:04
@carlopi carlopi marked this pull request as ready for review March 15, 2024 15:05
@@ -128,11 +128,39 @@ def get_relative_path(source_dir, target_file):
return target_file


def get_git_describe():
Copy link
Contributor

@Tishj Tishj Mar 15, 2024

Choose a reason for hiding this comment

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

Some useful generated simplification that I think would be nicer to work with:

from typing import NamedTuple

class VersionInfo(NamedTuple):
    major: str
    minor: str
    patch: str
    dev_version: str

def get_git_describe():
    # Assume package_build.get_git_describe() returns a string like "v1.2.3-dev1-abcdef"
    long_version = package_build.get_git_describe()
    
    # Split the version and dev_version parts
    version_parts, dev_version = long_version.split('-')[:-1], long_version.split('-')[-1]
    major, minor, patch = version_parts[0].lstrip('v').split('.')
    
    return VersionInfo(major, minor, patch, dev_version)

# Example usage:
version_info = get_git_describe()
print(version_info.major)  # Print major version
print(version_info.minor)  # Print minor version
print(version_info.patch)  # Print patch version
print(version_info.dev_version)  # Print development version

Not to be used verbatim obviously, but making a NamedTuple for this sounds like a good idea

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is cool, but I don't feel very comfortable adding infrastructure around this.

Also connected: #11187, that is currently just a copy.

@carlopi
Copy link
Contributor Author

carlopi commented Mar 15, 2024

This seems to work properly, see for example the results of this run: https://github.com/duckdb/duckdb-test-staging/actions/runs/8298231890/job/22711776901

@Mytherin Mytherin merged commit 1f42ae1 into duckdb:main Mar 15, 2024
@Mytherin
Copy link
Collaborator

Thanks!

github-actions bot pushed a commit to duckdb/duckdb-r that referenced this pull request Mar 17, 2024
Merge pull request duckdb/duckdb#11179 from carlopi/override_git_describe_in_scripts
@carlopi carlopi deleted the override_git_describe_in_scripts branch May 7, 2024 08:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants