Skip to content

Commit

Permalink
Improve Codegen Cleanup (#35642)
Browse files Browse the repository at this point in the history
Summary:
This PR adds a safety check in case the Cocoapod build script is not able to clean the build folder.
We had evidences where this process failed, and in this way the user has a clear and actionable message to fix its situation.

## Changelog

[iOS][Added] - Add message with instructions about what to do if the cleanup of the build folder fails.

Pull Request resolved: #35642

Test Plan:
I was not able to reproduce the issue locally.
The fix is not destructive, let's see if the amount of issues decreases.

Reviewed By: dmytrorykun

Differential Revision: D42067939

Pulled By: cipolleschi

fbshipit-source-id: 433dbfaec42a1bf460dc9a48051aa51ec6e12d16
  • Loading branch information
Riccardo Cipolleschi authored and facebook-github-bot committed Dec 16, 2022
1 parent f249bee commit 1b7127b
Show file tree
Hide file tree
Showing 4 changed files with 69 additions and 6 deletions.
51 changes: 49 additions & 2 deletions scripts/cocoapods/__tests__/codegen_utils-test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -423,12 +423,59 @@ def testCleanUpCodegenFolder_whenFolderExists_deleteItAndSetCleanupDone
CodegenUtils.clean_up_build_folder(@base_path, codegen_dir)

# Assert
assert_equal(Dir.exist_invocation_params, [codegen_path])
assert_equal(Dir.glob_invocation, ["#{codegen_path}/*"])
assert_equal(Dir.exist_invocation_params, [codegen_path, codegen_path])
assert_equal(Dir.glob_invocation, ["#{codegen_path}/*", "#{codegen_path}/*"])
assert_equal(FileUtils::FileUtilsStorage.rmrf_invocation_count, 1)
assert_equal(FileUtils::FileUtilsStorage.rmrf_paths, [globs])
assert_equal(CodegenUtils.cleanup_done(), true)
end

# ===================================== #
# Test - Assert Codegen Folder Is Empty #
# ===================================== #

def test_assertCodegenFolderIsEmpty_whenItDoesNotExists_doesNotAbort
# Arrange
codegen_dir = "build/generated/ios"
codegen_path = "#{@base_path}/#{codegen_dir}"

# Act
CodegenUtils.assert_codegen_folder_is_empty(@base_path, codegen_dir)

# Assert
assert_equal(Pod::UI.collected_warns, [])
end

def test_assertCodegenFolderIsEmpty_whenItExistsAndIsEmpty_doesNotAbort
# Arrange
codegen_dir = "build/generated/ios"
codegen_path = "#{@base_path}/#{codegen_dir}"
Dir.mocked_existing_dirs(codegen_path)
Dir.mocked_existing_globs([], "#{codegen_path}/*")

# Act
CodegenUtils.assert_codegen_folder_is_empty(@base_path, codegen_dir)

# Assert
assert_equal(Pod::UI.collected_warns, [])
end

def test_assertCodegenFolderIsEmpty_whenItIsNotEmpty_itAborts
# Arrange
codegen_dir = "build/generated/ios"
codegen_path = "#{@base_path}/#{codegen_dir}"
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)
}

# 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."
])
end

private
Expand Down
6 changes: 5 additions & 1 deletion scripts/cocoapods/__tests__/test_utils/DirMock.rb
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,11 @@ def self.glob_invocation

def self.glob(path)
@@glob_invocation.push(path)
return @@mocked_existing_globs[path]
return @@mocked_existing_globs[path] != nil ? @@mocked_existing_globs[path] : []
end

def self.remove_mocked_paths(path)
@@mocked_existing_globs = @@mocked_existing_globs.select { |k, v| v != path }
end

def self.set_pwd(pwd)
Expand Down
5 changes: 3 additions & 2 deletions scripts/cocoapods/__tests__/test_utils/FileUtilsMock.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,9 @@
# This source code is licensed under the MIT license found in the
# LICENSE file in the root directory of this source tree.

module FileUtils

require_relative './DirMock.rb'

module FileUtils
class FileUtilsStorage
@@RMRF_INVOCATION_COUNT = 0
@@RMRF_PATHS = []
Expand Down Expand Up @@ -35,5 +35,6 @@ def self.reset
def self.rm_rf(path)
FileUtilsStorage.push_rmrf_path(path)
FileUtilsStorage.increase_rmrfi_invocation_count
Dir.remove_mocked_paths(path)
end
end
13 changes: 12 additions & 1 deletion scripts/cocoapods/codegen_utils.rb
Original file line number Diff line number Diff line change
Expand Up @@ -314,6 +314,17 @@ def self.clean_up_build_folder(app_path, codegen_dir)
return if !Dir.exist?(codegen_path)

FileUtils.rm_rf(Dir.glob("#{codegen_path}/*"))
CodegenUtils.set_cleanup_done(true)
CodegenUtils.assert_codegen_folder_is_empty(app_path, 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)
# double check that the files have actually been deleted.
# Emit an error message if not.
codegen_path = File.join(app_path, 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
end
end
end

0 comments on commit 1b7127b

Please sign in to comment.