4:21 a.m.
As suggested in [1].
[1] https://listman.redhat.com/archives/libvir-list/2022-December/236211.html
*** BLURB HERE ***
Andrea Bolognani (2):
docs: Recommend better python3 shebang
syntax-check: Ensure Python is called via env(1)
build-aux/syntax-check.mk | 5 +++++
docs/hooks.rst | 2 +-
2 files changed, 6 insertions(+), 1 deletion(-)
--
2.39.2
4:21 a.m.
New subject: [libvirt PATCH 1/2] docs: Recommend better python3 shebang
Python scripts should always invoked the interpreter through
env(1) to ensure that they work on macOS and the BSDs, and at
this point not explicitly asking for Python 3 doesn't really
make sense.
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
docs/hooks.rst | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/docs/hooks.rst b/docs/hooks.rst
index 9c5f3ff456..9387676a55 100644
--- a/docs/hooks.rst
+++ b/docs/hooks.rst
@@ -78,7 +78,7 @@ or:
::
- #!/usr/bin/python
+ #!/usr/bin/env python3
Other command interpreters are equally valid, as is any executable binary, so
you are welcome to use your favourite languages.
--
2.39.2
4:21 a.m.
New subject: [libvirt PATCH 2/2] syntax-check: Ensure Python is called via env(1)
The syntax-check rule that calls flake8 on Python scripts
expects this to be the case, and it's the best practice anyway.
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
build-aux/syntax-check.mk | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/build-aux/syntax-check.mk b/build-aux/syntax-check.mk
index 96d322ee04..21f6b311ce 100644
--- a/build-aux/syntax-check.mk
+++ b/build-aux/syntax-check.mk
@@ -561,6 +561,11 @@ sc_require_enum_last_marker:
{ echo 'enum impl needs _LAST marker on second line' 1>&2; \
exit 1; } || :
+sc_prohibit_python_without_env:
+ @prohibit='#!/usr/.*/py''thon' \
+ halt='always call python via /usr/bin/env' \
+ $(_sc_search_regexp)
+
# We're intentionally ignoring a few warnings
#
# E501: Force breaking lines at < 80 characters results in
--
2.39.2
4:40 a.m.
New subject: [libvirt PATCH 2/2] syntax-check: Ensure Python is called via env(1)
On Mon, Feb 20, 2023 at 11:21:46AM +0100, Andrea Bolognani wrote:
The syntax-check rule that calls flake8 on Python scripts
expects this to be the case, and it's the best practice anyway.
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
build-aux/syntax-check.mk | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/build-aux/syntax-check.mk b/build-aux/syntax-check.mk
index 96d322ee04..21f6b311ce 100644
--- a/build-aux/syntax-check.mk
+++ b/build-aux/syntax-check.mk
@@ -561,6 +561,11 @@ sc_require_enum_last_marker:
{ echo 'enum impl needs _LAST marker on second line' 1>&2; \
exit 1; } || :
+sc_prohibit_python_without_env:
+ @prohibit='#!/usr/.*/py''thon' \
Shouldn't this be just '#!/usr/.*/python' ?
Erik
+ halt='always call python via /usr/bin/env' \
+ $(_sc_search_regexp)
+
# We're intentionally ignoring a few warnings
#
# E501: Force breaking lines at < 80 characters results in
--
2.39.2
5:02 a.m.
New subject: [libvirt PATCH 2/2] syntax-check: Ensure Python is called via env(1)
On Mon, Feb 20, 2023 at 11:40:34AM +0100, Erik Skultety wrote:
On Mon, Feb 20, 2023 at 11:21:46AM +0100, Andrea Bolognani wrote:
> +sc_prohibit_python_without_env:
> + @prohibit='#!/usr/.*/py''thon' \
Shouldn't this be just '#!/usr/.*/python' ?
Yes, but then the syntax-check rule would flag this very line :)
We could do the "exempt from syntax-check" dance, but that feels
unnecessarily complicated in this scenario. sc_copyright_usage
already uses a similar trick to prevent the regex to match itself.
--
Andrea Bolognani / Red Hat / Virtualization
7:33 a.m.
New subject: [libvirt PATCH 2/2] syntax-check: Ensure Python is called via env(1)
On Mon, Feb 20, 2023 at 03:02:42AM -0800, Andrea Bolognani wrote:
On Mon, Feb 20, 2023 at 11:40:34AM +0100, Erik Skultety wrote:
> On Mon, Feb 20, 2023 at 11:21:46AM +0100, Andrea Bolognani wrote:
> > +sc_prohibit_python_without_env:
> > + @prohibit='#!/usr/.*/py''thon' \
>
> Shouldn't this be just '#!/usr/.*/python' ?
Yes, but then the syntax-check rule would flag this very line :)
We could do the "exempt from syntax-check" dance, but that feels
unnecessarily complicated in this scenario. sc_copyright_usage
already uses a similar trick to prevent the regex to match itself.
Reviewed-by: Erik Skultety <eskultet@redhat>
10:38 a.m.
New subject: [libvirt PATCH 2/2] syntax-check: Ensure Python is called via env(1)
On Mon, Feb 20, 2023 at 02:33:55PM +0100, Erik Skultety wrote:
On Mon, Feb 20, 2023 at 03:02:42AM -0800, Andrea Bolognani wrote:
> On Mon, Feb 20, 2023 at 11:40:34AM +0100, Erik Skultety wrote:
> > On Mon, Feb 20, 2023 at 11:21:46AM +0100, Andrea Bolognani wrote:
> > > +sc_prohibit_python_without_env:
> > > + @prohibit='#!/usr/.*/py''thon' \
> >
> > Shouldn't this be just '#!/usr/.*/python' ?
>
> Yes, but then the syntax-check rule would flag this very line :)
>
> We could do the "exempt from syntax-check" dance, but that feels
> unnecessarily complicated in this scenario. sc_copyright_usage
> already uses a similar trick to prevent the regex to match itself.
Reviewed-by: Erik Skultety <eskultet@redhat>
Missing the .com here, you might have a misconfiguration somewhere.
Anyway, thanks for the review! Can you please ACK the first patch in
the series too? I can't push this one without it.
--
Andrea Bolognani / Red Hat / Virtualization
10:53 a.m.
New subject: [libvirt PATCH 2/2] syntax-check: Ensure Python is called via env(1)
On Mon, Feb 20, 2023 at 08:38:32AM -0800, Andrea Bolognani wrote:
On Mon, Feb 20, 2023 at 02:33:55PM +0100, Erik Skultety wrote:
> On Mon, Feb 20, 2023 at 03:02:42AM -0800, Andrea Bolognani wrote:
> > On Mon, Feb 20, 2023 at 11:40:34AM +0100, Erik Skultety wrote:
> > > On Mon, Feb 20, 2023 at 11:21:46AM +0100, Andrea Bolognani wrote:
> > > > +sc_prohibit_python_without_env:
> > > > + @prohibit='#!/usr/.*/py''thon' \
> > >
> > > Shouldn't this be just '#!/usr/.*/python' ?
> >
> > Yes, but then the syntax-check rule would flag this very line :)
> >
> > We could do the "exempt from syntax-check" dance, but that feels
> > unnecessarily complicated in this scenario. sc_copyright_usage
> > already uses a similar trick to prevent the regex to match itself.
>
> Reviewed-by: Erik Skultety <eskultet@redhat>
Missing the .com here, you might have a misconfiguration somewhere.
Anyway, thanks for the review! Can you please ACK the first patch in
the series too? I can't push this one without it.
Sorry, I thought I replied to the cover letter. You can assume my RB for patch
1 as well.
Erik
675
days inactive
675
days old
7 comments
2 participants
participants (2)
-
Andrea Bolognani -
Erik Skultety