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

[WIP/TESTING] mobile/deps: Bump rules_android -> 0.6.0 #37661

Closed
wants to merge 8 commits into from

Conversation

phlax
Copy link
Member

@phlax phlax commented Dec 14, 2024

No description provided.

@repokitteh-read-only repokitteh-read-only bot added the deps Approval required for changes to Envoy's external dependencies label Dec 14, 2024
Copy link

CC @envoyproxy/dependency-shepherds: Your approval is needed for changes made to (bazel/.*repos.*\.bzl)|(bazel/dependency_imports\.bzl)|(api/bazel/.*\.bzl)|(.*/requirements\.txt)|(.*\.patch).
envoyproxy/dependency-shepherds assignee is @RyanTheOptimist

🐱

Caused by: #37661 was opened by phlax.

see: more, trace.

Copy link
Contributor

@moderation moderation left a comment

Choose a reason for hiding this comment

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

Update dependency details

Comment on lines +100 to +102
urls = ["https://github.com/bazelbuild/rules_android/archive/refs/tags/v0.6.0.zip"],
sha256 = "691c90615ebf66aa474fdd5e17b92d54f62e5e3627f4eb224e9321cfcf5fe83f",
strip_prefix = "rules_android-0.6.0",
Copy link
Contributor

Choose a reason for hiding this comment

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

Seeing more /refs/tags/ creeping into the dependency URLs which makes sens as that is what GitHub uses on the page now. The links work without it though and are cleaner. Almost all the links omit the /refs/tags/. Wondering if we should standardardize?

As per dependency standards we should use the maintainer provided binary here

        urls = ["https://github.com/bazelbuild/rules_android/releases/download/v0.6.0/rules_android-v0.6.0.tar.gz"],
        sha256 = "af84b69ab3d16dd1a41056286e6511f147a94ccea995603e13e934c915c1631c",
        strip_prefix = "rules_android-0.6.0",

Copy link
Member Author

Choose a reason for hiding this comment

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

yeah - that works in this case so happy to update

in the general case refs/tags is preferable to direct archive downloads - although the latter is the only option where the upstream doesnt maintain regular releases and/or we want/need to pin to a particular commit

non-releasing upstream are a big pita - using hashes is very insecure because you have to actually check the commit is a real commit on the branch you are expecting - you can use those addresses with any commit in a fork branch without creating any pr or ever landing any commits on the target repos branches

@phlax phlax force-pushed the deps-bump-rules_android branch from 01cf74b to ad9ffbb Compare December 17, 2024 11:51
@phlax
Copy link
Member Author

phlax commented Dec 17, 2024

seems this is not compatible with bazel 6

@phlax phlax force-pushed the deps-bump-rules_android branch 2 times, most recently from df17c1e to eef5e6d Compare December 17, 2024 17:31
Copy link

This pull request has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in 7 days if no further activity occurs. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions!

@github-actions github-actions bot added the stale stalebot believes this issue/PR has not been touched recently label Jan 16, 2025
@phlax phlax force-pushed the deps-bump-rules_android branch from eef5e6d to 27d8675 Compare January 23, 2025 17:17
@phlax phlax removed the stale stalebot believes this issue/PR has not been touched recently label Jan 23, 2025
phlax added 2 commits January 23, 2025 18:15
Signed-off-by: Ryan Northey <[email protected]>
Signed-off-by: Ryan Northey <[email protected]>
@phlax phlax force-pushed the deps-bump-rules_android branch from fda2697 to 5705a14 Compare January 23, 2025 18:23
phlax added 5 commits January 23, 2025 18:31
Signed-off-by: Ryan Northey <[email protected]>
Signed-off-by: Ryan Northey <[email protected]>
Signed-off-by: Ryan Northey <[email protected]>
Signed-off-by: Ryan Northey <[email protected]>
Signed-off-by: Ryan Northey <[email protected]>
@fredyw
Copy link
Member

fredyw commented Jan 23, 2025

I'm afraid this won't work because of rules_kotlin still uses rules_android 0.1.1. There's an open PR to update rules_kotlin to a higher version (still not 0.6.0) of rules_android, but the PR looks stale.

@phlax
Copy link
Member Author

phlax commented Jan 23, 2025

ah - good to know - thanks

Copy link

This pull request has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in 7 days if no further activity occurs. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions!

@github-actions github-actions bot added the stale stalebot believes this issue/PR has not been touched recently label Feb 22, 2025
Copy link

github-actions bot commented Mar 2, 2025

This pull request has been automatically closed because it has not had activity in the last 37 days. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions!

@github-actions github-actions bot closed this Mar 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deps Approval required for changes to Envoy's external dependencies stale stalebot believes this issue/PR has not been touched recently
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants