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

Feature request: pull before push #84

Open
nsheff opened this issue Mar 16, 2021 · 10 comments
Open

Feature request: pull before push #84

nsheff opened this issue Mar 16, 2021 · 10 comments

Comments

@nsheff
Copy link

nsheff commented Mar 16, 2021

See also #79

This action works well for me, but occasionally I am running 2 of them at the same time and one will push changes in the middle of another one pulling the changes and then pushing.

It would be convenient if there was a pull_first: true option that would re-issue the git pull right before pushing to avoid a push conflict. Force is not the option as that will overwrite the other push changes (and ruin history).

@alexiswl
Copy link

alexiswl commented Jul 5, 2021

I just run code inside the runner directly now rather than calling this action:

jobs:
  my_job_name:
     runs-on: ubuntu-latest
     steps:
        # Standard checkout step
        - name: Checkout code
          id: git_checkout
          uses: actions/checkout@v2
          ....
        # Commit files
        ... # I use https://github.com/EndBug/add-and-commit
        # Push config files
        - name: Push code
          id: git_push
          run: |
            # Pull first
            git pull --rebase origin "${GITHUB_REF}"

            # Push after
            git push origin "${GITHUB_REF}" 

You may find this latest update https://github.blog/changelog/2021-04-19-github-actions-limit-workflow-run-or-job-concurrency/ quite useful. Setting concurrency on your jobs means your workflows are run in sequence rather than sequentially

@ad-m
Copy link
Owner

ad-m commented Jul 10, 2021

@alexiswl Could you provide pull request to README of github-push-action to add note about concurrency settings?

@alexiswl
Copy link

@ad-m while the concurrency means that the jobs don't run simultaneously, they still run from the one original commit.

Whilst concurrency will stop 'race conditions' of two actions pushing commits to the repo at the exact same time, it won't fix the issue because the second push will always fail because it hasn't merged the commit of the first push.

@nodesocket
Copy link

Any ideas on this? We are seeing this error when lots of GitHub actions fire closely together and errors with:

Run ad-m/github-push-action@master
Push to branch master
To https://github.com/Acme-Org/DevOps.git
 ! [rejected]          HEAD -> master (non-fast-forward)
error: failed to push some refs to 'https://github.com/Acme-Org/DevOps.git'
hint: Updates were rejected because the tip of your current branch is behind
hint: its remote counterpart. Integrate the remote changes (e.g.
hint: 'git pull ...') before pushing again.
hint: See the 'Note about fast-forwards' in 'git push --help' for details.
Error: Invalid exit code: 1
    at ChildProcess.<anonymous> (/home/runner/work/_actions/ad-m/github-push-action/master/start.js:29:21)
    at ChildProcess.emit (events.js:314:20)
    at maybeClose (internal/child_process.js:1022:16)
    at Process.ChildProcess._handle.onexit (internal/child_process.js:28[7](https://github.com/Acme-Org/DevOps/runs/6603223590?check_suite_focus=true#step:4:8):5) {
  code: 1
}
Error: Invalid exit code: 1
    at ChildProcess.<anonymous> (/home/runner/work/_actions/ad-m/github-push-action/master/start.js:29:21)
    at ChildProcess.emit (events.js:314:20)
    at maybeClose (internal/child_process.js:1022:16)
    at Process.ChildProcess._handle.onexit (internal/child_process.js:2[8](https://github.com/Acme-Org/DevOps/runs/6603223590?check_suite_focus=true#step:4:9)7:5)

Is there a way to perform a pull/rebase before?

@ZPascal
Copy link
Collaborator

ZPascal commented May 26, 2022

Hi @nodesocket, do you tried git pull or git pull --rebase origin "${GITHUB_REF}" beforehand? You can include the command inside the commit/ configuration area, e.g.

- name: Commit files
      run: |
        git pull
        git config --local user.email "41898282+github-actions[bot]@users.noreply.github.com"
        git config --local user.name "github-actions[bot]"
        git commit -m "Add changes" -a

@nsheff I think the point sounds valid to include a flag to pull the repository beforehand. I'll check how to include the functionality and prepare a PR.

@nodesocket
Copy link

nodesocket commented May 26, 2022

@ZPascal the action definition just does a checkout, runs a custom bash script which takes 1-2 seconds and then calls push. Do you spot anything or a solution? Are you recommending to call pull right before push?

jobs:
  update-helm-image-tags:
    name: Updated Helm image tags
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v2
        with:
          fetch-depth: 0
      - name: updateimagetags.sh
        run: helm/updateimagetags.sh ${{ join(github.event.client_payload.applications, ',') }} ${{ join(github.event.client_payload.environments, ',') }} ${{ github.event.client_payload.image_tag }}
      - name: Push
        uses: ad-m/github-push-action@master
        with:
          force: false

@ZPascal
Copy link
Collaborator

ZPascal commented May 31, 2022

Hi @nodesocket, I would recommend trying a pull beforehand. I've got one question: Your bash script does the rest (git config, git add, git commit)?

@ZPascal ZPascal linked a pull request May 31, 2022 that will close this issue
2 tasks
@nodesocket
Copy link

nodesocket commented May 31, 2022

@ZPascal correct, in my bash script I am doing the following:

git config --local user.name "GitHub Actions"
git config --local user.email "engineering+$GITHUB_ACTOR@acme.org"
git add --all
git commit -am "[gh-action] image tag for $APPLICATIONS in $ENVIRONMENTS to $IMAGE_TAG"

Should I just add git pull --rebase at the end, after the commit?

@ZPascal
Copy link
Collaborator

ZPascal commented May 31, 2022

Hi @nodesocket, yes. I think that should solve the issue.

@kevupton
Copy link

kevupton commented Aug 3, 2022

this package should totally have pull as an optional parameter.

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 a pull request may close this issue.

6 participants