Spell-checking job fails with "Argument list too long" when too many files are changed

Note that the "git diff" command might not work as expected if the MR needs rebasing.

🐛 Test-only job shouldn't require constant rebases to detect which files were changed. Active needs to change the git diff command.
git diff ${CI_MERGE_REQUEST_DIFF_BASE_SHA} seems to be working well, so maybe, if we need to update the command here, we can use the same.

I just hit this on https://git.drupalcode.org/project/drupal/-/merge_requests/5500

As you can see, there aren't many files changed at all. The issue was that the fork's 11.x branch was way out of date. That doesn't make sense to me since the MR's target is origin's 11.x. Why does it matter where the tip of the fork's 11.x branch is pointing?

Pushing 11.x to the fork fixed it, but then the 11.x branch shows up on the issue (which can easily be hidden now, but still shouldn't be required).

I made an MR based on my suggestion in #2.

Seems like a simple enough change

Tested the change here https://git.drupalcode.org/issue/drupal-3412420/-/jobs/651480 which errored out but passed the pipeline.

Could argue almost critical as some MRs aren't running now

Bumping the depth to 50 (as per recommendation via slack) has fixed it https://git.drupalcode.org/issue/drupal-3412420/-/jobs/651854

I agree that we should make this change, but it's not addressing the issue that was actually reported here. We would still have a fail when an MR actually does touch that many files (such as 📌 [PP-2] Add declare(strict_types=1) to all Kernel tests Postponed ).

For those hitting this (where you DONT have lots of files changed) - a temporary fix is to push the latest 11.x branch to your fork:

git checkout 11.x
git fetch
git pull origin 11.x
git push FORK 11.x

Where FORK is your issue's fork (i.e drupal-123123

You'll then need to manually hide the 11.x branch on your issue in the Issue fork section.

Re #10, why not looping through the files changed and run the spellcheck per file? Even if we make more calls, it should be safer, and for small MRs, the impact shouldn't be too big.

We don't need to loop because the issue summary already shows an example using a temp file.

Oh! My bad, I didn't read that part. Will revert the changes.

Using the suggested solution (thanks @mstrelan!) and the fetching fix, things are working as expected. See https://git.drupalcode.org/issue/drupal-3401988/-/jobs/654322

MR is ready for review: https://git.drupalcode.org/project/drupal/-/merge_requests/6209/diffs

👀 Ran into this too, I thought this was a recent regression 😅, when it clearly is not.

I ran this patch on the private fork used for testing security patches and confirmed it works as expected there.

Ran into this again in a core issue at ✨ Drastically improve the linking experience in CKEditor 5 Needs work .

Big +1 to get this committed, it's biting me on almost every issue I touch at the moment.

Again at #3379104-31: Add a "CKEditor 5 nightly" GitLab CI job → .

Added some nitpicks to the shell script.

@longwave - your suggestion differs from the one proposed in the issue summary and #13. Both work, so it's just a matter of agreeing to which one to choose.

If there are 100 files changed:
- The pipe+sed+yarn would trigger the command will trigger the spellcheck command 100 times for a single file each time
- The keeping changed files in a file+yarn would trigger the spellcheck command 1 time for the 100 files with just one argument

I'm fine either way but it'd be great to agree on the approach to do before making any more changes.

Seen a few more issues get hit by this today ✨ Continuation Add Views EntityReference filter to be available for all entity reference fields Needs work which is a large issue that's close. If this fixes it can we merge this and tweak in a follow up?

@fjgarlin Why would git diff | sed | yarn trigger it 100 times? The --file-list stdin argument takes the entire list directly from stdin, we're not using xargs.

That’s just me not knowing enough about bash.

takes the entire list directly from stdin

So won’t that pass “too many arguments” again if many files change? Or does it count as just one argument?

In fact testing locally we can simplify it to:

git diff $CI_MERGE_REQUEST_DIFF_BASE_SHA --name-only | sed "s_^_../_" | yarn --cwd=./core run -s spellcheck:core --no-must-find-files --file-list stdin

which fixes the directory issue using relative instead of absolute paths.

https://docs.gitlab.com/ee/ci/variables/#argument-list-too-long only mentions this affecting variables, I think the problem before was $MODIFIED was too long.

I thought that the limitation was from cspell, not gitlab!!

I'm going to try the suggested fix in the last failing reported MR.

Testing the suggested fix on an MR that was failing and is not rebased:

• Before: FAIL https://git.drupalcode.org/issue/drupal-3347343/-/pipelines/82849
• Then commit: https://git.drupalcode.org/project/drupal/-/commit/220ea3b8354aa34322b38...
• After: FAIL https://git.drupalcode.org/issue/drupal-3347343/-/pipelines/82882

Reason. The fork does not even have 11.x. This is not the problem on this issue.
I needed to push the 11.x branch, and the pipeline now works, but then I can't reproduce the issue here.

Trying to find another MR.

Next 🌱 [meta] Test against CKEditor 5 nightly Active .

• Before: FAIL https://git.drupalcode.org/issue/drupal-3379104/-/pipelines/82891
• Commit: https://git.drupalcode.org/project/drupal/-/commit/60bf9f07cfc9344f972d9...
• After: SUCCESS https://git.drupalcode.org/issue/drupal-3379104/-/pipelines/82892

Will apply the suggested fix in this issue's MR now.

Thanks for the explanations about the different variations of the commands.

I think this is ready for review again. #31 shows how it fixes it in a failing MR.

Everything is rebased too.

Committing from NR as this is affecting multiple merge requests.

Committed and pushed 7986c7c8d6 to 11.x and e874b9620b to 10.2.x. Thanks!

#11 🐛 Spell-checking job fails with "Argument list too long" when too many files are changed Active did not help unfortunately, but in the end a rebase did.

git switch 11.x
git pull
git switch ISSUE-BRANCH
git rebase 11.x
git push --force-with-lease

Automatically closed - issue fixed for 2 weeks with no activity.

✨ Add gitignore(s) to Composer-ready project templates Needs work just got hit by this, even though the MR only modifies a few files ...

       * [new branch]        11.x       -> 11.x
$ export MODIFIED=`git diff --name-only refs/heads/$TARGET_BRANCH|while read r;do echo "$CI_PROJECT_DIR/$r";done|tr "\n" " "`
$ echo $MODIFIED | tr ' ' '\n' | yarn --cwd=./core run -s spellcheck:core --no-must-find-files --file-list stdin
/scripts-81307-1019042/step_script: line 286: /usr/bin/tr: Argument list too long
/scripts-81307-1019042/step_script: line 286: /usr/bin/yarn: Argument list too long

Cleaning up project directory and file based variables
ERROR: Job failed: command terminated with exit code 1

https://git.drupalcode.org/issue/drupal-3082958/-/pipelines/108333/failures