pkgsrc-WIP-discuss archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: pkglint -Wall



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


-------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc. Do you grep through log files
for problems?  Stop!  Download the new AJAX search engine that makes
searching your log files as easy as surfing the  web.  DOWNLOAD SPLUNK!
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=103432&bid=230486&dat=121642
_______________________________________________
pkgsrc-wip-discuss mailing list
pkgsrc-wip-discuss%lists.sourceforge.net@localhost
https://lists.sourceforge.net/lists/listinfo/pkgsrc-wip-discuss



Home | Main Index | Thread Index | Old Index