Skip to content

Add simple cli to validate expressions#136

Closed
ahpook wants to merge 3 commits intogithub:mainfrom
ahpook:ahpook/cli
Closed

Add simple cli to validate expressions#136
ahpook wants to merge 3 commits intogithub:mainfrom
ahpook:ahpook/cli

Conversation

@ahpook
Copy link

@ahpook ahpook commented Mar 4, 2026

This adds a simple cli to the package called spdx-validate. It takes a string or an input file containing spdx expressions and runs the ValidateLicenses function on them, presenting errors if they fail validation.

(Disclosure: Coded with Claude Opus 4.6)

This adds a simple cli to the package called `spdx-validate`.  It takes a string or an input file containing spdx expressions and runs the `ValidateLicenses` function on them, presenting errors if they fail validation.

(Disclosure: Coded with Claude Opus 4.6)
@ahpook ahpook requested a review from elrayle as a code owner March 4, 2026 18:58
Copilot AI review requested due to automatic review settings March 4, 2026 18:58
@ahpook ahpook requested a review from dangoor as a code owner March 4, 2026 18:58
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds a new spdx-validate command-line tool to validate SPDX license expressions using the existing spdxexp.ValidateLicenses API, with README documentation and accompanying tests.

Changes:

  • Introduce cmd/spdx-validate Cobra-based CLI supporting stdin (single expression) and --file (newline-separated expressions).
  • Add unit/integration-style tests for CLI validation helpers.
  • Update module dependencies and README with build/usage instructions.

Reviewed changes

Copilot reviewed 4 out of 5 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
go.mod Adds Cobra (and indirect deps) needed for the new CLI.
go.sum Records new dependency checksums.
cmd/spdx-validate/main.go Implements the spdx-validate CLI and validation helpers.
cmd/spdx-validate/main_test.go Adds tests for the validation helper functions.
README.md Documents the new CLI build and usage.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +67 to +68
scanner := bufio.NewScanner(r)
if !scanner.Scan() {
Copy link

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

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

validateSingleExpression treats any scanner.Scan() failure as "no input provided" without checking scanner.Err(). This can hide real read errors (including bufio.ErrTooLong when the line exceeds the scanner token limit). Check scanner.Err() when Scan() returns false and return a wrapped read error when present; consider increasing the scanner buffer if long lines are plausible.

Suggested change
scanner := bufio.NewScanner(r)
if !scanner.Scan() {
scanner := bufio.NewScanner(r)
// Increase the scanner buffer to better handle long expressions.
scanner.Buffer(make([]byte, 0, 64*1024), 1024*1024)
if !scanner.Scan() {
if err := scanner.Err(); err != nil {
return false, fmt.Errorf("failed to read input: %w", err)
}

Copilot uses AI. Check for mistakes.
Copy link
Author

Choose a reason for hiding this comment

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

there's not going to be a 64mb spdx expression.

Copy link
Contributor

@dangoor dangoor left a comment

Choose a reason for hiding this comment

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

Copilot has some good suggestions here, but I'm not concerned about them if this is going to stay just a tool for spot checking things.

The previous split code path was left over from my initial attempts. now stdin is treated like a newline-separated file rather than being special-cased, so the tool can accept multiple lines from a shell pipeline or a file.
@ahpook
Copy link
Author

ahpook commented Mar 6, 2026

Closing this in favour of a local branch rather than a forked one. Note that the way the repo is configured, any external contributor's PR will be blocked due to the codeql setup, as @elrayle found on #129 . Copilot says:

PR #136 is a cross-fork PR (from ahpook/go-spdxgithub/go-spdx). CodeQL's "default setup" does not automatically run on pull requests originating from forks. This is a GitHub security restriction — fork PRs have limited permissions and cannot upload SARIF (code scanning) results to the base repository.

Meanwhile, branch protection rules on main likely require the CodeQL check to pass before merging, creating a deadlock: the check is required but can never run.

How to Unblock

There are a few options (roughly ordered by ease):

  1. Admin bypass: A repository admin can merge the PR using the "Merge without waiting for requirements to be met" (admin override) button on the PR, bypassing the CodeQL requirement.

  2. Push the branch directly to github/go-spdx instead of using a fork. If you (or someone with write access) push the ahpook/cli branch to the github/go-spdx repo and open the PR from there, CodeQL default setup will trigger normally:

git remote add upstream https://github.com/github/go-spdx.git
git push upstream ahpook/cli
Then re-target the PR to use the in-repo branch as the head.
  1. A repo admin can manually trigger the CodeQL analysis for the PR's head SHA, if the dynamic workflow supports manual dispatch.

  2. Switch from CodeQL default setup to an advanced setup (a .github/workflows/codeql.yml file). Advanced setup workflows do run on fork PRs (with pull_request_target or the standard pull_request trigger), though they require careful configuration to avoid security issues.

Recommended: Option 1 (admin bypass) is the quickest fix for this specific PR. For a longer-term solution, option 2 (pushing branches directly to the repo) avoids this problem entirely for contributors with write access.

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