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 duckdb version handling again #43762

Merged

Conversation

teaguesterling
Copy link
Contributor

After 0.10.0, DuckDB added the OVERRIDE_GIT_DESCRIBE variable that we can set to force a version when we're not in a git repository. This simplifies the logic for setting up the build environment for new versions, but also breaks the old method for new versions. This PR adds the latest version and updates the logic.

Copy link

spackbot-app bot commented Apr 20, 2024

@glentner can you review this PR?

This PR modifies the following package(s), for which you are listed as a maintainer:

  • duckdb

@teaguesterling teaguesterling marked this pull request as draft April 20, 2024 20:25
@teaguesterling teaguesterling marked this pull request as ready for review April 20, 2024 20:27
Copy link
Contributor

@tldahlgren tldahlgren left a comment

Choose a reason for hiding this comment

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

I have some questions about the changes.

var/spack/repos/builtin/packages/duckdb/package.py Outdated Show resolved Hide resolved
var/spack/repos/builtin/packages/duckdb/package.py Outdated Show resolved Hide resolved
var/spack/repos/builtin/packages/duckdb/package.py Outdated Show resolved Hide resolved
@tldahlgren tldahlgren self-assigned this Apr 22, 2024
- use f-string
- fix version specs to address inconsistencies
Fix spec definitions further
@teaguesterling
Copy link
Contributor Author

@spackbot fix styles

Copy link

spackbot-app bot commented Apr 23, 2024

Let me see if I can fix that for you!

Copy link

spackbot-app bot commented Apr 23, 2024

I was able to run spack style --fix for you!

spack style --fix
==> Running style checks on spack
  selected: isort, black, flake8, mypy
==> Modified files
  var/spack/repos/builtin/packages/duckdb/package.py
==> Running isort checks
  isort checks were clean
==> Running black checks
reformatted var/spack/repos/builtin/packages/duckdb/package.py
All done! ✨ 🍰 ✨
1 file reformatted.
  black checks were clean
==> Running flake8 checks
  flake8 checks were clean
==> Running mypy checks
lib/spack/spack/version/version_types.py:145: error: Argument 2 to "StandardVersion" has incompatible type "*Tuple[Tuple[Any, ...], Tuple[Any, ...]]"; expected "Tuple[Tuple[Any, ...], Tuple[Any, ...]]"  [arg-type]
lib/spack/spack/version/version_types.py:447: error: Argument 2 to "StandardVersion" has incompatible type "*Tuple[Tuple[Any, ...], Tuple[Any, ...]]"; expected "Tuple[Tuple[Any, ...], Tuple[Any, ...]]"  [arg-type]
lib/spack/spack/version/version_types.py:476: error: Argument 2 to "StandardVersion" has incompatible type "*Tuple[Tuple[Any, ...], Tuple[Any, ...]]"; expected "Tuple[Tuple[Any, ...], Tuple[Any, ...]]"  [arg-type]
Found 3 errors in 1 file (checked 621 source files)
  mypy found errors
Keep in mind that I cannot fix your flake8 or mypy errors, so if you have any you'll need to fix them and update the pull request. If I was able to push to your branch, if you make further changes you will need to pull from your updated branch before pushing again.

I've updated the branch with style fixes.

@teaguesterling
Copy link
Contributor Author

@tldahlgren and @glentner I've adjusted based on the comments provided.

Copy link
Contributor

@tldahlgren tldahlgren left a comment

Choose a reason for hiding this comment

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

LGTM except the typo in the comment.

var/spack/repos/builtin/packages/duckdb/package.py Outdated Show resolved Hide resolved
@tldahlgren tldahlgren merged commit f2bd0c5 into spack:develop Apr 30, 2024
14 checks passed
wdconinc pushed a commit that referenced this pull request May 4, 2024
* Added package to build Ollama
* Update package.py
   Add license and documentation
* [@spackbot] updating style on behalf of teaguesterling
* We can now use OVERRIDE_GIT_DESCRIBE to set the version in DuckDB
* Update duckdb/package.py
   - use f-string
   - fix version specs to address inconsistencies
* Update package.py
   Fix spec definitions further
* [@spackbot] updating style on behalf of teaguesterling

---------

Co-authored-by: Tamara Dahlgren <35777542+tldahlgren@users.noreply.github.com>
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.

None yet

2 participants