Skip to content

Add libs folder to bundled static libs #16655

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Mar 17, 2025
Merged

Add libs folder to bundled static libs #16655

merged 1 commit into from
Mar 17, 2025
+30 −21

Conversation

taniabogatsch
Copy link
Contributor

For some OS + architecture combinations, the bundled static libraries exceed 100MB, which can be inconvenient.

This PR adds a gather-libs target that collects the individual libraries (pre-bundling) and puts them in a libs folder. When publishing the existing artifact, it also zips that additional libs folder.

Follow-up to #16654.

@carlopi
Copy link
Contributor

carlopi commented Mar 14, 2025

I am not sure this is correct, the bundle-setup is still needed I think, probably as Makefile dependency.

@taniabogatsch
Copy link
Contributor Author

@carlopi, we must keep the bundle-library target in the Makefile, as some older builds use that. But the new gather-libs target works without any bundling; it just gathers all the available libs after make release, and that's all it needs.

https://github.com/duckdb/duckdb/actions/runs/13857853364?pr=16655, the artifacts look good, no?

carlopi
carlopi approved these changes Mar 17, 2025
View reviewed changes
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.

Looks right, thanks!!

Note that this changes the artifacts from static-lib-* to static-libs-*, but I do agree this is better given the artifact are differently packaged and it allow to more easily spot places to update (we are aware of this being used only in go client).

I think this is OK (and better) in v1.2-histrionicus, green from me.

@Mytherin Mytherin merged commit c8dcf26 into duckdb:v1.2-histrionicus Mar 17, 2025
55 checks passed
@Mytherin
Copy link
Collaborator

Thanks!

@taniabogatsch taniabogatsch deleted the static-libs branch March 17, 2025 13:44
krlmlr added a commit to duckdb/duckdb-r that referenced this pull request Apr 8, 2025
@krlmlr
vendor: Update vendored sources to duckdb/duckdb@d76de45
7bb3ec0 
Avoid UMA (duckdb/duckdb#16641)
Add libs folder to bundled static libs (duckdb/duckdb#16655)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
1 more reviewer

@carlopi carlopi carlopi approved these changes

Reviewers whose approvals may not affect merge requirements
Assignees
No one assigned
Labels
Ready For Review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@taniabogatsch @carlopi @Mytherin