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

bottle: fix adding bottle block to formulae with special license #9081

Merged
merged 1 commit into from Nov 9, 2020

Conversation

SeekingMeaning
Copy link
Contributor

@SeekingMeaning SeekingMeaning commented Nov 8, 2020

  • Have you followed the guidelines in our Contributing document?
  • Have you checked to ensure there aren't other open Pull Requests for the same change?
  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes? Here's an example.
  • Have you successfully run brew style with your changes locally?
  • Have you successfully run brew tests with your changes locally?
  • Have you successfully run brew man locally and committed any changes?

This adds support for license symbols (e.g.:cannot_represent) and hashes (e.g. "MIT" => { ... }) to the brew bottle command

  1. [^\[]+?\[[^\]]+?\] matches any_of: ["MIT", "0BSD"]
  2. [^{]+?{[^}]+?} matches "MIT" => { with: "LLVM-exception" }
  3. :\S+ matches :public_domain

Related:

@fxcoudert
Copy link
Member

How about license any_of: and license all_of:? I'm not sure from the regexp that these are handled.

I am also seeing some other weird things in some homebrew-core formulas:

  • license = "MIT" in nim: is that legal?
  • many formulas have arrays, without any_of or all_of, commented out like this in libical:
# license ["LGPL-2.1", "MPL-2.0"] - pending https://github.com/Homebrew/brew/pull/7953

@SeekingMeaning
Copy link
Contributor Author

SeekingMeaning commented Nov 8, 2020

How about license any_of: and license all_of:? I'm not sure from the regexp that these are handled.

They are; [^\[]+? matches any_of: and all_of: (this includes trailing spaces after the colon)

Although, expressions such as "MIT" => { with: "LLVM-exception" } don't appear to be supported

  • license = "MIT" in nim: is that legal?

I don't think so

  • many formulas have arrays, without any_of or all_of, commented out like this in libical

Those need to be updated (Homebrew/homebrew-core#58225 (comment))

@SeekingMeaning SeekingMeaning changed the title bottle: fix adding bottle block to formulae with license symbol bottle: fix adding bottle block to formulae with special license Nov 8, 2020
@Rylan12
Copy link
Member

Rylan12 commented Nov 8, 2020

I don't think this handles multiline licenses:

  license all_of: [
    :public_domain,
    "MIT",
    "GPL-3.0-or-later" => { with: "Autoconf-exception-3.0" },
  ]

@SeekingMeaning
Copy link
Contributor Author

SeekingMeaning commented Nov 8, 2020

I thought the same too but it turns out that the regex is actually multiline: https://github.com/Homebrew/brew/blob/50ce5ef/Library/Homebrew/dev-cmd/bottle.rb#L548

@MikeMcQuaid MikeMcQuaid merged commit 29160b9 into Homebrew:master Nov 9, 2020
@MikeMcQuaid
Copy link
Member

Thanks @SeekingMeaning! This will really need to be rewritten to use a proper Ruby parser at some point to stop it 💥ing so often.

@BrewTestBot BrewTestBot added the outdated PR was locked due to age label Dec 11, 2020
@Homebrew Homebrew locked as resolved and limited conversation to collaborators Dec 11, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
outdated PR was locked due to age
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants