Problem/Motivation

When building asset aggregate URLs, we use a combination of library versions for libraries that have them, and file contents for libraries that don't - this is to save hashing every file in an aggregate.

After #3505776: Retain asset aggregates for 45 days by default to reduce generation of identical files it is a bit more awkward if you update a file without updating a version for a library, because drush cr doesn't automatically clear the aggregate files. There are ways to disable the behaviour in local dev etc. but it can be a gotcha.

Thinking about whether to open this issue at all, I suddenly realised that all of core's own libraries are defined with VERSION which changes every patch release, even though the individual files often don't change for months or years.

This means that for core libraries, the logic is doing the opposite of what we want it to - results in more changes to aggregate filenames rather than less.

We can remove VERSION from all core libraries, but given it would also make things easier for contrib/custom themes and modules, I think always using the file contents is probably easier. Can profile it to see what the impact is.

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

Issue fork drupal-3589208

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

catch created an issue. See original summary.

catch’s picture

Status: Active » Needs review

smustgrave’s picture

Status: Needs review » Needs work
       ├ Exception: Warning: file_get_contents(themes/my_theme/css/dropbutton.css): Failed to open stream: No such file or directory
       ├ Drupal\Core\Asset\CssCollectionOptimizerLazy->generateHash()() (Line: 45)
       ├ Exception: Warning: file_get_contents(core/modules/system/tests/themes/test_theme/kitten.css): Failed to open stream: No such file or directory
       ├ Drupal\Core\Asset\CssCollectionOptimizerLazy->generateHash()() (Line: 45)

Seems related to the change here?

catch’s picture

Status: Needs work » Needs review

Yes very related. Added a file_exists() here because aggregate generation itself also has to cope with missing files already - so even though the failures are to some extent 'invalid' library definitions, I think we want to allow 'stub' library definitions to not break the hash generation logic.

smustgrave’s picture

Status: Needs review » Reviewed & tested by the community

Seems to have fixed the issue LGTM

alexpott’s picture

Status: Reviewed & tested by the community » Needs work

Small performance tweak suggested on the MR.