dannycjones commented on code in PR #3493: URL: https://github.com/apache/iceberg-rust/pull/3493#discussion_r4367064070
########## bindings/python/LICENSING.md: ########## @@ -0,0 +1,72 @@ +<!-- + ~ Licensed to the Apache Software Foundation (ASF) under one + ~ or more contributor license agreements. See the NOTICE file + ~ distributed with this work for additional information + ~ regarding copyright ownership. The ASF licenses this file + ~ to you under the Apache License, Version 2.0 (the + ~ "License"); you may not use this file except in compliance + ~ with the License. You may obtain a copy of the License at + ~ + ~ http://www.apache.org/licenses/LICENSE-2.0 + ~ + ~ Unless required by applicable law or agreed to in writing, + ~ software distributed under the License is distributed on an + ~ "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + ~ KIND, either express or implied. See the License for the + ~ specific language governing permissions and limitations + ~ under the License. +--> + +# LICENSE and NOTICE for pyiceberg-core + +The `pyiceberg-core` wheels are a convenience artifact. We build them and upload them to PyPI as part of every release. Review Comment: Does the idea of a 'convenience artifact' come from anywhere in particular? I see the artifacts as quite critical for any downstream user of PyIceberg, as we should not push the complexity of building on Rust on to them. (not blocking anything) ########## bindings/python/third-party-notices.txt: ########## Review Comment: I think this is OK. Doesn't match byte for byte but conveys the original notices intentions. Bits are dropped related to where to find Apache 2.0 license in the source of those crates, for example. ########## bindings/python/LICENSING.md: ########## @@ -0,0 +1,72 @@ +<!-- + ~ Licensed to the Apache Software Foundation (ASF) under one + ~ or more contributor license agreements. See the NOTICE file + ~ distributed with this work for additional information + ~ regarding copyright ownership. The ASF licenses this file + ~ to you under the Apache License, Version 2.0 (the + ~ "License"); you may not use this file except in compliance + ~ with the License. You may obtain a copy of the License at + ~ + ~ http://www.apache.org/licenses/LICENSE-2.0 + ~ + ~ Unless required by applicable law or agreed to in writing, + ~ software distributed under the License is distributed on an + ~ "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + ~ KIND, either express or implied. See the License for the + ~ specific language governing permissions and limitations + ~ under the License. +--> + +# LICENSE and NOTICE for pyiceberg-core + +The `pyiceberg-core` wheels are a convenience artifact. We build them and upload them to PyPI as part of every release. + +The Rust crates that `pyiceberg-core` depends on are statically linked into each wheel. So the `LICENSE` and `NOTICE` inside a wheel must cover those crates as well as iceberg-rust itself. + +The sdist only contains iceberg-rust's own source, so it ships the root `LICENSE` and `NOTICE` unchanged. + +## How it works + +`LICENSE` and `NOTICE` are generated at build time, before `maturin` builds and packages each wheel. + +The [`generate-python-license`](../../.github/actions/generate-python-license/action.yml) action regenerates the wheel's `LICENSE` and `NOTICE` in `bindings/python/`: + +- `LICENSE`: the root `LICENSE`, then the [`cargo-about`](https://github.com/EmbarkStudios/cargo-about) output, then the manual entries in `third-party-licenses.txt`. +- `NOTICE`: the root `NOTICE`, then the manual entries in `third-party-notices.txt`. + +The action is used in these workflows: + +- [`release_python.yml`](../../.github/workflows/release_python.yml): the release wheels published to PyPI. +- [`release_python_nightly.yml`](../../.github/workflows/release_python_nightly.yml): the nightly wheels published to TestPyPI. +- [`bindings_python_ci.yml`](../../.github/workflows/bindings_python_ci.yml): pull requests. It only checks that generation doesn't fail. The generated files are not published. + +Note that in the repository, `bindings/python/LICENSE` and `NOTICE` are symlinks to the root files, so builds outside the release workflows ship the plain root files. + +## Relevant files + +- `about.hbs`: the `cargo-about` output template. Keep it a plain template. +- `about.toml`: configures `cargo-about`. + - `accepted`: the licenses we bundle, matching `deny.toml`. Add new ones at the end, as the order is the preference when a crate offers a choice. + - `targets`: the wheel targets. Keep in sync with the build matrices in `release_python.yml` and `release_python_nightly.yml`. + - Clarifications, for crates whose license files `cargo-about` can't read (such as `libm`). They pin the file by checksum (`shasum -a 256 <file>`). See the [`cargo-about` docs](https://embarkstudios.github.io/cargo-about/cli/clarify.html). +- `third-party-licenses.txt`: otherwise, add the license text here. For example, `brotli-decompressor`'s MIT license, which the crate doesn't ship. It names the crate and version, so re-check it when the crate is upgraded. +- `third-party-notices.txt`: the `NOTICE` entries of dependencies that ship one (Arrow, Avro, moka). Find them from `bindings/python` (the repository's own crates appear too): + + ```shell + cargo about generate -c about.toml --locked --fail --manifest-path Cargo.toml --format json \ + | jq -r '.crates[].package.manifest_path' | xargs -n1 dirname \ + | xargs -I{} find {} -maxdepth 1 -iname 'NOTICE*' + ``` + +## Keeping entries up to date + +Pull request CI runs the action, so it fails when generation fails, for example on a license that isn't in `accepted`. + +Everything else needs a manual check and update before every release: the clarifications and `targets` in `about.toml`, `third-party-licenses.txt` and `third-party-notices.txt`. They don't change with the dependencies. Review Comment: We need an entry in the release guide for this. ########## bindings/python/LICENSING.md: ########## Review Comment: We do need to keep this this, so we can understand how to maintain these. ########## bindings/python/LICENSING.md: ########## @@ -0,0 +1,72 @@ +<!-- + ~ Licensed to the Apache Software Foundation (ASF) under one + ~ or more contributor license agreements. See the NOTICE file + ~ distributed with this work for additional information + ~ regarding copyright ownership. The ASF licenses this file + ~ to you under the Apache License, Version 2.0 (the + ~ "License"); you may not use this file except in compliance + ~ with the License. You may obtain a copy of the License at + ~ + ~ http://www.apache.org/licenses/LICENSE-2.0 + ~ + ~ Unless required by applicable law or agreed to in writing, + ~ software distributed under the License is distributed on an + ~ "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + ~ KIND, either express or implied. See the License for the + ~ specific language governing permissions and limitations + ~ under the License. +--> + +# LICENSE and NOTICE for pyiceberg-core + +The `pyiceberg-core` wheels are a convenience artifact. We build them and upload them to PyPI as part of every release. + +The Rust crates that `pyiceberg-core` depends on are statically linked into each wheel. So the `LICENSE` and `NOTICE` inside a wheel must cover those crates as well as iceberg-rust itself. + +The sdist only contains iceberg-rust's own source, so it ships the root `LICENSE` and `NOTICE` unchanged. + +## How it works + +`LICENSE` and `NOTICE` are generated at build time, before `maturin` builds and packages each wheel. + +The [`generate-python-license`](../../.github/actions/generate-python-license/action.yml) action regenerates the wheel's `LICENSE` and `NOTICE` in `bindings/python/`: + +- `LICENSE`: the root `LICENSE`, then the [`cargo-about`](https://github.com/EmbarkStudios/cargo-about) output, then the manual entries in `third-party-licenses.txt`. +- `NOTICE`: the root `NOTICE`, then the manual entries in `third-party-notices.txt`. + +The action is used in these workflows: + +- [`release_python.yml`](../../.github/workflows/release_python.yml): the release wheels published to PyPI. +- [`release_python_nightly.yml`](../../.github/workflows/release_python_nightly.yml): the nightly wheels published to TestPyPI. +- [`bindings_python_ci.yml`](../../.github/workflows/bindings_python_ci.yml): pull requests. It only checks that generation doesn't fail. The generated files are not published. Review Comment: nitpicky: we can just drop this, it is easy to find and this is easy to grow stale. ########## .github/actions/generate-python-license/action.yml: ########## @@ -0,0 +1,36 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +name: Generate wheel third-party licenses +description: 'Regenerates bindings/python/LICENSE and NOTICE with the root files plus the licenses and notices of the Rust dependencies bundled in the wheels. The sdist keeps the root files.' +runs: + using: "composite" + steps: + - name: Install cargo-about + uses: taiki-e/install-action@4cef1412cce204788f482e778a0b9187f9626a29 # v2.87.21 + with: + tool: [email protected] + + - name: Generate LICENSE and NOTICE + shell: bash + working-directory: bindings/python + run: | + cargo about generate -c about.toml --locked --fail --manifest-path Cargo.toml about.hbs > cargo-about-output.txt + # Both are symlinks to the root files, remove them so the redirects don't write through. + rm LICENSE NOTICE + cat ../../LICENSE cargo-about-output.txt third-party-licenses.txt > LICENSE + { cat ../../NOTICE; echo; cat third-party-notices.txt; } > NOTICE Review Comment: My preference on GHA is to have as much logic as we can in Python scripts, so GHA is a thin orchestration layer installing dependencies and executing individual scripts which can be reproduced locally. (I don't want to block on this, but I think this is the direction we should head.) ########## bindings/python/about.toml: ########## Review Comment: lgtm -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
