Still some wrong permissions vs. shebang

[kbabioch]

Describe the bug
#461 is still not totally resolved. With the latest release (4.5.6) these files are still broken:

[   18s] python2-pyfakefs.noarch: W: non-executable-script /usr/lib/python2.7/site-packages/pyfakefs/tests/all_tests.py 644 /usr/bin/env python
[   18s] python2-pyfakefs.noarch: W: non-executable-script /usr/lib/python2.7/site-packages/pyfakefs/tests/all_tests_without_extra_packages.py 644 /usr/bin/env python
[   18s] python2-pyfakefs.noarch: W: non-executable-script /usr/lib/python2.7/site-packages/pyfakefs/tests/example_test.py 644 /usr/bin/env python
[   18s] python3-pyfakefs.noarch: W: non-executable-script /usr/lib/python3.6/site-packages/pyfakefs/tests/all_tests.py 644 /usr/bin/env python
[   18s] python3-pyfakefs.noarch: W: non-executable-script /usr/lib/python3.6/site-packages/pyfakefs/tests/all_tests_without_extra_packages.py 644 /usr/bin/env python
[   18s] python3-pyfakefs.noarch: W: non-executable-script /usr/lib/python3.6/site-packages/pyfakefs/tests/example_test.py 644 /usr/bin/env python
[   18s] This text file contains a shebang or is located in a path dedicated for
[   18s] executables, but lacks the executable bits and cannot thus be executed.  If
[   18s] the file is meant to be an executable script, add the executable bits,
[   18s] otherwise remove the shebang or move the file elsewhere.

How To Reproduce
Check shebang lines and/or executable bits on those files.

Your enviroment
Does not matter.

[mrbean-bremen]

Hm, I re-added the shebang together with the executable bit to these files as proposed by @jmcgeheeiv. In the commit for all files is shown 100644 → 100755, so I thought that worked... strange.

[mrbean-bremen]

This is what I get (admittedly this is under Windows, but the permissions shall be the same):

$ git clone https://github.com/jmcgeheeiv/pyfakefs pyfakefs.release -b version-3.5.6
$ ll pyfakefs.release/pyfakefs/tests/
total 600
-rw-r--r-- 1 mrbean 197121      0 Jan 13 20:09 __init__.py
-rwxr-xr-x 1 mrbean 197121   2779 Jan 13 20:09 all_tests.py*
-rwxr-xr-x 1 mrbean 197121   1485 Jan 13 20:09 all_tests_without_extra_packages.py*
-rw-r--r-- 1 mrbean 197121   3056 Jan 13 20:09 dynamic_patch_test.py
-rw-r--r-- 1 mrbean 197121   4419 Jan 13 20:09 example.py
-rwxr-xr-x 1 mrbean 197121   7466 Jan 13 20:09 example_test.py*
-rw-r--r-- 1 mrbean 197121   2643 Jan 13 20:09 fake_filesystem_glob_test.py
...

This looks ok to me, so I'm a bit lost here...

[mrbean-bremen]

Ok, just to be sure, did the same under Ubuntu, with the same result.
@kbabioch - could it be that the executable bit gets lost during packaging, maybe due to some missing setting in pyfakefs?
EDIT: Forgot to mention that - I also installed the package and checked the mode in the installed files, with the same result.

[kbabioch]

Thanks for investigating. I've looked somewhat into this, and can confirm that the permissions for those files are correct in the release tarbal itself.

However, when using setuptools they are considered to be data files and will be copied/moved into place with 644 (i.e. with no permission bit). This has nothing to do with the packaging on our end, but is just intrinsic to setuptools itself.

I think you have multiple options here:

• Do not distribute those unit test files at all
• Remove the shebang altogether, since they are considered to be data files and won't be invoked directly, but always via pytest
• Fix the permissions after the fact using setuptools

[mrbean-bremen]

Ok, thanks - distributing the tests has been an request, and seems to be more or less the default now, so I think we will retain this. Fixing the permissions using setuptools sounds like a plan, I will look into this later.

[mrbean-bremen]

After reading up a bit about this it seems that there is no really clean solution here. I'm inclined to remove the shebang again instead... @jmcgeheeiv - what do you think?

[jmcgeheeiv]

Backing up a bit, how is it that these files are being considered data files? The purpose of setuptools is to install executable software, so I am surprised that executable files are difficult to handle.

[mrbean-bremen]

Well, there is actually the possibility to use entry_points for scripts, but that is not really suitable for these test scripts in my opinion.

[jmcgeheeiv]

Indeed entry_points does not sound right.

[mrbean-bremen]

Shall I remove the shebang in that case? I think that usually the tests are not run as a standalone script, anyway.

[jmcgeheeiv]

Yes, let's eliminate this issue in the most expedient manner possible.

[mrbean-bremen]

Ok, I backed out the respective commit.

[jmcgeheeiv]

I just found another reference that can guide us--nose wants the test files to be not executable:

nose, by default, follows a few simple rules for test discovery.

• ...
• Files with the executable bit set are ignored by default under Unix-style operating systems–use --exe to allow collection from them...

So, nose agrees that the test files should be not executable, and therefore should not contain a shebang.

[mrbean-bremen]

@kbabioch - the warning still seems to appear in the SLE_12_SP4 build, but not in the other builds with the latest version - have you any idea why this might happen?

[jmcgeheeiv]

I'm sorry, I have no insight into SLE.

…

On Sat, Feb 16, 2019 at 8:12 AM mrbean-bremen ***@***.***> wrote: @kbabioch <https://github.com/kbabioch> - the warning still seems to appear in the SLE_12_SP4 build, but not in the other builds with the latest version - have you any idea why this might happen? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#463 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ADjLAfoBiOtMwTZ3uC2P5LQSyEYhGwbyks5vOC3XgaJpZM4Z9SEU> .

[mrbean-bremen]

Ok, just noticed that the warning for the SLE_12 build is not the original problem, but another one:

python-pyfakefs.src:63: W: files-attr-not-set
python-pyfakefs.src:64: W: files-attr-not-set
python-pyfakefs.src:65: W: files-attr-not-set
A file or a directory entry in a %files section does not have attributes set
which may result in unexpected file permissions and thus security issues in
the resulting binary package depending on the build environment and rpmbuild
version (typically < 4.4).  Add default attributes using %defattr before it in
the %files section, or use per entry %attr's.

I think we can ignore this, as this seems to be specific to this version and not a regression.
@kbabioch - please reopen if you think this can/shall be handled.

[kbabioch]

@mrbean-bremen This problem is not related to yours. It is an RPM packaging issue. I will address it, once I touch the package again. Thank you once again for all the love you put into this ;-).