permissions of script files wrong/missing shebang lines

I will look through them.
Regards,
Leo Jackson

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

Sorry about the delay just testing took a bit.

Regards,
Leo Jackson

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

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

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

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.

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

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

With best wishes,

Wolfi

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

Thanks Again,
Leo Jackson

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

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.

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?

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

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.

> 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.

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.

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

trunk: r.10245

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

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.

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