[Gs-code-review] Re: Code review items

[Russell Lang]

Ray,

I originally took this bug because I had already developed a fix for 
it.  My original fix works on Linux but may not work on other Unix 
variants.  I suggest that this be reassigned to someone with better
knowledge of Unix make and scripting.  Who wants to take it?

Russell


From:           	Ray Johnston <ray at artifex.com>
Date sent:      	Mon, 30 Jul 2001 08:55:33 -0700

> Russell,
> 
> Russell Lang wrote:
> > The outstanding code review items are
> > http://www.ghostscript.com/pipermail/gs-code-review/2001-June/000777.html
> 
> This had been previously reviewed by Raph, so possibly he should be the
> one to review again (since he wanted to be reminded to check for the
> portability).
> 
> > On Fri, Apr 20, 2001 at 12:06:18PM +1000, Russell Lang wrote:
> > Content-Description: Mail message body
> > > Fix: change manual page symbolic links from absolute to relative,
> > > to allow installing to temporary location when building a 
> > > distribution.
> > > 
> > > This patch doesn't make change the working directory while creating 
> > > the symlinks.  This work on Linux.  If it will fail on other versions 
> > > of Unix, please speak up.
> > 
> > I agree with Ralph that I'd rather cd into the directory, just to be
> > conservative.
> > 
> > Other than that, the patch looks fine. I would like to see the revised
> > patch on code-review again, though, just so I can double check it for
> > portability issues.
> > 
> > Thanks,
> > 
> > Raph
> 
> From examinination, even though you changed the first arg to ln -s
> to remove the $$man1dir, you didn't on the second argument.
> 
> > --- ./unixinst.mak      Mon Jun 11 09:10:19 2001
> > !               ln -s $$man1dir/ps2ps.$(man1ext) $$man1dir/$$f.$(man1ext) ;\
> ----
> > !               cd $$man1dir; ln -s ps2ps.$(man1ext) $$man1dir/$$f.$(man1ext) ;\
> 
> Unless I'm missing something, this will NOT work with a relative
> path contained in $$main1dir.
> 
> Are you sure you didn't want:
> 
>          cd $$man1dir; ln -s ps2ps.$(man1ext) $$f.$(man1ext) ;\
> 
> Which is closer in function to the original.
> 
> Just for the record, I'm not sure what good the 'cd' does in the
> first place and if it is really needed, then should it precede the
> "rm -f ..." as well?
> 
> Lastly, the code with the 'cd' added is not in \( \) so this
> change of cwd will be in effect during the next pass through the
> 'for' causing the next pass to fail.

I did test this patch, but it probably isn't optimum.  The working 
directory is reset by gnu make after each line.

> I am not at all comfortable with this change.
> 
> Regards,
> Ray
> ___________________________________________________________________
> _______________________________________________
> Gs-code-review mailing list
> Gs-code-review at ghostscript.com
> http://www.ghostscript.com/mailman/listinfo/gs-code-review



Russell Lang                   gsview at ghostgum.com.au
Ghostgum Software Pty Ltd      http://www.ghostgum.com.au/