Repository navigation
[discussion] Reviewing current licensing and distribution files #3233
Description
Activity
Summary of some Slack discussion with @dannycjones, @xanderbailey, @kevinjqliu: https://apache-iceberg.slack.com/archives/C05HTENMJG4/p1789459352829739
@dannycjones was asking about why the
DEPENDENCIES.rust.tsvfiles exist, as they doubt if they are required under any license obligations.Three possible use cases raised:
- Provides a way for reviewers to understand license changes (albeit at release time)
- Provides a way for consumers to understand what licenses are used by the transitive dependencies of iceberg-rust
- Required under license obligations
@dannycjones recognizes that the pyiceberg-core Python package will likely need to maintain distribution of some kind of notice, as it distributes dependencies as part of a compiled wheel. Meanwhile, the crates themselves may be fine under license obligations as only source code is being distributed as part of the crates.
@kevinjqliu shared that the TSV process is likely adopted historically based on OpenDAL's release process.
We recognize that the current release process has a step to verify licenses according to a policy configured in deny.toml. @kevinjqliu suggests we move that into pull request checks (@dannycjones +1, opened here: #3234)
Related: original PR introducing the TSVs #147
Obviously, any decision here to change the notices should be reviewed by legal :) NAL
Re: whether we should keep the
DEPENDENCIES.rust.tsvfiles and what value they provide.The TSVs are meant to be regenerated during the release process so reviewers can inspect the dependency licenses. I personally don’t find the checked-in snapshots useful, especially when they’re stale. The actual license check is already handled by
cargo deny, which doesn’t read these files.I’m in favor of dropping them and running the same
cargo denycheck in PR CI (#3234), while keeping the release-time check. That way we catch dependencies that don’t meet our license policy at PR time, without requiring contributors to maintain generated files.Looks like the TSV process is inherited from OpenDAL, @Xuanwo do you have any suggestions on whether we can remove the TSV files? Am I missing anything regarding its use case?
- Reacted by Danny C Jones
Looks good! I'm open to remove the TSV files.
Reacted by Kevin LiuThanks @Xuanwo!
I think there's four things we need to do:
- Adopt the license check in CI, no brainer (Add cargo deny license checks to PRs #3234)
- Recommend a solution for the Python bindings, as that does distribute libraries and needs a notice
- Take this to the Iceberg dev@ mailing list
- Take this to the Apache legal-discuss@ mailing list
I identified I think an issue with the Python bindings that already existed, opened a report issue: #3239
Reacted by blackmwkOK, a little related to this: I'm planning to introduce a new Python script that handles the license checks (rejecting licenses we don't permit) and generating attribution and other files for the Python bindings.
This will be separate to the existing dependencies.sh - I'm going to narrow's its scope only to generating the TSVs mentioned in this issue.
I think the consensus in the community is that the TSVs aren't that useful for maintainer or consumer, so I'm effectively preparing to remove it by dropping the whole bash script, but I'll take that to the mailing list when that decision comes. If we decide instead to keep the TSVs, it can be folded into the new Python script.
The motivation for moving to the Python script is to ensure this stuff is maintainable. Drafting the necessary changes for the distribution notices felt practically unreadable in Bash.
thanks @dannycjones
lets get rid of the TSV and add
cargo denyto CI then we can follow up with the rest of the license fixesReacted by Danny C JonesSounds good. I have the change ready but it'll conflict with #3293, so that needs to go in first.
@dannycjones would you like to create a PR to remove the TSVs?
@dannycjones would you like to create a PR to remove the TSVs?
Thanks Kevin, that's up now: #3338
Thanks all, great to see this one closed. I'll get the changes up for the wheels soon.
Creating a discussion issue here, just to capture context. The purpose of this issue is not to make any major decision on the artifacts themselves, that should be taken to the mailing list. Instead, this just captures discussion on what artifacts we have and if we should pursue any changes.
Part of the motivation is that the release process is made clunky by trying to maintain these, and
mainhas no way to update these files right now.