Skip to content
/ rector-src Public

[Caching] Fix caching on change config cache not cleared#3479

Closed
samsonasik wants to merge 1 commit intomainfrom
caching-fix-change-config-cached
Closed

[Caching] Fix caching on change config cache not cleared#3479
samsonasik wants to merge 1 commit intomainfrom
caching-fix-change-config-cached

Conversation

@samsonasik
Copy link
Copy Markdown
Member

@samsonasik samsonasik commented Mar 14, 2023

@jackbentley this should fix rectorphp/rector#7835.

The invalidateCacheIfConfigurationChanged() is called before save(), so before save(), if it still null, the cache data need to be cleared to ensure no left over cache

rector-src/packages/Caching/Detector/ChangedFilesDetector.php

Lines 116 to 118 in c240c69

$this->invalidateCacheIfConfigurationChanged($key, $configurationHash);
$this->cache->save($key, CacheKey::CONFIGURATION_HASH_KEY, $configurationHash);

@samsonasik
Copy link
Copy Markdown
Member Author

samsonasik commented Mar 14, 2023

All checks have passed 🎉 @TomasVotruba it is ready for review.

@samsonasik
Copy link
Copy Markdown
Member Author

samsonasik commented Mar 16, 2023

@jackbentley the issue seems due to sha1_file() usage, which on lookup root rector.php, it got overlapped relative and absolute path, I will create alternative separate PR to use normal sha1() to ensure it works on both relative and absolute path.

@samsonasik samsonasik mentioned this pull request Mar 16, 2023
Merged
@TomasVotruba TomasVotruba deleted the caching-fix-change-config-cached branch March 16, 2023 12:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@TomasVotruba TomasVotruba Awaiting requested review from TomasVotruba

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

File passes after config cache when it should fail because of cache

1 participant

@samsonasik