permissions of script files wrong/missing shebang lines

[wolfi]

Hi Altogether,

while packaging for 0.48 pre1 for debian, I came back to an issue I should have reported long ago :-)
Lintian complains for the files listed below:

N: executable-not-elf-or-script
N:
N: This executable file is not an ELF format binary, and does not start
N: with the #! sequence that marks interpreted scripts. It might be a sh
N: script that fails to name /bin/sh as its shell, or it may be
N: incorrectly marked as executable. Sometimes upstream files developed
N: on Windows are marked unnecessarily as executable on other systems.
N:
N: If you are using debhelper to build your package, running dh_fixperms
N: will often correct this problem for you.
N:
N: Refer to Debian Policy Manual section 10.4 (Scripts) for details.
N:
N: Severity: normal, Certainty: certain
N:

Please go through the listed files and chef if they need the executable permission (than please add the shebang) or if they don't.

Thank you very much,

Wolfi

The affected files are:
W: inkscape: executable-not-elf-or-script ./usr/share/inkscape/extensions/text_randomcase.py
W: inkscape: executable-not-elf-or-script ./usr/share/inkscape/extensions/Barcode/__init__.py
W: inkscape: executable-not-elf-or-script ./usr/share/inkscape/extensions/color_blackandwhite.py
W: inkscape: executable-not-elf-or-script ./usr/share/inkscape/extensions/Barcode/Code93.py
W: inkscape: executable-not-elf-or-script ./usr/share/inkscape/extensions/text_lowercase.py
W: inkscape: executable-not-elf-or-script ./usr/share/inkscape/extensions/text_braille.py
W: inkscape: executable-not-elf-or-script ./usr/share/inkscape/extensions/render_alphabetsoup_config.py
W: inkscape: executable-not-elf-or-script ./usr/share/inkscape/extensions/color_replace.py
W: inkscape: executable-not-elf-or-script ./usr/share/inkscape/extensions/color_brighter.py
W: inkscape: executable-not-elf-or-script ./usr/share/inkscape/extensions/color_lesshue.py
W: inkscape: executable-not-elf-or-script ./usr/share/inkscape/extensions/dxf_templates.py
W: inkscape: executable-not-elf-or-script ./usr/share/inkscape/extensions/Barcode/RM4CC.py
W: inkscape: executable-not-elf-or-script ./usr/share/inkscape/extensions/color_rgbbarrel.py
W: inkscape: executable-not-elf-or-script ./usr/share/inkscape/extensions/Barcode/UPCE.py
W: inkscape: executable-not-elf-or-script ./usr/share/inkscape/extensions/inkweb.js
W: inkscape: executable-not-elf-or-script ./usr/share/inkscape/extensions/run_command.py
W: inkscape: executable-not-elf-or-script ./usr/share/inkscape/extensions/color_grayscale.py
W: inkscape: executable-not-elf-or-script ./usr/share/inkscape/extensions/color_lesslight.py
W: inkscape: executable-not-elf-or-script ./usr/share/inkscape/extensions/text_sentencecase.py
W: inkscape: executable-not-elf-or-script ./usr/share/inkscape/extensions/render_barcode.py
W: inkscape: executable-not-elf-or-script ./usr/share/inkscape/extensions/color_darker.py
W: inkscape: executable-not-elf-or-script ./usr/share/inkscape/extensions/text_titlecase.py
W: inkscape: executable-not-elf-or-script ./usr/share/inkscape/extensions/jessyInk_core_mouseHandler_zoomControl.js
W: inkscape: executable-not-elf-or-script ./usr/share/inkscape/extensions/jessyInk_core_mouseHandler_noclick.js
W: inkscape: executable-not-elf-or-script ./usr/share/inkscape/extensions/color_lesssaturation.py
W: inkscape: executable-not-elf-or-script ./usr/share/inkscape/extensions/color_desaturate.py
W: inkscape: executable-not-elf-or-script ./usr/share/inkscape/extensions/Barcode/Code128.py
W: inkscape: executable-not-elf-or-script ./usr/share/inkscape/extensions/Barcode/Base.py
W: inkscape: executable-not-elf-or-script ./usr/share/inkscape/extensions/color_randomize.py
W: inkscape: executable-not-elf-or-script ./usr/share/inkscape/extensions/color_custom.py
W: inkscape: executable-not-elf-or-script ./usr/share/inkscape/extensions/Barcode/Code39.py
W: inkscape: executable-not-elf-or-script ./usr/share/inkscape/extensions/svg_regex.py
W: inkscape: executable-not-elf-or-script ./usr/share/inkscape/extensions/color_removeblue.py
W: inkscape: executable-not-elf-or-script ./usr/share/inkscape/extensions/Barcode/EAN5.py
W: inkscape: executable-not-elf-or-script ./usr/share/inkscape/extensions/Barcode/Code39Ext.py
W: inkscape: executable-not-elf-or-script ./usr/share/inkscape/extensions/color_removered.py
W: inkscape: executable-not-elf-or-script ./usr/share/inkscape/extensions/Barcode/EAN8.py
W: inkscape: executable-not-elf-or-script ./usr/share/inkscape/extensions/color_morelight.py
W: inkscape: executable-not-elf-or-script ./usr/share/inkscape/extensions/text_uppercase.py
W: inkscape: executable-not-elf-or-script ./usr/share/inkscape/extensions/jessyInk.js
W: inkscape: executable-not-elf-or-script ./usr/share/inkscape/extensions/Barcode/UPCA.py
W: inkscape: executable-not-elf-or-script ./usr/share/inkscape/extensions/voronoi.py
W: inkscape: executable-not-elf-or-script ./usr/share/inkscape/extensions/text_replace.py
W: inkscape: executable-not-elf-or-script ./usr/share/inkscape/extensions/color_removegreen.py
W: inkscape: executable-not-elf-or-script ./usr/share/inkscape/extensions/Barcode/EAN13.py
W: inkscape: executable-not-elf-or-script ./usr/share/inkscape/extensions/color_morehue.py
W: inkscape: executable-not-elf-or-script ./usr/share/inkscape/extensions/text_flipcase.py
W: inkscape: executable-not-elf-or-script ./usr/share/inkscape/extensions/color_moresaturation.py
W: inkscape: executable-not-elf-or-script ./usr/share/inkscape/extensions/color_negative.py

[Joker Wild (lajjr-deactivatedaccount)]

I will look through them.
Regards,
Leo Jackson

[Joker Wild (lajjr-deactivatedaccount)]

Hello all,

I found some with missing shebangs as said. I tested and works either way so to conform to most OSs I will add shebangs to make less headaches. I will make a branch off .48 to get the fixes in it after review unless told otherwise. I tested after a compile on .46 I will also verify with .48, but seems to make no difference they still work.

Regards,
Leo Jackson

[Joker Wild (lajjr-deactivatedaccount)]

Sorry about the delay just testing took a bit.

Regards,
Leo Jackson

[Krzysztof Kosinski (tweenk)]

This is wrong. Those scripts are not supposed to be executed as standalone programs, so they should not be executable or have shebangs.

[Joker Wild (lajjr-deactivatedaccount)]

We can use overrides if this is not preferred. Like in Debian and Ubuntu lintian overrides. Under Fedora I didn't see these errors.
Under Windows they are just pushed as well.
where lintian is used is where the errors are. Or we can just ignore it as for the scripts are just pushed to the extensions directory.

Sorry about that.

Regards,
Leo Jackson

[Joker Wild (lajjr-deactivatedaccount)]

I know it's under debate, but I can switch to this even though it might not be pushed to trunk or focus development. It's just to give a possible solution to this bug.
Regards,
Leo Jackson

[Jon A. Cruz (jon-joncruz)]

The prior comment was not correct. By design the current inkscape extension system uses scripts that take SVG in on stdin and put replacement output SVG to stdout. These were intended to be reusable as stand-alone CGI-like components that are simple to reuse in other contexts, including invocations from other programs or scripts.

[Joker Wild (lajjr-deactivatedaccount)]

Hello,

I just want to add a small note as Jon A. Cruz said "These were intended to be reusable as stand-alone" This would be a good thing to add shebands if they are reused it another context in the development out of focus. I think they should have them added also seeing the code thus far, it can be a benefit to have.

Say maybe we want to make a toolbox for people to use for quick tasks sure would need them then. (We don't have this)
It sure would be useful and I would help create it. ;-)

With development you never know what road may come up. And for what ever reason you didn't add the shebands, and need them later you will have them.

Regards,
Leo Jackson

[wolfi (wolfi-sigxcpu)]

Thanks for sorting this out, I'll add the patch to the debian package in the next upload.

With best wishes,

Wolfi

[Joker Wild (lajjr-deactivatedaccount)]

Jazzy,
Thanks again reverted to committed as the release is for a released version to public.
Sorry about that.

Thanks Again,
Leo Jackson

[su_v (suv-lp)]

Jon A. Cruz wrote
> The prior comment was not correct. By design the current inkscape
> extension system uses scripts that take SVG in on stdin and put
> replacement output SVG to stdout. These were intended to be
> reusable as stand-alone CGI-like components that are simple
> to reuse in other contexts, including invocations from other
> programs or scripts.

There are different kind of script files included in the extensions directory, and not _all_ of them are intended to be used as stand-alone components. In my understanding this is limited to those scripts, which are referenced as <command/> in the corresponding INX file.

NOTE: If a shebang line is added e.g. to javascript files which are intended for embedded use in SVG files, it can break an extension (as happens with JessyInk.js [1]).

IMHO there should be rules defined, e.g.
With shebang:
- scripts which are called by INX files (as <command …></command>)
No shebang:
- (python/perl) scripts only imported as modules
- (java)script files to be embedded in SVG files

