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

Filtering paths & skipping concurrent runs can lead to problems #312

Open
bjchambers opened this issue Jan 12, 2023 · 4 comments
Open

Filtering paths & skipping concurrent runs can lead to problems #312

bjchambers opened this issue Jan 12, 2023 · 4 comments

Comments

@bjchambers
Copy link

  1. Push commit 1: Run two jobs. 1-push because of the push and 1-pr because of the pull request.
  2. Concurrent jobs skips 1-pr because it is concurrent and newer than 1-push.
  3. Cancel 1-push (see it's going to fail, etc.)
  4. Push commit 2 that touches ignored files.

What I'd expect: No successful, non-skipped run has occurred, so we need to run things.

What happens: Both jobs run on commit 2, but both decide that since (a) the only change is ignorable and (b) the most recent run (1-pr succeeded -- by skipping everything) there is nothing to do.

It seems like perhaps instead of "skip if there are no changed files since a successful run" it should be "skip if there are no changed files since a non-skipped successful run".

Is this a known issue / existing work arounds?

@paescuj
Copy link
Collaborator

paescuj commented Jan 12, 2023

Could you please share your config?

@bjchambers
Copy link
Author

Created a repro repository.

The workflow is setup to use skip jobs. It ignores changes to README.md but always fail to run tests (false).

https://github.com/bjchambers/skip-actions-repro/blob/main/.github/workflows/workflow.yml

I made a PR that first added a file. This behaves as follows:

I then added a commit that changed README.md. This shows all checks as having passed!

@paescuj
Copy link
Collaborator

paescuj commented Jan 12, 2023

Thanks! 👍 I'll have a look at this.

@fkirc
Copy link
Owner

fkirc commented Jan 12, 2023

Thank you for providing the reproducing repo!
I recognize that there are multiple problems with “concurrent skipping“, which is why I made the decision to disable concurrent skipping by default.
I made this decision many months ago because there have been several concurrency-issues, some of them are safety-critical.

In fact, I would consider concurrent skipping as an experimental feature.
For me, it is an open research question whether we could change the behaviour of concurrent skipping in a way that is useful for the general public.
Perhaps we might implement a new skipping-mode, similar to “same_content_newer“?

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

3 participants