Bug 128727 - CC and PKG_CONFIG should be quoted
Summary: CC and PKG_CONFIG should be quoted
Status: RESOLVED INVALID
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Other
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-02-12 22:55 PST by Ting-Wei Lan
Modified: 2014-03-25 15:34 PDT (History)
5 users (show)

See Also:


Attachments
[GTK] Quote the CC and PKG_CONFIG variable (1.28 KB, patch)
2014-02-13 05:36 PST, Ting-Wei Lan
no flags Details | Formatted Diff | Diff
[GTK] Quote the CC and PKG_CONFIG variable (updated) (1.48 KB, patch)
2014-02-16 02:39 PST, Ting-Wei Lan
no flags Details | Formatted Diff | Diff
[GTK] Quote the CC and PKG_CONFIG variable (updated again) (1.47 KB, patch)
2014-02-16 02:45 PST, Ting-Wei Lan
gustavo: review-
Details | Formatted Diff | Diff
[GTK] Quote the CC variable (989 bytes, patch)
2014-02-28 00:34 PST, Ting-Wei Lan
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.