Bug 128727

Summary: CC and PKG_CONFIG should be quoted
Product: WebKit Reporter: Ting-Wei Lan <lantw44>
Component: New BugsAssignee: Nobody <webkit-unassigned>
Status: RESOLVED INVALID    
Severity: Normal CC: berto, commit-queue, gustavo, mrobinson, zan
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: Other   
Attachments:
Description Flags
[GTK] Quote the CC and PKG_CONFIG variable
none
[GTK] Quote the CC and PKG_CONFIG variable (updated)
none
[GTK] Quote the CC and PKG_CONFIG variable (updated again)
gustavo: review-
[GTK] Quote the CC variable none

Description Ting-Wei Lan 2014-02-12 22:55:54 PST
In Tools/gtk/GNUmakefile.am, line 45:
CC=$(CC) $(srcdir)/Tools/gtk/generate-gtkdoc

This does not work if CC contains spaces. When I set CC to "clang -std=c11 -pedantic", I get the following error:
CC=clang -std=c11 -pedantic ./Tools/gtk/generate-gtkdoc
-std=c11: not found
gmake[1]: *** [docs-build.stamp] Error 127
gmake[1]: *** Waiting for unfinished jobs....

We should change it to CC="$(CC)" so it will work even if CC contains spaces.
Comment 1 Ting-Wei Lan 2014-02-13 05:36:44 PST
Created attachment 224059 [details]
[GTK] Quote the CC and PKG_CONFIG variable
Comment 2 Ting-Wei Lan 2014-02-16 02:39:14 PST
Created attachment 224317 [details]
[GTK] Quote the CC and PKG_CONFIG variable (updated)
Comment 3 WebKit Commit Bot 2014-02-16 02:41:06 PST
Attachment 224317 [details] did not pass style-queue:


ERROR: Tools/ChangeLog:319:  ChangeLog entry has no bug number  [changelog/bugnumber] [5]
Total errors found: 1 in 2 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 4 Zan Dobersek 2014-02-16 02:41:22 PST
Compiler flags should be placed into CFLAGS and CXXFLAGS environment variables.
Comment 5 Ting-Wei Lan 2014-02-16 02:45:49 PST
Created attachment 224318 [details]
[GTK] Quote the CC and PKG_CONFIG variable (updated again)
Comment 6 Alberto Garcia 2014-02-16 05:12:31 PST
(In reply to comment #5)
> Created an attachment (id=224318) [details]
> [GTK] Quote the CC and PKG_CONFIG variable (updated again)

In what case does an unquoted PKG_CONFIG break?
Comment 7 Ting-Wei Lan 2014-02-16 07:09:13 PST
(In reply to comment #6)
> (In reply to comment #5)
> > Created an attachment (id=224318) [details] [details]
> > [GTK] Quote the CC and PKG_CONFIG variable (updated again)
> 
> In what case does an unquoted PKG_CONFIG break?

It may not break. I found it when I found the unquoted CC, so I patched both CC and PKG_CONFIG.
Comment 8 Alberto Garcia 2014-02-16 14:40:01 PST
(In reply to comment #7)
> > In what case does an unquoted PKG_CONFIG break?
> It may not break. I found it when I found the unquoted CC, so I
> patched both CC and PKG_CONFIG.

Well, the Makefile is full of unquoted variables (e.g. PYTHON in that
very same line) :)

Quoting PKG_CONFIG doesn't harm, but it probably doesn't fix anything
either. I'd bet that if there was a case where PKG_CONFIG needed to be
quoted something else would break.

Anyway, quoting CC is fine. Altough, as Žan says, compiler flags
shouldn't generally go there, setting CC to "icecc gcc", "ccache gcc"
and friends is common practice.
Comment 9 Gustavo Noronha (kov) 2014-02-25 12:07:33 PST
Comment on attachment 224318 [details]
[GTK] Quote the CC and PKG_CONFIG variable (updated again)

I agree there's no point in changing PKG_CONFIG unless we're fixing an actual issue.
Comment 10 Gustavo Noronha (kov) 2014-02-25 12:08:04 PST
Comment on attachment 224318 [details]
[GTK] Quote the CC and PKG_CONFIG variable (updated again)

View in context: https://bugs.webkit.org/attachment.cgi?id=224318&action=review

> Tools/ChangeLog:319
>          * DumpRenderTree/mac/UIDelegate.mm: (-[UIDelegate webCryptoMasterKeyForWebView:]):
>          Use the SPI to specify a key.
>  
> +2014-02-13  Ting-Wei Lan  <lantw44@gmail.com>

This looks like it could break, btw, you want your ChangeLog entry at the top.
Comment 11 Ting-Wei Lan 2014-02-28 00:34:50 PST
Created attachment 225444 [details]
[GTK] Quote the CC variable
Comment 12 Alberto Garcia 2014-02-28 00:47:16 PST
(In reply to comment #11)
> Created an attachment (id=225444) [details]
> [GTK] Quote the CC variable

lgtm
Comment 13 Martin Robinson 2014-03-25 15:34:54 PDT
Autotools build is gone now. Feel free to reopen if this is an issue with the CMake build.