Merge lp:~intrigeri/apparmor/utils-keep-shebang into lp:apparmor/2.12

[intrigeri]

This patch by Adam Conrad <email address hidden> was added to the Ubuntu packaging in the "Patches backported from upstream fix submissions" section, so I assume it was meant to be merged upstream at some point?

I'm not sure I understand what's the rationale behind this change, the patch doesn't really tell.
And If we want this upstream, I guess we'll want to drop the "if False" thing.

So at this stage, this MR is more a request for info from the Ubuntu folks than a real "please apply upstream".

[Steve Beattie (sbeattie)]

What this patch is doing is disabling the shbang rewriting (converting '#!/usr/bin/env python' in the scripts to the python in the PYTHON variable) that occurs during the make install phase.

However, as of http://bazaar.launchpad.net/~apparmor-dev/apparmor/master/revision/3555 , all of the python utilities are hardcoded to use python3, so the snippet in question is redundant. So really what should happen is a patch like the following:

Index: b/utils/python-tools-setup.py
===================================================================
--- a/utils/python-tools-setup.py
+++ b/utils/python-tools-setup.py
@@ -41,14 +41,7 @@ class Install(_install, object):
         self.mkpath(prefix + os.path.dirname(scripts[0]))
         for s in scripts:
             f = prefix + s
- # If we have a defined python version, use it instead of the system
- # default
- if 'PYTHON' in os.environ:
- lines = open(os.path.basename(s)).readlines()
- lines[0] = '#! /usr/bin/env %s\n' % os.environ['PYTHON']
- open(f, 'w').write("".join(lines))
- else:
- self.copy_file(os.path.basename(s), f)
+ self.copy_file(os.path.basename(s), f)

         configs = ['easyprof/easyprof.conf']
         self.mkpath(prefix + "/etc/apparmor")

and dropping Adam's patch.

I'll submit the above to the list.

Thanks!

review: Disapprove

[Steve Beattie (sbeattie)]

Oh, I take what I said slightly back. If the PYTHON environment variable is set, it will always overwrite it (but just for aa-easyprof, as it's the only util covered by the setup-tools script, because the rest get installed in ${PREFIX}/sbin/). Given that we've deprecated python 2, going forward the only reason to use this would be to specify a specific version of python3 (or a hypothetical future python4) because the code was incompatible with the default python3. I don't think it's worth supporting that at this time, and still feel dropping the dead code is the preferred option here.

[Steve Beattie (sbeattie)]

Marking rejected, issue addressed as described above in http://bazaar.launchpad.net/~apparmor-dev/apparmor/master/revision/3724

[intrigeri (intrigeri)]

Steve Beattie:
> Marking rejected, issue addressed as described above in http://bazaar.launchpad.net/~apparmor-dev/apparmor/master/revision/3724

Thanks, I'm glad we can remove one more patch from the Debian/Ubuntu packaging once a new upstream release is out :)

Unmerged revisions

3717. By intrigeri on 2017-10-23

Stop inappropriately mangling script shebangs.

Patch by Adam Conrad <email address hidden>

=== modified file 'utils/python-tools-setup.py'
--- utils/python-tools-setup.py	2015-01-13 21:03:11 +0000
+++ utils/python-tools-setup.py	2017-10-23 14:28:31 +0000
@@ -43,7 +43,7 @@
             f = prefix + s
             # If we have a defined python version, use it instead of the system
             # default
-            if 'PYTHON' in os.environ:
+            if False:
                 lines = open(os.path.basename(s)).readlines()
                 lines[0] = '#! /usr/bin/env %s\n' % os.environ['PYTHON']
                 open(f, 'w').write("".join(lines))