Skip to content

github-actions does not have permissions to push to main #680

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

Closed
UebelAndre opened this issue Apr 7, 2021 · 15 comments
Closed

github-actions does not have permissions to push to main #680

UebelAndre opened this issue Apr 7, 2021 · 15 comments
Assignees

Comments

@UebelAndre
Copy link
Collaborator

In #663 a change was introduced to build new crate_universe_resolver binaries and push a change back to main containing the release URL where they were uploaded and the sha256 checksums of each binary. Unfortunately, the github-actions user cannot push to main.

https://github.com/bazelbuild/rules_rust/runs/2289735079?check_suite_focus=true

 [main 632d30b] github-actions: update crate_universe binaries from a3c2741207a205e5421f3b3632e578f07ccd26cc
 1 file changed, 6 insertions(+), 6 deletions(-)
remote: error: GH006: Protected branch update failed for refs/heads/main.        
remote: error: 2 of 2 required status checks are expected. At least 1 approving review is required by reviewers with write access.        
To https://github.com/bazelbuild/rules_rust
 ! [remote rejected] main -> main (protected branch hook declined)
error: failed to push some refs to 'https://github.com/bazelbuild/rules_rust'
@UebelAndre
Copy link
Collaborator Author

I think only @hlopko has the power to help out here.

@dfreese
Copy link
Collaborator

dfreese commented Apr 7, 2021

Is bootstrapping cargo_universe requiring auto-commits back into the repository?

@UebelAndre
Copy link
Collaborator Author

The bootstrapping process doesn't but being able to distribute the built binaries to users does. Otherwise users need to go get the URLs and sha256 values themselves which is not ideal.

@dfreese
Copy link
Collaborator

dfreese commented Apr 7, 2021

I'm not comfortable with the repository being auto-updated for a API that's experimental.

@UebelAndre
Copy link
Collaborator Author

Why are you uncomfortable? And what are the alternatives?

@dfreese
Copy link
Collaborator

dfreese commented Apr 7, 2021

The API takes the url and sha256 as an input. The SHAs could be generated as an additional artifact.

auto-commits are a significant jump in complexity to getting something working, and making sure they continue to work.

@UebelAndre
Copy link
Collaborator Author

I don't see how an additional artifact would solve for users having to copy/paste something in addition to having to go find the latest commit of the rules and the sha256 of these rules to use a built in rule.

How do we solve for the usability concerns?

@dfreese
Copy link
Collaborator

dfreese commented Apr 7, 2021

Given that most bazel repositories require a user to locate it's url and SHA, I don't see it as a huge usability hurdle.

@UebelAndre
Copy link
Collaborator Author

I do see it as a sizable usability hurdle. I'd like to avoid putting users in situations where they're using incompatible binaries with their current rev of the rules.

I hope to stabilize crate_universe as soon as possible and think it's appropriate to have the CI system commit updated info on the new binaries. If the rule being experimental is your primary concern, we could work something out but I feel it's best enable functionality like this so it has as much time as possible to mature.

@dfreese
Copy link
Collaborator

dfreese commented Apr 7, 2021

To clarify my concern, it being experimental is a secondary concern.

I think every commit of the repository should function appropriately, and if the API requires a secondary commit for every change, then it doesn't seem to be following that principle.

@UebelAndre
Copy link
Collaborator Author

Aside from committing binaries to the repo, what other options are available?

@illicitonion
Copy link
Collaborator

I agree that atomic self-contained commits are much nicer than separate ones (both ideologically, and for making things like bisects work properly). I can see the desire to avoid the complexity of auto-commits (at least for now) from a complexity perspective, but I don't think auto-commits are fundamentally problematic, other than that they break the self-contained-ness of commits.

It feels like the ideal way to address this long-term would be for a bot to perform the merges for us (and do the generation on the way), rather than for us to merge and then trigger another process.

In the mean time, I can imagine the now-existing "release" GitHub Action populating a copy-and-paste-able WORKSPACE block so that on https://github.com/bazelbuild/rules_rust/releases/tag/crate_universe-3 under where it says "From commit [blah]" it would then say:

load(..., "http_archive", "http_file")

http_archive(
    name = "rules_rust",
    url = "[rules_rust tar]",
    sha256 = "...",
    ...
)

http_file(
    name = "rules_rust_crate_universe_resolver_darwin_...",
    url = "[rules_rust tar]",
    sha256 = "...",
)

...

So that the primary way someone pulls in rules_rust is by copying this block. The rules could be written assuming you've registered the repos, and just consuming them assuming they have been.

Given bootstrapping for development and for PR builds needs to be separate anyway, changing from copying one http_archive to N http_* rules doesn't seem too onerous... Certainly while this is experimental, it seems good enough for us to use for development, at least :)

@UebelAndre
Copy link
Collaborator Author

That's a fine solution for right now but I'd like to hear ideas about a long term solution.

Eventually we should have something more streamline and I'd rather have conversations about what options there are vs things we don't like.

@UebelAndre
Copy link
Collaborator Author

Merging things into user's PR sounds risky and tough. Does anyone know of another repo that does this?

@UebelAndre
Copy link
Collaborator Author

UebelAndre commented Mar 23, 2022

This is no longer relevant due to #1195. Closing.

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

No branches or pull requests

4 participants