Iain Hibbert wrote:
> On Sat, 4 Feb 2006, Roland Illig wrote:
>>You may also want to run pkglint to catch some more possible errors. First,
>>simply run pkglint without any further options in the package directory, and
>>if that's done and you're brave, try "pkglint -Wall".
> 
> 
> Ok, I never tried this before..  am getting 'possibly misquoted' errors,
> eg
> 
> WARN: Makefile:56: Possibly misquoted make variable WRKSRC in unquoted string.
> WARN: Makefile:56: Possibly misquoted make variable dir in unquoted string.
> WARN: Makefile:56: Possibly misquoted make variable QTDIR in unquoted string.
> WARN: Makefile:56: Possibly misquoted make variable MKDIR in double quoted string.
> WARN: Makefile:56: Possibly misquoted make variable TEST in double quoted string.
> 
> and line 56 is like this:
> 
> 	cd ${WRKSRC}/${dir} && ${QTDIR}/bin/qmake "QMAKE_MKDIR=${MKDIR}" "QMAKE_CHK_DIR_EXISTS=${TEST} -d"
> 
> does this mean I should put some quotes in there?

Yes. First, these warnings are already very pedantic, which is the 
reason why they are not enabled by default. They serve as a preparation 
to make pkgsrc ready for pathnames containing spaces. But this is not 
the main concern. Another, maybe more important, effect of this is that 
the pkgsrc developers should get sensitive to correct quoting of 
everything. Together with proper error handling, this would improve 
pkgsrc's robustness enormously.

In particular, MKDIR might be defined as follows:

MKDIR=             ${SETENV} POSIXLY_CORRECT="yes" /usr/bin/mkdir -p

This definition should be valid and working for pkgsrc (it currently 
isn't). To handle this situation correctly, the above cannot simply be 
surrounded with double quotes, as the quotes around the "yes" would 
invert the quoting. Instead, the above should look like 
QMAKE_MKDIR=${MKDIR:Q} QMAKE_CHK_DIR_EXISTS=${TEST:Q}\ -d.

The other warnings can be fixed by running "pkglint -Wall --autofix". 
The only thing --autofix does is to add the :Q modifiers to variables 
without any modifiers that appear in unquoted strings. So far, I have 
not experienced any case where pkglint damaged anything by doing this, 
but who knows. Any other combination of variable modifiers and quoting 
context is not fixed automatically, as that seems too complicated to me 
at the moment.

> all the above technically could contain spaces, but I'm not sure how to
> quote them correctly to get rid of these warnings (tried a few
> permutations)

In general, use the :Q operator. But pay attention if the variable 
appears in 'single quotes', "double quotes" or `backticks`. There are 
subtle differences in the way the shell interprets those strings, and 
I'm not sure yet how to do it right. One popular example where pkglint 
could issue a warning but suppresses it are sed(1) replacements:

SUBST_SED.foo+=       -e 's,@PREFIX@,${PREFIX},g'

Here, PREFIX is first interpreted by the shell. For that, it would 
suffice to write ${PREFIX:Q}. But after that, the string gets further 
interpreted by sed(1), which handles backslashes and & specially. As 
long as we don't have backslashes and & in the ${PREFIX}, this doesn't 
matter. But if you replace PREFIX with CPPFLAGS in this example, things 
look different. It is not uncommon to say CPPFLAGS += -DLLONG=long\ 
long, and there you have a backslash. Of course, one could write 
s,@CPPFLAGS@,${CPPFLAGS:C/\\/\\\\/g:C/&/\\\&/:Q},g, but this looks 
terribly complicated, and casual packagers should not be required to 
write, read and understand this. I would favor ${CPPFLAGS:Qsed:Q}, which 
looks much friendlier, but all ancient and current make(1) versions do 
not know the :Qsed modifier yet. Or maybe define a variable 
Qsed=C/\\/\\\\/g:C/&/\\\&/ and then write ${EVIL:${Qsed}:Q}, but that is 
also not supported.

So, make(1) is able to quote shell strings very well and accurately, 
with nice syntax, so that I would like to have these modifiers in the 
shell as well (this won't happen either). For quoting to other languages 
than the shell, there is currently no support. So there are many 
remaining problems, they just don't show up yet.

Roland