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

ci(deps): fix git push during yarn.lock deduplication for Dependabot PRs [actions testing PR] #3

Closed
wants to merge 10 commits into from

Conversation

agilgur5
Copy link
Owner

@agilgur5 agilgur5 commented Apr 5, 2024

Fixes argoproj#12234 (comment), argoproj#12891 (comment)

Motivation

  • actions/checkout runs git checkout on a specific commit SHA, meaning there is no "branch" by default, it's on a detached HEAD
    • so without a branch specified, git push would error out

Modifications

  • specify a branch for git push using GH Actions env vars

    • note that $GITHUB_HEAD_REF should exist for PRs, and dependabot makes PRs (vs. direct pushes)
    • use origin as the remote name which is the default and also was mentioned in the error message
  • add permissions.contents: write to allow for this GHA job to push

  • add fetch-depth to actions/checkout so that it doesn't fail to push due to not having fetched (i.e. not knowing the state of the branch being pushed to)

  • also add if git diff --quiet -- ui/yarn.lock check to short-circuit the logic if there are no changes to yarn.lock (i.e. no deduplication necessary)

Verification

I tested the short-circuit logic locally:
  1. With no changes, exits properly:

    ❯ bash
    bash-5.1$ if ! git diff --exit-code -- ui/yarn.lock ; then
                exit 0
              fi
    exit
  2. With changes, does not exit:

    ❯ bash
    bash-5.1$ if ! git diff --exit-code -- ui/yarn.lock ; then
                exit 0
              fi
    bash-5.1$

Otherwise using this very PR in my own fork for testing purposes

- `actions/checkout` runs `git checkout` on a specific commit SHA, meaning there is no "branch" by default, it's on a detached `HEAD`
  - so without a branch specified, it would error out
  - so specify a branch using [GH Actions env vars](https://docs.github.com/en/actions/learn-github-actions/variables#default-environment-variables)
    - note that `$GITHUB_HEAD_REF` should exist for PRs, and dependabot makes PRs (vs. direct `push`es)
  - use `origin` as the remote name which is the default and also was mentioned in the error message

- also short-circuit the logic if there are no changes to `yarn.lock` (i.e. no deduplication neceessary)
  - this should also be less buggy as the later code will only execute when strictly necessary now (basically, when not needed, returns to the previous behavior before this step existed)

Signed-off-by: Anton Gilgur <agilgur5@gmail.com>
Signed-off-by: Anton Gilgur <agilgur5@gmail.com>
Signed-off-by: Anton Gilgur <agilgur5@gmail.com>
Signed-off-by: Anton Gilgur <agilgur5@gmail.com>
Signed-off-by: Anton Gilgur <agilgur5@gmail.com>
Signed-off-by: Anton Gilgur <agilgur5@gmail.com>
@agilgur5 agilgur5 force-pushed the ci-deps-fix-deduplicate-push branch from 6db5363 to 7f5047d Compare April 5, 2024 01:09
agilgur5 and others added 4 commits April 4, 2024 21:12
This reverts commit 7f5047d.

Signed-off-by: Anton Gilgur <agilgur5@gmail.com>
Signed-off-by: Anton Gilgur <agilgur5@gmail.com>
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
@agilgur5
Copy link
Owner Author

agilgur5 commented Apr 5, 2024

As can be seen in this PR and per argoproj#12234 (comment), checks don't run on commits made by GH Actions (in order to prevent an infinite loop, see GH docs). That means that the git push from this script will never pass branch protections and so will never be mergeable as a result.

As such, I will not be making a PR for this upstream and will instead follow-up upstream with a revert of argoproj#12234 entirely, since it is not workable. EDIT: reverted in argoproj#12892

Tbh, I'm not really sure how to get around these limitations, it would be great if dependabot automatically deduplicated itself per dependabot/dependabot-core#5830. Otherwise we might be stuck with manual deduplication necessary 😕

@agilgur5 agilgur5 closed this Apr 5, 2024
@agilgur5 agilgur5 deleted the ci-deps-fix-deduplicate-push branch April 5, 2024 03:47
@agilgur5 agilgur5 changed the title ci(deps): fix git push during yarn.lock deduplication for Dependabot PRs ci(deps): fix git push during yarn.lock deduplication for Dependabot PRs [actions testing PR] Apr 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
1 participant