Skip to content

tools/upgradedep: add support for Label based repositories.bzl#3051

Merged
linzhp merged 8 commits intobazel-contrib:masterfrom
JamyDev:fix/tools-upgradedep
Jan 20, 2022
Merged

tools/upgradedep: add support for Label based repositories.bzl#3051
linzhp merged 8 commits intobazel-contrib:masterfrom
JamyDev:fix/tools-upgradedep

Conversation

@JamyDev
Copy link
Copy Markdown
Contributor

@JamyDev JamyDev commented Jan 19, 2022

What type of PR is this?

Bug fix

What does this PR do? Why is it needed?

upgradedep.go was not working due to the change to Label() based patches in #3035. Modified the parser to support both string and Label() patches.

Added tests to ensure all cases are covered for the additional code.

Which issues(s) does this PR fix?

Fixes #3050

Other notes for review

@linzhp
Copy link
Copy Markdown
Contributor

linzhp commented Jan 20, 2022

The other way to test this PR is to extract the logic that reads repositories.bzl to a separate function, and read the real repositories.bzl to test that function. That way, future incompatible changes to repositories.bzl will fail that test.

@JamyDev
Copy link
Copy Markdown
Contributor Author

JamyDev commented Jan 20, 2022

I think reading the entire repositories file and doing a dry run of the upgrader would be a good integration test case, but that would bloat this PR, so that's a separate piece of work.

@blico
Copy link
Copy Markdown
Contributor

blico commented Jan 20, 2022

Looks good besides some mostly stylistic comments.

@linzhp linzhp merged commit a93cae3 into bazel-contrib:master Jan 20, 2022
@JamyDev JamyDev deleted the fix/tools-upgradedep branch January 20, 2022 22:42
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.

Dependency upgrader fails to run due to change to Label() dependencies

3 participants