mate-desktop / pluma

A powerful text editor for MATE
http://www.mate-desktop.org
GNU General Public License v2.0
158 stars 66 forks source link

build: fix shebangs #626

Closed raveit65 closed 3 years ago

raveit65 commented 3 years ago

From fedora build server with pluma-1.24.2 https://koji.fedoraproject.org/koji/getfile?taskID=64704645&volume=DEFAULT&name=build.log&offset=-4000

+ /usr/lib/rpm/redhat/brp-mangle-shebangs
mangling shebang in /usr/share/pluma/plugins/externaltools/tools/build from /bin/sh to #!/usr/bin/sh
mangling shebang in /usr/share/pluma/plugins/externaltools/tools/open-terminal-here from /bin/sh to #!/usr/bin/sh
mangling shebang in /usr/share/pluma/plugins/externaltools/tools/remove-trailing-spaces from /bin/sh to #!/usr/bin/sh
mangling shebang in /usr/share/pluma/plugins/externaltools/tools/run-command from /bin/sh to #!/usr/bin/sh
mangling shebang in /usr/share/pluma/plugins/externaltools/tools/search-recursive from /bin/bash to #!/usr/bin/bash
*** ERROR: ambiguous python shebang in /usr/share/pluma/plugins/externaltools/tools/switch-c: #!/usr/bin/python. Change it to python3 (or python2) explicitly.
mangling shebang in /usr/libexec/pluma/pluma-bugreport.sh from /bin/sh to #!/usr/bin/sh
error: Bad exit status from /var/tmp/rpm-tmp.3qkkmn (%install)
RPM build errors:
    Bad exit status from /var/tmp/rpm-tmp.3qkkmn (%install)
Child return code was: 1
EXCEPTION: [Error()]
lukefromdc commented 3 years ago

I see in plugins/externaltools/data/switch-c.tool.in tthe replacement of /usr/bin/python with /usr/bin/python3, a change which is needed throughout all code now at least in Debian where /usr/bin/python is no longer provided by any package. Bear in mind that Python 2 is on its way out almost everywhere now

Anywhere else in MATE we see this we need to make the same change

raveit65 commented 3 years ago

Well, the other warnings are created by a script when build for fedora/Redhat. /usr/lib/rpm/redhat/brp-mangle-shebangs. It's only a warning but i prefer clean builds.

raveit65 commented 3 years ago
[root@mother rave]# dnf provides /usr/lib/rpm/redhat/brp-mangle-shebangs
Last metadata expiration check: 1:04:15 ago on Mon Mar 29 21:21:02 2021.
redhat-rpm-config-182-1.fc34.noarch : Red Hat specific rpm configuration files
Repo        : @System
Matched from:
Filename    : /usr/lib/rpm/redhat/brp-mangle-shebangs
raveit65 commented 3 years ago

Np, if you don't like it, I can remove the fixes for mangling shebang and fix the warnings in my RPM for fedora ;)

jhgit commented 3 years ago

In this commit, it was claimed that changing #!/bin/sh to #!/usr/bin/sh was done to "fix shebangs":

https://github.com/mate-desktop/pluma/commit/9a99c3c7b0fbfa9510fb65d96ef70f96f7fd3c62

POSIX does not necessarily specify /bin/sh as THE location for the system's standard shell (I think it allows for it to be in different locations [1]), however it is FAR more common for the standard location to be /bin/sh.

This change breaks FreeBSD and many other systems (probably most all non-linux systems). [Note: this was noticed now because FreeBSD just updated its pluma port which pulled in this change from 6 months ago]

Can the packager "fix" this to accommodate its location of the standard POSIX shell? Yes, but I think the fedora script to "mangle" this shebang to use /usr/bin/sh is a poor choice for real world cross-platform compatibility. I also don't see many (any?) projects heeding this ill-advised (my opinion) change.

It certainly is not POSIX to have /bin and /usr/bin to be the same, so in short, this will break lots of systems that do not follow that practice.

While this change (/bin/sh -> /usr/bin/sh) MAY not merit a "bug" moniker, I feel this part of the change should be reverted as it does not reflect de facto practice.

[1] https://unix.stackexchange.com/questions/416725/does-sh-have-to-be-in-the-bin-directory

jhgit commented 3 years ago

See also this discussion which has links for the POSIX takes on the issue and the mandatory mention of using /usr/bin/env (which has its own set of problems such as location of 'env' and people who might have a different 'sh' in their path, accidentally, maliciously, or otherwise):

https://news.ycombinator.com/item?id=17069408

One take from that discussion is to just not have a shebang line at all which seems to be the most POSIX compliant solution [1] - POSIX seems to be standoff-ish regarding shebangs. I don't know if that would break on any real world systems, but it works on FreeBSD and Linux. Until I see a survey of how real world systems treat the "no shebang" case, I am not recommending that course of action. I still feel /bin/sh is the best choice (certainly better than /usr/bin/sh).

[1] https://pubs.opengroup.org/onlinepubs/009695399/utilities/xcu_chap02.html