[CI] implement job skipping for doc-only PRs

[stas00]

Let's save some time and money. This PR:

• skips most jobs when the only change is in \.(md|rst)$ files.

I tested this with various types of files and it seems to do the right thing. But if we merge let's monitor that I didn't miss some use case and we end up with broken master if some CI jobs didn't run.

• pros: obvious
• cons: I don't like that the skipped CI job status appear as completed normally, even though it didn't quite run. Let's hope circleci comes up with some better way of indicating that the job was skipped.

---

how it was done:

git merge-base --fork-point master to get the commit range didn't work at all, even though that's what we use for the fixup Makefile target.

Other suggestions I found didn't work either.

At the end I found https://circleci.com/docs/2.0/pipeline-variables/ to get the correct commit range:

git diff --name-only << pipeline.git.base_revision >>...<< pipeline.git.revision >> 

and now all is good.

credits: the circle step halt idea comes from this blog https://yu-ishikawa.medium.com/reusable-a-circleci-command-to-halt-if-no-changed-target-files-e87c6b0af82b

@LysandreJik, @sgugger

[LysandreJik]

That's a great idea!

[julien-c]

Btw this is why GitHub Actions are so cool: this is built-in

[stas00]

ok, this is good now.

[LysandreJik]

LGTM, thanks @stas00!

[LysandreJik]

Btw this is why GitHub Actions are so cool: this is built-in

This among others :) the only pain point is the lack of anchors, which would be a godsend given our current YAML files.

[sgugger]

There is a problem with the test, which seems to skip the tests as soon as there is at least one doc file, even if code files have also been modified. #8852 gives an example of this happening.
I have commented out the line skip-job-on-doc-only-changes in this commit to have the CI work while waiting for a fix on your side @stas00.

[stas00]

@sgugger, can you show me a a specific example of this behavior? Looking at PR you linked and other commits since the skip rule has been merged I don't see this happening.

For example, 75f8100 has both docs and code files and it has the skip rule activated - and none of the jobs were skipped, e.g. here is one job from that PR:
https://app.circleci.com/pipelines/github/huggingface/transformers/16543/workflows/ac01a5da-d9dc-4e23-8ee7-9e562263f030/jobs/128245

Thank you!

p.s. while we are sorting this new one out, you don't need to comment out all the invocation of the command, just the 'circleci halt' line in the command.

[sgugger]

Arg, I force-pushed my rebase so we lost the commit where the problem was happening. I assure you that PR had all tests skipped except build_doc.

[stas00]

I totally believe you what you saw, but this must have been some edge case that I haven't accounted for and I need to see what it was. As I have shown in the 2 links in the comment above yours the rule did work correctly for a PR with 2 docs and one py file, so what you suggested that it skips as soon as there is at least one doc doesn't seem to be the case.

Do you know at least what files were involved in that commit where the undesired skip has occurred? or perhaps it's still in your local branch?

The logic is simple:

1. it gets the modified file names
2. it then removes any docs that match \.(md|rst)$
3. • a. if there are any files left, we have non-docs - normal behavior ensues
   • b. if there are no files left, we have only docs - and it skips

[sgugger]

I've deleted that branch but there was only one commit with the 9 files you see in the PR.

[stas00]

OK, I created a PR #8853 with the exact same files by just reverting your commit 5530299 for the sake of the test (not intending to merge it) - plus .circleci/config.yml to restore the skipping rule - none of the checks has been skipped.

Moreover, you said:

There is a problem with the test, which seems to skip the tests as soon as there is at least one doc file

but your commit had no doc files.

Do you want to try to re-enable the rule and monitor to catch a potential edge case that you saw but we no longer know what it was? And if you run into it and I will monitor too, let's make sure to save the branch so that we could reproduce the problem.

To quickly disable the skip just this line needs to be commented out:

transformers/.circleci/config.yml

Line 19 in dfec84d

circleci step halt

The only tricky part with monitoring is that it won't affect older branches that weren't rebased or created after the skip was enabled.

Oh and I apologize if this causes a temporary potential hurdle in normal PR process - hopefully we will sort it out quickly and overall things will be better in the long run.

[sgugger]

If that helps, here is another commit with rst, md and py files where the tests were all skipped:
The corresponding PR is #8850

[stas00]

Ah, great! That helped a lot, @sgugger - Thank you for finding it!

It appears to be a bug in circleCI (https://app.circleci.com/pipelines/github/huggingface/transformers/16541/workflows/17b20230-8d7c-4b36-813c-2681f2c8a977/jobs/128232)

It's missing << pipeline.git.base_revision >> in

if git diff --name-only << pipeline.git.base_revision >>...<< pipeline.git.revision >> | egrep -qv '\.(md|rst)$'

resulting in:

if git diff --name-only ...5170e5381b9fccdfb9405d665ecee0515efc6453 | egrep -qv '\.(md|rst)$'

and hence fails the test. (it's missing the first hash before ...).

Back to the drawing board.

[stas00]

Can you think of why these few commits could be missing pipeline.git.base_revision - was there something special about those?

[sgugger]

I have no idea, but if CircleCI is flaky like this, I guess we won't be able to use this to determine whether the commit contains only doc files or not...

[stas00]

We still can, by checking whether pipeline.git.base_revision is defined, and never skip if it's not. If that's the best we can do, it won't always save resources.

But let me research first why is it not defined at times.

[stas00]

Workaround: #8853