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

Revert "Include changes to the target branch when action is re-run" #60

Merged

Conversation

simonjbeaumont
Copy link
Contributor

Reverts #58

This has resulted in a destructive outcome for developers using act to run the workflows locally, which is also in our public documentation now, particularly when used with --bind.

This patch recreates the entire repo and force updates to a ref. It literally deleted my Git repo and left me with a grafted shallow clone. It discarded all my local branches and no reflog was available.

We need to revert this one quickly.

I've verified that the revert mitigates the problem by making this change in my YAMLs locally, to the commit before this PR was merged:

-    uses: apple/swift-nio/.github/workflows/cxx_interop.yml@main
+    uses: swiftlang/github-workflows/.github/workflows/soundness.yml@814f5831ccc6aac159903e8db6daba726c8b62d5

I'm sympathetic to the motivation behind the PR but I think we should revert immediately to avoid others hitting this.

@simonjbeaumont simonjbeaumont requested a review from a team as a code owner November 7, 2024 17:47
@simonjbeaumont
Copy link
Contributor Author

//cc @shahmishal @ahoppen @ktoso @rnro

@shahmishal shahmishal merged commit 3fca720 into swiftlang:main Nov 7, 2024
9 checks passed
@simonjbeaumont
Copy link
Contributor Author

Thanks for merging promptly.

I'm finished for today now but I think we could investigate using a conditional here on the CI environment variable and only have this behaviour if running in CI. I believe we have a similar behaviour somewhere else already but I'm on a mobile device so can't validate right now.

@ahoppen
Copy link
Member

ahoppen commented Nov 7, 2024

Sorry for breaking the act workflow @simonjbeaumont and thanks for reverting my change.

If you have an example of how you guard behavior on the CI environment, I’d be interested to see that and check if we can apply it here as well.

@simonjbeaumont
Copy link
Contributor Author

simonjbeaumont commented Nov 8, 2024

@ahoppen Looks like we can use job.if here with an environment variable that's only set when using act:

jobs:
  foo:
    runs-on: ubuntu-latest
    name: Foo
    steps:
      - name: Dump env
        run: env | sort
      - name: Echo always
        run: echo "always"
      - name: Echo only if FOO=true
        if: env.FOO == 'true'
        run: echo "job running with FOO set to true"
      - name: Echo only if not FOO=true
        if: env.FOO != 'true'
        run: echo "job not running with FOO set to true"
      - name: Echo only if running with act
        if: env.ACT == 'true'
        run: echo "job running with ACT"
      - name: Echo only if not running with act
        if: env.ACT != 'true'
        run: echo "job not running with act"
% act -j foo
 [PR/Foo]   ✅  Success - Main Dump env
[PR/Foo] ⭐ Run Main Echo always
[PR/Foo]   🐳  docker exec cmd=[bash -e /var/run/act/workflow/1] user= workdir=
| always
[PR/Foo]   ✅  Success - Main Echo always
[PR/Foo] ⭐ Run Main Echo only if not FOO=true
[PR/Foo]   🐳  docker exec cmd=[bash -e /var/run/act/workflow/3] user= workdir=
| job not running with FOO set to true
[PR/Foo]   ✅  Success - Main Echo only if not FOO=true
[PR/Foo] ⭐ Run Main Echo only if running with act
[PR/Foo]   🐳  docker exec cmd=[bash -e /var/run/act/workflow/4] user= workdir=
| job running with ACT
[PR/Foo]   ✅  Success - Main Echo only if running with act
[PR/Foo] Cleaning up container for job Foo
[PR/Foo] 🏁  Job succeeded

% act -j foo --env FOO=false
[PR/Foo]   ✅  Success - Main Dump env
[PR/Foo] ⭐ Run Main Echo always
[PR/Foo]   🐳  docker exec cmd=[bash -e /var/run/act/workflow/1] user= workdir=
| always
[PR/Foo]   ✅  Success - Main Echo always
[PR/Foo] ⭐ Run Main Echo only if not FOO=true
[PR/Foo]   🐳  docker exec cmd=[bash -e /var/run/act/workflow/3] user= workdir=
| job not running with FOO set to true
[PR/Foo]   ✅  Success - Main Echo only if not FOO=true
[PR/Foo] ⭐ Run Main Echo only if running with act
[PR/Foo]   🐳  docker exec cmd=[bash -e /var/run/act/workflow/4] user= workdir=
| job running with ACT
[PR/Foo]   ✅  Success - Main Echo only if running with act
[PR/Foo] Cleaning up container for job Foo
[PR/Foo] 🏁  Job succeeded

% act -j foo --env FOO=true
[PR/Foo]   ✅  Success - Main Dump env
[PR/Foo] ⭐ Run Main Echo always
[PR/Foo]   🐳  docker exec cmd=[bash -e /var/run/act/workflow/1] user= workdir=
| always
[PR/Foo]   ✅  Success - Main Echo always
[PR/Foo] ⭐ Run Main Echo only if FOO=true
[PR/Foo]   🐳  docker exec cmd=[bash -e /var/run/act/workflow/2] user= workdir=
| job running with FOO set to true
[PR/Foo]   ✅  Success - Main Echo only if FOO=true
[PR/Foo] ⭐ Run Main Echo only if running with act
[PR/Foo]   🐳  docker exec cmd=[bash -e /var/run/act/workflow/4] user= workdir=
| job running with ACT
[PR/Foo]   ✅  Success - Main Echo only if running with act
[PR/Foo] Cleaning up container for job Foo
[PR/Foo] 🏁  Job succeeded

So I think it will just be adding if: env.ACT != 'true' to the steps. I'll open a PR to reintroduce the change.

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 this pull request may close these issues.

3 participants