From 81e1f65394f2d488770272f17d8cdc489f8d35d1 Mon Sep 17 00:00:00 2001 From: aandvalenzuela Date: Mon, 18 Nov 2024 12:59:36 +0100 Subject: [PATCH 1/3] Pushing changes proposed in https://github.com/cms-sw/cms-bot/pull/2319 to a clean branch --- PFA.py | 49 +++++++++++++++++++++++++++++++++ run-pr-code-checks | 65 ++++++++++++++++++++++++++++++++++++++++++++ run-python-format.py | 64 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 178 insertions(+) create mode 100644 PFA.py mode change 100755 => 100644 run-pr-code-checks create mode 100644 run-python-format.py diff --git a/PFA.py b/PFA.py new file mode 100644 index 000000000000..aeb86bca415a --- /dev/null +++ b/PFA.py @@ -0,0 +1,49 @@ +#!/usr/bin/python3 +import os +import argparse +def find_file_path(file): + if not os.path.isfile(file): + return None + return os.path.realpath(file) +def find_python_files(directory): + py_files = [] + for root, dirs, files in os.walk(directory): + for file in files: + if file.endswith(".py"): + py_files.append(os.path.join(root, file)) + return py_files +def CodeQualityChecks(python_files): + if python_files: + for file in python_files: + file_path = find_file_path(file) + if not file_path: + print("File {} not found or invalid path.".format(file)) + continue + + format_command = "ruff format {}".format(file_path) + print("Executing command: {}".format(format_command)) # Debug print + os.system(format_command) +def main(): + parser = argparse.ArgumentParser( + description="Linting and formatting Python files in directories or file paths." + ) + parser.add_argument( + "paths", + nargs="+", + help="List of directories or file paths to process" + ) + args = parser.parse_args() + + all_python_files = [] + for path in args.paths: + if os.path.isdir(path): + all_python_files.extend(find_python_files(path)) + elif os.path.isfile(path): + all_python_files.append(path) + else: + print("Error: {} is not a valid file or directory.".format(path)) + return + CodeQualityChecks(all_python_files) +if __name__ == "__main__": + main() + diff --git a/run-pr-code-checks b/run-pr-code-checks old mode 100755 new mode 100644 index c6c4ada6109d..f39e9ee49f3f --- a/run-pr-code-checks +++ b/run-pr-code-checks @@ -109,6 +109,7 @@ if $CODE_CHECKS ; then if [ -s ${UP_DIR}/code-checks-files.txt ] ; then ERR=false scram build -k -j $NUM_PROC code-checks USER_CODE_CHECKS_FILE="${UP_DIR}/code-checks-files.txt" > ${UP_DIR}/code-checks.log 2>&1 || ERR=true + if $ERR ; then echo '-code-checks' > ${UP_DIR}/code-checks.md echo -e "\nLogs: $UP_URL" >> ${UP_DIR}/code-checks.md @@ -120,6 +121,7 @@ if $CODE_CHECKS ; then send_jenkins_artifacts ${UP_DIR}/ pr-code-checks/${REPO_USER}-PR-${PULL_REQUEST}/${BUILD_NUMBER} ${CMS_BOT_DIR}/comment-gh-pr.py -r ${REPOSITORY} -p $PULL_REQUEST -R ${UP_DIR}/code-checks.md fi + #exit 0 exit 0 fi if [ -e ${CMSSW_BASE}/tmp/${SCRAM_ARCH}/code-checks-logs ] ; then @@ -143,6 +145,7 @@ if $CODE_FORMAT ; then scram build -k -j $NUM_PROC code-format USER_CODE_FORMAT_FILE="${UP_DIR}/code-format-files.txt" > ${UP_DIR}/code-format.log 2>&1 pushd $CMSSW_BASE/src git diff > ${UP_DIR}/code-format.patch + cat ${UP_DIR}/code-format.patch if [ -s ${UP_DIR}/code-format.patch ] ; then git commit -a -m 'auto applied code-formats changes' let COMMIT_CHG=${COMMIT_CHG}+1 @@ -152,6 +155,20 @@ if $CODE_FORMAT ; then fi fi +echo "We will now run formatting, and then linting checks" + +python3 ../cms-bot/run-python-format.py --inputfile "${UP_DIR}/code-checks-files.txt" --cmsswbase "${CMSSW_BASE}/src" + +pushd $CMSSW_BASE/src + git diff > ${UP_DIR}/python-code-format.patch || true + popd + +cp "python-linting.txt" "${UP_DIR}/python-linting.txt" || true + +if [ ! -f "python-linting.txt" ]; then + echo "Error: python-linting.txt was not created." +fi + if ${MULTIPLE_FILES_CHANGES} ; then ${CMS_BOT_DIR}/github_scripts/simultaneous_files_modifications_by_PRs.py ${PULL_REQUEST} > \ ${UP_DIR}/multiple_files_changes.txt || true @@ -176,6 +193,7 @@ if [ ${COMMIT_CHG} -gt 0 ] ; then popd exit 0 fi + RES="-code-checks" HOW_TO_RUN="\n\nCode check has found code style and quality issues which could be resolved by applying following patch(s)" if [ -s ${UP_DIR}/code-checks.patch ] ; then @@ -184,14 +202,24 @@ if [ ${COMMIT_CHG} -gt 0 ] ; then HOW_TO_RUN="${HOW_TO_RUN}\ne.g. \`curl -k https://cmssdt.cern.ch/SDT/${JENKINS_PREFIX_STR}code-checks/${REPO_USER}-PR-${PULL_REQUEST}/${BUILD_NUMBER}/code-checks.patch | patch -p1\`" HOW_TO_RUN="${HOW_TO_RUN}\nYou can also run \`scram build code-checks\` to apply code checks directly" fi + if [ -s ${UP_DIR}/code-format.patch ] ; then HOW_TO_RUN="${HOW_TO_RUN}\n\n- **code-format**:" HOW_TO_RUN="${HOW_TO_RUN}\nhttps://cmssdt.cern.ch/SDT/${JENKINS_PREFIX_STR}code-checks/${REPO_USER}-PR-${PULL_REQUEST}/${BUILD_NUMBER}/code-format.patch" HOW_TO_RUN="${HOW_TO_RUN}\ne.g. \`curl -k https://cmssdt.cern.ch/SDT/${JENKINS_PREFIX_STR}code-checks/${REPO_USER}-PR-${PULL_REQUEST}/${BUILD_NUMBER}/code-format.patch | patch -p1\`" HOW_TO_RUN="${HOW_TO_RUN}\nYou can also run \`scram build code-format\` to apply code format directly" fi + + if [ -s ${UP_DIR}/python-code-format.patch ] ; then + HOW_TO_RUN="${HOW_TO_RUN}\n\n- **python-code-format**:" + HOW_TO_RUN="${HOW_TO_RUN}\nhttps://cmssdt.cern.ch/SDT/${JENKINS_PREFIX_STR}code-checks/${REPO_USER}-PR-${PULL_REQUEST}/${BUILD_NUMBER}/python-code-format.patch" + HOW_TO_RUN="${HOW_TO_RUN}\ne.g. \`curl -k https://cmssdt.cern.ch/SDT/${JENKINS_PREFIX_STR}code-checks/${REPO_USER}-PR-${PULL_REQUEST}/${BUILD_NUMBER}/python-code-format.patch | patch -p1\`" + HOW_TO_RUN="${HOW_TO_RUN}\nYou can also run \`python3 PFA.py or python3 run-python-format.py --inputfile --cmsswbase \` to apply Python code formatting directly" + fi fi + + MSG="\n\nLogs: $UP_URL" if [ -s ${UP_DIR}/invalid-filenames.txt ] ; then RES="-code-checks" @@ -217,3 +245,40 @@ if ! $DRY_RUN ; then eval `scram unset -sh` ${CMS_BOT_DIR}/comment-gh-pr.py -r ${REPOSITORY} -p $PULL_REQUEST -R ${UP_DIR}/code-checks.md fi + +if [ -s ${UP_DIR}/python-code-format.patch ] ; then + echo "INFO: We're applying PEP8 formatting to CMSSW using Ruff; while Ruff found formatting errors in the PR's Python files, this non-blocking check may flag untouched lines, so feel free to contribute by applying the suggested patch." >> ${UP_DIR}/python-code-checks.md + + echo -e "\n\n- **python-code-format**:" >> ${UP_DIR}/python-code-checks.md + echo -e "https://cmssdt.cern.ch/SDT/${JENKINS_PREFIX_STR}code-checks/${REPO_USER}-PR-${PULL_REQUEST}/${BUILD_NUMBER}/python-code-format.patch" >> ${UP_DIR}/python-code-checks.md + echo -e "e.g. \`curl -k https://cmssdt.cern.ch/SDT/${JENKINS_PREFIX_STR}code-checks/${REPO_USER}-PR-${PULL_REQUEST}/${BUILD_NUMBER}/python-code-format.patch | patch -p1\`" >> ${UP_DIR}/python-code-checks.md + echo -e "You can also run \`python3 run-python-format.py --inputfile --cmsswbase \` to apply Python code formatting directly" >> ${UP_DIR}/python-code-checks.md + + echo -e "\n\n- **python-code-linting**:" >> ${UP_DIR}/python-code-checks.md + file="python-linting.txt" + content=$(<"$file") + if [ "$content" == "All checks passed!" ]; then + echo -e "Status: All linting checks passed! \xF0\x9F\x98\x8A " >> ${UP_DIR}/python-code-checks.md + else + echo -e "Status: There are some linting recommendations that need your attention. \xF0\x9F\x98\xA2" >> ${UP_DIR}/python-code-checks.md + echo -e "https://cmssdt.cern.ch/SDT/${JENKINS_PREFIX_STR}code-checks/${REPO_USER}-PR-${PULL_REQUEST}/${BUILD_NUMBER}/python-linting.txt" >> ${UP_DIR}/python-code-checks.md + echo -e "You can also run \`ruff check --fix\` to automatically apply fixes." >> ${UP_DIR}/python-code-checks.md + fi + if ! $DRY_RUN ; then + ${CMS_BOT_DIR}/comment-gh-pr.py -r ${REPOSITORY} -p $PULL_REQUEST -R ${UP_DIR}/python-code-checks.md + fi +else + echo "INFO: We're applying PEP8 formatting to CMSSW using Ruff; while Ruff found formatting errors in the PR's Python files, this non-blocking check may flag untouched lines, so feel free to contribute by applying the suggested patch." >> ${UP_DIR}/python-code-checks.md + echo -e "\n\n- **python-code-linting**:" >> ${UP_DIR}/python-code-checks.md + file="python-linting.txt" + content=$(<"$file") + if [ "$content" == "All checks passed!" ]; then + echo -e "Status: All linting checks passed! \xF0\x9F\x98\x8A " >> ${UP_DIR}/python-code-checks.md + else + echo -e "Status: There are some linting recommendations that need your attention. \xF0\x9F\x98\xA2" >> ${UP_DIR}/python-code-checks.md + echo -e "https://cmssdt.cern.ch/SDT/${JENKINS_PREFIX_STR}code-checks/${REPO_USER}-PR-${PULL_REQUEST}/${BUILD_NUMBER}/python-linting.txt" >> ${UP_DIR}/python-code-checks.md + echo -e "You can also run \`ruff check --fix\` to automatically apply fixes." >> ${UP_DIR}/python-code-checks.md + ${CMS_BOT_DIR}/comment-gh-pr.py -r ${REPOSITORY} -p $PULL_REQUEST -R ${UP_DIR}/python-code-checks.md + fi +fi + diff --git a/run-python-format.py b/run-python-format.py new file mode 100644 index 000000000000..a67948bc3ef1 --- /dev/null +++ b/run-python-format.py @@ -0,0 +1,64 @@ +#!/usr/bin/python3 +import os +import argparse +def main(): + parser = argparse.ArgumentParser(description="Run Python formatting and linting.") + parser.add_argument( + "--inputfile", + required=True, + help="Path to the file containing the list of files to process.", + ) + parser.add_argument( + "--cmsswbase", + required=True, + help="Path to the CMSSW base directory.", + ) + args = parser.parse_args() + + input_file = args.inputfile + cmssw_base = args.cmsswbase + + if not os.path.isfile(input_file): + print("Error: {} does not exist.".format(input_file)) + return + try: + with open(input_file, "r") as file: + files_list = [ + os.path.join(cmssw_base, line.strip()) for line in file if line.strip() + ] + except IOError as e: + print("Error reading {}: {}".format(input_file, e)) + return + with open("python-linting.txt", "w") as linting_output: + if not files_list: + linting_output.write("No files to check.\n") + print("No files to check. Exiting.") + return + + all_checks_passed = True + for file in files_list: + if os.path.isfile(file): + check_command = "ruff check {} >> python-linting.txt".format(file) + result = os.system(check_command) + if result != 0: + all_checks_passed = False + + if all_checks_passed: + linting_output.write("All checks passed!\n") + + print("Python linting completed. Check 'python-linting.txt' for details.") + pfa_command = ( + "python3 ../cms-bot/PFA.py " + + " ".join(files_list) + ) + + format_result = os.system(pfa_command) + + if format_result == 0: + print("Successfully formatted files.") + else: + print("An error occurred while running PFA.py. Exit code: {}".format(format_result)) + +if __name__ == "__main__": + main() + From 1c95ad846aec3d0c9c2dce204a5b04515589ce20 Mon Sep 17 00:00:00 2001 From: aandvalenzuela Date: Mon, 18 Nov 2024 13:00:38 +0100 Subject: [PATCH 2/3] Cleanup --- run-pr-code-checks | 1 - 1 file changed, 1 deletion(-) diff --git a/run-pr-code-checks b/run-pr-code-checks index f39e9ee49f3f..7dab93b890b8 100644 --- a/run-pr-code-checks +++ b/run-pr-code-checks @@ -121,7 +121,6 @@ if $CODE_CHECKS ; then send_jenkins_artifacts ${UP_DIR}/ pr-code-checks/${REPO_USER}-PR-${PULL_REQUEST}/${BUILD_NUMBER} ${CMS_BOT_DIR}/comment-gh-pr.py -r ${REPOSITORY} -p $PULL_REQUEST -R ${UP_DIR}/code-checks.md fi - #exit 0 exit 0 fi if [ -e ${CMSSW_BASE}/tmp/${SCRAM_ARCH}/code-checks-logs ] ; then From 5b9be8f529c7289cdd97f2c20d1f96e7e49dc7b7 Mon Sep 17 00:00:00 2001 From: aandvalenzuela Date: Mon, 18 Nov 2024 14:26:19 +0100 Subject: [PATCH 3/3] Apply formatting --- PFA.py | 17 +++++++++++------ run-python-format.py | 13 +++++-------- 2 files changed, 16 insertions(+), 14 deletions(-) diff --git a/PFA.py b/PFA.py index aeb86bca415a..65c36620f2c0 100644 --- a/PFA.py +++ b/PFA.py @@ -1,10 +1,14 @@ #!/usr/bin/python3 import os import argparse + + def find_file_path(file): if not os.path.isfile(file): return None return os.path.realpath(file) + + def find_python_files(directory): py_files = [] for root, dirs, files in os.walk(directory): @@ -12,6 +16,8 @@ def find_python_files(directory): if file.endswith(".py"): py_files.append(os.path.join(root, file)) return py_files + + def CodeQualityChecks(python_files): if python_files: for file in python_files: @@ -23,15 +29,13 @@ def CodeQualityChecks(python_files): format_command = "ruff format {}".format(file_path) print("Executing command: {}".format(format_command)) # Debug print os.system(format_command) + + def main(): parser = argparse.ArgumentParser( description="Linting and formatting Python files in directories or file paths." ) - parser.add_argument( - "paths", - nargs="+", - help="List of directories or file paths to process" - ) + parser.add_argument("paths", nargs="+", help="List of directories or file paths to process") args = parser.parse_args() all_python_files = [] @@ -44,6 +48,7 @@ def main(): print("Error: {} is not a valid file or directory.".format(path)) return CodeQualityChecks(all_python_files) + + if __name__ == "__main__": main() - diff --git a/run-python-format.py b/run-python-format.py index a67948bc3ef1..e8a86366dd96 100644 --- a/run-python-format.py +++ b/run-python-format.py @@ -1,6 +1,8 @@ #!/usr/bin/python3 import os import argparse + + def main(): parser = argparse.ArgumentParser(description="Run Python formatting and linting.") parser.add_argument( @@ -23,9 +25,7 @@ def main(): return try: with open(input_file, "r") as file: - files_list = [ - os.path.join(cmssw_base, line.strip()) for line in file if line.strip() - ] + files_list = [os.path.join(cmssw_base, line.strip()) for line in file if line.strip()] except IOError as e: print("Error reading {}: {}".format(input_file, e)) return @@ -47,10 +47,7 @@ def main(): linting_output.write("All checks passed!\n") print("Python linting completed. Check 'python-linting.txt' for details.") - pfa_command = ( - "python3 ../cms-bot/PFA.py " - + " ".join(files_list) - ) + pfa_command = "python3 ../cms-bot/PFA.py " + " ".join(files_list) format_result = os.system(pfa_command) @@ -59,6 +56,6 @@ def main(): else: print("An error occurred while running PFA.py. Exit code: {}".format(format_result)) + if __name__ == "__main__": main() -