Put build artifact tracking in integrity file - #3224
Conversation
bestander
left a comment
bestander
left a comment
There was a problem hiding this comment.
Reluctantly approve.
This fixes a bug which is a higher pri but integrity-checker needs some refactoring love.
I am eager to give it a look next week because I was planning to move integrity check to earlier before install starts.
| return true; | ||
| } | ||
|
|
||
| const {artifacts} = match; |
There was a problem hiding this comment.
Is it the right place?
bailout() function is overridden in add.js.
In the next couple of weeks I'd like to do integrity check earlier in install phase.
| integrityFileMissing: false, | ||
| integrityMatches, | ||
| missingPatterns, | ||
| artifacts: expected ? expected.artifacts : null, |
There was a problem hiding this comment.
Returning artifacts from check seems to break cohesion of the check function.
Makes sense from performance point of view because we already parse integrity file here but I would rather have a separate function that returns integrity file.
Side note: my fault that check command has side effects (reporter logs), I plan to refactor it.
|
Let's merge it now, I'll be refactoring integrity-checker next week |
2 participants
Summary
This moves build artifact tracking away from the cache metadata and into the integrity file.
I suspect this should fix #1981 as the cause seems to be that they restore
node_modulesand not Yarn's internal cache.Test plan
Existing tests.