-
Notifications
You must be signed in to change notification settings - Fork 6
build: only include zstd license distributing zstd #51
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
base: master
Are you sure you want to change the base?
Conversation
When zstd is dynamically linked, the package doesn't contain zstd. Therefore, we only need to include its license when zstd isn't dynamically linked.
|
Thank you for the PR! I need to think about wether I want to merge this or not. On one hand, I am unsure if it is legally required for dynamic builds or not. I mean that even with On the other hand, I feel like trying to find the truth here is a waste of energy. This is especially true as my vision for the future |
|
Thanks for your feedback!
The BSD license (used by both zstd and pyzstd) requires "attribution" when using a BSD-licensed project. You achieve this by having an acknowledgements section in your README, package descriptions, The BSD license only requires distributing the license itself when you re-distribute the actual code (binary or source). E.g. static linking, or dynamic linking but bundling the If you're dynamically linking and relying on the library being installed separately on the system, then you are not distributing the library and don't need to distribute its license.
Correct, you build against the library, but you don't include the library in the resulting package. No re-distribution = no need to distribute the license. Simple acknowledgement is fine. Copyleft licenses like the GPL usually have specific clauses about this, such that if you link against a GPL project you must use a compatible license yourself. However permissive licenses like the BSD license do not do this. |
|
If you're unsure, closing this is a valid solution. I can work-around the nixpkgs build failure in several ways, including pulling in the actual LICENSE file: ln -s ${zstd-c.src}/LICENSE zstd |
|
I tried your PR locally, and it seems that it will still fail when building with |
|
Interesting. I didn't run into that issue when I tested this PR against the nix package. In the nix package we run setuptools via pyproject, and configure pypaBuildFlags = [
"--config-setting=--global-option=--dynamic-link-zstd"
];Maybe having it configured as a "global option" is the difference? |
|
On my side it fails with each of the following commands, saying that the $ python -m pip install --config-settings=--build-option=--dynamic-link-zstd .
$ python -m build --wheel --config-setting=--build-option=--dynamic-link-zstd . |
|
The only obvious difference I can see is |
|
Oh woah I'm blind 🤦 it works with However the documentation has been pointing to Maybe I'm making our lives difficult for nothing and I should just commit the |
|
I personally don't have a strong opinion. The nixpkgs package is currently symlinking # pyzst needs a copy of upstream zstd's license
ln -s ${zstd-c.src}/LICENSE zstdI opened this PR to discuss whether the LICENSE should be skipped when dynamically linking, but if you're unsure you're welcome to close it. Committing the file may simplify things for anyone else who decides they don't want/need the zstd git-submodule, but I don't think that's critically important. I guess your preference will depend on whether you consider building without the submodule to be a supported build method. |
As discussed in #43 (comment), the upstream zstd license should only be included in the package when the package is distributing upstream zstd code.
This PR disables #43 when zstd is being dynamically linked. I.e., when the build is configured with
--dynamic-link-zstd.I've tested this PR's patches against NixOS/nixpkgs#449557 and it seems to be working.