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

Conversation

vimanyu
Copy link
Contributor

@vimanyu vimanyu commented Jan 27, 2021

  • Adds support for using vcpkg openssl in build_desktop.py.
  • Added openssl installation for mac and linux as a step if integration tests depend on openssl instead of boringssl.

Copy link
Contributor

@DellaBitta DellaBitta left a comment

Choose a reason for hiding this comment

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

Out of curiosity, what is this exactly?

"Submodule vcpkg updated 3754 files"

Does github do a deep comparison of submodule version change implications?

@@ -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.

@vimanyu vimanyu requested a review from DellaBitta January 27, 2021 20:12
@vimanyu vimanyu merged commit 1349ace into dev Jan 28, 2021
@vimanyu vimanyu deleted the bugfix/integration-tests-mac-openssl-error branch January 28, 2021 00:07
@firebase firebase locked and limited conversation to collaborators Feb 27, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants