Fix concurrency issue in static content deploy

[convenient]

Summary

This fixes a bug in which multiple concurrent processes spin up to handle the same theme package, depending on how the themes are defined with their parents.

We saw a lot of issues like

DEBUG: STDERR:Compilation from source /path/to/some.js failed
Magento\Framework\Exception\FileSystemException: The file or directory "/app/var/view_preprocessed/pub/static/frontend/path/to/some.js" cannot be copied to "/app/pub/static/frontend/path/to/some.js"

We also saw less compilation intermittently failing to generate complete styles-m.css and styles-l.css files, as multiple processes were fighting and doing work on the same file at the same time. This cause the frontend to be deployed in a broken manner which needed a redeploy to solve.

For affected setups this may also improve the performance of this process, by the idea that we have a process no longer doing duplicated work.

To reproduce

Reproduce

Clone https://github.com/convenient/magento-bug-reproduction-static-content-concurrency then run composer install, you can now generate static content.

Without the bugfix we can see multiple executions of the same package, see that frontend/Custom/ztheme/default is executed multiple times

$ rm -rf pub/static var/view_preprocessed/ var/cache ; php bin/magento > /dev/null ;  php ./bin/magento setup:static-content:deploy --no-ansi --no-interaction -f -s compact -vvv --jobs 4 --no-html-minify en_US en_GB en_IE -vvv | grep -E 'Execute: frontend/Custom/ztheme|Prevent'
Execute: frontend/Custom/ztheme/default
Execute: frontend/Custom/ztheme/default
Execute: frontend/Custom/ztheme/en_US
Execute: frontend/Custom/ztheme/en_GB
Execute: frontend/Custom/ztheme/en_IE

With the bugfix we can see each is only executed once

$ rm -rf pub/static var/view_preprocessed/ var/cache ; php bin/magento > /dev/null ;  php ./bin/magento setup:static-content:deploy --no-ansi --no-interaction -f -s compact -vvv --jobs 4 --no-html-minify en_US en_GB en_IE -vvv | grep -E 'Execute: frontend/Custom/ztheme|Prevent'
Execute: frontend/Custom/ztheme/default
Preventing duplicate execution of package as it is in progress: frontend/Custom/ztheme/default (pid: 67830)
Execute: frontend/Custom/ztheme/en_US
Execute: frontend/Custom/ztheme/en_GB
Execute: frontend/Custom/ztheme/en_IE

Verification

I have used this fix on production for over a year without any issues. But we can also double check on the example reproduction repository provided above.

Get a baseline

rm -rf pub/static/
php ./bin/magento setup:static-content:deploy --no-ansi --no-interaction -f -s compact -vvv --jobs 4 --no-html-minify en_US en_GB
find ./pub/static -type f -exec md5sum {} + | awk '{print $1, $2}' > before.txt

Apply the bugfix then do the following

rm -rf pub/static/
php ./bin/magento setup:static-content:deploy --no-ansi --no-interaction -f -s compact -vvv --jobs 4 --no-html-minify en_US en_GB
find ./pub/static -type f -exec md5sum {} + | awk '{print $1, $2}' > after.txt

See that all files (that matter) were generated the same

$ diff  before.txt after.txt
1c1
< 04d5dfe8b06b35834eeb526128ef11ba ./pub/static/deployed_version.txt
---
> b5e5f1122730b93197d32286a8dc65ab ./pub/static/deployed_version.txt

Contribution checklist (*)

• Pull request has a meaningful description of its purpose
• All commits are accompanied by meaningful commit messages
• All new or changed code is covered with unit/integration tests (if applicable)
• README.md files for modified modules are updated and included in the pull request if any README.md predefined sections require an update
• All automated tests passed successfully (all builds are green)

[m2-assistant]

Hi @convenient. Thank you for your contribution!
Here are some useful tips on how you can test your changes using Magento test environment.
❗ Automated tests can be triggered manually with an appropriate comment:

• @magento run all tests - run or re-run all required tests against the PR changes
• @magento run <test-build(s)> - run or re-run specific test build(s)
  For example: @magento run Unit Tests

<test-build(s)> is a comma-separated list of build names.

Allowed build names are:

1. Database Compare
2. Functional Tests CE
3. Functional Tests EE
4. Functional Tests B2B
5. Integration Tests
6. Magento Health Index
7. Sample Data Tests CE
8. Sample Data Tests EE
9. Sample Data Tests B2B
10. Static Tests
11. Unit Tests
12. WebAPI Tests
13. Semantic Version Checker

You can find more information about the builds here
ℹ️ Run only required test builds during development. Run all test builds before sending your pull request for review.

---

For more details, review the Code Contributions documentation.
Join Magento Community Engineering Slack and ask your questions in #github channel.

[convenient]

@magento run all tests

[convenient]

@magento run Functional Tests B2B, Functional Tests CE, Functional Tests EE

[convenient]

@magento run all tests

[convenient]

The static analysis is failing with the following.

Can I please have a recommendation for the correct approach here, given that none of these issues relate to my code change.

    | WARNING | Copyright is missing or Copyright content/year is not valid
 27 | WARNING | Visibility must be declared on all constants if your project supports PHP 7.1 or later
 32 | WARNING | Visibility must be declared on all constants if your project supports PHP 7.1 or later

[hostep]

Every file touched in a PR should adhere to the latest static test checks, even if it's not code you touched, annoying, but it is what it is I'm afraid.

• for the constants, just prepend them with public so they remain backwards compatible in case people use them in customisations
• About copyright header: Please make copyright header check better magento-coding-standard#492

[engcom-Hotel]

@magento run all tests

[Copilot]

Pull Request Overview

This PR fixes a concurrency bug during static content deployment, ensuring that the same theme package is not handled by multiple concurrent processes.

• Prevents duplicate execution of a theme package by checking if it has already been scheduled.
• Adds a debug log to indicate when duplicate execution is prevented.

Comments suppressed due to low confidence (1)

app/code/Magento/Deploy/Process/Queue.php:266

• Add a brief comment explaining that the absence of the package in the $packages array indicates it is already in progress, to aid future maintainers in understanding why duplicate execution is being prevented.

if (!isset($packages[$name])) {

[ihor-sviziev]

Hi @convenient,
Thank you so much for the contribution! Looks good

Could you please fix static test failures as described in #39954 (comment)

[engcom-Dash]

@magento run all tests

[engcom-Dash]

@convenient & @ihor-sviziev I have fixed the static test cases also added the comment above the if statement. The other failing test cases seems flaky to me. Could you please review the changes ?

Moving it back to pending review. Thanks!

[convenient]

@magento run Unit Tests, Functional Tests B2B, Functional Tests CE, Functional Tests EE