Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Yoga.podspec has unstable checksum #43220

Closed
stianjensen opened this issue Feb 27, 2024 · 15 comments
Closed

Yoga.podspec has unstable checksum #43220

stianjensen opened this issue Feb 27, 2024 · 15 comments
Labels
Type: Upgrade Issue Issues reported from upgrade issue form

Comments

@stianjensen
Copy link
Contributor

stianjensen commented Feb 27, 2024

Old Version

0.72.10

New Version

0.73.5

Description

When perform an upgrade from 0.72.x to 0.73.x I get a difference in build output between my local build and the build we're running on our CI. The checksum for the Yoga.podspec is different (I have ran pod update Yoga locally to ensure this is not due to caching).

After comparing the generated podspec json file for Yoga locally with the resolved one on CI I can see that the ordering of the elements of the private_header_files key is different. On my CI the file names are sorted alphabetically, while locally the order I've gotten seems sort of arbitrary.

My local ruby version is:
ruby --version
ruby 2.7.8p225 (2023-03-30 revision 1f4d455848) [arm64-darwin22]

On CI we're running:
Default Ruby: 3.1.4

So the difference ruby versions may or may not be relevant here.

According to the ruby documentation, Dir.glob does not guarantee the order of its returned values:
https://ruby-doc.org/core-2.6.3/Dir.html#method-c-glob

Case sensitivity depends on your system (File::FNM_CASEFOLD is ignored), as does the order in which the results are returned.

Use of that was introduced in 33ebb5d, which first shipped as part of RN 0.73

Steps to reproduce

Run pod install in a fresh react native 0.73 repo locally and commit the lockfile.
When running on CI or another computer, you'll get a diff in the lockfile (or an error if you're running with the flag --deployment).

Affected Platforms

Build - MacOS

Output of npx react-native info

System:
  OS: macOS 13.6.4
  CPU: (10) arm64 Apple M2 Pro
  Memory: 94.03 MB / 16.00 GB
  Shell:
    version: "5.9"
    path: /bin/zsh
Binaries:
  Node:
    version: 20.10.0
    path: ~/.nvm/versions/node/v20.10.0/bin/node
  Yarn:
    version: 1.22.19
    path: /opt/homebrew/bin/yarn
  npm:
    version: 10.2.3
    path: ~/.nvm/versions/node/v20.10.0/bin/npm
  Watchman:
    version: 2023.12.04.00
    path: /opt/homebrew/bin/watchman
Managers:
  CocoaPods:
    version: 1.13.0
    path: /Users/stiaje/.rbenv/shims/pod
SDKs:
  iOS SDK:
    Platforms:
      - DriverKit 23.2
      - iOS 17.2
      - macOS 14.2
      - tvOS 17.2
      - visionOS 1.0
      - watchOS 10.2
  Android SDK:
    API Levels:
      - "23"
      - "26"
      - "27"
      - "28"
      - "29"
      - "30"
      - "31"
      - "33"
    Build Tools:
      - 23.0.1
      - 25.0.2
      - 25.0.3
      - 26.0.1
      - 26.0.2
      - 27.0.0
      - 27.0.3
      - 28.0.2
      - 28.0.3
      - 29.0.2
      - 30.0.2
      - 30.0.3
      - 31.0.0
      - 33.0.0
    System Images:
      - android-30 | Google APIs Intel x86 Atom
      - android-31 | Google APIs Intel x86 Atom_64
    Android NDK: Not Found
IDEs:
  Android Studio: 2022.3 AI-223.8836.35.2231.11005911
  Xcode:
    version: 15.2/15C500b
    path: /usr/bin/xcodebuild
Languages:
  Java:
    version: 17.0.6
    path: /Applications/Android Studio.app/Contents/jbr/Contents/Home/bin/javac
  Ruby:
    version: 2.7.8
    path: /Users/stiaje/.rbenv/shims/ruby
npmPackages:
  "@react-native-community/cli": Not Found
  react:
    installed: 18.2.0
    wanted: 18.2.0
  react-native:
    installed: 0.73.5
    wanted: 0.73.5
  react-native-macos: Not Found
