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

Handle SPDX licences starting with a digit #10356

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

ffaf1
Copy link
Collaborator

@ffaf1 ffaf1 commented Sep 14, 2024

cabal has a devscript that parses a json file (containing SPDX licences) and returns a number of of Haskell data constructors.

There have been problems when the SPDX short identifier starts with a digit (e.g. “0BSD”), as that would generate an data constructor starting with a digit, which is not valid Haskell. The way to handle such occourrences was in an ad-hoc basis.

This patch prepends N_ to the beginning of every SPDX licence starting with a digit, future-proofing the script.


  • Patches conform to the coding conventions.
  • Any changes that could be relevant to users have been recorded in the changelog.
  • The documentation has been updated, if necessary.
  • n/a Manual QA notes have been included.
  • I don't think this can have meaningful tests Tests have been added. (Ask for help if you don’t know how to write them! Ask for an exemption if tests are too complex for too little coverage!)

This has not been done in #10341 not to incour in bugs or problems during release.

The patch is very simple but not trivial (we are retroactively changing some data constructors). Voice any concerns!

This should not be merged before 3.14 is released.

This should not need backbort. It needs a backport (specificaly: the changelog files) since it will case breaking changes to a datatype. No need for backport, the changelog files will just be picked normally during next release.

@ffaf1 ffaf1 added the re: devx Improving the cabal developer experience (internal issue) label Sep 14, 2024
@ffaf1 ffaf1 added the attention: needs-backport in the future e.g., to a point release after the main release label Sep 14, 2024
Copy link
Collaborator

@ulysses4ever ulysses4ever left a comment

Choose a reason for hiding this comment

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

Does NN have any precedent? I'm surprised it's not just N, for example...

@ffaf1
Copy link
Collaborator Author

ffaf1 commented Sep 14, 2024

Does NN have any precedent? I'm surprised it's not just N, for example...

Any string that is short/recognizeable enough and does not clash is ok. I will change it to N_.

@ffaf1 ffaf1 added squash+merge me Tell Mergify Bot to squash-merge and removed attention: needs-backport in the future e.g., to a point release after the main release labels Sep 14, 2024
@mergify mergify bot added the merge delay passed Applied (usually by Mergify) when PR approved and received no updates for 2 days label Sep 18, 2024
cabal has a devscript that parses a json file (containing SPDX licences)
and returns a number of of Haskell data constructors.

There have been problems when the SPDX short identifier starts with
a digit (e.g. “0BSD”), as that would generate an data constructor
starting with a digit, which is not valid Haskell.
The way to handle such occourrences was in an ad-hoc basis.

This patch prepends “N_” to the beginning of every SPDX licence starting
with a digit, future-proofing the script.
Useless since we are going to have to do it again before a release,
needed to make CI green (“Check that diff is clean”).
@ffaf1
Copy link
Collaborator Author

ffaf1 commented Sep 29, 2024

@Kleidukos we decided to postpone this after the release.

What is your opinion now? I remember you had some doubts about this having to involve the parser, but I forgot the specifics.

@Kleidukos
Copy link
Member

My opinion was "let's not rush the modification of the SPDX generator, because it will fail us when we need to use it for lib:Cabal 3.14". Now the modification window is open and will close before 3.16 when we will have more urgent problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merge delay passed Applied (usually by Mergify) when PR approved and received no updates for 2 days re: devx Improving the cabal developer experience (internal issue) squash+merge me Tell Mergify Bot to squash-merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants