From 0c56fc4d6604f34619fbb66516b02cb5c6544578 Mon Sep 17 00:00:00 2001 From: Peter Dillinger Date: Fri, 29 May 2020 11:24:19 -0700 Subject: [PATCH] Allow missing "unversioned" python, as in CentOS 8 (#6883) Summary: RocksDB Makefile was assuming existence of 'python' command, which is not present in CentOS 8. We avoid using 'python' if 'python3' is available. Also added fancy logic to format-diff.sh to make clang-format-diff.py for Python2 work even with Python3 only (as some CentOS 8 FB machines come equipped) Also, now use just 'python3' for PYTHON if not found so that an informative "command not found" error will result rather than something weird. Pull Request resolved: https://github.com/facebook/rocksdb/pull/6883 Test Plan: manually tried some variants, 'make check' on a fresh CentOS 8 machine without 'python' executable or Python2 but with clang-format-diff.py for Python2. Reviewed By: gg814 Differential Revision: D21767029 Pulled By: pdillinger fbshipit-source-id: 54761b376b140a3922407bdc462f3572f461d0e9 --- .gitignore | 2 + Makefile | 4 +- TARGETS | 2 +- buckifier/buckify_rocksdb.py | 4 +- buckifier/check_buck_targets.sh | 6 +- buckifier/targets_cfg.py | 2 +- build_tools/format-diff.sh | 104 ++++++++++++++++++++------------ tools/check_all_python.py | 2 +- tools/db_crashtest.py | 2 +- tools/ldb_test.py | 2 +- tools/write_stress_runner.py | 2 +- 11 files changed, 80 insertions(+), 52 deletions(-) mode change 100644 => 100755 tools/check_all_python.py diff --git a/.gitignore b/.gitignore index 0238a4ab78..218ac5e8e9 100644 --- a/.gitignore +++ b/.gitignore @@ -85,3 +85,5 @@ buckifier/*.pyc buckifier/__pycache__ compile_commands.json +clang-format-diff.py +.py3/ diff --git a/Makefile b/Makefile index 3141c59b44..51569ffc62 100644 --- a/Makefile +++ b/Makefile @@ -8,7 +8,9 @@ BASH_EXISTS := $(shell which bash) SHELL := $(shell which bash) -PYTHON?=$(shell which python) +# Default to python3. Some distros like CentOS 8 do not have `python`. +PYTHON?=$(shell which python3 || which python || echo python3) +export PYTHON CLEAN_FILES = # deliberately empty, so we can append below. CFLAGS += ${EXTRA_CFLAGS} diff --git a/TARGETS b/TARGETS index 5df9ae0e90..6a68b3c693 100644 --- a/TARGETS +++ b/TARGETS @@ -1,4 +1,4 @@ -# This file @generated by `python buckifier/buckify_rocksdb.py` +# This file @generated by `python3 buckifier/buckify_rocksdb.py` # --> DO NOT EDIT MANUALLY <-- # This file is a Facebook-specific integration for buck builds, so can # only be validated by Facebook employees. diff --git a/buckifier/buckify_rocksdb.py b/buckifier/buckify_rocksdb.py index 6c13f6ebfa..870505b53a 100644 --- a/buckifier/buckify_rocksdb.py +++ b/buckifier/buckify_rocksdb.py @@ -20,10 +20,10 @@ from util import ColorString # User can pass extra dependencies as a JSON object via command line, and this # script can include these dependencies in the generate TARGETS file. # Usage: -# $python buckifier/buckify_rocksdb.py +# $python3 buckifier/buckify_rocksdb.py # (This generates a TARGET file without user-specified dependency for unit # tests.) -# $python buckifier/buckify_rocksdb.py \ +# $python3 buckifier/buckify_rocksdb.py \ # '{"fake": { \ # "extra_deps": [":test_dep", "//fakes/module:mock1"], \ # "extra_compiler_flags": ["-DROCKSDB_LITE", "-Os"], \ diff --git a/buckifier/check_buck_targets.sh b/buckifier/check_buck_targets.sh index ccf77f91d7..66c83c52f3 100755 --- a/buckifier/check_buck_targets.sh +++ b/buckifier/check_buck_targets.sh @@ -15,7 +15,7 @@ echo Backup original TARGETS file. cp TARGETS TARGETS.bkp -python buckifier/buckify_rocksdb.py +${PYTHON:-python3} buckifier/buckify_rocksdb.py TGT_DIFF=`git diff TARGETS | head -n 1` @@ -24,9 +24,9 @@ then mv TARGETS.bkp TARGETS exit 0 else - echo "Please run 'python buckifier/buckify_rocksdb.py' to update TARGETS file." + echo "Please run '${PYTHON:-python3} buckifier/buckify_rocksdb.py' to update TARGETS file." echo "Do not manually update TARGETS file." - python --version + ${PYTHON:-python3} --version mv TARGETS.bkp TARGETS exit 1 fi diff --git a/buckifier/targets_cfg.py b/buckifier/targets_cfg.py index 62ef6f6c74..f4e2973bf7 100644 --- a/buckifier/targets_cfg.py +++ b/buckifier/targets_cfg.py @@ -5,7 +5,7 @@ from __future__ import print_function from __future__ import unicode_literals rocksdb_target_header_template = \ - """# This file \100generated by `python buckifier/buckify_rocksdb.py` + """# This file \100generated by `python3 buckifier/buckify_rocksdb.py` # --> DO NOT EDIT MANUALLY <-- # This file is a Facebook-specific integration for buck builds, so can # only be validated by Facebook employees. diff --git a/build_tools/format-diff.sh b/build_tools/format-diff.sh index 70da9a5782..386885b578 100755 --- a/build_tools/format-diff.sh +++ b/build_tools/format-diff.sh @@ -26,53 +26,77 @@ while getopts ':ch' OPTION; do esac done -if [ -z $CLANG_FORMAT_DIFF ] -then -CLANG_FORMAT_DIFF="clang-format-diff.py" -fi +REPO_ROOT="$(git rev-parse --show-toplevel)" -# Check clang-format-diff.py -if ! which $CLANG_FORMAT_DIFF &> /dev/null -then - if [ ! -f ./clang-format-diff.py ] - then - echo "You didn't have clang-format-diff.py and/or clang-format available in your computer!" - echo "You can download clang-format-diff.py by running: " - echo " curl --location http://goo.gl/iUW1u2 -o ${CLANG_FORMAT_DIFF}" - echo "You can download clang-format by running:" - echo " brew install clang-format" - echo " Or" - echo " apt install clang-format" - echo " This might work too:" - echo " yum install git-clang-format" - echo "Then, move both files (i.e. ${CLANG_FORMAT_DIFF} and clang-format) to some directory within PATH=${PATH}" - echo "and make sure ${CLANG_FORMAT_DIFF} is executable." - exit 128 +if [ "$CLANG_FORMAT_DIFF" ]; then + echo "Note: CLANG_FORMAT_DIFF='$CLANG_FORMAT_DIFF'" + # Dry run to confirm dependencies like argparse + if $CLANG_FORMAT_DIFF --help >/dev/null < /dev/null; then + true #Good else - if [ -x ./clang-format-diff.py ] - then - PATH=$PATH:. + exit 128 + fi +else + # First try directly executing the possibilities + if clang-format-diff.py --help &> /dev/null < /dev/null; then + CLANG_FORMAT_DIFF=clang-format-diff.py + elif $REPO_ROOT/clang-format-diff.py --help &> /dev/null < /dev/null; then + CLANG_FORMAT_DIFF=$REPO_ROOT/clang-format-diff.py + else + # This probably means we need to directly invoke the interpreter. + # But first find clang-format-diff.py + if [ -f "$REPO_ROOT/clang-format-diff.py" ]; then + CFD_PATH="$REPO_ROOT/clang-format-diff.py" + elif which clang-format-diff.py &> /dev/null; then + CFD_PATH="$(which clang-format-diff.py)" else - CLANG_FORMAT_DIFF="python ./clang-format-diff.py" + echo "You didn't have clang-format-diff.py and/or clang-format available in your computer!" + echo "You can download clang-format-diff.py by running: " + echo " curl --location http://goo.gl/iUW1u2 -o ${CLANG_FORMAT_DIFF}" + echo "You can download clang-format by running:" + echo " brew install clang-format" + echo " Or" + echo " apt install clang-format" + echo " This might work too:" + echo " yum install git-clang-format" + echo "Then, move both files (i.e. ${CLANG_FORMAT_DIFF} and clang-format) to some directory within PATH=${PATH}" + echo "and make sure ${CLANG_FORMAT_DIFF} is executable." + exit 128 + fi + # Check argparse pre-req on interpreter, or it will fail + if echo import argparse | ${PYTHON:-python3}; then + true # Good + else + echo "To run clang-format-diff.py, we'll need the library "argparse" to be" + echo "installed. You can try either of the follow ways to install it:" + echo " 1. Manually download argparse: https://pypi.python.org/pypi/argparse" + echo " 2. easy_install argparse (if you have easy_install)" + echo " 3. pip install argparse (if you have pip)" + exit 129 + fi + # Unfortunately, some machines have a Python2 clang-format-diff.py + # installed but only a Python3 interpreter installed. Rather than trying + # different Python versions that might be installed, we can try migrating + # the code to Python3 if it looks like Python2 + if grep -q "print '" "$CFD_PATH" && \ + ${PYTHON:-python3} --version | grep -q 'ython 3'; then + if [ ! -f "$REPO_ROOT/.py3/clang-format-diff.py" ]; then + echo "Migrating $CFD_PATH to Python3 in a hidden file" + mkdir -p "$REPO_ROOT/.py3" + ${PYTHON:-python3} -m lib2to3 -w -n -o "$REPO_ROOT/.py3" "$CFD_PATH" > /dev/null || exit 128 + fi + CFD_PATH="$REPO_ROOT/.py3/clang-format-diff.py" + fi + CLANG_FORMAT_DIFF="${PYTHON:-python3} $CFD_PATH" + # This had better work after all those checks + if $CLANG_FORMAT_DIFF --help >/dev/null < /dev/null; then + true #Good + else + exit 128 fi fi fi -# Check argparse, a library that clang-format-diff.py requires. -python 2>/dev/null << EOF -import argparse -EOF - -if [ "$?" != 0 ] -then - echo "To run clang-format-diff.py, we'll need the library "argparse" to be" - echo "installed. You can try either of the follow ways to install it:" - echo " 1. Manually download argparse: https://pypi.python.org/pypi/argparse" - echo " 2. easy_install argparse (if you have easy_install)" - echo " 3. pip install argparse (if you have pip)" - exit 129 -fi - # TODO(kailiu) following work is not complete since we still need to figure # out how to add the modified files done pre-commit hook to git's commit index. # diff --git a/tools/check_all_python.py b/tools/check_all_python.py old mode 100644 new mode 100755 index 17fe95eab9..b4a41c10e5 --- a/tools/check_all_python.py +++ b/tools/check_all_python.py @@ -1,4 +1,4 @@ -#!/usr/bin/env python2 +#!/usr/bin/env python3 # Copyright (c) Facebook, Inc. and its affiliates. All Rights Reserved. import glob diff --git a/tools/db_crashtest.py b/tools/db_crashtest.py index 630b96b041..25d38c4f28 100644 --- a/tools/db_crashtest.py +++ b/tools/db_crashtest.py @@ -1,4 +1,4 @@ -#!/usr/bin/env python +#!/usr/bin/env python3 # Copyright (c) Facebook, Inc. and its affiliates. All Rights Reserved. from __future__ import absolute_import, division, print_function, unicode_literals diff --git a/tools/ldb_test.py b/tools/ldb_test.py index 9773b85f81..46edfa4e57 100644 --- a/tools/ldb_test.py +++ b/tools/ldb_test.py @@ -1,4 +1,4 @@ -#!/usr/bin/env python +#!/usr/bin/env python3 # Copyright (c) Facebook, Inc. and its affiliates. All Rights Reserved. from __future__ import absolute_import, division, print_function, unicode_literals diff --git a/tools/write_stress_runner.py b/tools/write_stress_runner.py index 51daa24e83..962515dfb5 100644 --- a/tools/write_stress_runner.py +++ b/tools/write_stress_runner.py @@ -1,4 +1,4 @@ -#!/usr/bin/env python +#!/usr/bin/env python3 # Copyright (c) Facebook, Inc. and its affiliates. All Rights Reserved. from __future__ import absolute_import, division, print_function, unicode_literals