Add python3 script shebang lint #12972

ken2812221 · 2018-04-13T03:43:12Z

base58.py can executed by python3

Empact · 2018-04-13T10:31:19Z

Good candidate for a lint?

maflcko · 2018-04-13T13:37:42Z

To the best of my knowledge, the script is outdated and won't produce test vectors in the current format. Since you are touching this file, might as well fix this in one go?

maflcko · 2018-04-13T13:47:56Z

Also the readme should be updated to reflect the new names, i.e. "src/test/data/key_io_valid.json" and "src/test/data/key_io_invalid.json"

practicalswift · 2018-04-13T14:00:28Z

@Empact The linter you're thinking about would check that all *.py files start with #!/usr/bin/env python3? Anything more?

ken2812221 · 2018-04-13T14:26:10Z

@MarcoFalke since we don't generate testdata dynamically, should we delete this testgen directory?

ken2812221 · 2018-04-13T14:36:11Z

I don't know how to deal with contrib/devtools/test-security-check.py, it seems outdated, and not python3 code

jnewbery · 2018-04-13T14:56:16Z

Concept ACK adding the linter, however it should only return an error if there's a python or python2 shebang. It's fine to have no shebang if the module is not intended to be run as a standalone executable.

I don't think we should add shebangs to modules which aren't intended to be run as standalone executables.

test-security-check.py was overlooked in #11881. You can convert it to Python3 with the following commit: jnewbery@35cb5da

practicalswift · 2018-04-13T15:01:00Z

@ken2812221 What about this linter?

#!/bin/bash

EXIT_CODE=0
for PYTHON_FILE in $(git ls-files -- "*.py"); do
    if [[ $(head -c 2 "${PYTHON_FILE}") == "#!" &&
          $(head -n 1 "${PYTHON_FILE}") != "#!/usr/bin/env python3" ]]; then
        echo "Missing shebang \"#!/usr/bin/env python3\" in ${PYTHON_FILE}"
        EXIT_CODE=1
    fi
done
exit ${EXIT_CODE}

ken2812221 · 2018-04-13T15:31:26Z

@practicalswift Seems your lint is flexible

ken2812221 · 2018-04-13T16:20:32Z

@jnewbery thanks

practicalswift · 2018-04-13T16:24:02Z

utACK 294e59675bf326eb0e81a6685410f25df5619be4

jnewbery · 2018-04-13T17:24:06Z

contrib/testgen/base58.py

@@ -1,3 +1,4 @@
+#!/usr/bin/env python3


I still don't think base58.py requires a shebang. It's not executable:

→ ls -l contrib/testgen/base58.py -rw-rw-r-- 1 ubuntu ubuntu 3092 Apr 13 14:37 contrib/testgen/base58.py

jnewbery · 2018-04-13T17:24:44Z

contrib/devtools/lint-python-shebang.sh

+for PYTHON_FILE in $(git ls-files -- "*.py"); do
+    if [[ $(head -c 2 "${PYTHON_FILE}") == "#!" &&
+          $(head -n 1 "${PYTHON_FILE}") != "#!/usr/bin/env python3" ]]; then
+        echo "Missing shebang \"#!/usr/bin/env python3\" in ${PYTHON_FILE}"


Comment could be clearer. Something like "Shebang must use python3 (not python or python2)"

practicalswift · 2018-04-14T05:13:38Z

utACK 2bff472

Empact · 2018-04-15T23:46:13Z

utACK 2bff472

jnewbery · 2018-04-16T14:31:21Z

utACK 2bff472

2bff472 [contrib] convert test-security-check to python3 (John Newbery) 958bf40 add lint tool to check python3 shebang (practicalswift) Pull request description: base58.py can executed by python3 Tree-SHA512: 30511204feefd4ccd5b4bf698fb88e516633e692dc95d31fe957b1c0c4879de25906355b28a5a0522171887315c8464a611e601ff00540db172d5bd463ee13d9

fanquake added the Scripts and tools label Apr 13, 2018

jnewbery reviewed Apr 13, 2018

View reviewed changes

practicalswift and others added 2 commits April 14, 2018 02:13

add lint tool to check python3 shebang

958bf40

[contrib] convert test-security-check to python3

2bff472

ken2812221 changed the title ~~trivial: Use python3 explicitly~~ Add python3 script shebang lint Apr 13, 2018

maflcko merged commit 2bff472 into bitcoin:master Apr 16, 2018

ken2812221 deleted the explicit_python3 branch April 17, 2018 09:29

Bushstar mentioned this pull request Apr 17, 2018

commits from bitcoin/master FeatherCoin/Feathercoin#298

Merged

str4d mentioned this pull request Oct 27, 2020

Backport useful lints from upstream zcash/zcash#4829

Merged

bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021

Add python3 script shebang lint #12972

Add python3 script shebang lint #12972

Uh oh!

Conversation

ken2812221 commented Apr 13, 2018

Uh oh!

Empact commented Apr 13, 2018

Uh oh!

maflcko commented Apr 13, 2018

Uh oh!

maflcko commented Apr 13, 2018

Uh oh!

practicalswift commented Apr 13, 2018

Uh oh!

ken2812221 commented Apr 13, 2018

Uh oh!

ken2812221 commented Apr 13, 2018

Uh oh!

jnewbery commented Apr 13, 2018

Uh oh!

practicalswift commented Apr 13, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ken2812221 commented Apr 13, 2018

Uh oh!

ken2812221 commented Apr 13, 2018

Uh oh!

practicalswift commented Apr 13, 2018

Uh oh!

jnewbery Apr 13, 2018

Choose a reason for hiding this comment

Uh oh!

jnewbery Apr 13, 2018

Choose a reason for hiding this comment

Uh oh!

practicalswift commented Apr 14, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Empact commented Apr 15, 2018

Uh oh!

jnewbery commented Apr 16, 2018

Uh oh!

Uh oh!

practicalswift commented Apr 13, 2018 •

edited

Loading

practicalswift commented Apr 14, 2018 •

edited

Loading