npmGlobalPackages:
  "*react-native*": Not Found
Android:
  hermesEnabled: true
  newArchEnabled: false
iOS:
  hermesEnabled: true
  newArchEnabled: false

Stacktrace or Logs

> pod install
[...]
Verifying no changes
[!] There were changes to the lockfile in deployment mode:
SPEC CHECKSUMS:
  Yoga:
    New Lockfile: a716eea57d0d3430219c0a5a233e1e93ee931eb7
    Old Lockfile: 9e6a04eacbd94f97d94577017e9f23b3ab41cf6c

make: *** [install] Error 1

Reproducer

https://github.com/stianjensen/test-rn-73-pod-install

(The reproduce also shows a diff in the glog spec. That is caused by GitHub actions CI running Xcode 14.2 while I'm running Xcode 15 locally. This difference affects the DEFINES_MODULE flag in the glog podspec)

Screenshots and Videos

No response

@stianjensen stianjensen added Needs: Triage 🔍 Type: Upgrade Issue Issues reported from upgrade issue form labels Feb 27, 2024
@stianjensen
Copy link
Contributor Author

I applied the following patch locally (with patch-package), and it seems to have made things more stable for me (with a new checksum 134d218bd3cc6a4efa72eccf363aa13817749e92):

diff --git a/node_modules/react-native/ReactCommon/yoga/Yoga.podspec b/node_modules/react-native/ReactCommon/yoga/Yoga.podspec
index 2288fbc..61bd492 100644
--- a/node_modules/react-native/ReactCommon/yoga/Yoga.podspec
+++ b/node_modules/react-native/ReactCommon/yoga/Yoga.podspec
@@ -59,6 +59,6 @@ Pod::Spec.new do |spec|
   # Fabric must be able to access private headers (which should not be included in the umbrella header)
   all_header_files = 'yoga/**/*.h'
   all_header_files = File.join('ReactCommon/yoga', all_header_files) if ENV['INSTALL_YOGA_WITHOUT_PATH_OPTION']
-  spec.private_header_files = Dir.glob(all_header_files) - Dir.glob(public_header_files)
+  spec.private_header_files = Dir.glob(all_header_files).sort_by(&:upcase) - Dir.glob(public_header_files).sort_by(&:upcase)
   spec.preserve_paths = [all_header_files]
 end

@cortinico
Copy link
Contributor

Related to #31121

I recall that @cipolleschi fixed this type of issues in the entire codebase. Perhaps we still have some absolute paths in the Yoga podspec? Also cc @NickGerleman

@stianjensen
Copy link
Contributor Author

To me it looks like this particular case is not related to absolute paths, but rather something with the ordering (The checksum is a sha1 calculated from the json representation of the podspec, so ordering of content in that file is also important).

Checking for absolute paths was the first thing I looked for, though, given the history of this problem.

@cortinico
Copy link
Contributor

The checksum is a sha1 calculated from the json representation of the podspec, so ordering of content in that file is also important

I'll defer to the CocoaPods experts to answer here.

@cipolleschi
Copy link
Contributor

Yeah, @stianjensen is right. I think that something changed in how glob works. To exclude the problem, can you try to use the same ruby version you have in CI? You can use rbenv or any other ruby version manager to check that.

@stianjensen
Copy link
Contributor Author

stianjensen commented Mar 5, 2024

Good point!

Upgrading from 2.7.8, which I've been running locally, to 3.0.6, which is what is default on the GitHub actions runners did indeed make the checksums match up:
stianjensen/test-rn-73-pod-install#2

With that PR I'm down to only the glog issue (which is because of a mismatch in Xcode version between locally and CI (over vs under Xcode 14.3 – that one may be more tricky to resolve, and I guess it's more fair to require all builds of your project to run on the same or similar Xcode version, for more reasons than this).

@cipolleschi
Copy link
Contributor

Can you lionk the glog issue please? 😅

@stianjensen
Copy link
Contributor Author

Can you lionk the glog issue please? 😅

I haven't created one for that, yet, I actually just discovered that once I was setting up a reproduce repository for this issue!

Not sure if it's reported by anyone else yet! I can create a new separate issue for that, though, with a link to a separate branch on my reproduce repo!

@stianjensen
Copy link
Contributor Author

I did mention the issue here:
#31121 (comment)

But that thread is a long list of several unrelated sources of unstable checksums, which may make it not very actionable anymore.
So here is a new issue specifically for that issue with the glog podspec:
reactwg/react-native-releases#122

(I don't know how that ended up in another repo than this issue, I'm pretty sure I just clicked 'new issue' in the same way as last week 🤷)

@Mutai-Gilbert
Copy link

Did you find a solution to the above issue?

@cipolleschi
Copy link
Contributor

@Mutai-Gilbert there is no solution, sadly.
The checksum depends on how glob is implemented in ruby. So I suggest you update your app adding a .ruby-version file to pin ruby to the specific version you and your team is using.
That way, you all will be using the same version of ruby that CI is using and the checksum will be stable.

We can't do that in React Native: we tried already but it created more problem than the one it solved. We can set a ruby version that is good for everyone that's using React Native.

@NickGerleman
Copy link
Contributor

NickGerleman commented Apr 25, 2024

@Mutai-Gilbert feel free to contribute a PR which sorts glob() result. Though, I bet there might be other areas of non-determinism in the build.

@zgordon02
Copy link

I have the same issue with RN .74.3. Yoga checksum is different from GH Actions/my machine. Ruby, pod, and xcode versions are all the same. I'm at a loss of how to solve this

@zgordon02
Copy link

I applied the following patch locally (with patch-package), and it seems to have made things more stable for me (with a new checksum 134d218bd3cc6a4efa72eccf363aa13817749e92):

diff --git a/node_modules/react-native/ReactCommon/yoga/Yoga.podspec b/node_modules/react-native/ReactCommon/yoga/Yoga.podspec
index 2288fbc..61bd492 100644
--- a/node_modules/react-native/ReactCommon/yoga/Yoga.podspec
+++ b/node_modules/react-native/ReactCommon/yoga/Yoga.podspec
@@ -59,6 +59,6 @@ Pod::Spec.new do |spec|
   # Fabric must be able to access private headers (which should not be included in the umbrella header)
   all_header_files = 'yoga/**/*.h'
   all_header_files = File.join('ReactCommon/yoga', all_header_files) if ENV['INSTALL_YOGA_WITHOUT_PATH_OPTION']
-  spec.private_header_files = Dir.glob(all_header_files) - Dir.glob(public_header_files)
+  spec.private_header_files = Dir.glob(all_header_files).sort_by(&:upcase) - Dir.glob(public_header_files).sort_by(&:upcase)
   spec.preserve_paths = [all_header_files]
 end

This worked for me

@sheltonsuen
Copy link

I applied the following patch locally (with patch-package), and it seems to have made things more stable for me (with a new checksum 134d218bd3cc6a4efa72eccf363aa13817749e92):

diff --git a/node_modules/react-native/ReactCommon/yoga/Yoga.podspec b/node_modules/react-native/ReactCommon/yoga/Yoga.podspec
index 2288fbc..61bd492 100644
--- a/node_modules/react-native/ReactCommon/yoga/Yoga.podspec
+++ b/node_modules/react-native/ReactCommon/yoga/Yoga.podspec
@@ -59,6 +59,6 @@ Pod::Spec.new do |spec|
   # Fabric must be able to access private headers (which should not be included in the umbrella header)
   all_header_files = 'yoga/**/*.h'
   all_header_files = File.join('ReactCommon/yoga', all_header_files) if ENV['INSTALL_YOGA_WITHOUT_PATH_OPTION']
-  spec.private_header_files = Dir.glob(all_header_files) - Dir.glob(public_header_files)
+  spec.private_header_files = Dir.glob(all_header_files).sort_by(&:upcase) - Dir.glob(public_header_files).sort_by(&:upcase)
   spec.preserve_paths = [all_header_files]
 end

This worked for me

This worked for me too

lourou added a commit to ephemeraHQ/converse-app that referenced this issue Oct 25, 2024
lourou added a commit to ephemeraHQ/converse-app that referenced this issue Oct 25, 2024
lourou added a commit to ephemeraHQ/converse-app that referenced this issue Oct 28, 2024
lourou added a commit to ephemeraHQ/converse-app that referenced this issue Oct 29, 2024
lourou added a commit to ephemeraHQ/converse-app that referenced this issue Oct 29, 2024
lourou added a commit to ephemeraHQ/converse-app that referenced this issue Oct 31, 2024
* Add `colors.scrim` to design system for bottom sheet backdrop component

* Implement bottom sheet backdrop, top safe area, dynamic height/size

* Add icon variant `subtle` for bottom sheet closing button style

* Update the reaction detail types and display the reactions in the bottom sheet

* Render the reactions drawer only when needed

* Cleanup unused code

* Auto-close reactions drawer when index reaches 0, add left and right padding to reactions ScrollView

The reactions ScrollView did not scroll far enough to the right, hiding some of the reaction emojis. This commit adds a filler View with a fixed width of 64px as the last child of the ScrollView to ensure it can scroll all the way to the right, revealing the previously hidden content

* refactor and fix modal

* make cleaner

* oop

* remove background on scrollview since we have on bottom sheeet

* Revert because Yoga.podspec has unstable checksum

facebook/react-native#43220

* Fix typo and add accessibility

* Use path alias, add comment on reaction types for clarity

* Small fixes, add more accessiblity

* Zustand logic update

* fix: EAS Build Fixes (#1099)

* remove testflight action

* fix eas

* oops

* Set to remote

* add platform checks

* fixes

* Expo is great

---------

Co-authored-by: Thierry <[email protected]>

* WIP implement `BottomSheetModal`

* Add `BottomSheetModalProvider`

* Make icon slightly smaller to fit figma's icon size

* Move the bottom sheet modal provider at the app root, so it shows up on top of everything else

* Dismiss keyboard when reactions drawer opens, better error handling

* Refactor `MessageReactionsDrawer` component and zustand store for updated `RolledUpReactions` structure

- Updated `RolledUpReactions` type definition to remove deprecated fields and add `count` in `DetailedReaction`
- Fetch profile information for reactors
- Modified `useMemo` logic in `MessageReactionsDrawer` to include `preview` with top reactions sorted by count
- Adjusted `zustand` store initial state to match the updated `RolledUpReactions` structure
- Fixed map error in component by ensuring `detailed` array has data before rendering

* Fix android build config

* Optimizing memo comparison

The current memo comparison checks each field individually. Consider using a shallow comparison of the entire reactions object since it's already immutable (as evident from the Zustand store pattern mentioned in the PR summary)

* Updated `RolledUpReactions` data model to include:
 - `preview` with counts for each reaction type
 - `detailed` as a sorted array, with user’s own reactions appearing first

* Implement drawer logic and UI for filtered reactions view in `MessageReactionsDrawer`

- Added filter functionality to display specific reactions based on selected content
- Introduced "All" button in preview section to reset filter and show all reactions
- Refactored `BottomSheetScrollView` to:
    - Display horizontal preview of all reactions with counts
    - Render detailed list of reactors for each reaction type
    - Apply filtering based on selected reaction type, with user’s own reactions highlighted at the top
- Adjusted styling to indicate active filters, improving user interaction and readability

* Reset reaction filter on dismiss, replace `ScrollView` with the one from `react-native-gesture-handler`

* Reaction chips styling and UI states

* Extract `ReactionPreview` type

* Detailed reactions styling, fallback with `shortAddress$ if no username is set, simplify type

* Small fixes

* Memoizing the backgroundStyle array

* Refactor MessageReactionsDrawer to improve scrolling behavior

- Implement `FlashList`
- Wrapped `FlashList` in `BottomSheetScrollView` to enhance scroll management
- Enabled nested scrolling for FlashList to ensure smooth scrolling within BottomSheet
- Adjusted `contentContainerStyle` to respect safe area insets

* Resolve diff for android build

* GestureHandlerRootView styling

* Build config revert

---------

Co-authored-by: Thierry <[email protected]>
Co-authored-by: Alex Risch <[email protected]>
technoplato pushed a commit to ephemeraHQ/converse-app that referenced this issue Dec 17, 2024
* Add `colors.scrim` to design system for bottom sheet backdrop component

* Implement bottom sheet backdrop, top safe area, dynamic height/size

* Add icon variant `subtle` for bottom sheet closing button style

* Update the reaction detail types and display the reactions in the bottom sheet

* Render the reactions drawer only when needed

* Cleanup unused code

* Auto-close reactions drawer when index reaches 0, add left and right padding to reactions ScrollView

The reactions ScrollView did not scroll far enough to the right, hiding some of the reaction emojis. This commit adds a filler View with a fixed width of 64px as the last child of the ScrollView to ensure it can scroll all the way to the right, revealing the previously hidden content

* refactor and fix modal

* make cleaner

* oop

* remove background on scrollview since we have on bottom sheeet

* Revert because Yoga.podspec has unstable checksum

facebook/react-native#43220

* Fix typo and add accessibility

* Use path alias, add comment on reaction types for clarity

* Small fixes, add more accessiblity

* Zustand logic update

* fix: EAS Build Fixes (#1099)

* remove testflight action

* fix eas

* oops

* Set to remote

* add platform checks

* fixes

* Expo is great

---------

Co-authored-by: Thierry <[email protected]>

* WIP implement `BottomSheetModal`

* Add `BottomSheetModalProvider`

* Make icon slightly smaller to fit figma's icon size

* Move the bottom sheet modal provider at the app root, so it shows up on top of everything else

* Dismiss keyboard when reactions drawer opens, better error handling

* Refactor `MessageReactionsDrawer` component and zustand store for updated `RolledUpReactions` structure

- Updated `RolledUpReactions` type definition to remove deprecated fields and add `count` in `DetailedReaction`
- Fetch profile information for reactors
- Modified `useMemo` logic in `MessageReactionsDrawer` to include `preview` with top reactions sorted by count
- Adjusted `zustand` store initial state to match the updated `RolledUpReactions` structure
- Fixed map error in component by ensuring `detailed` array has data before rendering

* Fix android build config

* Optimizing memo comparison

The current memo comparison checks each field individually. Consider using a shallow comparison of the entire reactions object since it's already immutable (as evident from the Zustand store pattern mentioned in the PR summary)

* Updated `RolledUpReactions` data model to include:
 - `preview` with counts for each reaction type
 - `detailed` as a sorted array, with user’s own reactions appearing first

* Implement drawer logic and UI for filtered reactions view in `MessageReactionsDrawer`

- Added filter functionality to display specific reactions based on selected content
- Introduced "All" button in preview section to reset filter and show all reactions
- Refactored `BottomSheetScrollView` to:
    - Display horizontal preview of all reactions with counts
    - Render detailed list of reactors for each reaction type
    - Apply filtering based on selected reaction type, with user’s own reactions highlighted at the top
- Adjusted styling to indicate active filters, improving user interaction and readability

* Reset reaction filter on dismiss, replace `ScrollView` with the one from `react-native-gesture-handler`

* Reaction chips styling and UI states

* Extract `ReactionPreview` type

* Detailed reactions styling, fallback with `shortAddress$ if no username is set, simplify type

* Small fixes

* Memoizing the backgroundStyle array

* Refactor MessageReactionsDrawer to improve scrolling behavior

- Implement `FlashList`
- Wrapped `FlashList` in `BottomSheetScrollView` to enhance scroll management
- Enabled nested scrolling for FlashList to ensure smooth scrolling within BottomSheet
- Adjusted `contentContainerStyle` to respect safe area insets

* Resolve diff for android build

* GestureHandlerRootView styling

* Build config revert

---------

Co-authored-by: Thierry <[email protected]>
Co-authored-by: Alex Risch <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Upgrade Issue Issues reported from upgrade issue form
Projects
None yet
Development

No branches or pull requests

7 participants