New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unable to compile with GCC 4.8 #433

Closed
eleshar opened this Issue Jul 11, 2013 · 11 comments

Comments

Projects
None yet
6 participants
@eleshar
Copy link

eleshar commented Jul 11, 2013

Hi,

I'm attempting to install the latest release as outlined on https://developers.google.com/speed/pagespeed/module/build_ngx_pagespeed_from_source but my installation keeps failing with:

In file included from /root/sources/ngx_pagespeed/psol/include/third_party/chromium/src/base/logging.h:14:0,
from /root/sources/ngx_pagespeed/src/log_message_handler.cc:30:
/root/sources/ngx_pagespeed/psol/include/third_party/chromium/src/base/basictypes.h: In function âDest bit_cast(const Source&)â:
/root/sources/ngx_pagespeed/psol/include/third_party/chromium/src/base/basictypes.h:318:16: error: typedef âVerifySizesAreEqualâ locally defined but not used [-Werror=unused-local-typedefs]
typedef char VerifySizesAreEqual [sizeof(Dest) == sizeof(Source) ? 1 : -1];
^
cc1plus: all warnings being treated as errors
make[1]: *** [objs/addon/src/log_message_handler.o] Error 1
make[1]: Leaving directory `/root/sources/nginx-1.5.2'
make: *** [install] Error 2

I'm on Ubuntu 13.04 and building with:
./configure --prefix=/opt/nginx --conf-path=/etc/nginx/nginx.conf --pid-path=/var/run/nginx.pid --lock-path=/var/lock/nginx.lock --http-log-path=/var/log/nginx/access.log --error-log-path=/var/log/nginx/error.log --http-client-body-temp-path=/var/lib/nginx/body --http-proxy-temp-path=/var/lib/nginx/proxy --http-fastcgi-temp-path=/var/lib/nginx/fastcgi --http-uwsgi-temp-path=/var/lib/nginx/uwsgi --http-scgi-temp-path=/var/lib/nginx/scgi --with-http_stub_status_module --with-http_ssl_module --with-http_realip_module --with-http_gzip_static_module --user=www-data --group=www-data --with-debug --without-mail_pop3_module --without-mail_imap_module --without-mail_smtp_module --add-module=/root/sources/ngx_pagespeed-release-1.6.29.3-beta --with-http_spdy_module

@chaizhenhua

This comment has been minimized.

Copy link
Contributor

chaizhenhua commented Jul 11, 2013

@eleshar can you follow the instruction here https://github.com/pagespeed/ngx_pagespeed/wiki/Building-PSOL-From-Source with tag name 1.6.29.2 instead of 1.5.27.3

@jeffkaufman

This comment has been minimized.

Copy link
Contributor

jeffkaufman commented Jul 11, 2013

I don't think building from source as @chaizhenhua will help, sorry!

The problem is that -Wall in gcc 4.8 now enables -Wunused-local-typedefs and one of our dependencies fails that test.

We either need to fix the dependency or figure out how to build with that warning disabled.

@eleshar

This comment has been minimized.

Copy link

eleshar commented Jul 11, 2013

Thanks @jeffkaufman downgrading gcc to version 4.7.3 resolved the issue.

@jeffkaufman

This comment has been minimized.

Copy link
Contributor

jeffkaufman commented Jul 11, 2013

With gcc 4.8 I needed to add -Wno-unused-local-typedefs to CFLAGS:

diff --git a/config b/config
index 05e06d2..2505a60 100644
--- a/config
+++ b/config
@@ -87,6 +87,11 @@ fi
 # linker errors, so disable it here.
 CFLAGS="$CFLAGS -DSERF_HTTPS_FETCHING=0"

+# On GCC 4.8 and above, -Wall enables -Wunused-local-typedefs.  This breaks on
+# VerifySizesAreEqual in bit_cast in chromium/src/base/basictypes.h which has a
+# typedef that is intentionally unused.
+CFLAGS="$CFLAGS -Wno-unused-local-typedefs"
+
 pagespeed_include="\
   $mod_pagespeed_dir \
   $mod_pagespeed_dir/third_party/chromium/src \

While this got past the error with VerifySizesAreEqual it gave a new set of errors:

In file included from /home/jefftk/ngx_ps/ngx_pagespeed/psol/include/third_party/chromium/src/base/memory/scoped_ptr.h:100:0,
                 from /home/jefftk/ngx_ps/ngx_pagespeed/psol/include/pagespeed/kernel/base/scoped_ptr.h:26,
                 from /home/jefftk/ngx_ps/ngx_pagespeed/psol/include/net/instaweb/util/public/scoped_ptr.h:23,
                 from /home/jefftk/ngx_ps/ngx_pagespeed/psol/include/net/instaweb/http/public/headers.h:22,
                 from /home/jefftk/ngx_ps/ngx_pagespeed/psol/include/net/instaweb/http/public/response_headers.h:21,
                 from /home/jefftk/ngx_ps/ngx_pagespeed/src/ngx_pagespeed.h:37,
                 from /home/jefftk/ngx_ps/ngx_pagespeed/src/ngx_base_fetch.h:46,
                 from /home/jefftk/ngx_ps/ngx_pagespeed/src/ngx_base_fetch.cc:19:
/home/jefftk/ngx_ps/ngx_pagespeed/psol/include/third_party/chromium/src/base/memory/scoped_ptr.h:134:29: error: invalid use of incomplete type ‘class scoped_ptr<C>’ [-Werror]
   MOVE_ONLY_TYPE_FOR_CPP_03(scoped_ptr, RValue)
                             ^
/home/jefftk/ngx_ps/ngx_pagespeed/psol/include/third_party/chromium/src/base/move.h:198:31: note: in definition of macro ‘MOVE_ONLY_TYPE_FOR_CPP_03’
   struct rvalue_type : public type { \
                               ^
In file included from /home/jefftk/ngx_ps/ngx_pagespeed/psol/include/pagespeed/kernel/base/scoped_ptr.h:26:0,
                 from /home/jefftk/ngx_ps/ngx_pagespeed/psol/include/net/instaweb/util/public/scoped_ptr.h:23,
                 from /home/jefftk/ngx_ps/ngx_pagespeed/psol/include/net/instaweb/http/public/headers.h:22,
                 from /home/jefftk/ngx_ps/ngx_pagespeed/psol/include/net/instaweb/http/public/response_headers.h:21,
                 from /home/jefftk/ngx_ps/ngx_pagespeed/src/ngx_pagespeed.h:37,
                 from /home/jefftk/ngx_ps/ngx_pagespeed/src/ngx_base_fetch.h:46,
                 from /home/jefftk/ngx_ps/ngx_pagespeed/src/ngx_base_fetch.cc:19:
/home/jefftk/ngx_ps/ngx_pagespeed/psol/include/third_party/chromium/src/base/memory/scoped_ptr.h:133:7: error: declaration of ‘class scoped_ptr<C>’ [-Werror]
 class scoped_ptr {
@rbu

This comment has been minimized.

Copy link

rbu commented Sep 5, 2013

WebKit has this fixed, what about updating this code copy?

@rcarl01

This comment has been minimized.

Copy link

rcarl01 commented Sep 17, 2013

Is there any update? as it mentioned that Webkit has this fixed!

please share the info

@jlporter

This comment has been minimized.

Copy link
Contributor

jlporter commented Sep 24, 2013

There is a discussion on the gcc mailing list about this at http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54055. I haven't tried, but looking at the trunk version of scoped_ptr.h in chromium, I don't think this has been fixed there yet.

For now, it looks like the only way to continue is disabling treating warnings as errors, since there isn't an option to disable just that warning. Together with Jeff's change, the following diff compiles for me now.

diff --git a/config b/config
index c523aaa..9dac721 100644
--- a/config
+++ b/config
@@ -93,6 +93,19 @@ fi
 # linker errors, so disable it here.
 CFLAGS="$CFLAGS -DSERF_HTTPS_FETCHING=0 $FLAG_MARCH"

+# On GCC 4.8 and above, -Wall enables -Wunused-local-typedefs.  This breaks on
+# VerifySizesAreEqual in bit_cast in chromium/src/base/basictypes.h which has a
+# typedef that is intentionally unused.
+CFLAGS="$CFLAGS -Wno-unused-local-typedefs"
+
+# On GCC 4.8 and above, we get the following compiler warning:
+# chromium/src/base/memory/scoped_ptr.h:133:7: warning: declaration of ‘class scoped_ptr<C>’ [enabled by default]
+# Based on discussion at http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54055, this
+# is invalid code, but hasn't been fixed yet in chromium. Unfortunately, there
+# also does not appear to be a flag for just disabling that warning, so we add
+# Wno-error to override nginx's default -Werror option.
+CFLAGS="$CFLAGS -Wno-error"
+
 pagespeed_include="\
   $mod_pagespeed_dir \
   $mod_pagespeed_dir/third_party/chromium/src \
@jeffkaufman

This comment has been minimized.

Copy link
Contributor

jeffkaufman commented Sep 24, 2013

Thanks for looking into this!

Can we do these relaxations only for gcc 4.8? Something like:

if [ $(gcc --version | head -n 1 | awk '{print $NF}') \> '4.8' ]; then
  # comments
  CFLAGS="$CFLAGS -Wno-unused-local-typedefs"
  # comments
  CFLAGS="$CFLAGS -Wno-error"
fi
@jlporter

This comment has been minimized.

Copy link
Contributor

jlporter commented Sep 24, 2013

That didn't quite work on my system. Looks like there is some variability in the gcc version strings.

$ gcc --version
gcc (GCC) 4.8.1 20130725 (prerelease)
$ gcc --version | head -n 1 | awk '{print $NF}'                                                                                            
(prerelease)

I came up with the following based on nginx's auto/cc/gcc

diff --git a/config b/config
index c523aaa..11ac24b 100644
--- a/config
+++ b/config
@@ -93,6 +93,23 @@ fi
 # linker errors, so disable it here.
 CFLAGS="$CFLAGS -DSERF_HTTPS_FETCHING=0 $FLAG_MARCH"

+case "$NGX_GCC_VER" in
+  4.8*)
+    # On GCC 4.8 and above, -Wall enables -Wunused-local-typedefs.  This breaks on
+    # VerifySizesAreEqual in bit_cast in chromium/src/base/basictypes.h which has a
+    # typedef that is intentionally unused.
+    CFLAGS="$CFLAGS -Wno-unused-local-typedefs"
+
+    # On GCC 4.8 and above, we get the following compiler warning:
+    # chromium/src/base/memory/scoped_ptr.h:133:7: warning: declaration of ‘class scoped_ptr<C>’ [enabled by default]
+    # Based on discussion at http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54055, this
+    # is invalid code, but hasn't been fixed yet in chromium. Unfortunately, there
+    # also does not appear to be a flag for just disabling that warning, so we add
+    # Wno-error to override nginx's default -Werror option.
+    CFLAGS="$CFLAGS -Wno-error"
+  ;;
+esac
+
 pagespeed_include="\
   $mod_pagespeed_dir \
   $mod_pagespeed_dir/third_party/chromium/src \
@jeffkaufman

This comment has been minimized.

Copy link
Contributor

jeffkaufman commented Sep 24, 2013

That is so much better; thanks!

Do you want to create the pull request or should I?

@jeffkaufman

This comment has been minimized.

Copy link
Contributor

jeffkaufman commented Sep 24, 2013

Fixed by #528

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment