View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0000060 | Cinelerra-GG | Bug | public | 2018-12-13 23:33 | 2019-02-01 00:16 |
| Reporter | ferdnyc | Assigned To | goodguy | ||
| Priority | normal | Severity | major | Reproducibility | always |
| Status | closed | Resolution | fixed | ||
| Product Version | 2018-11 | ||||
| Summary | 0000060: [PATCH] cinelerra-gg sets unnecessary executable bits during install to system locations | ||||
| Description | When the cinelerra install process triggers the target defined as `sys_install` in cinelerra-5.1/Makefile.am, which installs files in a location outside of the source tree (as opposed to `bin_install` which simply copies files to "./bin/" in the tree), the cinelerra-5.1/inst.sh script is used to take the contents of entire source directory trees and reproduce them at the destination path. It does this by traversing the source directory using recursive-descent, repeatedly calling the $(install_sh) script — GNU Autotool's standard installer, transferred to the source directory as `install-sh` — to recreate every directory and file found. Because inst.sh calls install-sh with its default options, each regular source file will be given the default file mode 0755, meaning the executable bits will be SET for every file installed. This results in entire directories full of files being set executable in /usr/lib64/cinelerra-gg and /usr/share/cinelerra-gg, even though none of their contents is an executable binary. This is a major violation of software policy in many Linux distros', and completely prohibited under their packaging guidelines. | ||||
| Additional Information | The attached patches, made against the curent GIT head, address this issue in a twofold manner: (1): First, the `inst.sh` script is modified to install regular files ($f) in destination $dir via `$install_sh -m 0644 -c "$f" "$dir"`, so that every file installed recursively in this manner will explicitly _not_ have its executable bits set. The install command for directories is unchanged, and will still default to the proper 0755 mode. (2): In concert with this, the rules in cinelerra-5.1/Makefile.am are modified, and every executable binary which **does** need to have its executable bits set is explicitly installed with `$(install_sh)` directly, rather than relying on the $(inst_sh) mass-install script. As a result of this change, the only files installed executable in our current cinelerra-gg package are: $ rpmls -p ~/rpmbuild/RPMS/x86_64/cinelerra-gg-5.1-55.20181208git230f4fd.fc29.x86_64.rpm|egrep '^\-rwx' -rwxr-xr-x /usr/bin/bdwrite -rwxr-xr-x /usr/bin/cin_db -rwxr-xr-x /usr/bin/cinelerra-gg -rwxr-xr-x /usr/bin/zmpeg3cat -rwxr-xr-x /usr/bin/zmpeg3cc2txt -rwxr-xr-x /usr/bin/zmpeg3ifochk -rwxr-xr-x /usr/bin/zmpeg3show -rwxr-xr-x /usr/bin/zmpeg3toc -rwxr-xr-x /usr/lib64/cinelerra-gg/cutads -rwxr-xr-x /usr/lib64/cinelerra-gg/hveg2enc -rwxr-xr-x /usr/lib64/cinelerra-gg/mpeg2enc -rwxr-xr-x /usr/lib64/cinelerra-gg/mplex -rwxr-xr-x /usr/lib64/cinelerra-gg/mplexlo Prior to this change, the package contained 941 (!) regular files with the executable bit set: $ rpmls -p ~/rpmbuild/RPMS/x86_64/cinelerra-gg-5.1-55.20181208git230f4fd.ferd29.x86_64.rpm|egrep '^\-rwx'|wc -l 941 | ||||
| Tags | No tags attached. | ||||
| Attached Files | 0001-inst.sh-Don-t-set-files-executable.patch (1,072 bytes)
From 1f5dfe033fdc1f677b6ffca7d2a0fd208730a62d Mon Sep 17 00:00:00 2001 From: "FeRD (Frank Dana)" <ferdnyc@gmail.com> Date: Wed, 12 Dec 2018 19:26:17 -0500 Subject: [PATCH] inst.sh: Don't set files executable inst.sh was running install-sh on directory contents recursively, without setting a file mode. Since its default is 0755, that left far too many data files executable. Changed to pass `-m 0644` when installing regular files. (Directories will still get 0755.) --- cinelerra-5.1/inst.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cinelerra-5.1/inst.sh b/cinelerra-5.1/inst.sh index bbfee54..ec20eee 100755 --- a/cinelerra-5.1/inst.sh +++ b/cinelerra-5.1/inst.sh @@ -7,7 +7,7 @@ $mkinstalldirs "$dir" if [ "$*" = "*" ]; then exit; fi for f in "$@"; do - if [ -f "$f" ]; then $install_sh -c "$f" "$dir"; continue; fi + if [ -f "$f" ]; then $install_sh -m 0644 -c "$f" "$dir"; continue; fi if [ -d "$f" ]; then ( cd $f; $inst_sh "$dir/$f" * ) else echo "*** Error - install $f in $dir failed." 1>&2; exit 1; fi done -- 2.19.2 0002-Makefile.am-Explicitly-install-mod-x-binaries.patch (2,426 bytes)
From 49f78ccdf337d7b03f4e69e33ac0aa6a4a9d53f5 Mon Sep 17 00:00:00 2001
From: "FeRD (Frank Dana)" <ferdnyc@gmail.com>
Date: Thu, 13 Dec 2018 00:27:46 -0500
Subject: [PATCH] Makefile.am: Explicitly install mod +x binaries
With the default file mode for $(inst_sh) changed to 0644,
it's now necessary to explicitly install the files we want
to be executable.
This commit replaces $(inst_sh) for those files (only) with
individual calls to the default autotools $(install_sh) script.
Destination directory paths are first created, if necessary.
---
cinelerra-5.1/Makefile.am | 26 ++++++++++++++++++--------
1 file changed, 18 insertions(+), 8 deletions(-)
diff --git a/cinelerra-5.1/Makefile.am b/cinelerra-5.1/Makefile.am
index a61379c..e38ba2d 100644
--- a/cinelerra-5.1/Makefile.am
+++ b/cinelerra-5.1/Makefile.am
@@ -50,7 +50,10 @@ bin_uninstall:
# with system_build
sys_install: $(CIN_INSTALLS)
- cd bin; $(inst_sh) "$(DESTDIR)$(bindir)" "$(WANT_CIN)" bdwrite
+ $(install_sh) -d "$(DESTDIR)$(bindir)"
+ cd bin; $(install_sh) -t "$(DESTDIR)$(bindir)" "$(WANT_CIN)";\
+ $(install_sh) -t "$(DESTDIR)$(bindir)" bdwrite
+
cd bin; $(inst_sh) "$(DESTDIR)$(datadir)/$(WANT_CIN)" \
COPYING README models Cinelerra_factory expanders.txt \
ffmpeg msg info tips doc
@@ -77,18 +80,26 @@ dvl_uninstall:
# with-libzmpeg3
zmp_install:
- cd bin; $(inst_sh) "$(DESTDIR)$(bindir)" \
- zmpeg3{cat,cc2txt,ifochk,show,toc}
- cd bin; $(inst_sh) "$(DESTDIR)$(WANT_CINLIB_DIR)" \
- hveg2enc mpeg2enc mplex mplexlo
+ $(install_sh) -d "$(DESTDIR)$(bindir)"
+ cd bin; for b in zmpeg3{cat,cc2txt,ifochk,show,toc}; do\
+ $(install_sh) -t "$(DESTDIR)$(bindir)" $$b;\
+ done
+
+ $(install_sh) -d "$(DESTDIR)$(WANT_CINLIB_DIR)"
+ cd bin; for b in hveg2enc mpeg2enc mplex mplexlo; do\
+ $(install_sh) -t "$(DESTDIR)$(WANT_CINLIB_DIR)" $$b;\
+ done
zmp_uninstall:
rm -f "$(DESTDIR)$(bindir)"/zmpeg3{cat,cc2txt,ifochk,show,toc}
# with-commercial
com_install:
- cd bin; $(inst_sh) "$(DESTDIR)$(bindir)" cin_db
- cd bin; $(inst_sh) "$(DESTDIR)$(WANT_CINLIB_DIR)" cutads
+ $(install_sh) -d "$(DESTDIR)$(bindir)"
+ cd bin; $(install_sh) -t "$(DESTDIR)$(bindir)" cin_db
+
+ $(install_sh) -d "$(DESTDIR)$(WANT_CINLIB_DIR)"
+ cd bin; $(install_sh) -t "$(DESTDIR)$(WANT_CINLIB_DIR)" cutads
com_uninstall:
rm -f "$(DESTDIR)$(bindir)/cin_db"
@@ -110,4 +121,3 @@ lv2_uninstall:
val-%:
@echo $($(subst val-,,$@))
-
--
2.19.2
| ||||
|
|
For now, gg ended up implementing the simpler method of "simply copies files to "./bin/" in the tree". |
|
|
Any method that solves the core issue works for me, I'm far less hung up on method than outcome. I'll prepare a new build off the latest commits and submit it for review, thanks! |
|
|
Hi, I'm afraid the umask was inverted on this fix — in the current source, all regular files install with file mode 0000 (no permissions of any kind). The issue is in inst.sh, which now contains: for f in "$@"; do if [ -f "$f" ]; then ( umask 755; cp "$f" "$dir" ); continue; fi if [ -d "$f" ]; then ( cd $f; $inst_sh "$dir/$f" * ) else echo "*** Error - inst.sh $f in $dir failed." 1>&2; exit 1; fi done Since umask is the mask of permission bits that will NOT be set, it should be the inverse of the maximum desired permissions — the default is umask 022 (which would correspond to a mode of 755). However, even using umask 022 to manage install permissions would result in all of the plugin files being installed with their executable bits set, as that's how they're output by the compiler. The mismatch between build process output and desired final state is why permissions are typically set explicitly by the install process, rather than relying on the source or build trees to have the correct permissions at the time of install. |
|
|
A fix for inst.sh has just now been checked into the GIT repository. Sorry about putting in the inverse. Plugin binaries are shared object libraries and have to be executable. |
|
|
Appears to be OK now from our point of view. If further information becomes available, can re-open. |
| Date Modified | Username | Field | Change |
|---|---|---|---|
| 2018-12-13 23:33 | ferdnyc | New Issue | |
| 2018-12-13 23:33 | ferdnyc | File Added: 0001-inst.sh-Don-t-set-files-executable.patch | |
| 2018-12-13 23:33 | ferdnyc | File Added: 0002-Makefile.am-Explicitly-install-mod-x-binaries.patch | |
| 2018-12-14 03:54 | PhyllisSmith | Assigned To | => goodguy |
| 2018-12-14 03:54 | PhyllisSmith | Status | new => assigned |
| 2018-12-15 14:14 | PhyllisSmith | Note Added: 0000202 | |
| 2018-12-15 17:54 | ferdnyc | Note Added: 0000212 | |
| 2018-12-17 06:38 | ferdnyc | Note Added: 0000243 | |
| 2018-12-18 02:51 | PhyllisSmith | Note Added: 0000255 | |
| 2019-02-01 00:16 | PhyllisSmith | Status | assigned => closed |
| 2019-02-01 00:16 | PhyllisSmith | Resolution | open => fixed |
| 2019-02-01 00:16 | PhyllisSmith | Note Added: 0000733 |