Skip to content

Conversation

hmeriann
Copy link
Contributor

@hmeriann hmeriann commented Feb 11, 2025

Currently, the naming pattern for DuckDB extensions is inconsistent with the naming pattern used for DuckDB binaries.
Specifically:

  • Extension names:
linux-extensions-64
duckdb-extensions-${{ matrix.duckdb_arch }} (OSX and Wasm)
windows_amd64_mingw-extensions

This PR aligns the naming pattern for extensions across all platforms to use duckdb-extensions-${{ matrix.duckdb_arch }}

@carlopi
Copy link
Contributor

carlopi commented Feb 11, 2025

I think there are 2 change-sets in this PR, the one connected to extensions and the one connected to binaries. I would separate the 2 since they are logically independent.

Extensions: LGTM, change should not be visible to external world (since it changed already a few times / it's recent / extensions you can get them through regular INSTALL)

Binaries: I am not sure I see it AND it's independent, so I would remove from PR and discuss that independently.

In particular, Linux binaries are distributed like:

    - uses: actions/upload-artifact@v4
      with:
        name: duckdb-binaries-linux-${{ matrix.config.arch }}
        path: |
          libduckdb-linux-${{ matrix.config.arch }}.zip
          duckdb_cli-linux-${{ matrix.config.arch }}.zip

that seems consistent to the Windows pre-this change.

Change can have sense, but it's more visible (since nightly download endpoint uses that) and would make more sense across the board.

Copy link
Contributor

@carlopi carlopi left a comment

Choose a reason for hiding this comment

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

Thanks, great the extension part, I would revert the Windows binaries part.

@hmeriann
Copy link
Contributor Author

libduckdb-linux-${{ matrix.config.arch }}.zip

yes! And duckdb_arch is linux_arm64 or linux_amd64, but for windows it is windows-amd64 or windows-arm64 https://github.com/duckdb/duckdb/blob/main/.github/workflows/Windows.yml#L109C27-L109C40

It makes sense that changes to the binaries names and to the extensions names should be PR'ed separately. I'll remove changes to binaries names from this PR and will open a follow-up PR with them

@carlopi
Copy link
Contributor

carlopi commented Feb 11, 2025

I think in the case of LinuxRelease:

arch: arm64, image: aarch64

so it then becomes libduckdb-linux-${{ matrix.config.arch }}.zip -> `libduckdb-linux-aarch64.zip (or the other one)

@hmeriann hmeriann requested a review from carlopi February 11, 2025 13:32
@duckdb-draftbot duckdb-draftbot marked this pull request as draft February 11, 2025 13:33
@carlopi
Copy link
Contributor

carlopi commented Feb 11, 2025

Thanks!!

@hmeriann
Copy link
Contributor Author

I think in the case of LinuxRelease:

arch: arm64, image: aarch64

so it then becomes libduckdb-linux-${{ matrix.config.arch }}.zip -> `libduckdb-linux-aarch64.zip (or the other one)

I don't think that image variable is used in LinuxRelease workflow to create an archive file containing a binary, please take a look here

image variable is used once and not in the binaries names

@hmeriann hmeriann marked this pull request as ready for review February 11, 2025 13:38
@Mytherin Mytherin merged commit 60c9442 into duckdb:main Feb 11, 2025
35 checks passed
@Mytherin
Copy link
Collaborator

Thanks!

Antonov548 added a commit to Antonov548/duckdb-r that referenced this pull request Feb 27, 2025
Fix duckdb/duckdb#16163: COLUMNS should not treat identifiers as strings (duckdb/duckdb#16193)
Only delete test directory when `--test-temp-dir` is not specified (duckdb/duckdb#16192)
unified names for duckdb-extensions (duckdb/duckdb#16179)
krlmlr pushed a commit to duckdb/duckdb-r that referenced this pull request Mar 5, 2025
Fix duckdb/duckdb#16163: COLUMNS should not treat identifiers as strings (duckdb/duckdb#16193)
Only delete test directory when `--test-temp-dir` is not specified (duckdb/duckdb#16192)
unified names for duckdb-extensions (duckdb/duckdb#16179)
krlmlr added a commit to duckdb/duckdb-r that referenced this pull request May 15, 2025
Fix duckdb/duckdb#16163: COLUMNS should not treat identifiers as strings (duckdb/duckdb#16193)
Only delete test directory when `--test-temp-dir` is not specified (duckdb/duckdb#16192)
unified names for duckdb-extensions (duckdb/duckdb#16179)
krlmlr added a commit to duckdb/duckdb-r that referenced this pull request May 15, 2025
Fix duckdb/duckdb#16163: COLUMNS should not treat identifiers as strings (duckdb/duckdb#16193)
Only delete test directory when `--test-temp-dir` is not specified (duckdb/duckdb#16192)
unified names for duckdb-extensions (duckdb/duckdb#16179)
krlmlr added a commit to duckdb/duckdb-r that referenced this pull request May 17, 2025
Fix duckdb/duckdb#16163: COLUMNS should not treat identifiers as strings (duckdb/duckdb#16193)
Only delete test directory when `--test-temp-dir` is not specified (duckdb/duckdb#16192)
unified names for duckdb-extensions (duckdb/duckdb#16179)
krlmlr added a commit to duckdb/duckdb-r that referenced this pull request May 18, 2025
Fix duckdb/duckdb#16163: COLUMNS should not treat identifiers as strings (duckdb/duckdb#16193)
Only delete test directory when `--test-temp-dir` is not specified (duckdb/duckdb#16192)
unified names for duckdb-extensions (duckdb/duckdb#16179)
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