fix(homebrew): handle new github archive url format #29138
Conversation
nikaro
force-pushed
the
fix/homebrew-url-path-parsing
branch
from
052904f
to
2874c02
Compare
Head branch was pushed to by a user without write access
nikaro
force-pushed
the
fix/homebrew-url-path-parsing
branch
from
2874c02
to
7dcbb4a
Compare
|
@rarkins i modified some forgotten urls in tests and force pushed. |
nikaro
force-pushed
the
fix/homebrew-url-path-parsing
branch
from
7dcbb4a
to
e1e65af
Compare
|
Ah my bad, i didn't saw about these snapshots things. |
Old archive urls were in form: https://github.com/<owner/<repo>/archive/<tag>.tar.gz New ones are in form: https://github.com/<owner/<repo>/archive/refs/tags/<tag>.tar.gz So with the current parsing `currentValue` ends up being filled with the `refs` string instead of the tag value. This change handles both forms by checking the length of the url paths slice.
Head branch was pushed to by a user without write access
nikaro
force-pushed
the
fix/homebrew-url-path-parsing
branch
from
e1e65af
to
f4e53df
Compare
|
This time it should be good, i ran the Sorry for the inconvenience. |
|
I may need some help here, or someone taking over the PR. As i don't really know what to do to fix the coverage decrease. Is it just a matter of adding a case in the Or maybe to keep things simple i can remove the handling of old URLs format, what do you think? |
Head branch was pushed to by a user without write access
|
I'd prefer to keep it, unless we know that no GitHub instance (including self hosted) that anyone uses ever generates the old style any more. Most likely it's 1-2 additional test cases. Maybe one which was changed can be reverted to the old format. |
|
I added a test case, " |
|
There is update coverage missed: https://app.codecov.io/gh/renovatebot/renovate/pull/29138/blob/lib/modules/manager/homebrew/update.ts#L195 
If this is not new code then it implies that it used to be reachable via a test, so again my advice would be to locate that test and duplicate it instead of replace it fully |
|
I've simplified the way how i check for the new URL format, i guess the "issue" had to do with the null return for unhandled "archive" URLs. This should pass now. The tests and code coverage are good locally. |
| const res = extractPackageFile(content); | ||
| expect(res).not.toBeNull(); | ||
| expect(res?.deps[0].skipReason).toBeUndefined(); | ||
| expect(res).toMatchSnapshot(); |
There was a problem hiding this comment.
It is the GitHub diff that is a bit messed up.
The "handles old "archive" github url format" has the same content than the old "handles no space before class header" one (with the old URL format). So there is a "new" snapshot.
|
🎉 This PR is included in version 37.368.9 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Changes
This change fix the GitHub archive tag URL path parsing for Hombrew manager.
Context
Old archive urls were in form:
https://github.com/<owner>/<repo>/archive/<tag>.tar.gzNew ones are in form:
https://github.com/<owner>/<repo>/archive/refs/tags/<tag>.tar.gzSo with the current parsing
currentValueends up being filled with therefsstring instead of the tag value.Also,
brew stylecommand complains about the old url format, cf.: https://github.com/nikaro/homebrew-tap/actions/runs/9131845923/job/25111949942#step:7:15Documentation (please check one with an [x])
How I've tested my work (please select one)
I have verified these changes via: