Skip to content

[py] Fix license in package metadata and include copyright notices #16114

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

Merged
merged 8 commits into from
Aug 4, 2025

Conversation

cgoldberg
Copy link
Contributor

@cgoldberg cgoldberg commented Jul 31, 2025

User description

🔗 Related Issues

#16113

💥 What does this PR do?

This PR updates Python packaging:

  • fixes license metadata in py/pyproject.toml to conform to PEP 639: https://peps.python.org/pep-0639 (the current way we define license metadata is deprecated and will no longer work after February 2026)
  • updates Bazel configuration to include copyright notice file (NOTICE) in the packages we build for distribution (wheel/sdist)

🔄 Types of changes

  • Build/Packaging

PR Type

Other


Description

  • Update Python license metadata to PEP 639 compliance

  • Include NOTICE file in distribution packages

  • Add Bazel rules for copyright notice handling

  • Create symlinks for LICENSE and NOTICE files


Diagram Walkthrough

flowchart LR
  A["License Metadata"] --> B["PEP 639 Compliance"]
  C["NOTICE File"] --> D["Distribution Package"]
  E["Bazel Rules"] --> F["Copyright Handling"]
  B --> G["Updated pyproject.toml"]
  D --> G
  F --> G
Loading

File Walkthrough

Relevant files
Configuration changes
BUILD.bazel
Add NOTICE file handling to Bazel build                                   

py/BUILD.bazel

  • Add Bazel rules for NOTICE file handling
  • Include NOTICE file in wheel and sdist packages
  • Update license string format in py_wheel rule
+21/-1   
LICENSE
Add LICENSE symlink                                                                           

py/LICENSE

  • Create symlink to root LICENSE file
+1/-0     
NOTICE
Add NOTICE symlink                                                                             

py/NOTICE

  • Create symlink to root NOTICE file
+1/-0     
pyproject.toml
Update license metadata for PEP 639                                           

py/pyproject.toml

  • Update license format to PEP 639 standard
  • Add license-files array with LICENSE and NOTICE
  • Include NOTICE in package data
+4/-2     

@selenium-ci selenium-ci added C-py Python Bindings B-build Includes scripting, bazel and CI integrations labels Jul 31, 2025
Copy link
Contributor

qodo-merge-pro bot commented Jul 31, 2025

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Copy link
Contributor

qodo-merge-pro bot commented Jul 31, 2025

PR Code Suggestions ✨

Latest suggestions up to fdcddcd

CategorySuggestion                                                                                                                                    Impact
Incremental [*]
Include NOTICE in license-files array

Consider adding the NOTICE file to the license-files array as well, since it
contains copyright notices that should be distributed with the package metadata

py/pyproject.toml [71-72]

+"LICENSE",
+"NOTICE",
 
-
  • Apply / Chat
Suggestion importance[1-10]: 7

__

Why: The suggestion correctly points out that the NOTICE file should be added to the license-files key in pyproject.toml for packaging correctness, which was missed in the PR.

Medium
  • Update

Previous suggestions

✅ Suggestions up to commit 1eac865
CategorySuggestion                                                                                                                                    Impact
Possible issue
Fix missing comma in array
Suggestion Impact:The suggestion was directly implemented - a comma was added after "LICENSE" and also after "NOTICE" to properly format the TOML array

code diff:

-    "LICENSE"
-    "NOTICE"
+    "LICENSE",
+    "NOTICE",

Add a missing comma after "LICENSE" to properly separate the list items. Without
the comma, this creates a syntax error in the TOML array.

py/pyproject.toml [71-72]

-"LICENSE"
+"LICENSE",
 "NOTICE"

[Suggestion processed]

Suggestion importance[1-10]: 9

__

Why: The suggestion correctly identifies a missing comma in the TOML array, which is a syntax error that would cause the package build to fail.

High

@cgoldberg cgoldberg marked this pull request as draft August 1, 2025 18:19
@cgoldberg cgoldberg marked this pull request as ready for review August 4, 2025 12:14
Copy link
Contributor

qodo-merge-pro bot commented Aug 4, 2025

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

License Format

The license field format change from object to string should be validated to ensure it properly conforms to PEP 639 and works with all packaging tools

license = "Apache-2.0"
license-files = ["LICENSE", "NOTICE"]
File Dependencies

The new NOTICE file handling assumes the NOTICE file exists in the root license target. Should verify this dependency is correctly resolved

select_file(
    name = "global-notice",
    srcs = "//:license",
    subpath = "NOTICE",
)

Copy link
Contributor

qodo-merge-pro bot commented Aug 4, 2025

PR Code Suggestions ✨

No code suggestions found for the PR.

@cgoldberg
Copy link
Contributor Author

this seems to work.. I'm merging it, and will validate the package published to TestPyPI tomorrow.

@cgoldberg cgoldberg merged commit 2a74c64 into SeleniumHQ:trunk Aug 4, 2025
15 of 16 checks passed
@cgoldberg cgoldberg deleted the py-package-licensing branch August 4, 2025 12:16
@cgoldberg
Copy link
Contributor Author

packages looks good on TestPyPI and now include NOTICE file.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
B-build Includes scripting, bazel and CI integrations C-py Python Bindings Review effort 2/5
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants