Skip to content

Conversation

@Saadnajmi
Copy link
Collaborator

Please select one of the following

  • I am removing an existing difference between facebook/react-native and microsoft/react-native-macos 👍
  • I am cherry-picking a change from Facebook's react-native into microsoft/react-native-macos 👍
  • I am making a fix / change for the macOS implementation of react-native
  • I am making a change required for Microsoft usage of react-native

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.

@Saadnajmi Saadnajmi requested a review from a team as a code owner January 9, 2024 06:20
@FalseLobster
Copy link

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
Copy link
Member

tido64 commented Jan 10, 2024

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
Copy link
Collaborator Author

@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.

@Saadnajmi Saadnajmi merged commit 55f94d6 into microsoft:main Jan 13, 2024
@Saadnajmi Saadnajmi deleted the brew branch January 13, 2024 22:55
Saadnajmi added a commit to Saadnajmi/react-native-macos that referenced this pull request Jan 23, 2024
Saadnajmi added a commit to Saadnajmi/react-native-macos that referenced this pull request Jan 23, 2024
Saadnajmi added a commit that referenced this pull request Jan 23, 2024
…2048)

* [visionOS] Add local podspecs to override supported platforms

* Bump fmt to 9.1.0 (facebook#39799)

Summary:
This is what Folly is built against internally. Bump the version we use, and the standard we compile with, to take some different paths, and see if we fix some warnings caused by FMT with the ndk bump.

Changelog:
[General][Breaking] - Bump fmt to 9.1.0

Pull Request resolved: facebook#39799

Test Plan: Passes in CircleCI

Reviewed By: cortinico, yungsters

Differential Revision: D49900112

Pulled By: NickGerleman

fbshipit-source-id: 3f11080555ef20aeb9291d1096ffa6077b3b3bbd

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

* [CI] Remove watchman

---------

Co-authored-by: Nick Gerleman <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants