View Revisions: Issue #59

Summary 0000059: Packaging: Issues identified in source tree / build process
Revision 2018-12-13 22:01 by ferdnyc
Description The review process for Cinelerra-GG's inclusion in RPMfusion continues apace (see package review bug):
https://bugzilla.rpmfusion.org/show_bug.cgi?id=5093

Our always-diligent package reviewers have identified a number of issues, most relatively minor, that we've had to work around in packaging the software to meet Fedora guidelines. Policy is that we also bring these issues to the attention of the upstream project, so here I am.

0000001. The text of the GPL in cinelerra-5.1/COPYING is out of date, and uses an out of date street address for the FSF offices.

0000002. The application's .desktop file template at cinelerra-5.1/image/cin.desktop is outdated, and does not follow the current Freedesktop specification:
http://freedesktop.org/wiki/Specifications/desktop-entry-spec

The following errors are produced by the desktop-file-validate tool:

$ desktop-file-validate image/cin.desktop
image/cin.desktop: warning: value "Application;AudioVideo;Multimedia;VideoEditing;" for key "Categories" in group "Desktop Entry" contains a deprecated value "Application"

image/cin.desktop: error: value "Application;AudioVideo;Multimedia;VideoEditing;" for key "Categories" in group "Desktop Entry" contains an unregistered value "Multimedia"; values extending the format should start with "X-"

image/cin.desktop: error: value "Application;AudioVideo;Multimedia;VideoEditing;" for key "Categories" in group "Desktop Entry" contains an unregistered value "VideoEditing"; values extending the format should start with "X-"

image/cin.desktop: warning: key "Encoding" in group "Desktop Entry" is deprecated

image/cin.desktop: error: (will be fatal in the future): value "cin.svg" for key "Icon" in group "Desktop Entry" is an icon name with an extension, but there should be no extension as described in the Icon Theme Specification if the value is not an absolute path

0000003. Many installed files have their executable bits set unnecessarily.

#4. Certain files are built with executable stack, a dangerous code practice which is normally not necessary and merely the result of an oversight in development.

Thanks very much, as I said I'll report 0000003 separately (I have patches to fix it), and I think that should be about it for now.
Revision 2018-12-14 00:33 by administrator
Description The review process for Cinelerra-GG's inclusion in RPMfusion continues apace (see package review bug):
https://bugzilla.rpmfusion.org/show_bug.cgi?id=5093

Our always-diligent package reviewers have identified a number of issues, most relatively minor, that we've had to work around in packaging the software to meet Fedora guidelines. Policy is that we also bring these issues to the attention of the upstream project, so here I am.

(1) The text of the GPL in cinelerra-5.1/COPYING is out of date, and uses an out of date street address for the FSF offices.

(2) The application's .desktop file template at cinelerra-5.1/image/cin.desktop is outdated, and does not follow the current Freedesktop specification:
http://freedesktop.org/wiki/Specifications/desktop-entry-spec

The following errors are produced by the desktop-file-validate tool:

$ desktop-file-validate image/cin.desktop
image/cin.desktop: warning: value "Application;AudioVideo;Multimedia;VideoEditing;" for key "Categories" in group "Desktop Entry" contains a deprecated value "Application"

image/cin.desktop: error: value "Application;AudioVideo;Multimedia;VideoEditing;" for key "Categories" in group "Desktop Entry" contains an unregistered value "Multimedia"; values extending the format should start with "X-"

image/cin.desktop: error: value "Application;AudioVideo;Multimedia;VideoEditing;" for key "Categories" in group "Desktop Entry" contains an unregistered value "VideoEditing"; values extending the format should start with "X-"

image/cin.desktop: warning: key "Encoding" in group "Desktop Entry" is deprecated

image/cin.desktop: error: (will be fatal in the future): value "cin.svg" for key "Icon" in group "Desktop Entry" is an icon name with an extension, but there should be no extension as described in the Icon Theme Specification if the value is not an absolute path

(3) Many installed files have their executable bits set unnecessarily.

(4) Certain files are built with executable stack, a dangerous code practice which is normally not necessary and merely the result of an oversight in development.

Thanks very much, as I said I'll report 0000003 separately (I have patches to fix it), and I think that should be about it for now.
Revision 2018-12-13 22:01 by ferdnyc
Additional Information 0000001. If you wish to continue with the GPLv2, a more current version is found here:
https://www.gnu.org/licenses/old-licenses/gpl-2.0.txt

Or you can switch to GPLv3, which wisely eschews the physical address in favor of a URL, since it's the 21st Century and all:
https://www.gnu.org/licenses/gpl-3.0.txt


0000002. Currently I'm fixing up the .desktop file during packaging with the following desktop-file-edit command:

$ desktop-file-edit --set-icon="cinelerra-gg" --remove-key=Encoding \
    --remove-category=Application --remove-category=VideoEditing \
    --remove-category=Multimedia --add-category=AudioVideoEditing \
    --add-category=X-Multimedia $RPM_BUILD_ROOT%{_datadir}/applications/%{name}.desktop

I'll attach a patch which makes the same modifications to the file in the source tree, which would make this step unnecessary. (The patch changes the Icon field from "Icon=cin.svg" to "Icon=cin", since at that stage the file is only a template. The important part of that change is that icon names should be listed in the .desktop file without any extension, since the exact file format used is left as an implementation decision.)

After these modifications, the file passes the desktop-file-validate checks.

(Note: While I've migrated the "Multimedia" category listed in the original .desktop file to "X-Multimedia", both here and in the attached patch, it's not a standard category defined in the Freedesktop standard. As such, it probably isn't going to be recognized anywhere, and simply removing it may be the better course of action.

0000003. The install process uses the cinelerra-5.1/inst.sh script to recursively install directory trees with the executable bits set on every file, resulting in most of /usr/lib64/cinelerra-gg/ and large chunks of /usr/share/cinelerra-gg/ containing unnecessarily executable files, even things like the image files in /usr/share/cinelerra-gg/models/. From a packaging standpoint this is not acceptable. I'll file a separate report to address this issue in particular.

#4. Our automated execstack checks report the following:


cinelerra-gg.x86_64: W: executable-stack /usr/bin/cinelerra-gg
cinelerra-gg.x86_64: W: executable-stack /usr/lib64/cinelerra-gg/plugins/themes/theme_blond.plugin
cinelerra-gg.x86_64: W: executable-stack /usr/lib64/cinelerra-gg/plugins/themes/theme_blond_cv.plugin
cinelerra-gg.x86_64: W: executable-stack /usr/lib64/cinelerra-gg/plugins/themes/theme_blue.plugin
cinelerra-gg.x86_64: W: executable-stack /usr/lib64/cinelerra-gg/plugins/themes/theme_blue_dot.plugin
cinelerra-gg.x86_64: W: executable-stack /usr/lib64/cinelerra-gg/plugins/themes/theme_bright.plugin
cinelerra-gg.x86_64: W: executable-stack /usr/lib64/cinelerra-gg/plugins/themes/theme_hulk.plugin
cinelerra-gg.x86_64: W: executable-stack /usr/lib64/cinelerra-gg/plugins/themes/theme_neophyte.plugin
cinelerra-gg.x86_64: W: executable-stack /usr/lib64/cinelerra-gg/plugins/themes/theme_pinklady.plugin
cinelerra-gg.x86_64: W: executable-stack /usr/lib64/cinelerra-gg/plugins/themes/theme_suv.plugin
cinelerra-gg.x86_64: W: executable-stack /usr/lib64/cinelerra-gg/plugins/themes/theme_unflat.plugin
cinelerra-gg.x86_64: W: executable-stack /usr/lib64/cinelerra-gg/plugins/video/chromakeyhsv.plugin

executable-stack:
The binary declares the stack as executable. Executable stack is usually an
error as it is only needed if the code contains GCC trampolines or similar
constructs which uses code on the stack. One common source for needlessly
executable stack cases are object files built from assembler files which don't
define a proper .note.GNU-stack section.

This can easily be cleared using `execstack -c` on each affected file, but that assumes that the executable stack flag is actually the result of mere oversight, and not actively in use by the software for any reason. So I wanted to check whether any of the listed files genuinely have need to execute operations from their stack, or if it's safe to unset the executable-stack permission using `execstack -c`? If the code isn't consciously making use of stack execution, then it should be safe to clear it. (Obviously, it would be preferable to make the necessary source changes so that the flag is never set to begin with.)
Revision 2018-12-14 00:33 by administrator
Additional Information (1) If you wish to continue with the GPLv2, a more current version is found here:
https://www.gnu.org/licenses/old-licenses/gpl-2.0.txt

Or you can switch to GPLv3, which wisely eschews the physical address in favor of a URL, since it's the 21st Century and all:
https://www.gnu.org/licenses/gpl-3.0.txt


(2) Currently I'm fixing up the .desktop file during packaging with the following desktop-file-edit command:

$ desktop-file-edit --set-icon="cinelerra-gg" --remove-key=Encoding \
    --remove-category=Application --remove-category=VideoEditing \
    --remove-category=Multimedia --add-category=AudioVideoEditing \
    --add-category=X-Multimedia $RPM_BUILD_ROOT%{_datadir}/applications/%{name}.desktop

I'll attach a patch which makes the same modifications to the file in the source tree, which would make this step unnecessary. (The patch changes the Icon field from "Icon=cin.svg" to "Icon=cin", since at that stage the file is only a template. The important part of that change is that icon names should be listed in the .desktop file without any extension, since the exact file format used is left as an implementation decision.)

After these modifications, the file passes the desktop-file-validate checks.

(Note: While I've migrated the "Multimedia" category listed in the original .desktop file to "X-Multimedia", both here and in the attached patch, it's not a standard category defined in the Freedesktop standard. As such, it probably isn't going to be recognized anywhere, and simply removing it may be the better course of action.

(3) The install process uses the cinelerra-5.1/inst.sh script to recursively install directory trees with the executable bits set on every file, resulting in most of /usr/lib64/cinelerra-gg/ and large chunks of /usr/share/cinelerra-gg/ containing unnecessarily executable files, even things like the image files in /usr/share/cinelerra-gg/models/. From a packaging standpoint this is not acceptable. I'll file a separate report to address this issue in particular.

(4) Our automated execstack checks report the following:


cinelerra-gg.x86_64: W: executable-stack /usr/bin/cinelerra-gg
cinelerra-gg.x86_64: W: executable-stack /usr/lib64/cinelerra-gg/plugins/themes/theme_blond.plugin
cinelerra-gg.x86_64: W: executable-stack /usr/lib64/cinelerra-gg/plugins/themes/theme_blond_cv.plugin
cinelerra-gg.x86_64: W: executable-stack /usr/lib64/cinelerra-gg/plugins/themes/theme_blue.plugin
cinelerra-gg.x86_64: W: executable-stack /usr/lib64/cinelerra-gg/plugins/themes/theme_blue_dot.plugin
cinelerra-gg.x86_64: W: executable-stack /usr/lib64/cinelerra-gg/plugins/themes/theme_bright.plugin
cinelerra-gg.x86_64: W: executable-stack /usr/lib64/cinelerra-gg/plugins/themes/theme_hulk.plugin
cinelerra-gg.x86_64: W: executable-stack /usr/lib64/cinelerra-gg/plugins/themes/theme_neophyte.plugin
cinelerra-gg.x86_64: W: executable-stack /usr/lib64/cinelerra-gg/plugins/themes/theme_pinklady.plugin
cinelerra-gg.x86_64: W: executable-stack /usr/lib64/cinelerra-gg/plugins/themes/theme_suv.plugin
cinelerra-gg.x86_64: W: executable-stack /usr/lib64/cinelerra-gg/plugins/themes/theme_unflat.plugin
cinelerra-gg.x86_64: W: executable-stack /usr/lib64/cinelerra-gg/plugins/video/chromakeyhsv.plugin

executable-stack:
The binary declares the stack as executable. Executable stack is usually an
error as it is only needed if the code contains GCC trampolines or similar
constructs which uses code on the stack. One common source for needlessly
executable stack cases are object files built from assembler files which don't
define a proper .note.GNU-stack section.

This can easily be cleared using `execstack -c` on each affected file, but that assumes that the executable stack flag is actually the result of mere oversight, and not actively in use by the software for any reason. So I wanted to check whether any of the listed files genuinely have need to execute operations from their stack, or if it's safe to unset the executable-stack permission using `execstack -c`? If the code isn't consciously making use of stack execution, then it should be safe to clear it. (Obviously, it would be preferable to make the necessary source changes so that the flag is never set to begin with.)