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: > > james...@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.
