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

bump-formula-pr: handle url with specs hash #13474

Merged

Conversation

samford
Copy link
Member

@samford samford commented Jun 24, 2022

  • 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 typecheck with your changes locally?
  • Have you successfully run brew tests with your changes locally?

As mentioned in #13471, the existing bump-formula-pr regexes expect a url string to only be followed by a newline. However, url also accepts a specs hash, which can appear after the url string. For example:

url "https://www.example.com/1.2.3.tar.gz", using: :homebrew_curl

This commit modifies the regexes to capture anything after the url string and before the newline. This works for the aforementioned example, where the trailing hash is on the same line, but it won't work if the hash appears on a subsequent line. For example:

url "https://www.example.com/1.2.3.tar.gz",
  using: :homebrew_curl

I spent some time trying to create a regex that would also work for the latter case but didn't meet with success (my brain is a bit too tired tonight). Since we don't have any instances of the multiline format in homebrew/core yet, there's something to be said for merging this PR in the short-term to fix brew bump-formula-pr and we can adjust this in a follow-up PR if we come up with a more comprehensive solution.

Fixes #13471.

The existing `bump-formula-pr` regexes expect a `url` string to only
be followed by a newline. However, `url` also accepts a `specs` hash,
which can appear after the `url` string. For example:

```
url "https://www.example.com/1.2.3.tar.gz", using: :homebrew_curl
```

This commit modifies the regexes to capture anything after the `url`
string and before the newline. This works for the aforementioned
example, where the trailing hash is on the same line, but it won't
work if the hash appears on a subsequent line. For example:

```
url "https://www.example.com/1.2.3.tar.gz",
  using: :homebrew_curl
```
@samford samford added bug Reproducible Homebrew/brew bug critical Critical change which should be shipped as soon as possible. labels Jun 24, 2022
@BrewTestBot
Copy link
Member

Review period skipped due to critical label.

@Bo98
Copy link
Member

Bo98 commented Jun 25, 2022

I spent some time trying to create a regex that would also work for the latter case but didn't meet with success (my brain is a bit too tired tonight).

The answer there would be to do proper AST parsing instead, but that's a bigger rewrite.

@Bo98 Bo98 merged commit 38e1af8 into Homebrew:master Jun 25, 2022
@github-actions github-actions bot added the outdated PR was locked due to age label Jul 26, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 26, 2022
@samford samford deleted the bump-formula-pr-handle-url-wth-specs-hash branch April 6, 2024 01:35
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Reproducible Homebrew/brew bug critical Critical change which should be shipped as soon as possible. outdated PR was locked due to age
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bump-formula-pr fails for Formula using :homebrew_curl
3 participants