Allow packaging step to specify an explicit binutils format, and add some configuration options to build_desktop (#241)

Specifying an explicit binutils format fixes an issue in Windows builds where some libraries would be autodetected, but then would create invalid output libraries since the target format wasn't specified.

This was causing a linker issue with absl in Firestore with vector deleting destructors not being found, because certain absl libraries were not created valid by merge_libraries. In particular, Windows libraries are now set to be output in pe-bigobj-x86-64/pe-bigobj-i386 format, since Firestore in particular has >65K sections and requires bigobj. There's no harm in making all the other libraries the same format.

This PR also adds a couple of flags to build_desktop.py:

It adds support for performing a verbose build during packaging, in case it's needed for further debugging.
Adds a flag for disabling vcpkg, although we don't yet use it as we require vcpkg to get the "protoc" executable.
It also includes a bit of code in the workflow to handle Mac arm64 although we don't yet build for that platform.

Author: jonsimantov

.github/workflows/cpp-packaging.yml

@@ -11,6 +11,9 @@ on:
       preserveIntermediateArtifacts:
         description: 'preserve intermediate artifacts?'
         default: 0
+      verboseBuild:
+        description: 'verbose build?'
+        default: 0
       skipIntegrationTests:
         description: 'skip integration tests?'
         default: 0
@@ -60,6 +63,12 @@ jobs:
           github.event.inputs.downloadPublicVersion == '' && github.event.inputs.downloadPreviousRun == ''
         run: echo "::warning ::Intermediate artifacts will be preserved."
 
+      - name: log if verbose build enabled
+        if: |
+          github.event.inputs.verboseBuild != 0 && github.event.inputs.verboseBuild != '' &&
+          github.event.inputs.downloadPublicVersion == '' && github.event.inputs.downloadPreviousRun == ''
+        run: echo "::warning ::Verbose build enabled."
+
   build_tools:
     name: build-tools-${{ matrix.tools_platform }}
     runs-on: ${{ matrix.os }}
@@ -320,7 +329,11 @@ jobs:
       - name: Build desktop SDK
         shell: bash
         run: |
-          python scripts/gha/build_desktop.py --arch "${{ matrix.architecture }}" --config "${{ matrix.build_type }}" --msvc_runtime_library "${{ matrix.msvc_runtime }}" --linux_abi "${{ matrix.linux_abi }}" --build_dir out-${{ env.SDK_NAME }} ${{ matrix.additional_build_flags }}
+          verbose_flag=
+          if [[ -n "${{ github.event.inputs.verboseBuild }}" && "${{ github.event.inputs.verboseBuild }}" -ne 0 ]]; then
+            verbose_flag=--verbose
+          fi
+          python scripts/gha/build_desktop.py --arch "${{ matrix.architecture }}" --config "${{ matrix.build_type }}" --msvc_runtime_library "${{ matrix.msvc_runtime }}" --linux_abi "${{ matrix.linux_abi }}" --build_dir out-${{ env.SDK_NAME }} ${verbose_flag} ${{ matrix.additional_build_flags }}
           # Make a list of all the source files, for debugging purposes.
           cd out-${{ env.SDK_NAME }}
           find .. -type f -print > src_file_list.txt
@@ -433,12 +446,30 @@ jobs:
           else
             tools_platform=darwin
           fi
+          verbose_flag=
+          if [[ -n "${{ github.event.inputs.verboseBuild }}" && "${{ github.event.inputs.verboseBuild }}" -ne 0 ]]; then
+            verbose_flag=-v
+          fi
+          declare -a additional_flags
           tar -xvzf artifacts/packaging-tools-${tools_platform}/packaging-tools.tgz -C bin
           chmod -R u+x bin
           for pkg in artifacts/firebase-cpp-sdk-${{ matrix.sdk_platform }}${{ matrix.suffix }}*-build/*.tgz; do
             # determine the build variant based on the artifact filename
             variant=$(sdk-src/build_scripts/desktop/get_variant.sh "${pkg}")
-            sdk-src/build_scripts/desktop/package.sh -b ${pkg} -o firebase-cpp-sdk-${{ matrix.sdk_platform }}${{ matrix.suffix }}-package -p ${{ matrix.sdk_platform }} -t bin -d ${variant} -P python3 -j
+            additional_flags=(${verbose_flag})
+            # Several build targets require explicitly-set binutils format to be passed
+            # to package.sh (and thus, to merge_libraries).
+            if [[ "${{ matrix.sdk_platform }}" == "darwin" && "${variant}" == "arm64" ]]; then
+              # MacOS ARM
+              additional_flags+=(-f mach-o-arm64)
+            elif [[ "${{ matrix.sdk_platform }}" == "windows" && "${variant}" == *"/x64/"* ]]; then
+              # Windows x64
+              additional_flags+=(-f pe-x86-64,pe-bigobj-x86-64)
+            elif [[ "${{ matrix.sdk_platform }}" == "windows" && "${variant}" == *"/x86/"* ]]; then
+              # Windows x86
+              additional_flags+=(-f pe-i386,pe-bigobj-i386)
+            fi
+            sdk-src/build_scripts/desktop/package.sh -b ${pkg} -o firebase-cpp-sdk-${{ matrix.sdk_platform }}${{ matrix.suffix }}-package -p ${{ matrix.sdk_platform }} -t bin -d ${variant} -P python3 -j ${additional_flags[*]}
           done
           if [[ "${{ matrix.sdk_platform }}" == "darwin" ]]; then
             # Darwin has a final step after all the variants are done,

build_scripts/desktop/package.sh

@@ -13,6 +13,7 @@ options:
   -m, merge_libraries.py path                     default: <script dir>/../../scripts/merge_libraries.py
   -P, python command                              default: python
   -t, packaging tools directory                   default: ~/bin
+  -f, binutils format                             default: [auto-detect]
   -j, run merge_libraries jobs in parallel
   -v, enable verbose mode
 example:
@@ -29,6 +30,7 @@ root_dir=$(cd $(dirname $0)/../..; pwd -P)
 merge_libraries_script=${root_dir}/scripts/merge_libraries.py
 tools_path=~/bin
 built_sdk_tarfile=
+binutils_format=
 temp_dir=
 run_in_parallel=0
 
@@ -44,8 +46,11 @@ abspath(){
     fi
 }
 
-while getopts "b:o:p:d:m:P:t:hjv" opt; do
+while getopts "f:b:o:p:d:m:P:t:hjv" opt; do
     case $opt in
+        f)
+            binutils_format=$OPTARG
+            ;;
         b)
             built_sdk_path=$OPTARG
             ;;
@@ -252,6 +257,11 @@ fi
 if [[ ${verbose} -eq 1 ]]; then
     merge_libraries_params+=(--verbosity=3)
 fi
+if [[ -n "${binutils_format}" ]]; then
+    merge_libraries_params+=(--force_binutils_target="${binutils_format}")
+fi
+
+
 
 if [[ ! -x "${binutils_objcopy}" || ! -x "${binutils_ar}" || ! -x "${binutils_nm}" ]]; then
     echo "Packaging tools not found at path '${tools_path}'."
@@ -327,12 +337,12 @@ for product in ${product_list[*]}; do
     rm -f "${outfile}"
     if [[ ${verbose} -eq 1 ]]; then
       echo "${python_cmd}" "${merge_libraries_script}" \
-		    ${merge_libraries_params[*]} \
+                    ${merge_libraries_params[*]} \
                     ${cache_param} \
-		    --output="${outfile}" \
-		    --scan_libs="${allfiles}" \
-		    --hide_c_symbols="${deps_hidden}" \
-		    ${libfile_src} ${deps[*]}
+                    --output="${outfile}" \
+                    --scan_libs="${allfiles}" \
+                    --hide_c_symbols="${deps_hidden}" \
+                    ${libfile_src} ${deps[*]}
     fi
     # Place the merge command in a script so we can optionally run them in parallel.
     echo "#!/bin/bash -e" > "${merge_libraries_tmp}/merge_${product}.sh"
@@ -342,7 +352,7 @@ for product in ${product_list[*]}; do
       echo "echo \"${libfile_out}\"" >> "${merge_libraries_tmp}/merge_${product}.sh"
     fi
     if [[ ! -z ${deps_basenames[*]} ]]; then
-	echo -n  >> "${merge_libraries_tmp}/merge_${product}.sh"
+        echo -n  >> "${merge_libraries_tmp}/merge_${product}.sh"
     fi
     echo >> "${merge_libraries_tmp}/merge_${product}.sh"
     echo "\"${python_cmd}\" \\

cmake/external_rules.cmake

@@ -148,6 +148,12 @@ function(build_external_dependencies)
         -DCMAKE_BUILD_TYPE="${CMAKE_BUILD_TYPE}")
   endif()
 
+  if (CMAKE_VERBOSE_MAKEFILE)
+    # If verbose mode was enabled, pass it on
+    set(CMAKE_SUB_CONFIGURE_OPTIONS ${CMAKE_SUB_CONFIGURE_OPTIONS}
+        "-DCMAKE_VERBOSE_MAKEFILE=${CMAKE_VERBOSE_MAKEFILE}")
+  endif()
+
   if(APPLE)
     # Propagate MacOS build flags.
     if(CMAKE_OSX_ARCHITECTURES)

scripts/gha/build_desktop.py

@@ -141,7 +141,8 @@ def install_cpp_dependencies_with_vcpkg(arch, msvc_runtime_library, cleanup=True
     utils.clean_vcpkg_temp_data()
 
 def cmake_configure(build_dir, arch, msvc_runtime_library='static', linux_abi='legacy',
-                    build_tests=True, config=None, target_format=None):
+                    build_tests=True, config=None, target_format=None,
+                    disable_vcpkg=False, verbose=False):
   """ CMake configure.
 
   If you are seeing problems when running this multiple times,
@@ -156,6 +157,8 @@ def cmake_configure(build_dir, arch, msvc_runtime_library='static', linux_abi='l
    config (str): Release/Debug config.
           If its not specified, cmake's default is used (most likely Debug).
    target_format (str): If specified, build for this targetformat ('frameworks' or 'libraries').
+   disable_vcpkg (bool): If True, skip vcpkg and just use CMake for deps.
+   verbose (bool): If True, enable verbose mode in the CMake file.
   """
   cmd = ['cmake', '-S', '.', '-B', build_dir]
 
@@ -170,19 +173,18 @@ def cmake_configure(build_dir, arch, msvc_runtime_library='static', linux_abi='l
     # workaround, absl doesn't build without tests enabled
     cmd.append('-DBUILD_TESTING=off')
 
-  if utils.is_linux_os() and arch == 'x86':
-    # Use a separate cmake toolchain for cross compiling linux x86 builds
-    vcpkg_toolchain_file_path = os.path.join(os.getcwd(), 'external', 'vcpkg',
-                                             'scripts', 'buildsystems', 'linux_32.cmake')
-  else:
-    vcpkg_toolchain_file_path = os.path.join(os.getcwd(), 'external',
-                                           'vcpkg', 'scripts',
-                                           'buildsystems', 'vcpkg.cmake')
-
-  cmd.append('-DCMAKE_TOOLCHAIN_FILE={0}'.format(vcpkg_toolchain_file_path))
-
-  vcpkg_triplet = utils.get_vcpkg_triplet(arch, msvc_runtime_library)
-  cmd.append('-DVCPKG_TARGET_TRIPLET={0}'.format(vcpkg_triplet))
+  if not disable_vcpkg:
+    if utils.is_linux_os() and arch == 'x86':
+      # Use a separate cmake toolchain for cross compiling linux x86 builds
+      vcpkg_toolchain_file_path = os.path.join(os.getcwd(), 'external', 'vcpkg',
+                                               'scripts', 'buildsystems', 'linux_32.cmake')
+    else:
+      vcpkg_toolchain_file_path = os.path.join(os.getcwd(), 'external',
+                                               'vcpkg', 'scripts',
+                                               'buildsystems', 'vcpkg.cmake')
+    cmd.append('-DCMAKE_TOOLCHAIN_FILE={0}'.format(vcpkg_toolchain_file_path))
+    vcpkg_triplet = utils.get_vcpkg_triplet(arch, msvc_runtime_library)
+    cmd.append('-DVCPKG_TARGET_TRIPLET={0}'.format(vcpkg_triplet))
 
   if utils.is_windows_os():
     # If building for x86, we should supply -A Win32 to cmake configure
@@ -202,31 +204,38 @@ def cmake_configure(build_dir, arch, msvc_runtime_library='static', linux_abi='l
     cmd.append('-DFIREBASE_XCODE_TARGET_FORMAT={0}'.format(target_format))
 
   cmd.append('-DFIREBASE_USE_BORINGSSL=ON')
+
+  # Print out every command while building.
+  if verbose:
+    cmd.append('-DCMAKE_VERBOSE_MAKEFILE=1')
+
   utils.run_command(cmd)
 
 def main():
   args = parse_cmdline_args()
 
   # Ensure that the submodules are initialized and updated
   # Example: vcpkg is a submodule (external/vcpkg)
-  utils.run_command(['git', 'submodule', 'init'])
-  utils.run_command(['git', 'submodule', 'update'])
+  if not args.disable_vcpkg:
+    utils.run_command(['git', 'submodule', 'init'])
+    utils.run_command(['git', 'submodule', 'update'])
 
   # To build x86 on x86_64 linux hosts, we also need x86 support libraries
   if args.arch == 'x86' and utils.is_linux_os():
     install_x86_support_libraries()
 
-  # Install C++ dependencies using vcpkg
-  install_cpp_dependencies_with_vcpkg(args.arch, args.msvc_runtime_library,
-                                      cleanup=True)
+  if not args.disable_vcpkg:
+    # Install C++ dependencies using vcpkg
+    install_cpp_dependencies_with_vcpkg(args.arch, args.msvc_runtime_library,
+                                        cleanup=True)
 
   if args.vcpkg_step_only:
     print("Exiting without building the Firebase C++ SDK as just vcpkg step was requested.")
     return
 
   # CMake configure
   cmake_configure(args.build_dir, args.arch, args.msvc_runtime_library, args.linux_abi,
-                  args.build_tests, args.config, args.target_format)
+                  args.build_tests, args.config, args.target_format, args.disable_vcpkg, args.verbose)
 
   # CMake build
   # cmake --build build -j 8
@@ -238,14 +247,6 @@ def main():
     cmd.append('--target')
     cmd.extend(args.target)
   utils.run_command(cmd)
-  # Copy libraries from appropriate vcpkg directory to build output
-  # directory for later inclusion.
-  vcpkg_path = ('external/vcpkg/installed/%s/%slib/' %
-                (utils.get_vcpkg_triplet(args.arch, args.msvc_runtime_library),
-                 'debug/' if args.config == 'Debug' else ''))
-  if (os.path.exists(vcpkg_path)):
-    shutil.rmtree('vcpkg-libs', ignore_errors=True)
-    shutil.copytree(vcpkg_path, os.path.join(args.build_dir, 'vcpkg-libs'), )
 
 
 def parse_cmdline_args():
@@ -257,6 +258,8 @@ def parse_cmdline_args():
                       help='C++ ABI for Linux (legacy or c++11)')
   parser.add_argument('--build_dir', default='build', help='Output build directory')
   parser.add_argument('--build_tests', action='store_true', help='Build unit tests too')
+  parser.add_argument('--verbose', action='store_true', help='Enable verbose CMake builds.')
+  parser.add_argument('--disable_vcpkg', action='store_true', help='Disable vcpkg and just use CMake.')
   parser.add_argument('--vcpkg_step_only', action='store_true', help='Just install cpp packages using vcpkg and exit.')
   parser.add_argument('--config', default='Release', help='Release/Debug config')
   parser.add_argument('--target', nargs='+', help='A list of CMake build targets (eg: firebase_app firebase_auth)')

scripts/merge_libraries.py

@@ -99,6 +99,10 @@
     "skip_creating_archives", False,
     "Skip creating archive files (.a or .lib) and instead just leave the object "
     "files (.o or .obj) in the output directory.")
+flags.DEFINE_string("force_binutils_target", None, "Force all binutils calls to "
+                    "use the given target, via the --target flag. If not set, "
+                    "will autodetect target format. If you want to specify "
+                    "different input and output formats, separate them with a comma.")
 
 # Never rename 'std::' by default when --auto_hide_cpp_namespaces is enabled.
 IMPLICIT_CPP_NAMESPACES_TO_IGNORE = {"std"}
@@ -266,7 +270,7 @@ def create_archive(output_archive_file, object_files, old_archive=None):
     Empty list if there are no errors, or error text if there was an error.
   """
   errors = []
-  if old_archive and FLAGS.platform != "windows":
+  if old_archive and FLAGS.platform != "windows" and FLAGS.platform != "darwin":
     # Copy the old archive to the new archive, then clear the files from it.
     # This preserves the file format of the old archive file.
     # On Windows, we'll always create a new archive.
@@ -641,6 +645,9 @@ def rename_symbol(symbol):
           new_symbol = re.sub("@%s@@" % ns, "@%s@@" % new_ns, new_symbol)
       new_renames[symbol] = new_symbol
   else:
+    if FLAGS.platform == "windows" and symbol.startswith("$LN"):
+      # Don't rename $LN*, those are local symbols.
+      return new_renames
     # C symbol. Just split, rename, and re-join.
     (prefix, remainder) = split_symbol(symbol)
     new_symbol = prefix + FLAGS.rename_string + remainder
@@ -705,9 +712,10 @@ def move_object_file(src_obj_file, dest_obj_file, redefinition_file=None):
   # If we created the output file, remove the input file.
   if os.path.isfile(dest_obj_file):
     # But first...
-    if os.path.getsize(src_obj_file) >= 16 and os.path.getsize(
-        dest_obj_file) >= 16 and (FLAGS.platform == "ios" or
-                                  FLAGS.platform == "darwin"):
+    if (os.path.getsize(src_obj_file) >= 16 and
+        os.path.getsize(dest_obj_file) >= 16 and
+        (FLAGS.platform == "ios" or FLAGS.platform == "darwin") and
+        not binutils_force_target_format):
       # Ugly hack time: objcopy doesn't set the CPU subtype correctly on the
       # header for Mach-O files. So just overwrite the first 16 bytes of the
       # output file with the first 16 bytes of the input file.
@@ -753,7 +761,13 @@ def run_binutils_command(cmdline, error_output=None, ignore_errors=False):
     # files use the same format. Also we will need to explicitly specify this
     # format when creating an archive with "ar".
     # If we've never had to force a format, let binutils autodetect.
-    output = run_command([cmdline[0]] + ["--target=%s" % binutils_force_target_format] + cmdline[1:],
+    # Also, we can force a separate input and output target for objcopy, splitting on comma.
+    target_list = binutils_force_target_format.split(",")
+    if cmdline[0] == FLAGS.binutils_objcopy_cmd and len(target_list) > 1:
+      target_params = ["--input-target=%s" % target_list[0], "--output-target=%s" % target_list[1]]
+    else:
+      target_params = ["--target=%s" % target_list[0]]
+    output = run_command([cmdline[0]] + target_params + cmdline[1:],
                          error_output, ignore_errors)
   else:
     # Otherwise, if we've never had to force a format, use the default.
@@ -896,6 +910,8 @@ def shutdown_cache():
 
 
 def main(argv):
+  global binutils_force_target_format
+  binutils_force_target_format = FLAGS.force_binutils_target
   try:
     working_root = None
     input_paths = []