Skip to content

Pass along FIREBASE_PYTHON_HOST_EXECUTABLE for leveldb.cmake #9847

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 1 commit into from
May 27, 2022

Conversation

dconeybe
Copy link
Contributor

@dconeybe dconeybe commented May 26, 2022

Fix a bug where setting up the leveldb build in cmake would sometimes use an incorrect python interpreter when patching its source code. This happened because the leveldb build is invoked in a child cmake process, to which the cache variables are explicitly passed in via cmake's -D command-line argument.

The FIREBASE_PYTHON_HOST_EXECUTABLE cache variable was introduced in #9662, and the leveldb build was modified in #9596 to use it; however, it must have gone unnoticed that it was ignoring the FIREBASE_PYTHON_HOST_EXECUTABLE cache variable. This PR fixes this.

Here is where this bug was discovered: firebase/firebase-cpp-sdk#967 (comment)

@google-oss-bot
Copy link

Coverage Report 1

Affected Products

  • FirebaseFirestore-iOS-FirebaseFirestore.framework

    FilenameBase (55a8146)Merge (6368ef0)Diff
    leveldb_key.cc98.33%98.14%-0.20%
    write_stream.cc91.55%94.37%+2.82%

Test Logs

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

@dconeybe dconeybe marked this pull request as ready for review May 27, 2022 02:38
@dconeybe dconeybe requested a review from wu-hui May 27, 2022 02:38
@dconeybe dconeybe removed their assignment May 27, 2022
@wu-hui wu-hui assigned dconeybe and unassigned wu-hui May 27, 2022
@dconeybe dconeybe merged commit 68bc0b5 into master May 27, 2022
@dconeybe dconeybe deleted the dconeybe/PythonHostExecutableFix branch May 27, 2022 14:13
@firebase firebase locked and limited conversation to collaborators Jun 27, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants