bugGNUstep - Bugs: bug #33741, GSGhostscriptImageRep uses...

 
 

bug #33741: GSGhostscriptImageRep uses hardcoded path /usr/bin/gs

Submitted by:  Wolfgang Lux <wlux>
Submitted on:  Sun 10 Jul 2011 03:56:37 PM UTC  
 
Category:  Gui/AppKit Severity:  3 - Normal
Item Group:  Bug Status:  None
Privacy:  Public Assigned to:  None
Open/Closed:  In Test

Add a New Comment (Rich Markup)
   

You are not logged in

Please log in, so followups can be emailed to you.

 

( Jump to the original submission)

Tue 19 Jul 2011 10:31:01 AM UTC, comment #9: 

I haven't looked at the code yet, but the last commens seem to imply you still run which gs. Instead of relying on the existence of external tools (OK, OK, which is fairly common ;)
it might work using sth. like this:

[[NSFileManager defaultManager] locateExecutable:@"gs"];

The Zipper uses this to figure out where the external archiver tools are, so I guess it should also work to find gs.

Sebastian Reitenbach <buzzdee>
Project Member
Mon 11 Jul 2011 07:07:52 PM UTC, comment #8: 

Okay, that looks good now.

> Are you running autoreconf (which runs aclocal)?


Ah, no. I was using plain autoconf instead. Autoreconf did the job. Thanks.

Wolfgang Lux <wlux>
Mon 11 Jul 2011 06:51:23 PM UTC, comment #7: 

Good point, it will only invoke 'which gs' once now.

Eric Wasylishen <ericw>
Project Member
Mon 11 Jul 2011 06:37:42 PM UTC, comment #6: 

Sorry, my bad. I changed them to NSLog, and adjusted the behaviour so the message that an exception occurred is only printed once, so people who don't have Ghostscript installed don't get spammed with NSLog's.

For PKG_CHECK_MODULES, I found this blog post:
http://myitcorner.com/blog/?p=207 which states that aclocal is responsible for including the system's pkg.m4 so it can be used in our configure script. Are you running autoreconf (which runs aclocal)?

Eric Wasylishen <ericw>
Project Member
Mon 11 Jul 2011 06:06:42 PM UTC, comment #5: 

Just noticed that I forgot another point. I think you would want to cache the path of the ghostscript executable in _ghostscriptExecutablePath so that which gs is executed at most once.

Wolfgang Lux <wlux>
Mon 11 Jul 2011 05:42:19 PM UTC, comment #4: 

> Good point; I added NSWarnLog's to the two handlers. Does that look OK?


Do you test your code before committing? During compilation I get a warning about an implicit declaration of NSWarnLog. If the code path is actually taken (e.g., by using defaults write NSGlobalDomain GSGhostscriptExecutablePath /usr/bin/no-gs on the command line), programs will crash with an undefined symbol: NSWarnLog.
Besides, I think that plain NSLog would be better than NSWarnLog. I don't see a reason why you would want the ability to hide the exception message.

> Regarding PKG_CHECK_MODULES, autoconf is supposed to make it available if you have pkg-config installed. Do you have pkg-config?


I have pkg-config installed. There's even a .../share/aclocal/pkg.m4 file, but nevertheless autoconf complains and doesn't expand PKG_CHECK_MODULES. Happens for me on Ubuntu 10.04, FreeBSD, and OS X.

Wolfgang Lux <wlux>
Mon 11 Jul 2011 04:21:17 PM UTC, comment #3: 

Good point; I added NSWarnLog's to the two handlers. Does that look OK?

Regarding PKG_CHECK_MODULES, autoconf is supposed to make it available if you have pkg-config installed. Do you have pkg-config?

Eric Wasylishen <ericw>
Project Member
Mon 11 Jul 2011 10:09:49 AM UTC, comment #2: 

It works for me now, but looking at your code I'm not totally happy with it.

In particular, you now catch exceptions thrown by NSTask, but your handlers are empty. This is BAD (even worse than not catching the exception at all)! The minimum here is to log the exception, so users have at least a chance to find a cause if a postscript is not displayed as expected. This might happen, e.g., if a user has set the GSGhostscriptExecutablePath default manually, but mistyped the path.

In the meantime, I was thinking about adding a configure time check to determine the default path for gs, but autoconf (incorrectly) complains about a possibly undefined AC_DEFINE macro at line 122. At the end, it turns out that the real problem is the ImageMagick test you added in r33399. This uses the PKG_CHECK_MODULES macro, which is not defined. I'm wondering how you managed to update configure yourself.

Wolfgang Lux <wlux>
Sun 10 Jul 2011 10:47:53 PM UTC, comment #1: 

Now it will use the default GSGhostscriptExecutablePath, or if that is not set, it will try to run "which gs" using the shell specified in the SHELL environment variable. Feel free to improve if there is a more portable or correct way of doing it..

Eric Wasylishen <ericw>
Project Member
Sun 10 Jul 2011 03:56:37 PM UTC, original submission:  

Oh, how I hate those dumb Linuxism's.

There are systems out there which are not Linux and where gs may be installed somewhere else (e.g., /usr/local/bin/gs on FreeBSD, /usr/pkg/bin on NetBSD, /opt/local/bin/gs on OS X with MacPorts, etc.). The path should at least be configurable with a user default. Maybe configure should also try to look up the correct path. And a task which executes env gs would at least find the gs executable when it is located in the PATH.

Wolfgang Lux <wlux>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by buzzdee (Posted a comment)
  • -email is unavailable- added by ericw (Posted a comment)
  • -email is unavailable- added by wlux (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

     

     

    Follows 1 latest change.

    Date Changed by Updated Field Previous Value => Replaced by
    2011-07-10 ericw Open/ClosedOpen => In Test

    Back to the top


    Powered by Savane 3.6