search

microsoft / react-native-macos

A framework for building native macOS apps with React.
https://microsoft.github.io/react-native-windows/
MIT License
3.45k stars 132 forks source link

[CI] Install the latest version of packages in the brewfile #2024

Closed Saadnajmi closed 7 months ago

Saadnajmi commented 8 months ago

Please select one of the following

Summary:

I noticed that the CI as written would:

  1. Run brew bundle, and attempt to install the packages in .ado/brewfile
  2. Notice that there was a new version available, and try to upgrade in place
  3. Update the lock file in the CI job and continue on.

There's no point updating a lock file during a CI run, its point is to lock files. Furthermore, we don't actually care about the versions of the packages installed in our CI's brew file (in this case, just xcbeautify. Really, it's just a shorthand for brew install x; brew install y; ... which would have installed the latest version anyway. Let's just always install the latest version of things and ditch the lock file altogether.

Changelog:

[INTERNAL] [CHANGED] - Install the latest version of packages in the brewfile in CI

Test Plan:

CI should pass.

FalseLobster commented 8 months ago

Weirdly, it looks like the primary purpose of brewfile.lock.json isn't to lock specific versions, but rather serve as a debugging utility to evaluate how environment changes might break homebrew. I think it it's only read on failures or useful for doing a diff or something to facilitate error reporting. So we should probably have this in our azure artifacts as part of CI for brew bundle failures and I'm not sure it even makes sense in git. Although tbh, I'm not opposed to removing it from CI either, but I just wanted to share with you the reading I had already done:

See the docs here and discussion here.

tido64 commented 8 months ago

Not checking in Brewfile.lock.json sounds fine to me. We should probably keep the generation of it in case we need to debug.

Saadnajmi commented 8 months ago

@Falselobster @tido64 Interesting, thanks for the insight! I updated the PR to still generate the lock file, and also print it out during CI. This PR was inspired by seeing a one-off segmentation fault during one brew bundle step in an earlier job, so I see the point in adding more debug information.