Skip to content

Fix LavaMoat config check#10514

Merged
Gudahtt merged 1 commit intodevelopfrom
fix-lavamoat-config-check
Feb 25, 2021
Merged

Fix LavaMoat config check#10514
Gudahtt merged 1 commit intodevelopfrom
fix-lavamoat-config-check

Conversation

@Gudahtt
Copy link
Copy Markdown
Member

@Gudahtt Gudahtt commented Feb 24, 2021

The CI script to ensure no LavaMoat policy changes are required has been failing despite there being no changes. It turns out that the command used to check for changes (git diff-index) was failing despite the lack of changes because the file was written again by yarn lavamoat:auto but git hadn't gotten around to updating its index since the write occurred, so it was considering it as changed until it verified it wasn't.

The command has been replaced by git diff --exit-code --quiet, whichshould do exactly the same thing except that it forces git to update its internal cache to verify whether changes are present.

The CI script to ensure no LavaMoat policy changes are required has
been failing despite there being no changes. It turns out that the
command used to check for changes (`git diff-index`) was failing
despite the lack of changes because the file was written again by
`yarn lavamoat:auto` but git hadn't gotten around to updating its index
since the write occurred, so it was considering it as changed until it
verified it wasn't [1].

The command has been replaced by `git diff --exit-code --quiet`, which
should do exactly the same thing except that it forces git to update
its internal cache to verify whether changes are present.

[1]: https://stackoverflow.com/questions/34807971/why-does-git-diff-index-head-result-change-for-touched-files-after-git-diff-or-g
@Gudahtt Gudahtt requested review from a team and kumavis as code owners February 24, 2021 21:12
@Gudahtt Gudahtt requested a review from darkwing February 24, 2021 21:12
@Gudahtt
Copy link
Copy Markdown
Member Author

Gudahtt commented Feb 24, 2021

I was able to test this by running the command:

touch package.json && git diff-index --quiet HEAD || echo "failed"

The command printed "failed" demonstrating a non-zero exit code.

I tested the new command like this:

touch package.json && git diff --exit-code --quiet || echo "failed"

The "failed" message was not printed, demonstrating a zero exit code.

@metamaskbot
Copy link
Copy Markdown
Collaborator

Builds ready [95bdb1c]
Page Load Metrics (610 ± 41 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint47796184
domContentLoaded3917656088641
load3927666108641
domInteractive3917646088641

@darkwing
Copy link
Copy Markdown
Contributor

Worked as described!

@Gudahtt Gudahtt merged commit 20b2c5f into develop Feb 25, 2021
@Gudahtt Gudahtt deleted the fix-lavamoat-config-check branch February 25, 2021 14:43
@github-actions github-actions bot locked and limited conversation to collaborators Feb 25, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants