Skip to content

Conversation

akien-mga
Copy link
Contributor

Rebase of @ondys' #228 with one missed include changed too.
Supersedes #228. (This PR can be closed if #228 is updated with the transcoder change.)

We have the same need in the Godot project, also with a non-CMake build (relevant build file for SCons: https://github.com/godotengine/godot/blob/master/modules/basis_universal/SCsub).

CMake users may also want to have an easy way to use their own zstd implementation, but for this PR I followed @ondys' steps to do the minimal changes needed by existing downstream projects which shouldn't break the workflow for the main app.

@past-due
Copy link
Contributor

I would also like to see this (or something similar to this) merged. @richgel999

@alexmyczko
Copy link

alexmyczko commented Jun 17, 2023

so do I, for the Debian package!

for the impatient: http://sid.ethz.ch/debian/basis-universal/

@past-due
Copy link
Contributor

@akien-mga Looks like the latest commits to master have yielded some small conflicts, but hopefully @richgel999 might be willing to take a look at this once resolved?

@akien-mga
Copy link
Contributor Author

Rebased on latest master.

…ns (mostly in non CMake builds).

In our internal repository, we have our own version of zstdlib and introducing extra copy is both undesirable and potentially dangerous (due to ODR violations).

Co-authored-by: Rémi Verschelde <[email protected]>
@akien-mga
Copy link
Contributor Author

Rebased to get AppVeyor CI to pass, the previous commit failed due to a random connectivity error.

@past-due
Copy link
Contributor

@richgel999 Just wanted to bump this - would be great to be able to get something like this merged!

@Ono-Sendai
Copy link

I'd like to see this merged too, I have to make the same changes for my build.

@past-due
Copy link
Contributor

@richgel999 Just a gentle reminder of this PR, and a kind request to please consider merging it.

(Wanting to use zstd as well as basis_universal in the same project is an ever-more-common case.)

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.

5 participants