From 944eb252c3f15e2835de112c4bf66e0f9c14e185 Mon Sep 17 00:00:00 2001
From: rachguo <rachguo@rachguos-Mini.attlocal.net>
Date: Thu, 14 Mar 2024 22:40:25 -0700
Subject: [PATCH] address pr comments

---
 ...> maccatalyst_prepare_objects_for_prelink.py} |  8 +++++---
 cmake/onnxruntime.cmake                          | 16 +++++++++-------
 2 files changed, 14 insertions(+), 10 deletions(-)
 rename cmake/{handle_duplicate_object_files.py => maccatalyst_prepare_objects_for_prelink.py} (83%)

diff --git a/cmake/handle_duplicate_object_files.py b/cmake/maccatalyst_prepare_objects_for_prelink.py
similarity index 83%
rename from cmake/handle_duplicate_object_files.py
rename to cmake/maccatalyst_prepare_objects_for_prelink.py
index a87c53502a926..34664b4e05237 100644
--- a/cmake/handle_duplicate_object_files.py
+++ b/cmake/maccatalyst_prepare_objects_for_prelink.py
@@ -7,7 +7,8 @@
 import sys
 
 
-# Note: This script is mainly used for handling extracting duplicate named .o files under different subdirectories for
+# Note: This script is mainly used for sanity checking/validating the files in the .a library equal to the .o files
+# in the source dir to handle the case of source files having duplicate names under different subdirectories for
 # each onnxruntime library. (Only applicable when doing a Mac Catalyst build.)
 def main():
     source_dir = sys.argv[1]
@@ -23,16 +24,17 @@ def main():
 
                 dest_file = f"{dest_name_without_extension}.o"
                 while os.path.exists(os.path.join(dest_dir, dest_file)):
-                    print("Duplicate named files: " + os.path.join(dest_dir, dest_file))
+                    print("Duplicate file name from source: " + os.path.join(source_dir, subdir, file_name))
                     counter += 1
                     dest_file = f"{dest_name_without_extension}_{counter}.o"
+                    print("Renamed file name in destination: " + os.path.join(dest_dir, dest_file))
 
                 destination_path = os.path.join(dest_dir, dest_file)
                 source_file = os.path.join(source_dir, subdir, file_name)
                 shutil.copy(source_file, destination_path)
 
     # Sanity check to ensure the number of .o object from the original cmake source directory matches with the number
-    # of .o files extracted from each onnxruntime library
+    # of .o files extracted from each .a onnxruntime library
     file_lists_from_static_lib = []
     with open(files_from_static_lib) as file:
         filenames = file.readlines()
diff --git a/cmake/onnxruntime.cmake b/cmake/onnxruntime.cmake
index 6fac840be22df..e15c8a046dc20 100644
--- a/cmake/onnxruntime.cmake
+++ b/cmake/onnxruntime.cmake
@@ -314,17 +314,19 @@ if(onnxruntime_BUILD_APPLE_FRAMEWORK)
       add_custom_command(TARGET onnxruntime POST_BUILD
                          COMMAND ${CMAKE_COMMAND} -E make_directory ${CUR_STATIC_LIB_OBJ_DIR})
       if (PLATFORM_NAME STREQUAL "macabi")
-        # Handle the case where extracting .o files with duplicate names under different subdirectories within
+        # There exists several duplicate names for source files under different subdirectories within
         # each onnxruntime library. (e.g. onnxruntime/contrib_ops/cpu/element_wise_ops.o
         # vs. onnxruntime/providers/core/cpu/math/element_wise_ops.o)
-        # Simply use 'ar ARGS -x' for extracting the .o files would possibly cause duplicate naming files being overwritten.
+        # In that case, using 'ar ARGS -x' to extract the .o files from .a lib would possibly cause duplicate naming files being overwritten
+        # and lead to missing undefined symbol error in the generated binary.
+        # So we use the below python script as a sanity check to do a recursive find of all .o files in ${CUR_TARGET_CMAKE_SOURCE_LIB_DIR}
+        # and verifies that matches the content of the .a, and then copy from the source dir.
+        # TODO: The copying action here isn't really necessary. For future fix, consider using the script extracts from the ar with the rename to potentially
+        # make both maccatalyst and other builds do the same thing.
         set(CUR_TARGET_CMAKE_SOURCE_LIB_DIR ${CMAKE_CURRENT_BINARY_DIR}/CMakeFiles/${_LIB}.dir)
         add_custom_command(TARGET onnxruntime POST_BUILD
-                          COMMAND ar -t $<TARGET_FILE:${_LIB}> | grep "\.o$"  > object_file_list.txt
-                          WORKING_DIRECTORY ${CUR_STATIC_LIB_OBJ_DIR})
-        add_custom_command(TARGET onnxruntime POST_BUILD
-                          COMMAND mkdir -p ${CUR_STATIC_LIB_OBJ_DIR}
-                          COMMAND ${CMAKE_COMMAND} -E env python3 ${CMAKE_CURRENT_SOURCE_DIR}/handle_duplicate_object_files.py ${CUR_TARGET_CMAKE_SOURCE_LIB_DIR} ${CUR_STATIC_LIB_OBJ_DIR} ${CUR_STATIC_LIB_OBJ_DIR}/object_file_list.txt
+                          COMMAND ar -t $<TARGET_FILE:${_LIB}> | grep "\.o$"  > ${_LIB}.object_file_list.txt
+                          COMMAND ${CMAKE_COMMAND} -E env python3 ${CMAKE_CURRENT_SOURCE_DIR}/maccatalyst_prepare_objects_for_prelink.py ${CUR_TARGET_CMAKE_SOURCE_LIB_DIR} ${CUR_STATIC_LIB_OBJ_DIR} ${CUR_STATIC_LIB_OBJ_DIR}/${_LIB}.object_file_list.txt
                           WORKING_DIRECTORY ${CUR_STATIC_LIB_OBJ_DIR})
       else()
         add_custom_command(TARGET onnxruntime POST_BUILD