Skip to content

Conversation

@sumanth-manchala
Copy link
Contributor

@sumanth-manchala sumanth-manchala commented Sep 6, 2025

Which issue does this PR close?

  • None.

What changes are included in this PR?

This adds LICENSE and NOTICE files to the PyPI sdist which is needed for distributing conda package. Also add those files to the wheel distribution

Are these changes tested?

Copy link
Contributor

@kevinjqliu kevinjqliu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i see in https://pypi.org/project/pyiceberg-core/ the metadata already mentions

License: Apache Software License

am i missing something?

@sumanth-manchala
Copy link
Contributor Author

i see in https://pypi.org/project/pyiceberg-core/ the metadata already mentions

License: Apache Software License

am i missing something?

Conda forge recommends LICENSE files to be available as part of sdist for re-distribution as conda packages

Copy link
Contributor

@kevinjqliu kevinjqliu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great catch! Having the LICENSE and NOTICE in the artifacts is a requirement for every ASF release, lets fix this asap
https://www.apache.org/legal/release-policy.html#licensing-documentation

Could you also include the NOTICE file in this change?

@kevinjqliu
Copy link
Contributor

cc @Fokko @liurenjie1024 @Xuanwo
we dont have LICENSE and NOTICE file included in past pyiceberg-core releases

@kevinjqliu
Copy link
Contributor

kevinjqliu commented Sep 7, 2025

Quick command check for LICENSE and NOTICE file when building sdist and wheel using main branch.

sdist

maturin sdist -o dist        
tar -tzf dist/pyiceberg_core-*.tar.gz | grep -E '/(LICENSE|NOTICE)$'
pyiceberg_core-0.6.0/crates/integrations/datafusion/LICENSE
pyiceberg_core-0.6.0/crates/integrations/datafusion/NOTICE
pyiceberg_core-0.6.0/crates/iceberg/LICENSE
pyiceberg_core-0.6.0/crates/iceberg/NOTICE

LICENSE and NOTICE should be in top-level here.

wheel

maturin build --release -o dist
tar -tzf dist/pyiceberg_core-*.whl | grep -E '/(LICENSE|NOTICE)$'

No LICENSE and NOTICE at all

@sumanth-manchala
Copy link
Contributor Author

sumanth-manchala commented Sep 7, 2025

Great catch! Having the LICENSE and NOTICE in the artifacts is a requirement for every ASF release, lets fix this asap https://www.apache.org/legal/release-policy.html#licensing-documentation

Could you also include the NOTICE file in this change?

Done and verified the same in both sdit and whl!

image

kevinjqliu
kevinjqliu previously approved these changes Sep 7, 2025
Copy link
Contributor

@kevinjqliu kevinjqliu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

I verified locally for both sdist and wheel

maturin sdist -o dist        
tar -tzf dist/pyiceberg_core-*.tar.gz | grep -E '/(LICENSE|NOTICE)$'

pyiceberg_core-0.6.0/crates/iceberg/LICENSE
pyiceberg_core-0.6.0/crates/iceberg/NOTICE
pyiceberg_core-0.6.0/crates/integrations/datafusion/LICENSE
pyiceberg_core-0.6.0/crates/integrations/datafusion/NOTICE
pyiceberg_core-0.6.0/bindings/python/LICENSE
pyiceberg_core-0.6.0/bindings/python/NOTICE
pyiceberg_core-0.6.0/LICENSE
pyiceberg_core-0.6.0/NOTICE

maturin build --release -o dist
tar -tzf dist/pyiceberg_core-*.whl | grep -E '/(LICENSE|NOTICE)$'

pyiceberg_core-0.6.0.dist-info/licenses/LICENSE
pyiceberg_core-0.6.0.dist-info/licenses/NOTICE

Pasting the ASF policy,

For source packages, LICENSE and NOTICE MUST be located at the root of the distribution. For additional packages, they MUST be located in the distribution format's customary location for licensing materials, such as the META-INF directory of Java "jar" files.

source package is located at the root.
wheel package is in the licenses/ folder

@kevinjqliu
Copy link
Contributor

CI issue should be resolved by #1654

@kevinjqliu kevinjqliu changed the title Add license for pyiceberg-core Add LICENSE and NOTICE files for pyiceberg-core source distribution and wheel Sep 7, 2025
@sumanth-manchala
Copy link
Contributor Author

LGTM!

I verified locally for both sdist and wheel

maturin sdist -o dist        
tar -tzf dist/pyiceberg_core-*.tar.gz | grep -E '/(LICENSE|NOTICE)$'

pyiceberg_core-0.6.0/crates/iceberg/LICENSE
pyiceberg_core-0.6.0/crates/iceberg/NOTICE
pyiceberg_core-0.6.0/crates/integrations/datafusion/LICENSE
pyiceberg_core-0.6.0/crates/integrations/datafusion/NOTICE
pyiceberg_core-0.6.0/bindings/python/LICENSE
pyiceberg_core-0.6.0/bindings/python/NOTICE
pyiceberg_core-0.6.0/LICENSE
pyiceberg_core-0.6.0/NOTICE

maturin build --release -o dist
tar -tzf dist/pyiceberg_core-*.whl | grep -E '/(LICENSE|NOTICE)$'

pyiceberg_core-0.6.0.dist-info/licenses/LICENSE
pyiceberg_core-0.6.0.dist-info/licenses/NOTICE

Pasting the ASF policy,

For source packages, LICENSE and NOTICE MUST be located at the root of the distribution. For additional packages, they MUST be located in the distribution format's customary location for licensing materials, such as the META-INF directory of Java "jar" files.

source package is located at the root.
wheel package is in the licenses/ folder

Thanks @kevinjqliu !!

liurenjie1024
liurenjie1024 previously approved these changes Sep 8, 2025
Copy link
Contributor

@liurenjie1024 liurenjie1024 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @sumanth-manchala for adding this!

@kevinjqliu kevinjqliu merged commit b67caf2 into apache:main Sep 8, 2025
17 checks passed
@kevinjqliu
Copy link
Contributor

Thanks again @sumanth-manchala and thanks for the review @liurenjie1024

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.

3 participants