Skip to content

Bugfix/integration tests mac openssl error #255

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 14 commits into from
Jan 28, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions .github/workflows/integration_tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,20 @@ jobs:
run: |
choco install openssl -r

- name: Install OpenSSL (MacOS)
if: matrix.target_platform == 'Desktop' &&
matrix.ssl_variant == 'openssl' &&
startsWith(matrix.os, 'macos')
run: |
brew install openssl

- name: Install OpenSSL (Linux)
if: matrix.target_platform == 'Desktop' &&
matrix.ssl_variant == 'openssl' &&
startsWith(matrix.os, 'ubuntu')
run: |
sudo apt install openssl

- name: Build integration tests
shell: bash
run: |
Expand Down
2 changes: 1 addition & 1 deletion external/vcpkg
Submodule vcpkg updated 3753 files
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
openssl
protobuf
zlib
--triplet
x64-linux
5 changes: 5 additions & 0 deletions external/vcpkg_custom_data/response_files/openssl/x64-osx.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
openssl
protobuf
zlib
--triplet
x64-osx
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
openssl
protobuf
zlib
--triplet
x64-windows-static-md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
openssl
protobuf
zlib
--triplet
x64-windows-static
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
openssl
protobuf
zlib
--triplet
x86-linux
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
openssl
protobuf
zlib
--triplet
x86-windows-static-md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
openssl
protobuf
zlib
--triplet
x86-windows-static
32 changes: 23 additions & 9 deletions scripts/gha/build_desktop.py
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ def install_x86_support_libraries():
utils.run_command(['apt', 'install', '-y'] + packages, as_root=True)


def _install_cpp_dependencies_with_vcpkg(arch, msvc_runtime_library):
def _install_cpp_dependencies_with_vcpkg(arch, msvc_runtime_library, use_openssl=False):
"""Install packages with vcpkg.

This does the following,
Expand All @@ -85,6 +85,7 @@ def _install_cpp_dependencies_with_vcpkg(arch, msvc_runtime_library):
Args:
arch (str): Architecture (eg: 'x86', 'x64').
msvc_runtime_library (str): Runtime library for MSVC (eg: 'static', 'dynamic').
use_openssl (bool): Use OpenSSL based vcpkg response files.
"""

# Install vcpkg executable if its not installed already
Expand All @@ -101,15 +102,21 @@ def _install_cpp_dependencies_with_vcpkg(arch, msvc_runtime_library):
# for each desktop platform, there exists a vcpkg response file in the repo
# (external/vcpkg_<triplet>_response_file.txt) defined for each target triplet
vcpkg_triplet = utils.get_vcpkg_triplet(arch, msvc_runtime_library)
vcpkg_response_file_path = os.path.join(os.getcwd(), 'external', 'vcpkg_custom_data',
'response_files', '{0}.txt'.format(vcpkg_triplet))
vcpkg_response_files_dir_path = os.path.join(os.getcwd(), 'external', 'vcpkg_custom_data',
'response_files')
if use_openssl:
vcpkg_response_files_dir_path = os.path.join(vcpkg_response_files_dir_path, 'openssl')

vcpkg_response_file_path = os.path.join(vcpkg_response_files_dir_path,
'{0}.txt'.format(vcpkg_triplet))

# Eg: ./external/vcpkg/vcpkg install @external/vcpkg_x64-osx_response_file.txt
# --disable-metrics
utils.run_command([vcpkg_executable_file_path, 'install',
'@' + vcpkg_response_file_path, '--disable-metrics'])

def install_cpp_dependencies_with_vcpkg(arch, msvc_runtime_library, cleanup=True):
def install_cpp_dependencies_with_vcpkg(arch, msvc_runtime_library, cleanup=True,
use_openssl=False):
"""Install packages with vcpkg and optionally cleanup any intermediates.

This is a wrapper over a low level installation function and attempts the
Expand All @@ -119,11 +126,12 @@ def install_cpp_dependencies_with_vcpkg(arch, msvc_runtime_library, cleanup=True
arch (str): Architecture (eg: 'x86', 'x64').
msvc_runtime_library (str): Runtime library for MSVC (eg: 'static', 'dynamic').
cleanup (bool): Clean up intermediate files used during installation.
use_openssl (bool): Use OpenSSL based vcpkg response files.

Raises:
(ValueError) If installation wasn't successful.
"""
_install_cpp_dependencies_with_vcpkg(arch, msvc_runtime_library)
_install_cpp_dependencies_with_vcpkg(arch, msvc_runtime_library, use_openssl)
vcpkg_triplet = utils.get_vcpkg_triplet(arch, msvc_runtime_library)
# Verify the installation with an attempt to auto fix any issues.
success = utils.verify_vcpkg_build(vcpkg_triplet, attempt_auto_fix=True)
Expand All @@ -142,7 +150,7 @@ def install_cpp_dependencies_with_vcpkg(arch, msvc_runtime_library, cleanup=True

def cmake_configure(build_dir, arch, msvc_runtime_library='static', linux_abi='legacy',
build_tests=True, config=None, target_format=None,
disable_vcpkg=False, verbose=False):
use_openssl=False, disable_vcpkg=False, verbose=False):
""" CMake configure.

If you are seeing problems when running this multiple times,
Expand All @@ -157,6 +165,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').
use_openssl (bool) : Use prebuilt OpenSSL library instead of using boringssl
downloaded and built during the cmake configure step.
disable_vcpkg (bool): If True, skip vcpkg and just use CMake for deps.
verbose (bool): If True, enable verbose mode in the CMake file.
"""
Expand Down Expand Up @@ -203,7 +213,8 @@ def cmake_configure(build_dir, arch, msvc_runtime_library='static', linux_abi='l
if (target_format):
cmd.append('-DFIREBASE_XCODE_TARGET_FORMAT={0}'.format(target_format))

cmd.append('-DFIREBASE_USE_BORINGSSL=ON')
if not use_openssl:
cmd.append('-DFIREBASE_USE_BORINGSSL=ON')

# Print out every command while building.
if verbose:
Expand All @@ -224,18 +235,20 @@ def main():
if args.arch == 'x86' and utils.is_linux_os():
install_x86_support_libraries()

# Install C++ dependencies using vcpkg
if not args.disable_vcpkg:
# Install C++ dependencies using vcpkg
install_cpp_dependencies_with_vcpkg(args.arch, args.msvc_runtime_library,
cleanup=True)
cleanup=True, use_openssl=args.use_openssl)

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.disable_vcpkg, args.verbose)
args.build_tests, args.config, args.target_format,
args.use_openssl, args.disable_vcpkg, args.verbose)

# CMake build
# cmake --build build -j 8
Expand Down Expand Up @@ -264,6 +277,7 @@ def parse_cmdline_args():
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)')
parser.add_argument('--target_format', default=None, help='(Mac only) whether to output frameworks (default) or libraries.')
parser.add_argument('--use_openssl', action='store_true', default=None, help='Use openssl for build instead of boringssl')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you thin about just adding an openssl flag to the prereqs script also? (And we can move the Windows openssl setup into there too.)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thats a good point but as per our original design, we wanted to keep all architecture specific steps under build_desktop. And if we are using vcpkg, we have to specify the exact architecture to build. Something to discuss in our "various ways to build" meeting but if we want to continue using vcpkg, it makes sense to not have openssl under the prereqs script. For now, I will leave it here but it should be trivial to move this to prereqs if we decide to go that way after the meeting.

args = parser.parse_args()
return args

Expand Down