falkTX / Cadence

Collection of tools useful for audio production
GNU General Public License v2.0
368 stars 80 forks source link

Unset executable bits & remove !#shebang #306

Closed ratijas closed 3 years ago

ratijas commented 3 years ago

Patch 1/3

I'm not an expert, but those XMLs do not look like any kind of normally executable scripts. And even if they were -- XML syntax does not allow shebangs, as far as standard1 is concerned.

Patch 2/3

Drop unused global var WINEASIO_PREFIX

It was mis-formatted anyway. Looks like a copy-paste from wineasio which is broken in the exact same way1.

1 https://github.com/wineasio/wineasio/blob/638ce56a7f872e8556b45949f7dfa734f4bd366c/gui/settings.py#L41

Patch 3/3

Unset executable bit & remove !#shebang

Unset executable bit from all python sources upon installation, and remove shebang from all python modules w/o if __name__ == '__main__'.

There are corresponding wrappers provided in the $(PREFIX)/bin/ for each python script (not ui_*.py), so there should be no need to call these source files directly as commands. Even those python sources which do not have a script in $(PREFIX)/bin/ -- they are just being imported into other modules, and their __name__ == '__main__' exists simply for debugging purposes.

Also, restricting to only single valid entry point adheres to

The Zen of Python

There should be one-- and preferably only one --obvious way to do it.

Fixes #240

ratijas commented 3 years ago

Ping, @falkTX

falkTX commented 3 years ago

The removal of the python3 shebang is not wanted, the rest is fine. if you restore the shebang stuff, I will happily merge

ratijas commented 3 years ago

The removal of the python3 shebang is not wanted, the rest is fine.

Mind if I ask why?

In the meantime, it's OK to merge just two out of three patches manually, and leave the rest for the follow-up PRs.

falkTX commented 3 years ago

Mind if I ask why?

because that sets the filetype in the header. there is no real point on removing it.

$ cat test.py 
# hello
$ file test.py
test.py: ASCII text

$ cat test2.py 
#!/usr/bin/env python3
# hello
$ file test2.py
test2.py: Python script, ASCII text executable
ratijas commented 3 years ago

uhm... what's the point of using file command? Shebang was meant to specify an interpreter for executable scripts, not to handle mime types or whatnot.

ratijas commented 3 years ago

By the way. The script cadence-session-start starts with a switch which chooses python interpreter.

if [ -f /usr/bin/python3 ]; then
  PYTHON=/usr/bin/python3
else
  PYTHON=python
fi

This is redundant and unneeded, since the only Python script it calls is cadence_session_start.py, which in turn has correct shebang on top:

#!/usr/bin/env python3
# -*- coding: utf-8 -*-

# Imports (Global)
import dbus, sys
...

So, the project doesn't even use those shebangs as designed, but you are worried about file utility mistaking python for plain ASCII text. lol wat?

falkTX commented 3 years ago

python3 is not always available as binary to run, sometimes it is just python.

I am not worried about the file tool, I just care that the files have the right filetype. I posted that as an example of a tool that reads the file as regular text file instead of a python file as it really is.

PS: the snarky tone is not necessary or welcome here.

ratijas commented 3 years ago

First of all,

PS: the snarky tone is not necessary or welcome here.

I'm sorry, I failed at trying my best.

python3 is not always available as binary to run, sometimes it is just python.

Looking at what internet has to say on topic, this issue has been long sorted out in Python community, and installing python3 to PATH is officially recommended way.

https://stackoverflow.com/questions/62214293/is-python3-always-installed-with-python-3

There is an answer on SO which links to PEP 394. It was created at 2011, so I'd expect most major distributions to follow it by now. Can you provide supported counter-examples where python3 is not available in PATH?

I am not worried about the file tool, I just care that the files have the right filetype. I posted that as an example of a tool that reads the file as regular text file instead of a python file as it really is.

"filetype" is purely informative/non-normative, and shouldn't be relied upon in any way. Most files on *NIX are either pre-compiled binaries, text files or devices. There is no need in getting some phantom filetype of application's sources.

falkTX commented 3 years ago

"filetype" is purely informative/non-normative, and shouldn't be relied upon in any way.

it is precisely for UI/informative reasons that I keep the shebang, I mean why not. I see no reason to remove them, it is more pleasant if they are in there.

the python/python3 naming thing comes from past experience with archlinux where python was available but not python3. I dont use archlinux, so that changed now? if yes, good to know.

ratijas commented 3 years ago

it is precisely for UI/informative reasons that I keep the shebang, I mean why not. I see no reason to remove them, it is more pleasant if they are in there.

As a personal preference, that might make sense. As a technical decision, it doesn't hold water. Shebang marks library/component file as an executable, which is logically wrong and a misleading information (UI which you care about). I only tried argue about the technical part; but if that's how you like them to be — OK, whatever makes you happy.

the python/python3 naming thing comes from past experience with archlinux where python was available but not python3. I dont use archlinux, so that changed now? if yes, good to know.

Nowdays in Arch Linux, all three variants are available, with plain old python defaulting to python3.

falkTX commented 3 years ago

Thank you

ratijas commented 3 years ago

Ah, thank you too, of course. What a relieve. 🥳

For future reference and note to myself: the link in second patch turned out to be mis-formatted (in terms of markdown) — it should've been [N]: with double-colon at the end. Otherwise, it'd just get converted to a link itself instead of defining one.