Code Monkey home page Code Monkey logo

Comments (19)

yelizariev avatar yelizariev commented on July 20, 2024 2

I don't think that it will work -- secrets are not available in forks, so you cannot use custom token

from automerge-action.

pascalgn avatar pascalgn commented on July 20, 2024

GH doesn't let me clone my own repository, so I cannot quickly test this right now.

Could you possibly adapt your actions workflow file to enable tracing and then try again?

      - name: automerge
        uses: pascalgn/automerge-action@...
        with:
          args: "--trace"

from automerge-action.

planetf1 avatar planetf1 commented on July 20, 2024

Thanks - will do so & update here

from automerge-action.

planetf1 avatar planetf1 commented on July 20, 2024

Here's a log of a failing run -> https://gist.github.com/3ae5899ae6886ddf64f97abaaeae0126

from automerge-action.

pascalgn avatar pascalgn commented on July 20, 2024

Yeah, that was a bug because I used pullRequest.head... where instead pullRequest.base... should have been used!

This should be fixed in the latest version, you can try it like

    steps:
      - name: automerge
        uses: "pascalgn/automerge-action@7854d3bd607dccdaf0b2c134b699a812c8960213"

from automerge-action.

planetf1 avatar planetf1 commented on July 20, 2024

Sincere thanks for addressing that - out of interest I wonder if it would be reasonable to use use the latest ? presumably opascalgn/automerge-action? Would you recommend that? (new to github actions, and I guess there's no confirmed version yet?)

from automerge-action.

pascalgn avatar pascalgn commented on July 20, 2024

I would definitely recommend against using latest. We did that once accidentally for another action (provided by GH) and the API of that action changed, without us noticing, breaking our build.

If you want, you can point to the releases (which is also what GH is recommending, e.g. when looking at the action on the marketplace). However, I would recommend to always point to the full SHA, so that you can be reasonably safe that the version doesn't change (accidentally or on purpose)

from automerge-action.

planetf1 avatar planetf1 commented on July 20, 2024

Thanks for the clarification.

I tried a new automerge & hit a problem:

2020-01-27T09:38:11.7325143Z INFO  Updating PR #2496 Add notes on issue management & update autoclosing
2020-01-27T09:38:11.7325338Z INFO  No update necessary, mergeable_state: clean
2020-01-27T09:38:11.7325613Z INFO  Merging PR #2496 Add notes on issue management & update autoclosing
2020-01-27T09:38:11.7325809Z INFO  PR is probably ready: mergeable_state: clean
2020-01-27T09:38:11.7325970Z INFO  Failed to merge PR: Resource not accessible by integration

I posted the full trace to https://gist.github.com/04e7f4103cbd4346cb207504165ab107

I'm not sure if this is a followon or not.

I didn't see any evidence of the automerge rebasing the PR - is it intended to do this?

from automerge-action.

pascalgn avatar pascalgn commented on July 20, 2024

I'm afraid that's a general problem with GH actions and forks. From the docs:

When you create a pull request from a forked repository to the base repository, GitHub sends the pull_request event to the base repository and no pull request events occur on the forked repository.
[...]
The permissions for the GITHUB_TOKEN in forked repositories is read-only.

As I understand it, this means:

  1. The action is not triggered in the forked repository, so it cannot rebase the branch there (as it never runs)
  2. The action is triggered in the base repository, but it cannot rebase the branch, because the branch belongs to the fork and the action only has read-only access to the fork

The docs also state

Workflows don't run on forked repositories by default. You must enable GitHub Actions in the Actions tab of the forked repository.

So if you enable actions for the fork, the action could be triggered by the push event (this should get sent to the fork) and rebase should happen.

Merging should work correctly, as the pull_request event is sent to the base repository (which has the correct permissions to do the actual merging)

from automerge-action.

planetf1 avatar planetf1 commented on July 20, 2024

Ah! Thanks for the explanation.

Yes our process is all centered around forks (fairly common open source model).

In our case dependabot is the one bot that actually creates branches in the main repo -- and it is able therefore to auto rebase & merge

Although I could enable updates on my repo, it won't scale as a general solution. I'll probably have to think again how we want to handle this.

Thanks for the time though.

from automerge-action.

pascalgn avatar pascalgn commented on July 20, 2024

Yes our process is all centered around forks (fairly common open source model).

Yes, that's why I think it makes sense to support this case correctly.

I'm not entirely sure if it's really required or even desired to update branches in the forks. When I fork a repository, I would not expect some automation to update my branch. That could be one of the reasons why GH requires actions to be explicitly enabled.

If you want the PR to be up-to-date before merging (which makes sense IMO), maybe it could be enough to indicate that it's behind. Currently this is made visible via the failed automerge check, but if that's not enough, one could think about adding a comment like "Cannot merge, as branch is behind". I think the permissions should be enough for this

from automerge-action.

planetf1 avatar planetf1 commented on July 20, 2024

Though just indicating with a message - whilst it helps the user understand why it isn't merged, I don't think it completely addresses the problem that occurs in forked repos where github protections require branches to be up to date before merging.(understand why of course)

If we have a backlog of 11 PRs to merge (like this morning) I have to chose my first one, click 'update branch' on github (of course also in cli, or could be done by owner going a pull or rebase & push). wait for checks to complete (30 mins - build/tests). Then I can merge.

In the meantime if anyone else has done the same and remerged I have to start again, since the 'update' will need to be done again, then another 30 mins of checks.

So with the automerge behaviour described we might get the first one through but no more. So it could eliminate that 30 min period of checking before merging, but what it can't do is mark a whole set of PRs and get them effectively queued up for merging.

from automerge-action.

GMNGeoffrey avatar GMNGeoffrey commented on July 20, 2024

So to be clear about the current support for forks, if I set this up to just do auto-merging and do not set "Require branches to be up to date before merging", will it work for merging a PR coming from a fork?

Something like this:

automerge.yml
name: automerge	
on:	
  pull_request:	
    types:	
      - labeled	
      - unlabeled	
      - synchronize	
      - opened	
      - edited	
      - ready_for_review	
      - reopened	
      - unlocked	
  pull_request_review:	
    types:	
      - submitted	
  check_suite:	
    types:	
      - completed	
  status: {}	
jobs:	
  automerge:	
    runs-on: ubuntu-18.04
    steps:	
      - name: automerge	
        uses: "pascalgn/automerge-action@135f0bdb927d9807b5446f7ca9ecc2c51de03c4a"	
        env:	
          GITHUB_TOKEN: "${{ secrets.GITHUB_WRITE_ACCESS_TOKEN }}"  # personal access token
          MERGE_LABELS: "automerge-squash,!wip"	
          MERGE_METHOD: "squash"	
          MERGE_FORKS: true
          MERGE_COMMIT_MESSAGE: "pull-request-title-and-description"

from automerge-action.

pascalgn avatar pascalgn commented on July 20, 2024

Yes, that should work, see some comment above

Merging should work correctly, as the pull_request event is sent to the base repository (which has the correct permissions to do the actual merging)

from automerge-action.

michaelbeaumont avatar michaelbeaumont commented on July 20, 2024

It seems like this ultimately stems from the mismatch between "this code is safe to run workflows on with a secret token because it's already in the main repo" vs "it's safe to run because I added a label that implies it's allowed to be in the repo (but I need that secret token to make it so)". It appears to be a failure of Github Actions but the problem isn't easy to solve using the generic "events trigger some arbitrary code" workflow interface.

I would suppose this trust implication (label from maintainers -> safe to run workflows with secrets and/or merge) needs to be special cased in Github (or some interface for expressing the idea "if such and such is true, this code is safe" needs to be introduced). Alternatively, treat a PR action not as either "running in a fork" or "running in base" but rather "running with this base and this fork, take into account what the base says about PRs to itself"

But would creating a workflow that just runs periodically to loop over open PRs and their labels solve this problem until then?

from automerge-action.

planetf1 avatar planetf1 commented on July 20, 2024

I think a periodic/scheduled workflow would be a suitable alternative. Certainly for me the main value I see in automerge is avoiding the need to rebuild, wait for a 30min cycle, go to merge, find another one has got in and start again. I'm not really so bothered if it takes 30 mins or a day, it's more the automation and reducing manual effort for PRs we think are good to go.

Indeed allowing to run only in less active hours could be seen as a feature (avoiding interaction with any more urgent fixes being manually driven)!

from automerge-action.

pascalgn avatar pascalgn commented on July 20, 2024

I would suppose this trust implication (label from maintainers -> safe to run workflows with secrets and/or merge) needs to be special cased in Github

Yes, it's nice that this action is useful for people, but when you really think about it, it should actually be a built-in feature, as it already is in GitLab or Azure DevOps (if I'm not mistaken). Now this action supports much more obscure workflows then initially thought necessary, but I still hope that GH will at least implement the core functionality (merge when all checks pass) some day.

I think a periodic/scheduled workflow would be a suitable alternative.

Yes, I think that's a good idea. I even thought about switching our workflow to "scheduled," as the action will otherwise be triggered unnecessarily many times. Checking every 15 or 30 minutes or so, based on build speed, should be fine. I haven't tested it much with the "schedule" event trigger, but it should already work

from automerge-action.

michaelbeaumont avatar michaelbeaumont commented on July 20, 2024

Well, well! Certainly relevant: github/roadmap#107

from automerge-action.

michaelbeaumont avatar michaelbeaumont commented on July 20, 2024

Hmm it seems check_suite/check_run events for forked repositories are also useless, since they don't contain a reference to the pull request.

from automerge-action.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.