Skip to content

Commit

Permalink
Fix cleanup not working on template app (#35679)
Browse files Browse the repository at this point in the history
Summary:
Pull Request resolved: #35679

The Codegen cleanup step was not always working due to an issue with the codegen folder path. It was working for RNTester, but not for apps created from the Template.

## Changelog:
[iOS][Fixed] - Fix path issue to properly run the codegen cleanup step

Reviewed By: jacdebug

Differential Revision: D42137600

fbshipit-source-id: ba4cb03d4c6eb17fda70a4aff383908d2e468429
  • Loading branch information
cipolleschi authored and Riccardo Cipolleschi committed Dec 19, 2022
1 parent f7b35c0 commit ce3eefe
Show file tree
Hide file tree
Showing 4 changed files with 29 additions and 19 deletions.
3 changes: 2 additions & 1 deletion packages/rn-tester/Podfile
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,8 @@ def pods(target_name, options = {}, use_flipper: !IN_CI && !USE_FRAMEWORKS)
flipper_configuration: use_flipper ? FlipperConfiguration.enabled : FlipperConfiguration.disabled,
app_path: "#{Dir.pwd}",
config_file_dir: "#{Dir.pwd}/node_modules",
production: !ENV['PRODUCTION'].nil?
production: !ENV['PRODUCTION'].nil?,
ios_folder: '.',
)
pod 'ReactCommon/turbomodule/samples', :path => "#{@prefix_path}/ReactCommon"

Expand Down
28 changes: 17 additions & 11 deletions scripts/cocoapods/__tests__/codegen_utils-test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -380,9 +380,10 @@ def testCleanUpCodegenFolder_whenCleanupDone_doNothing
# Arrange
CodegenUtils.set_cleanup_done(true)
codegen_dir = "build/generated/ios"
ios_folder = '.'

# Act
CodegenUtils.clean_up_build_folder(@base_path, codegen_dir)
CodegenUtils.clean_up_build_folder(@base_path, ios_folder, codegen_dir)

# Assert
assert_equal(FileUtils::FileUtilsStorage.rmrf_invocation_count, 0)
Expand All @@ -394,9 +395,10 @@ def testCleanUpCodegenFolder_whenFolderDoesNotExists_markAsCleanupDone
# Arrange
CodegenUtils.set_cleanup_done(false)
codegen_dir = "build/generated/ios"
ios_folder = '.'

# Act
CodegenUtils.clean_up_build_folder(@base_path, codegen_dir)
CodegenUtils.clean_up_build_folder(@base_path, ios_folder, codegen_dir)

# Assert
assert_equal(FileUtils::FileUtilsStorage.rmrf_invocation_count, 0)
Expand All @@ -409,7 +411,8 @@ def testCleanUpCodegenFolder_whenFolderExists_deleteItAndSetCleanupDone
# Arrange
CodegenUtils.set_cleanup_done(false)
codegen_dir = "build/generated/ios"
codegen_path = "#{@base_path}/#{codegen_dir}"
ios_folder = '.'
codegen_path = "#{@base_path}/./#{codegen_dir}"
globs = [
"/MyModuleSpecs/MyModule.h",
"#{codegen_path}/MyModuleSpecs/MyModule.mm",
Expand All @@ -420,7 +423,7 @@ def testCleanUpCodegenFolder_whenFolderExists_deleteItAndSetCleanupDone
Dir.mocked_existing_globs(globs, "#{codegen_path}/*")

# Act
CodegenUtils.clean_up_build_folder(@base_path, codegen_dir)
CodegenUtils.clean_up_build_folder(@base_path, ios_folder, codegen_dir)

# Assert
assert_equal(Dir.exist_invocation_params, [codegen_path, codegen_path])
Expand All @@ -437,10 +440,11 @@ def testCleanUpCodegenFolder_whenFolderExists_deleteItAndSetCleanupDone
def test_assertCodegenFolderIsEmpty_whenItDoesNotExists_doesNotAbort
# Arrange
codegen_dir = "build/generated/ios"
codegen_path = "#{@base_path}/#{codegen_dir}"
codegen_path = "#{@base_path}/./#{codegen_dir}"
ios_folder = '.'

# Act
CodegenUtils.assert_codegen_folder_is_empty(@base_path, codegen_dir)
CodegenUtils.assert_codegen_folder_is_empty(@base_path, ios_folder, codegen_dir)

# Assert
assert_equal(Pod::UI.collected_warns, [])
Expand All @@ -449,12 +453,13 @@ def test_assertCodegenFolderIsEmpty_whenItDoesNotExists_doesNotAbort
def test_assertCodegenFolderIsEmpty_whenItExistsAndIsEmpty_doesNotAbort
# Arrange
codegen_dir = "build/generated/ios"
codegen_path = "#{@base_path}/#{codegen_dir}"
codegen_path = "#{@base_path}/./#{codegen_dir}"
ios_folder = '.'
Dir.mocked_existing_dirs(codegen_path)
Dir.mocked_existing_globs([], "#{codegen_path}/*")

# Act
CodegenUtils.assert_codegen_folder_is_empty(@base_path, codegen_dir)
CodegenUtils.assert_codegen_folder_is_empty(@base_path, ios_folder, codegen_dir)

# Assert
assert_equal(Pod::UI.collected_warns, [])
Expand All @@ -463,18 +468,19 @@ def test_assertCodegenFolderIsEmpty_whenItExistsAndIsEmpty_doesNotAbort
def test_assertCodegenFolderIsEmpty_whenItIsNotEmpty_itAborts
# Arrange
codegen_dir = "build/generated/ios"
codegen_path = "#{@base_path}/#{codegen_dir}"
codegen_path = "#{@base_path}/./#{codegen_dir}"
ios_folder = '.'
Dir.mocked_existing_dirs(codegen_path)
Dir.mocked_existing_globs(["#{codegen_path}/MyModuleSpecs/MyModule.mm",], "#{codegen_path}/*")

# Act
assert_raises() {
CodegenUtils.assert_codegen_folder_is_empty(@base_path, codegen_dir)
CodegenUtils.assert_codegen_folder_is_empty(@base_path, ios_folder, codegen_dir)
}

# Assert
assert_equal(Pod::UI.collected_warns, [
"Unable to remove the content of ~/app/ios/build/generated/ios folder. Please run rm -rf ~/app/ios/build/generated/ios and try again."
"Unable to remove the content of ~/app/ios/./build/generated/ios folder. Please run rm -rf ~/app/ios/./build/generated/ios and try again."
])
end

Expand Down
10 changes: 5 additions & 5 deletions scripts/cocoapods/codegen_utils.rb
Original file line number Diff line number Diff line change
Expand Up @@ -306,22 +306,22 @@ def self.cleanup_done
return @@CLEANUP_DONE
end

def self.clean_up_build_folder(app_path, codegen_dir)
def self.clean_up_build_folder(app_path, ios_folder, codegen_dir)
return if CodegenUtils.cleanup_done()
CodegenUtils.set_cleanup_done(true)

codegen_path = File.join(app_path, codegen_dir)
codegen_path = File.join(app_path, ios_folder, codegen_dir)
return if !Dir.exist?(codegen_path)

FileUtils.rm_rf(Dir.glob("#{codegen_path}/*"))
CodegenUtils.assert_codegen_folder_is_empty(app_path, codegen_dir)
CodegenUtils.assert_codegen_folder_is_empty(app_path, ios_folder, codegen_dir)
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, codegen_dir)
def self.assert_codegen_folder_is_empty(app_path, ios_folder, codegen_dir)
# double check that the files have actually been deleted.
# Emit an error message if not.
codegen_path = File.join(app_path, codegen_dir)
codegen_path = File.join(app_path, ios_folder, codegen_dir)
if Dir.exist?(codegen_path) && Dir.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
Expand Down
7 changes: 5 additions & 2 deletions scripts/react_native_pods.rb
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ def prepare_react_native_project!
# - flipper_configuration: The configuration to use for flipper.
# - 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,
Expand All @@ -59,13 +60,15 @@ def use_react_native! (
hermes_enabled: ENV['USE_HERMES'] && ENV['USE_HERMES'] == '0' ? false : true,
flipper_configuration: FlipperConfiguration.disabled,
app_path: '..',
config_file_dir: '')
config_file_dir: '',
ios_folder: 'ios'
)

# Current target definition is provided by Cocoapods and it refers to the target
# that has invoked the `use_react_native!` function.
ReactNativePodsUtils.detect_use_frameworks(current_target_definition)

CodegenUtils.clean_up_build_folder(app_path, $CODEGEN_OUTPUT_DIR)
CodegenUtils.clean_up_build_folder(app_path, ios_folder, $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.
Expand Down

0 comments on commit ce3eefe

Please sign in to comment.