Skip to content
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

Partially revert "Do not depend on unversioned python binary (#1496)" #1501

Merged
merged 1 commit into from
Oct 13, 2022

Conversation

LebedevRI
Copy link
Collaborator

As predicted, the cmake part of the change is contentious. #1496 (comment)

This partially reverts commit 229bc5a.

…1496)"

As predicted, the cmake part of the change is contentious.
google#1496 (comment)

This partially reverts commit 229bc5a.
@LebedevRI LebedevRI requested a review from dmah42 October 12, 2022 17:47
@dmah42
Copy link
Member

dmah42 commented Oct 13, 2022

i wonder how this is not failing already due to missing python. there must be some configuration edge case where testing is enabled but the python scripts don't run.

@dmah42 dmah42 merged commit db4f581 into google:main Oct 13, 2022
@LebedevRI LebedevRI deleted the unbreak branch October 13, 2022 10:18
@joshmcorreia
Copy link

i wonder how this is not failing already due to missing python. there must be some configuration edge case where testing is enabled but the python scripts don't run.

I didn't set up any of the testing at my company so I'm not very familiar with this but here are the logs from before the recent breaking change if it helps you diagnose.

[ 33%] No update step for 'googletest'
[ 44%] No patch step for 'googletest'
[ 55%] No configure step for 'googletest'
[ 66%] No build step for 'googletest'
[ 77%] No install step for 'googletest'
[ 88%] No test step for 'googletest'
[100%] Completed 'googletest'
[100%] Built target googletest
-- Could NOT find Python (missing: Python_EXECUTABLE Interpreter) 
-- ##### Building BENCHMARK for:ARCH:  x86_64  
-- Configuring done
-- Generating done
-- Build files have been written to: proj/build/test/benchmark_tests/googlebenchmark-download
[ 11%] Creating directories for 'googlebenchmark'
[ 22%] Performing download step (git clone) for 'googlebenchmark'
Cloning into 'googlebenchmark-src'...
Already on 'main'
Your branch is up to date with 'origin/main'.
[ 33%] Performing update step for 'googlebenchmark'
HEAD is now at d2a8a4e Support for QuRT OS (Hexagon RTOS) (#1497)
[ 44%] No patch step for 'googlebenchmark'
[ 55%] No configure step for 'googlebenchmark'
[ 66%] No build step for 'googlebenchmark'
[ 77%] No install step for 'googlebenchmark'
[ 88%] No test step for 'googlebenchmark'
[100%] Completed 'googlebenchmark'
[100%] Built target googlebenchmark

I see the warning about not finding Python but it doesn't seem to be fatal. This is on a rocky linux 8.6 Docker container, which doesn't come with Python.

@MatzeB
Copy link
Contributor

MatzeB commented Oct 17, 2022

Thanks for fixing! It seems the only python use from cmake was in tools/strip_asm.py, which in turn is only enabled with BENCHMARK_ENABLE_ASSEMBLY_TESTS and LLVM_FILECHECK_EXE being found. Having llvms filecheck in the path is probably pretty unusual, but explains why I was running into python trouble...

Anyway I think I'm gonna change the LLVM builds to disable all the testing code instead of trying to fix things here. Thanks for the revert.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants