James McCoy wrote on Wed, Aug 02, 2017 at 20:51:20 -0400:
> On Wed, Aug 02, 2017 at 02:07:20PM +0000, Daniel Shahaf wrote:
> > jamessan_at_apache.org wrote on Wed, Aug 02, 2017 at 01:35:31 -0000:
> > > * r1802032
> > > Install 'fsfs-stats' as a wrapper to 'svnfsfs', to which it was renamed in
> > > r1618848.
> > > Justification:
> > > Backwards compatibility with 1.8.x tools/.
> > > Votes:
> > > + -0: jamessan ($(bindir) and $$1 should be quoted in case they contain shell metacharacters)
> >
> > Thanks for the review. I'll fix $1 in a moment, but why does $(bindir)
> > need to be quoted? The makefiles use $(bindir) unquoted [1],
> > so I assumed what was safe for shell commands in Makefile is safe for
> > shell commands on the installed system.
>
> Just because no one has run into trouble with the existing code, doesn't
> mean it's right. :) Sure, $(bindir) is _typically_ going to be safe to
> use unquoted, but why rely on that?
For consistency with every other use of «$(bindir)» in Makefile.
If someone actually wanted to install to a directory with spaces today,
they'd probably invoke configure as «./configure --prefix=/foo\\\ bar»,
or change «prefix = /foo bar» to «prefix = "/foo bar"» in Makefile after
running configure. That would cause all uses of «$(bindir)» unquoted to
work, but would break wherever «"$(bindir)"» double-quoted is used.
So perhaps we should quote _all_ uses of «$(bindir)»; but not just one.
Makes sense?
Cheers,
Daniel
> If I run "./configure --prefix
> '/home/jamessan/things & stuff'" and then "make", the Makefile breaks
> horribly. I don't even need to try installing. It fails while creating
> the .la files.
Received on 2017-08-03 03:44:52 CEST