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

build: less node_modules recopy from nix #19120

Merged
merged 2 commits into from
Mar 7, 2024

Conversation

yqrashawn
Copy link
Contributor

@yqrashawn yqrashawn commented Mar 6, 2024

Summary

rebuild clojure nix shell won't break running build after this PR

  • these files in node_modules always got changed after make run-ios (maybe by rn?)

    • react-native/ReactCommon/react/renderer/components/rncore/ShadowNodes.cpp
    • react-native/ReactCommon/react/renderer/components/rncore/Props.h
    • react-native/ReactCommon/react/renderer/components/rncore/Props.cpp
    • react-native/ReactCommon/react/renderer/components/rncore/ComponentDescriptors.h
    • react-native/ReactCommon/react/renderer/components/rncore/ShadowNodes.h
    • react-native/ReactCommon/react/renderer/components/rncore/EventEmitters.h
    • react-native/ReactCommon/react/renderer/components/rncore/States.h
    • react-native/ReactCommon/react/renderer/components/rncore/EventEmitters.cpp
    • react-native/ReactCommon/react/renderer/components/rncore/States.cpp
    • react-native/ReactCommon/react/renderer/components/rncore/RCTComponentViewHelpers.h
    • react-native-config/ios/ReactNativeConfig/GeneratedDotEnv.m

    skip these file in nix/scripts/node_modules

  • yarn install is used in tests to trigger node-pre-gyp rebuild for modules/react-native-status/nodejs/bindings.js
    changed yarn install to node-pre-gpy rebuild

status: ready

@status-im-auto
Copy link
Member

status-im-auto commented Mar 6, 2024

Jenkins Builds

Click to see older builds (12)
Commit #️⃣ Finished (UTC) Duration Platform Result
2add473 #1 2024-03-06 13:35:03 ~5 min tests 📄log
✔️ 2add473 #1 2024-03-06 13:37:35 ~7 min android-e2e 🤖apk 📲
✔️ 2add473 #1 2024-03-06 13:39:24 ~9 min android 🤖apk 📲
✔️ 2add473 #1 2024-03-06 13:40:16 ~10 min ios 📱ipa 📲
88d7e70 #2 2024-03-06 13:45:12 ~1 min tests 📄log
✔️ 88d7e70 #2 2024-03-06 13:51:04 ~7 min ios 📱ipa 📲
✔️ 88d7e70 #2 2024-03-06 13:51:28 ~8 min android-e2e 🤖apk 📲
✔️ 88d7e70 #2 2024-03-06 13:51:51 ~8 min android 🤖apk 📲
✔️ 8d86f7d #3 2024-03-06 14:08:20 ~6 min android-e2e 🤖apk 📲
✔️ 8d86f7d #3 2024-03-06 14:08:22 ~6 min tests 📄log
✔️ 8d86f7d #3 2024-03-06 14:08:26 ~6 min android 🤖apk 📲
✔️ 8d86f7d #3 2024-03-06 14:09:25 ~7 min ios 📱ipa 📲
Commit #️⃣ Finished (UTC) Duration Platform Result
17c4933 #4 2024-03-07 01:54:51 ~1 min tests 📄log
✔️ 17c4933 #4 2024-03-07 01:59:20 ~6 min android-e2e 🤖apk 📲
✔️ 17c4933 #4 2024-03-07 01:59:27 ~6 min android 🤖apk 📲
✔️ 17c4933 #4 2024-03-07 02:00:37 ~7 min ios 📱ipa 📲
✔️ 1096e6c #5 2024-03-07 04:53:13 ~6 min tests 📄log
✔️ 1096e6c #5 2024-03-07 04:53:23 ~6 min android-e2e 🤖apk 📲
✔️ 1096e6c #5 2024-03-07 04:54:18 ~7 min android 🤖apk 📲
✔️ 1096e6c #5 2024-03-07 04:54:40 ~7 min ios 📱ipa 📲

@yqrashawn yqrashawn force-pushed the build/less-node_modules-recopy branch from 2add473 to 88d7e70 Compare March 6, 2024 13:43
@yqrashawn yqrashawn changed the title build: less recopy node_modules from nix build: less node_modules recopy from nix Mar 6, 2024
@yqrashawn
Copy link
Contributor Author

[2024-03-06T13:43:57.603Z] ../modules/react-native-status/nodejs/status.cpp:8:10: fatal error: ../../../result/libstatus.h: No such file or directory
missing make status-go-library

@yqrashawn yqrashawn force-pushed the build/less-node_modules-recopy branch from 88d7e70 to 8d86f7d Compare March 6, 2024 14:01
@yqrashawn yqrashawn self-assigned this Mar 6, 2024
@yqrashawn yqrashawn marked this pull request as ready for review March 6, 2024 15:00
@yqrashawn yqrashawn requested a review from jakubgs as a code owner March 6, 2024 15:00
@yqrashawn yqrashawn force-pushed the build/less-node_modules-recopy branch from 8d86f7d to 17c4933 Compare March 7, 2024 01:52
Copy link
Contributor

@siddarthkay siddarthkay left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for improving this bit @yqrashawn

@siddarthkay
Copy link
Contributor

@yqrashawn : I also saw unit tests fail, I have not verified the node-gyp-pre rebuild stuff.
can you pls check?

@yqrashawn yqrashawn force-pushed the build/less-node_modules-recopy branch from 17c4933 to 1096e6c Compare March 7, 2024 04:46
@yqrashawn
Copy link
Contributor Author

Thanks for the review @siddarthkay doc added and bug fixed. I misspelled pre-gyp to gyp-pre

@yqrashawn yqrashawn merged commit 29e69c5 into develop Mar 7, 2024
6 checks passed
@yqrashawn yqrashawn deleted the build/less-node_modules-recopy branch March 7, 2024 05:30
Copy link
Member

@jakubgs jakubgs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Neat. But it would make more sense to comment next to the lines the comment affects.
Otherwise you can miss it, or accidentally remove it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants