Skip to content
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

Update SPDX Expression Parsing #719

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

Update SPDX Expression Parsing #719

wants to merge 7 commits into from

Conversation

febuiles
Copy link
Contributor

@febuiles febuiles commented Mar 21, 2024

Closes #263
Closes #670
Closes #575
Closes #635

Context

Since we introduced SPDX licenses around 2022, we've had issues dealing with SPDX expression validations due to the library we use for checking whether one expression satisfies another one.

Folks reached out to the maintainer in 2022 to fix some of these changes, but set a clear direction that does not fit our purposes anymore. The @onebeyond/spdx-license-satisfies is a fork of the original project, created by people who encountered the same issues as us.

Changes

This PR moves the Action away from spdx-satisfies.js and uses @onebeyond/spdx-license-satisfies instead to check whether an SPDX license satisfies an expression or not: The MIT license satisfies the expression MIT OR GPL-2.0, but it does not satisfy MIT AND GPL-2.0.

In the process of making these changes I:

  • Moved all the SPDX-related logic to spdx.ts.
  • Added a few TODOs to spdx.ts noting the things we still need to support.
  • Updated the tests that were using invalid SPDX identifiers.
  • Removed unnecessary stubs for SPDX license validation.
  • Updated tsconfig.json to fix a duplicate entry in the compiler options.

@febuiles febuiles self-assigned this Mar 21, 2024
@febuiles febuiles marked this pull request as ready for review March 22, 2024 13:37
@febuiles febuiles requested a review from a team as a code owner March 22, 2024 13:37
src/config.ts Outdated Show resolved Hide resolved
src/config.ts Outdated Show resolved Hide resolved
src/config.ts Outdated Show resolved Hide resolved

expect(spdx.satisfies(license, expr)).toBe(false)
})
})
Copy link
Contributor

Choose a reason for hiding this comment

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

😍

src/spdx.ts Outdated Show resolved Hide resolved
@elireisman
Copy link
Contributor

Couple questions, but this is looking good so far, thanks! 🍻

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants