From be0c353e18da3a0996f6e2fd3fecbe3852d3d240 Mon Sep 17 00:00:00 2001 From: Dmitry Rykun Date: Wed, 10 Jan 2024 08:38:50 -0800 Subject: [PATCH] CocoaPods: simplify iOS build directory cleanup (#42233) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/42233 This diff removes the need for providing the `ios_folder` argument to `use_react_native`. We no longer do any manual path tranformations to get the iOS project root. Instead we use `Pod::Config.instance.installation_root` which always points to the correct directory. Changelog: [iOS][Breaking] - CocoaPods: remove the `ios_folder` argument from the `use_react_native` function. Reviewed By: cipolleschi Differential Revision: D52659429 fbshipit-source-id: 67c79cd9d74a0351ad2c242b74cbd48b6bd2dc94 --- .../cocoapods/__tests__/codegen_utils-test.rb | 20 +++++++------------ .../scripts/cocoapods/codegen_utils.rb | 10 +++++----- .../react-native/scripts/react_native_pods.rb | 6 ++---- packages/rn-tester/Podfile | 1 - .../remove-new-arch-flags-fixture.js | 3 --- 5 files changed, 14 insertions(+), 26 deletions(-) diff --git a/packages/react-native/scripts/cocoapods/__tests__/codegen_utils-test.rb b/packages/react-native/scripts/cocoapods/__tests__/codegen_utils-test.rb index d2b650d8383b9e..cdb7474dbbf63f 100644 --- a/packages/react-native/scripts/cocoapods/__tests__/codegen_utils-test.rb +++ b/packages/react-native/scripts/cocoapods/__tests__/codegen_utils-test.rb @@ -391,11 +391,10 @@ def testCleanUpCodegenFolder_whenCleanupDone_doNothing # Arrange CodegenUtils.set_cleanup_done(true) codegen_dir = "build/generated/ios" - ios_folder = '.' rn_path = '../node_modules/react-native' # Act - CodegenUtils.clean_up_build_folder(rn_path, @base_path, ios_folder, codegen_dir, dir_manager: DirMock, file_manager: FileMock) + CodegenUtils.clean_up_build_folder(rn_path, codegen_dir, dir_manager: DirMock, file_manager: FileMock) # Assert assert_equal(FileUtils::FileUtilsStorage.rmrf_invocation_count, 0) @@ -407,11 +406,10 @@ def testCleanUpCodegenFolder_whenFolderDoesNotExists_markAsCleanupDone # Arrange CodegenUtils.set_cleanup_done(false) codegen_dir = "build/generated/ios" - ios_folder = '.' rn_path = '../node_modules/react-native' # Act - CodegenUtils.clean_up_build_folder(rn_path, @base_path, ios_folder, codegen_dir, dir_manager: DirMock, file_manager: FileMock) + CodegenUtils.clean_up_build_folder(rn_path, codegen_dir, dir_manager: DirMock, file_manager: FileMock) # Assert assert_equal(FileUtils::FileUtilsStorage.rmrf_invocation_count, 0) @@ -424,8 +422,7 @@ def testCleanUpCodegenFolder_whenFolderExists_deleteItAndSetCleanupDone # Arrange CodegenUtils.set_cleanup_done(false) codegen_dir = "build/generated/ios" - ios_folder = '.' - codegen_path = "#{@base_path}/./#{codegen_dir}" + codegen_path = "#{@base_path}/#{codegen_dir}" globs = [ "/MyModuleSpecs/MyModule.h", "#{codegen_path}/MyModuleSpecs/MyModule.mm", @@ -438,7 +435,7 @@ def testCleanUpCodegenFolder_whenFolderExists_deleteItAndSetCleanupDone DirMock.mocked_existing_globs(globs, "#{codegen_path}/*") # Act - CodegenUtils.clean_up_build_folder(rn_path, @base_path, ios_folder, codegen_dir, dir_manager: DirMock, file_manager: FileMock) + CodegenUtils.clean_up_build_folder(rn_path, codegen_dir, dir_manager: DirMock, file_manager: FileMock) # Assert assert_equal(DirMock.exist_invocation_params, [codegen_path, codegen_path]) @@ -460,10 +457,9 @@ def test_assertCodegenFolderIsEmpty_whenItDoesNotExists_doesNotAbort # Arrange codegen_dir = "build/generated/ios" codegen_path = "#{@base_path}/./#{codegen_dir}" - ios_folder = '.' # Act - CodegenUtils.assert_codegen_folder_is_empty(@base_path, ios_folder, codegen_dir, dir_manager: DirMock, file_manager: FileMock) + CodegenUtils.assert_codegen_folder_is_empty(codegen_path, dir_manager: DirMock) # Assert assert_equal(Pod::UI.collected_warns, []) @@ -473,12 +469,11 @@ def test_assertCodegenFolderIsEmpty_whenItExistsAndIsEmpty_doesNotAbort # Arrange codegen_dir = "build/generated/ios" codegen_path = "#{@base_path}/./#{codegen_dir}" - ios_folder = '.' DirMock.mocked_existing_dirs(codegen_path) DirMock.mocked_existing_globs([], "#{codegen_path}/*") # Act - CodegenUtils.assert_codegen_folder_is_empty(@base_path, ios_folder, codegen_dir, dir_manager: DirMock, file_manager: FileMock) + CodegenUtils.assert_codegen_folder_is_empty(codegen_path, dir_manager: DirMock) # Assert assert_equal(Pod::UI.collected_warns, []) @@ -488,13 +483,12 @@ def test_assertCodegenFolderIsEmpty_whenItIsNotEmpty_itAborts # Arrange codegen_dir = "build/generated/ios" codegen_path = "#{@base_path}/./#{codegen_dir}" - ios_folder = '.' DirMock.mocked_existing_dirs(codegen_path) DirMock.mocked_existing_globs(["#{codegen_path}/MyModuleSpecs/MyModule.mm",], "#{codegen_path}/*") # Act assert_raises() { - CodegenUtils.assert_codegen_folder_is_empty(@base_path, ios_folder, codegen_dir, dir_manager: DirMock, file_manager: FileMock) + CodegenUtils.assert_codegen_folder_is_empty(codegen_path, dir_manager: DirMock) } # Assert diff --git a/packages/react-native/scripts/cocoapods/codegen_utils.rb b/packages/react-native/scripts/cocoapods/codegen_utils.rb index bfae865e2ec252..f37864d7d42293 100644 --- a/packages/react-native/scripts/cocoapods/codegen_utils.rb +++ b/packages/react-native/scripts/cocoapods/codegen_utils.rb @@ -336,25 +336,25 @@ def self.cleanup_done return @@CLEANUP_DONE end - def self.clean_up_build_folder(rn_path, app_path, ios_folder, codegen_dir, dir_manager: Dir, file_manager: File) + def self.clean_up_build_folder(rn_path, codegen_dir, dir_manager: Dir, file_manager: File) return if CodegenUtils.cleanup_done() CodegenUtils.set_cleanup_done(true) - codegen_path = file_manager.join(app_path, ios_folder, codegen_dir) + ios_folder = Pod::Config.instance.installation_root.relative_path_from(Pathname.pwd) + codegen_path = file_manager.join(ios_folder, codegen_dir) return if !dir_manager.exist?(codegen_path) FileUtils.rm_rf(dir_manager.glob("#{codegen_path}/*")) base_provider_path = file_manager.join(rn_path, 'React', 'Fabric', 'RCTThirdPartyFabricComponentsProvider') FileUtils.rm_rf("#{base_provider_path}.h") FileUtils.rm_rf("#{base_provider_path}.mm") - CodegenUtils.assert_codegen_folder_is_empty(app_path, ios_folder, codegen_dir, dir_manager: dir_manager, file_manager: file_manager) + CodegenUtils.assert_codegen_folder_is_empty(codegen_path, dir_manager: dir_manager) end # Need to split this function from the previous one to be able to test it properly. - def self.assert_codegen_folder_is_empty(app_path, ios_folder, codegen_dir, dir_manager: Dir, file_manager: File) + def self.assert_codegen_folder_is_empty(codegen_path, dir_manager: Dir) # double check that the files have actually been deleted. # Emit an error message if not. - codegen_path = file_manager.join(app_path, ios_folder, codegen_dir) if dir_manager.exist?(codegen_path) && dir_manager.glob("#{codegen_path}/*").length() != 0 Pod::UI.warn "Unable to remove the content of #{codegen_path} folder. Please run rm -rf #{codegen_path} and try again." abort diff --git a/packages/react-native/scripts/react_native_pods.rb b/packages/react-native/scripts/react_native_pods.rb index 2f55c920cb709d..5944bb0aed73de 100644 --- a/packages/react-native/scripts/react_native_pods.rb +++ b/packages/react-native/scripts/react_native_pods.rb @@ -66,7 +66,6 @@ def prepare_react_native_project! # - hermes_enabled: whether Hermes should be enabled or not. # - app_path: path to the React Native app. Required by the New Architecture. # - config_file_dir: directory of the `package.json` file, required by the New Architecture. -# - ios_folder: the folder where the iOS code base lives. For a template app, it is `ios`, the default. For RNTester, it is `.`. def use_react_native! ( path: "../node_modules/react-native", fabric_enabled: false, @@ -74,8 +73,7 @@ def use_react_native! ( production: false, # deprecated hermes_enabled: ENV['USE_HERMES'] && ENV['USE_HERMES'] == '0' ? false : true, app_path: '..', - config_file_dir: '', - ios_folder: 'ios' + config_file_dir: '' ) # Set the app_path as env variable so the podspecs can access it. @@ -86,7 +84,7 @@ def use_react_native! ( # that has invoked the `use_react_native!` function. ReactNativePodsUtils.detect_use_frameworks(current_target_definition) - CodegenUtils.clean_up_build_folder(path, app_path, ios_folder, $CODEGEN_OUTPUT_DIR) + CodegenUtils.clean_up_build_folder(path, $CODEGEN_OUTPUT_DIR) # We are relying on this flag also in third parties libraries to proper install dependencies. # Better to rely and enable this environment flag if the new architecture is turned on using flags. diff --git a/packages/rn-tester/Podfile b/packages/rn-tester/Podfile index 7a9bda7f3f9ffa..38b8c251da3862 100644 --- a/packages/rn-tester/Podfile +++ b/packages/rn-tester/Podfile @@ -49,7 +49,6 @@ def pods(target_name, options = {}) app_path: "#{Dir.pwd}", config_file_dir: "#{Dir.pwd}/node_modules", production: false, #deprecated - ios_folder: '.', ) pod 'ReactCommon-Samples', :path => "#{@prefix_path}/ReactCommon/react/nativemodule/samples" diff --git a/scripts/releases/__tests__/__fixtures__/remove-new-arch-flags-fixture.js b/scripts/releases/__tests__/__fixtures__/remove-new-arch-flags-fixture.js index f04486b05e3ba6..b84ea462682c94 100644 --- a/scripts/releases/__tests__/__fixtures__/remove-new-arch-flags-fixture.js +++ b/scripts/releases/__tests__/__fixtures__/remove-new-arch-flags-fixture.js @@ -18,7 +18,6 @@ def use_react_native! ( flipper_configuration: FlipperConfiguration.disabled, app_path: '..', config_file_dir: '', - ios_folder: 'ios' ) end `; @@ -32,7 +31,6 @@ def use_react_native! ( flipper_configuration: FlipperConfiguration.disabled, app_path: '..', config_file_dir: '', - ios_folder: 'ios' ) end `; @@ -45,7 +43,6 @@ def use_react_native! ( flipper_configuration: FlipperConfiguration.disabled, app_path: '..', config_file_dir: '', - ios_folder: 'ios' ) end `;