The merged changes to trunk
<http://bazaar.launchpad.net/~inkscape.dev/inkscape/trunk/revision/9734>
does not include all changes from the merge proposal
<http://bazaar.launchpad.net/~lajjr/inkscape/shebangs/revision/9637>
but apparently on some distributions (reported for 0.48.1 on Debian), Inkscape is shipped with the changes from the branch instead of the committed changes (causing locally created JessyInk presentations to fail in web broswers).

[1] reported on irc, with Inkscape 0.48.1 distributed by Debian: new presentations don't run e.g. in Firefox, with this error:

Error: illegal character
Source File: http://imgh.us/187_test.svg
Line: 98
Source Code:
#!/usr/bin/env js

[Alex Valavanis (valavanisalex)]

Thanks ~suv. I just received a debian bug report email about this, and can confirm that the problem has propagated through to Ubuntu. I have added a comment to the Debian tracker, and I will take care of it in Ubuntu.

[Alex Valavanis (valavanisalex)]

I'm stripping out the shebang lines from each of the *.js files in the Debian patch. As far as I can tell, none of them are supposed to ever be run as a standalone script. The only other discrepancy I can see between the Debian version and the trunk is that the voronoi.py script has had a shebang added in Debian.

In this case, it looks like voronoi.py *is* intended to be executable, even though it is not called directly by the extension. It handles __main__ invocations, provides command-line help, etc. Perhaps it would be appropriate to include a shebang in this script upstream too?

[Alex Valavanis (valavanisalex)]

Part of the problem here is that the JavaScript extension files are installed using a _SCRIPTS Automake rule. This adds the "executable" flag to the files, but it is not possible to run any of the *.js files as standalone scripts. It would be better to use a _DATA rule, which just copies the file without changing the permissions.

If no one objects, I will commit the following changes to trunk:
* Add shebang to voronoi.py
* Move *.js files to the _DATA rule in share/extensions/Makefile.am

[su_v (suv-lp)]

The python script 'voronoi.py' originates from <http://www.billsimons.com/> and is used unedited (as far as I know) as an imported module in Inkscape's extension to generate a Voronoi pattern fill. Executing the script by itself does not generate output relevant for Inkscape (no SVG output), and users who want to use the script for different purposes will most likely install it in their $PATH.

Beyond that I see no reason against adding the shebang (but for using the script with Inkscape, it's not required, and it is not part of the original version).

@JazzyNico - since you comitted the original merge proposal, any comments on handling *.js files? As far as I can tell, all included javascript files are intended for embedded use in SVG files.

[jazzynico (jazzynico)]

> As far as I can tell, all included javascript files are intended for embedded use in SVG files.

Confirmed. Thus adding a shebang here is totally useless.

[Alex Valavanis (valavanisalex)]

As I see it, it doesn't really matter to the Inkscape project whether voronoi.py is classed as "executable" or not. However, we need to make a decision about which way to go. At the moment, we are distributing the script with executable permission, but not telling the system how to interpret it. If the user tries to run the script in Ubuntu, it is interpreted using the dash interpreter, and obviously fails. We either need to remove the executable flag, or add a shebang.

[Alex Valavanis (valavanisalex)]

I've updated trunk as described previously. Please feel free to revert the changes if you think it's a bad idea

[Alex Valavanis (valavanisalex)]

trunk: r.10245

[Launchpad Janitor (janitor)]

This bug was fixed in the package inkscape - 0.48.1-2ubuntu5

---------------
inkscape (0.48.1-2ubuntu5) oneiric; urgency=low

  * debian/rules: No need to manually call intltool-update on build. This
    is now handled by dh_translations.
  * debian/control: No longer suggest python and skencil. They do not
    offer any enhancement to Inkscape (LP: #762541)
  * debian/control: Suggest transfig to allow import of xfig files
    (LP: #668300)
  * debian/patches/02-add-shebangs-and-fix-permissions.dpatch:
    No longer mark JavaScript extension files as standalone executables
    (LP: #602005):
    - share/extensions/*.js: Revert addition of shebangs
    - share/extensions/Makefile.am: Install *.js using _DATA rule rather
      than _SCRIPTS rule to avoid giving executable permission
  * debian/patches/fix-fontforge-glyph-template.dpatch: Cherry-pick
    patch from uptream trunk to fix layer structure in template (LP: #565296)
 -- Alex Valavanis <email address hidden> Mon, 30 May 2011 18:50:49 +0100

[su_v (suv-lp)]

Alex Valavanis wrote on 2011-05-24:
> * Add shebang to voronoi.py
> * Move *.js files to the _DATA rule in share/extensions/Makefile.am

The fix was committed to trunk in r10245 - this report should have been tagged 'backport-proposed' (for the 0.48.x branch).
@Hannes - sorry to have missed that in time for 0.48.2.

[jazzynico (jazzynico)]

Back-ported to the 0.48.x branch, revision 9829.