Create individual Python virtualenv's in cmake builds #9662

dconeybe · 2022-04-15T22:28:44Z

During cmake builds, instead of using the python interpreter installed locally, use that python interpreter to create a virtualenv for each python script run by cmake. Each virtualenv can then have its own isolated dependencies installed. This removes the dependence on having the correct python libraries installed locally. All you need is a vanilla python 3 interpreter. This also helps fix some unusual python dependency issues when this repository is incorporated into the firebase-cpp-sdk.

Here is an example of the build error in the C++ SDK that this fixes (https://github.com/firebase/firebase-cpp-sdk/runs/6065960193):

[ 83%] Generating nanopb sources
google/firestore/v1/document.proto:20:1: warning: Import google/api/annotations.proto is unused.
google/firestore/v1/query.proto:20:1: warning: Import google/api/annotations.proto is unused.
google/firestore/v1/common.proto:20:1: warning: Import google/api/annotations.proto is unused.
google/firestore/v1/write.proto:20:1: warning: Import google/api/annotations.proto is unused.
google/firestore/admin/index.proto:21:1: warning: Import google/api/annotations.proto is unused.

         *************************************************************
         *** Could not import the Google protobuf Python libraries ***
         *** Try installing package 'python-protobuf' or similar.  ***
         *************************************************************
    
Traceback (most recent call last):
  File "external/src/firestore/Firestore/Protos/tmpuwkOue.py", line 25
    import nanopb_generator as nanopb
  File "external/src/firestore-build/external/src/nanopb/generator/nanopb_generator.py", line 25
    import google.protobuf.text_format as text_format
  File "external/src/firestore-build/external/src/protobuf/python/google/protobuf/text_format.py", line 51
    import six
ImportError: No module named six
--nanopb_out: protoc-gen-nanopb: Plugin failed with status code 1.

For some reason the build is using a Python interpreter that does not have the six module installed; however, the six module is installed during setup of the runner. So it must be choosing a different Python interpreter. This PR fixes this by setting up a dedicated virtualenv with six installed for running the Python script.

#no-changelog

google-oss-bot · 2022-04-15T23:34:54Z

Coverage Report ¹

Affected Products

FirebaseFirestore-iOS-FirebaseFirestore.framework
Overall coverage changed from 86.78% (caac2ce) to 86.77% (9565bbc) by -0.02%.
Filename Base (caac2ce) Merge (9565bbc) Diff
exception.cc 23.68% 73.68% +50.00%
exception_apple.mm 89.66% 58.62% -31.03%
leveldb_key.cc 98.81% 97.73% -1.08%
leveldb_mutation_queue.cc 93.56% 92.05% -1.52%

Test Logs

https://storage.googleapis.com/firebase-sdk-metric-reports/u6wEN71zZD.html

wu-hui

Thank you for doing this!

…rror. This commit patches in firebase/firebase-ios-sdk#9662, which creates dedicated virtualenv instances for each Python script that is run by cmake. By doing this, each script can have its own set of dependencies installed and does not rely on the host system's Python interpreter having any particular set of third-party modules installed. This is not specifically related to Snappy, but, for convenience, is being incorporated into the patch file for snappy. This commit fixes the following build error (https://github.com/firebase/firebase-cpp-sdk/runs/6065960193): ``` [ 83%] Generating nanopb sources google/firestore/v1/document.proto:20:1: warning: Import google/api/annotations.proto is unused. google/firestore/v1/query.proto:20:1: warning: Import google/api/annotations.proto is unused. google/firestore/v1/common.proto:20:1: warning: Import google/api/annotations.proto is unused. google/firestore/v1/write.proto:20:1: warning: Import google/api/annotations.proto is unused. google/firestore/admin/index.proto:21:1: warning: Import google/api/annotations.proto is unused. ************************************************************* *** Could not import the Google protobuf Python libraries *** *** Try installing package 'python-protobuf' or similar. *** ************************************************************* Traceback (most recent call last): File "external/src/firestore/Firestore/Protos/tmpuwkOue.py", line 25 import nanopb_generator as nanopb File "external/src/firestore-build/external/src/nanopb/generator/nanopb_generator.py", line 25 import google.protobuf.text_format as text_format File "external/src/firestore-build/external/src/protobuf/python/google/protobuf/text_format.py", line 51 import six ImportError: No module named six --nanopb_out: protoc-gen-nanopb: Plugin failed with status code 1. ```

…ror can be fixed by using a temporary directory. A previous commit, bc4a657, patched firebase-ios-sdk with firebase/firebase-ios-sdk#9662, which modified cmake builds to create a python virtualenv for each python script. This, however, ran into a strange problem in the Windows GitHub Actions runners, where creating the virtualenv failed when it attempted to call ensurepip. The suspected root cause is a MAX_PATH length violation. To test out this hypothesis, this commit modifies the FirebaseSetupPythonInterpreter() to create the Python virtualenv in a temporary directory instead of a well-defined subdirectory of cmake's binary directory. This should have a shorter path length for the virtualenv. If this works, then possibly we will need to provide a way to override the base directory for the pyvenv's. Here is the actual failure from https://github.com/firebase/firebase-cpp-sdk/runs/6083486670 ``` -- Found PythonInterp: C:/ProgramData/chocolatey/bin/python2.7.exe (found suitable version "2.7.9", minimum required is "2.7") -- Found Python3: C:/hostedtoolcache/windows/Python/3.10.4/x64/python3.exe (found version "3.10.4") found components: Interpreter -- FirebaseSetupPythonInterpreter(FirestoreProtos): Creating Python virtualenv in D:/a/firebase-cpp-sdk/firebase-cpp-sdk/ta/admob/it/bin/external/src/firestore-build/Firestore/Protos/pyvenv/FirestoreProtos using C:/hostedtoolcache/windows/Python/3.10.4/x64/python3.exe Error: Command '['D:\\a\\firebase-cpp-sdk\\firebase-cpp-sdk\\ta\\admob\\it\\bin\\external\\src\\firestore-build\\Firestore\\Protos\\pyvenv\\FirestoreProtos\\Scripts\\python3.exe', '-Im', 'ensurepip', '--upgrade', '--default-pip']' returned non-zero exit status 3221225781. CMake Error at bin/external/src/firestore/cmake/python_setup.cmake:133 (message): Failed to create a Python virtualenv in D:/a/firebase-cpp-sdk/firebase-cpp-sdk/ta/admob/it/bin/external/src/firestore-build/Firestore/Protos/pyvenv/FirestoreProtos using C:/hostedtoolcache/windows/Python/3.10.4/x64/python3.exe Call Stack (most recent call first): bin/external/src/firestore/Firestore/Protos/CMakeLists.txt:16 (FirebaseSetupPythonInterpreter) ```

…rebase/firebase-ios-sdk#9662)

dconeybe · 2022-04-20T06:48:53Z

FYI This script has been copied into the firebase-cpp-sdk as well: firebase/firebase-cpp-sdk#908

* Update iOS dependencies - Mon Apr 18 2022 ### iOS - Firebase/Analytics → 8.15.0 - Firebase/Auth → 8.15.0 - Firebase/Core → 8.15.0 - Firebase/Crashlytics → 8.15.0 - Firebase/Database → 8.15.0 - Firebase/DynamicLinks → 8.15.0 - Firebase/Firestore → 8.15.0 - Firebase/Functions → 8.15.0 - Firebase/Installations → 8.15.0 - Firebase/Messaging → 8.15.0 - Firebase/RemoteConfig → 8.15.0 - Firebase/Storage → 8.15.0 > Created by [Update Android and iOS dependencies workflow](https://github.com/firebase/firebase-cpp-sdk/actions/runs/2185021604). * query_main.h: fix build error due to Filter being renamed to FieldFilter. This fixes the following build error in `ubuntu-latest-Release-x64-static`: ``` firestore/src/main/query_main.h:185:34: error: no type named 'Operator' in 'firebase::firestore::core::Filter' using Operator = core::Filter::Operator; ~~~~~~~~~~~~~~^ ``` e.g. https://github.com/firebase/firebase-cpp-sdk/runs/6065960625 * firestore_snappy.patch.txt: patch in a PR that fixes a Python build error. This commit patches in firebase/firebase-ios-sdk#9662, which creates dedicated virtualenv instances for each Python script that is run by cmake. By doing this, each script can have its own set of dependencies installed and does not rely on the host system's Python interpreter having any particular set of third-party modules installed. This is not specifically related to Snappy, but, for convenience, is being incorporated into the patch file for snappy. This commit fixes the following build error (https://github.com/firebase/firebase-cpp-sdk/runs/6065960193): ``` [ 83%] Generating nanopb sources google/firestore/v1/document.proto:20:1: warning: Import google/api/annotations.proto is unused. google/firestore/v1/query.proto:20:1: warning: Import google/api/annotations.proto is unused. google/firestore/v1/common.proto:20:1: warning: Import google/api/annotations.proto is unused. google/firestore/v1/write.proto:20:1: warning: Import google/api/annotations.proto is unused. google/firestore/admin/index.proto:21:1: warning: Import google/api/annotations.proto is unused. ************************************************************* *** Could not import the Google protobuf Python libraries *** *** Try installing package 'python-protobuf' or similar. *** ************************************************************* Traceback (most recent call last): File "external/src/firestore/Firestore/Protos/tmpuwkOue.py", line 25 import nanopb_generator as nanopb File "external/src/firestore-build/external/src/nanopb/generator/nanopb_generator.py", line 25 import google.protobuf.text_format as text_format File "external/src/firestore-build/external/src/protobuf/python/google/protobuf/text_format.py", line 51 import six ImportError: No module named six --nanopb_out: protoc-gen-nanopb: Plugin failed with status code 1. ``` * firestore_snappy.patch.txt: hack test to see if the windows python error can be fixed by using a temporary directory. A previous commit, bc4a657, patched firebase-ios-sdk with firebase/firebase-ios-sdk#9662, which modified cmake builds to create a python virtualenv for each python script. This, however, ran into a strange problem in the Windows GitHub Actions runners, where creating the virtualenv failed when it attempted to call ensurepip. The suspected root cause is a MAX_PATH length violation. To test out this hypothesis, this commit modifies the FirebaseSetupPythonInterpreter() to create the Python virtualenv in a temporary directory instead of a well-defined subdirectory of cmake's binary directory. This should have a shorter path length for the virtualenv. If this works, then possibly we will need to provide a way to override the base directory for the pyvenv's. Here is the actual failure from https://github.com/firebase/firebase-cpp-sdk/runs/6083486670 ``` -- Found PythonInterp: C:/ProgramData/chocolatey/bin/python2.7.exe (found suitable version "2.7.9", minimum required is "2.7") -- Found Python3: C:/hostedtoolcache/windows/Python/3.10.4/x64/python3.exe (found version "3.10.4") found components: Interpreter -- FirebaseSetupPythonInterpreter(FirestoreProtos): Creating Python virtualenv in D:/a/firebase-cpp-sdk/firebase-cpp-sdk/ta/admob/it/bin/external/src/firestore-build/Firestore/Protos/pyvenv/FirestoreProtos using C:/hostedtoolcache/windows/Python/3.10.4/x64/python3.exe Error: Command '['D:\\a\\firebase-cpp-sdk\\firebase-cpp-sdk\\ta\\admob\\it\\bin\\external\\src\\firestore-build\\Firestore\\Protos\\pyvenv\\FirestoreProtos\\Scripts\\python3.exe', '-Im', 'ensurepip', '--upgrade', '--default-pip']' returned non-zero exit status 3221225781. CMake Error at bin/external/src/firestore/cmake/python_setup.cmake:133 (message): Failed to create a Python virtualenv in D:/a/firebase-cpp-sdk/firebase-cpp-sdk/ta/admob/it/bin/external/src/firestore-build/Firestore/Protos/pyvenv/FirestoreProtos using C:/hostedtoolcache/windows/Python/3.10.4/x64/python3.exe Call Stack (most recent call first): bin/external/src/firestore/Firestore/Protos/CMakeLists.txt:16 (FirebaseSetupPythonInterpreter) ``` * Revert "firestore_snappy.patch.txt: hack test to see if the windows python error can be fixed by using a temporary directory." This hack didn't fix anything: the same problem occurred: https://github.com/firebase/firebase-cpp-sdk/runs/6087940880 This reverts commit 3936b10. * Capture stderr when creating the virtualenv to see if it adds any extra info * Revert "Capture stderr when creating the virtualenv to see if it adds any extra info" Didn't help. This reverts commit 3030e74. * build_testapps.py: Specify `FIREBASE_PYTHON_HOST_EXECUTABLE` to cmake This _may_ fix the following `build-desktop-windows-latest-openssl` build error: ``` -- Found PythonInterp: C:/ProgramData/chocolatey/bin/python2.7.exe -- Found Python3: C:/hostedtoolcache/windows/Python/3.10.4/x64/python3.exe -- FirebaseSetupPythonInterpreter(FirestoreProtos): Creating Python virtualenv in D:/a/firebase-cpp-sdk/firebase-cpp-sdk/ta/admob/it/bin/external/src/firestore-build/Firestore/Protos/pyvenv/FirestoreProtos using C:/hostedtoolcache/windows/Python/3.10.4/x64/python3.exe CMake Error at bin/external/src/firestore/cmake/python_setup.cmake:136 (message): Failed to create a Python virtualenv in D:/a/firebase-cpp-sdk/firebase-cpp-sdk/ta/admob/it/bin/external/src/firestore-build/Firestore/Protos/pyvenv/FirestoreProtos using C:/hostedtoolcache/windows/Python/3.10.4/x64/python3.exe Command '['D:\\a\\firebase-cpp-sdk\\firebase-cpp-sdk\\ta\\admob\\it\\bin\\external\\src\\firestore-build\\Firestore\\Protos\\pyvenv\\FirestoreProtos\\Scripts\\python3.exe', '-Im', 'ensurepip', '--upgrade', '--default-pip']' returned non-zero exit status 3221225781. Call Stack (most recent call first): bin/external/src/firestore/Firestore/Protos/CMakeLists.txt:16 (FirebaseSetupPythonInterpreter) ``` It appears that `find_package(Python3)` is finding an undesirable Python interpreter (i.e. the one in C:/hostedtoolcache). Instead, it should find the one installed by the GitHub Actions workflow. Probably this incorrect Python doesn't correctly support virtualenv. By setting the `FIREBASE_PYTHON_HOST_EXECUTABLE` cmake cache variable, the `FirebaseSetupPythonInterpreter()` function will use that interpreter, the correct interpreter, instead of searching for one and finding a wrong one. Co-authored-by: firebase-workflow-trigger-bot <[email protected]> Co-authored-by: Denver Coneybeare <[email protected]> Co-authored-by: Jon Simantov <[email protected]>

…-ios-sdk#9662 (Create individual Python virtualenv's in cmake builds) since it's incorporated into the CocoaPods-9.0.0.nightly tag

…port and Firestore (#915) * Remove the patch to enable Snappy support * firestore_patch.py: also copy the URL_HASH * query_main.h: fix build error due to Filter being renamed to FieldFilter. * user_data_converter_main.cc: fix build error when calling NullValue(), which now just returns a google_firestore_v1_Value instead of a Message<google_firestore_v1_Value> * Merge in #896 (leveldb_snappy_test.cc: run the test on iOS as well, and other improvements) * leveldb_snappy_test.cc: re-use the test from the iOS SDK instead of re-writing it. * leveldb_snappy_test.cc: add a newline at the end of the file. * query_main.h: add missing #include "Firestore/core/src/core/field_filter.h" * Set the FIRESTORE_INCLUDE_OBJC cmake cache var to NO to avoid building Objective-C parts of the iOS SDK This fixes the following build error: ``` FIRFirestore.h:147:20: error: unknown attribute 'swift_async' ignored [-Werror,-Wunknown-attributes] __attribute__((swift_async(none))); // Disable async import due to #9426. ^ 1 error generated. ``` which was introduced by firebase/firebase-ios-sdk#9502 * external/pip_requirements.txt: add six This is an attempt to fix the following build error: ``` Traceback (most recent call last): File "Firestore/Protos/tmphmjWeu.py", line 25, in <module> import nanopb_generator as nanopb File "nanopb/generator/nanopb_generator.py", line 25, in <module> import google.protobuf.text_format as text_format File "protobuf/python/google/protobuf/text_format.py", line 51, in <module> import six ImportError: No module named six ``` e.g. https://github.com/firebase/firebase-cpp-sdk/runs/6034310890 * Improve FIREBASE_PYTHON_EXECUTABLE cmake cache var and make its usage ubiquitous * firestore.cmake: bump pinned commit * Revert "external/pip_requirements.txt: add six" since it didn't fix the problem. This reverts commit 56553d9. * firestore.cmake: fix pinned commit * Revert "Improve FIREBASE_PYTHON_EXECUTABLE cmake cache var and make its usage ubiquitous" It seemed to cause problems with the build. There is a better way in the iOS SDK anyways. This reverts commit c6346fd. * firestore.cmake: update pinned commit * CMakeLists.txt: Set FIRESTORE_INCLUDE_OBJC as a cache variable so it actually works * query_main.cc: IsBefore(BoundPosition) -> IsInclusive(BoundPosition) This fixes several test failures due to incorrect handling of startAfter and endBefore resulting from a change in core::Bound from "before" to "inclusive" in firebase/firebase-ios-sdk#9519 Namely, this fixes the following failure: ``` cursor_test.cc:250: Failure Expected equality of these values: std::vector<std::string>({"c", "f", "b", "e"}) Which is: { "c", "f", "b", "e" } QuerySnapshotToIds(snapshot) Which is: { "c", "f" } [ FAILED ] FirestoreIntegrationTest.TimestampsCanBePassedToQueriesAsLimits ``` e.g. https://github.com/firebase/firebase-cpp-sdk/runs/6044737005 * firestore.cmake: update pinned commit * Use staging repo and update to 9.0 podfiles. Add Swift headers from 9.0.0 zip release. * Remove streaming support, as it fails with the new Swift SDK. * Enable Firestore and Admob. * Remove AdMob for now - it's broken in 7.69.0-cppsdk. * Don't process swift headers if already processed * fix nightly version tag * Fix update dependencies script * Support directories with spaces * Exclude swift headers from linter. * Fix whitespace * Fix comma * Fix whitespace. * Fix Firestore external file. * Restore new external file and fix include path. * Fix include files and paths to be compatible with C++ build. * Format code. * Add NOLINT * Remove included test with bad include path. * Fix internal test podfile * Fix podfiles * Revert some junk that got pulled in when #898 was merged into this branch * Temporarily remove admob from integration_tests.yml too. * integration_tests.yml: remove admob from another place, since it still seems to be being built * Revert "integration_tests.yml: remove admob from another place, since it still seems to be being built" This reverts commit fc6b845. * Revert "Temporarily remove admob from integration_tests.yml too." This reverts commit 32aec7e. * Add AdMob back in and refer to new version in staging. * Speed up update-dependencies zip distribution usage * Fix substitution. * Fix header formatting * Fix spacing. * Update Xcode version to 12.5.1 and add tests to 13.2.1 * Add new version to readme * Change Functions iOS SDK and tvOS SDK version to 12.4 (minimum for Swift). * Since FIRStorageMetadata no longer implements NSCopying, copy it a different way. * Change deployment version to 12.4 for Storage, required for Swift support. * Update analytics headers from 9.0.0 Analytics RC. * Add note about removal of deprecated analytics constants. * Set analytics deployment target * Set deployment targets * Add empty Swift file to sample framework. * Switch Podfile postprocessing to use python3 * Add empty Swift file to integration test project. * Add empty Swift file to analytics project * Add empty.swift to remaining integration test projects. * Check to make sure that the controller can pause/resume/etc. * Workaround for pause/cancel/resume semantics changing in 9.x. * Switch from nightly to release tag for Firestore external dep. * Update Xcode version to 13.2.1, required version for iOS SDK. * Change deployment target back to 10.x. * Fix iOS deployment version back to 10.0 * Add notes about including a swift file * Change comparison to lower-case to prevent restore_secrets overriding it. * Clean up the verbose log of file size * firestore_snappy.patch.txt: remove the patching for firebase/firebase-ios-sdk#9662 (Create individual Python virtualenv's in cmake builds) since it's incorporated into the CocoaPods-9.0.0.nightly tag * CHERRY PICK from main branch: leveldb_snappy_test.cc: run the test on iOS as well, and other improvements (#896) This change improves the leveldb_snappy_test.cc to provide stronger guarantees of expected behavior. * Set block storage for task variable. * Fix server key. * Format code. * test iOS simulator devices * Windows compatibility for strcasecmp * test iOS simulator devices * Update Swift header to include copy method; revert Storage to use it. * upload ios projects artifacts * Temporarily disable a test that fails on device when built from cmdline. * Update the testing Xcode version to 13.3.1 * Change xcode version to 13.3.1 and macos runners to macos-12. * Restore simulator versions back to previous values * Reworded some release notes. (Also replaced a couple tabs with spaces.) * Revert "upload ios projects artifacts" This reverts commit e04d6a6. * Fix Swift header script to work with multiple URLs. * Update Swift headers for final 9.0.0 release * Add readme about ios_pod directory and its swift_headers subdirectory * Remove staging repo from podfiles. (#923) * If the zip file can't be downloaded, just skip that step. Co-authored-by: Denver Coneybeare <[email protected]> Co-authored-by: Mou <[email protected]> Co-authored-by: Mou Sun <[email protected]> Co-authored-by: a-maurice <[email protected]>

Create individual Python virtualenv's in cmake builds

11e3c7b

dconeybe added the api: firestore label Apr 15, 2022

dconeybe self-assigned this Apr 15, 2022

wu-hui approved these changes Apr 19, 2022

View reviewed changes

dconeybe merged commit 2ab9dcb into master Apr 19, 2022

dconeybe deleted the dconeybe/PythonCmakeSetup branch April 19, 2022 17:18

dconeybe mentioned this pull request Apr 20, 2022

python_setup.cmake: add FIREBASE_PYTHON_PYVENV_DEST_DIR cmake cache var #9669

Closed

dconeybe added a commit to firebase/firebase-cpp-sdk that referenced this pull request Apr 20, 2022

Create individual Python virtualenv's in cmake builds (copied from fi…

b4e29c3

…rebase/firebase-ios-sdk#9662)

dconeybe mentioned this pull request Apr 20, 2022

Create individual Python virtualenv's in cmake builds firebase/firebase-cpp-sdk#908

Closed

firebase locked and limited conversation to collaborators May 20, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Create individual Python virtualenv's in cmake builds #9662

Create individual Python virtualenv's in cmake builds #9662

Uh oh!

dconeybe commented Apr 15, 2022 •

edited

Loading

Uh oh!

google-oss-bot commented Apr 15, 2022

`FirebaseFirestore-iOS-FirebaseFirestore.framework`

Uh oh!

wu-hui left a comment

Uh oh!

dconeybe commented Apr 20, 2022

Uh oh!

Uh oh!

Create individual Python virtualenv's in cmake builds #9662

Create individual Python virtualenv's in cmake builds #9662

Uh oh!

Conversation

dconeybe commented Apr 15, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

google-oss-bot commented Apr 15, 2022

Coverage Report 1

Affected Products

FirebaseFirestore-iOS-FirebaseFirestore.framework

Test Logs

Uh oh!

wu-hui left a comment

Choose a reason for hiding this comment

Uh oh!

dconeybe commented Apr 20, 2022

Uh oh!

Uh oh!

dconeybe commented Apr 15, 2022 •

edited

Loading

Coverage Report ¹

`FirebaseFirestore-iOS-FirebaseFirestore.framework`