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

Allow setting spdx_license_id in module upload #134

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

Conversation

mfridman
Copy link
Member

This PR adds spdx_license_id field to module UploadRequest. The goal is to offer users more control over the resulting spdx license identifier that gets exposed once a module has been published. Such as in Generated SDKs.

The intention is to add a --spdx-license-id flag (or --license if we wanted to keep it short) to the buf push command.

For this to be useful, we need to expand the spdx expression logic to support (in addition to the SPDX License List):

a user defined license reference denoted by the LicenseRef-[idString]

... idstring = 1*(ALPHA / DIGIT / "-" / "." )

Reference

Copy link

github-actions bot commented Oct 25, 2024

The latest Buf updates on your PR. Results from workflow Buf CI / buf (pull_request).

BuildFormatLintBreakingUpdated (UTC)
✅ passed✅ passed✅ passed✅ passedOct 31, 2024, 3:09 PM

// May be empty if the SPDX License Identifier was not explicitly set during upload, if the
// license file was not present, or if it was not possible to detect the SPDX License Identifier
// from the license file.
string spdx_license_id = 8;
Copy link
Member

Choose a reason for hiding this comment

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

should we validate these strings w/ regex?

Copy link
Member

Choose a reason for hiding this comment

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

I think so. I went through the IDs and concluded they only contain characters a-z, A-Z, 0-9, dashes, periods, plusses. I added bufbuild/spdx-go#2 to verify this whenever we update the list. This should be the regex for this.

Copy link
Member

@bufdev bufdev left a comment

Choose a reason for hiding this comment

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

I can't remember what the thinking was but I explicitly didn't want to do this for some reason when I looked into it #18

@mfridman
Copy link
Member Author

There's probably more of an argument to do add it now, especially since we allow this field to be explicitly set in PluginInfo.

@mfridman mfridman requested a review from bufdev October 30, 2024 14:07
@bufdev
Copy link
Member

bufdev commented Oct 31, 2024

Also added to v1beta1 as these need to be kept in sync. I added the validation to bufplugin as well.

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