Skip to content

[rust] Add license and notice links to Selenium Manager help - #17986

Merged
diemol merged 7 commits into
SeleniumHQ:trunkfrom
shubmittal:fix-selenium-manager-legal-info-16113
Sep 9, 2026
Merged

[rust] Add license and notice links to Selenium Manager help#17986
diemol merged 7 commits into
SeleniumHQ:trunkfrom
shubmittal:fix-selenium-manager-legal-info-16113

Conversation

@shubmittal

@shubmittal shubmittal commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

🔗 Related Issues

Related to #16113

💥 What does this PR do?

Embeds the full LICENSE and NOTICE texts in Selenium Manager and adds a --license flag to print them without network access.

🔧 Implementation Notes

Uses package-local LICENSE and NOTICE copies with Rust's include_str! and Bazel compile_data, so Cargo archives and each binary carry the legal texts. A repository-only test keeps the package copies synchronized with the canonical root files, and a focused regression test verifies the complete output.

🤖 AI assistance

  • No substantial AI assistance used
  • AI assisted (complete below)
    • Tool(s): GitHub Copilot CLI
    • What was generated: Embedded legal-output implementation, regression test, validation guidance, and PR text.
    • I reviewed all AI output and can explain the change

💡 Additional Considerations

This covers Selenium Manager only. Other bindings and components remain tracked by #16113.

🔄 Types of changes

  • New feature (non-breaking change which adds functionality and tests)

@qodo-code-review

Copy link
Copy Markdown
Contributor

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more →

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account →

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us →

@CLAassistant

CLAassistant commented Sep 6, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@selenium-ci selenium-ci added C-rust Rust code is mostly Selenium Manager B-manager Selenium Manager labels Sep 6, 2026
Expose the Selenium LICENSE and NOTICE URLs in --help and cover the output with a focused regression test.

Related to SeleniumHQ#16113
@shubmittal
shubmittal force-pushed the fix-selenium-manager-legal-info-16113 branch from c9e5a0e to 8d573c2 Compare September 6, 2026 04:36

@diemol diemol left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We'd prefer a different approach here: embed LICENSE/NOTICE into the binary via include_str! + compile_data, and add a --license flag to print them in full. A link to
blob/trunk/LICENSE needs network access and isn't pinned to this binary's version, so it doesn't fully satisfy #16113.

Add --license to print the full embedded LICENSE and NOTICE texts without requiring network access.

Related to SeleniumHQ#16113
@shubmittal

Copy link
Copy Markdown
Contributor Author

We'd prefer a different approach here: embed LICENSE/NOTICE into the binary via include_str! + compile_data, and add a --license flag to print them in full.

Updated to embed both files at build time with include_str! and Bazel compile_data. The new --license flag prints the complete LICENSE and NOTICE contents offline, with exact-output regression coverage.

@qodo-code-review

qodo-code-review Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📜 Skill insights (0)

Grey Divider


Remediation recommended

1. Packaged manager builds fail ✓ Resolved 🐞 Bug ≡ Correctness
Description
include_str! resolves both legal files two directories above the Cargo package root, so those
inputs are absent from the packaged crate. Running cargo package performs its verification build
from that isolated crate, and the manager binary then fails to compile before it can be published or
installed from the archive.
Code

rust/src/main.rs[R42-43]

+const LICENSE: &str = include_str!("../../LICENSE");
+const NOTICE: &str = include_str!("../../NOTICE");
Evidence
The new source paths traverse above rust, while rust/Cargo.toml defines the package and has no
package-local copies or packaging configuration for these inputs. The canonical files belong to the
repository-root Bazel filegroup, and Cargo documents that cargo package verifies the package by
building it from a pristine packaged source tree.

rust/src/main.rs[42-43]
rust/Cargo.toml[2-12]
BUILD.bazel[17-24]
🌐 Cargo documents that packaging selects files for the package and verifies that package by building it from a pristine state.

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The binary embeds files outside the `rust` Cargo package. Cargo's package verification builds from an isolated archive that does not contain those repository-root paths, causing compilation to fail.

## Issue Context
Make the embedded LICENSE and NOTICE available within the packaged crate while retaining Bazel's declared compile-time inputs. Prefer a package-local source that remains synchronized with the canonical repository files.

## Fix Focus Areas
- rust/src/main.rs[42-43]
- rust/Cargo.toml[2-12]
- rust/BUILD.bazel[83-90]
- rust/tests/output_tests.rs[25-34]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Context sources
Review mode: ⚖️ Balanced: This push changes runtime legal-file embedding and CLI output plus Bazel/Cargo packaging and CI configuration, creating real cross-build and behavioral risk but not enough independent logic for extended review.

Grey Divider

Tip of the day
💡 Did you know, you can commit Qodo's fix in one click with committable suggestions (GitHub & GitLab)

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment thread rust/src/main.rs Outdated
Include package-local LICENSE and NOTICE copies for Cargo archives and
verify they stay synchronized with the canonical repository files.

Related to SeleniumHQ#16113
@qodo-code-review

Copy link
Copy Markdown
Contributor

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more →

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account →

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us →

@qodo-code-review

Copy link
Copy Markdown
Contributor

Code review by qodo was updated up to the latest commit 0dfd73c

@qodo-code-review

qodo-code-review Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

No code changes since the last review — review skipped

Qodo Logo

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

B-manager Selenium Manager C-rust Rust code is mostly Selenium Manager

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants