apache / incubator-pagespeed-ngx

Automatic PageSpeed optimization module for Nginx
http://ngxpagespeed.com/
Apache License 2.0
4.37k stars 363 forks source link

Error during making of debian package. #197

Open v4l3r10 opened 11 years ago

v4l3r10 commented 11 years ago

I added pagespeed module into standard nginx-full debian package, but during compile of package i receive this error:

/tmp/ngx_pagespeed/psol/include/third_party/chromium/src/base/logging.h:289: error: expected unqualified-id before numeric constant /tmp/ngx_pagespeed/psol/include/third_party/chromium/src/base/logging.h:290: error: expected unqualified-id before numeric constant make[2]: * [objs/addon/src/ngx_pagespeed.o] Error 1 make[2]: Leaving directory `/usr/src/nginx/source/nginx-1.2.7/debian/build-full' make[1]: * [build] Error 2 make[1]: Leaving directory`/usr/src/nginx/source/nginx-1.2.7/debian/build-full' make: *\ [build-arch.full] Error 2 dpkg-buildpackage: error: debian/rules build gave error exit status 2

jeffkaufman commented 11 years ago

Could you give more details of what you're doing?

That error might be some sort of name collision? Maybe LOG_INFO and LOG_WARNING are defined as preprocessor macros somewhere else?

v4l3r10 commented 11 years ago

I'm trying to make a deb package from nginx-full deb src package... i have added in debian/rules this line to add modules on nginx-full package. --add-module=$(MODULESDIR)/ngx_http_substitutions_filter_module \ --add-module=$(MODULESDIR)/ngx_pagespeed \

after i execute dpkg-buildpackage -uc -b

p.s.: I have fight with variable name and after many rename on many file te compile was successfull.. please if you can fix variable name:D

jeffkaufman commented 11 years ago

I have fight with variable name and after many rename on many file te compile was successfull

Are you saying that after renaming LOG_INFO and LOG_WARNING to something else, through all of the pagespeed code, you got it to compile? Do you know what file defined the variables they were conflicting with?

v4l3r10 commented 11 years ago

after a grep in debian src.:

:/usr/src/nginx/source/nginx-1.2.7# grep -R " LOG_INFO" * debian/build-full/src/core/ngx_log.c: {ngx_string("info"), LOG_INFO}, debian/build-light/src/core/ngx_log.c: {ngx_string("info"), LOG_INFO}, debian/patches/syslog_1.3.11.patch:+ {ngx_string("info"), LOG_INFO}, debian/build-extras/src/core/ngx_log.c: {ngx_string("info"), LOG_INFO}, src/core/ngx_log.c: {ngx_string("info"), LOG_INFO},

jeffkaufman commented 11 years ago

Are these debian specific changes to nginx to use LOG_INFO? I'm only seeing NGX_LOG_INFO:

$ wget http://nginx.org/download/nginx-1.2.7.tar.gz
$ tar -xvzf nginx-1.2.7.tar.gz
$ cd nginx-1.2.7/
$ grep -R " LOG_INFO" *
[ no results ]    
v4l3r10 commented 11 years ago

Yes bro, apt-get source nginx

jeffkaufman commented 11 years ago

I'm sorry, I'm not that familiar with apt-get. How do I make apt-get source nginx get me 1.2.7 and not an older version?

v4l3r10 commented 11 years ago

I use debian squeeze Linux version 2.6.32-5-amd64 (Debian 2.6.32-48squeeze1) (dannf@debian.org) (gcc version 4.3.5 (Debian 4.3.5-4) ) #1 SMP Mon Feb 25 00:26:11 UTC 2013 and the nginx default version was 1.2.7 when i use apt-get source nginx it download nginx_1.2.7-1~dotdeb.1.debian.tar.gz and nginx_1.2.7.orig.tar.gz see here http://apt.cmjscripter.net/dotdeb/dists/squeeze/nginx/source/

v4l3r10 commented 11 years ago

i Suggest to add a modulesname to variable ex.: NGX_PAGESPEED_LOG_INFO :) thk for your work! :D

jeffkaufman commented 11 years ago

Unfortunately logging.h where we get LOG_INFO etc is part of the chromium project:

http://src.chromium.org/viewvc/chrome/trunk/src/base/logging.h

Because LOG_INFO is defined inside the logging namespace I doubt they would be willing to change it.

I think fixing the nginx debian package not to #define LOG_INFO would probably make more sense.

jeffkaufman commented 11 years ago

What the debian package is doing is more complex than just #defineing LOG_INFO. They add a compile-time option NGX_ENABLE_SYSLOG which does an #include <syslog.h>, and it's syslog.h that has LOG_INFO etc.

nginx-1.2.7/src/http/modules/ngx_http_log_module.c:
+#if (NGX_ENABLE_SYSLOG)                                                         
+#include <syslog.h>                                                             
...  

If you compile nginx without syslog that should fix this error.

jeffkaufman commented 11 years ago

(And "incompatible with syslog.h" sounds like something I could report to chromium.)

jeffkaufman commented 11 years ago

Another way to fix this might be to convert syslog's #defines to const ints in the preprocessor, as in fixx11h.h:

#ifdef LOG_INFO
#ifndef NGX_PS_LOG_INFO
#define NGX_PS_LOG_INFO
const int NGX_PS_LOG_INFO_TMP = LOG_INFO;
#undef LOG_INFO;
const int LOG_INFO = NGX_PS_LOG_INFO_TMP;
#endif
#undef LOG_INFO
#endif

(Repeating for LOG_WARNING)

Because chromium's logging.h namespaces their use of LOG_INFO and LOG_WARNING I think this should work. It would need to run some time between #include <syslog.h> and #include "base/logging.h".

Is this worth it?

jeffkaufman commented 11 years ago

Actually, talking to @morlovich, can we just put:

#undef LOG_INFO
#undef LOG_WARNING

Somewhere before #include "base/logging.h"?

v4l3r10 commented 11 years ago

OK bro i have added this :

// FIX for compile problem with chromium

ifdef LOG_INFO

undef LOG_INFO

endif

ifdef LOG_WARNING

undef LOG_WARNING

endif

into ngx_pagespeed/psol/include/third_party/chromium/src/base/logging.h

and all worked fine! thk for ur help!

Tombar commented 11 years ago

I Got the same issue @v4l3r10 had while building an nginx package for 1.4.0 with syslog module and pagespeed, add i can confirm the recommended fix and it work!

// FIX for compile problem with chromium

ifdef LOG_INFO

undef LOG_INFO

endif

ifdef LOG_WARNING

undef LOG_WARNING

endif

Tombar commented 11 years ago

Hello Jeff

I see some of the code change regarding this issue on 1.5.27.3 but it can't compile as is when syslog module is enabled unfortunately

/home/vagrant/nginx-custom/nginx-1.4.1/debian/modules/ngx_pagespeed-release-1.5.27.3-beta/psol/include/third_party/chromium/src/base/logging.h:289: error: expected unqualified-id before numeric constant /home/vagrant/nginx-custom/nginx-1.4.1/debian/modules/ngx_pagespeed-release-1.5.27.3-beta/psol/include/third_party/chromium/src/base/logging.h:290: error: expected unqualified-id before numeric constant /home/vagrant/nginx-custom/nginx-1.4.1/debian/modules/ngx_pagespeed-release-1.5.27.3-beta/src/ngx_pagespeed.cc: In function 'bool ngx_psol::::ps_apply_x_forwarded_proto(ngx_http_requestt, GoogleString_)': /home/vagrant/nginx-custom/nginx-1.4.1/debian/modules/ngx_pagespeed-release-1.5.27.3-beta/src/ngx_pagespeed.cc:1288: error: expected unqualified-id before numeric constant /home/vagrant/nginx-custom/nginx-1.4.1/debian/modules/ngx_pagespeed-release-1.5.27.3-beta/src/ngx_pagespeed.cc:1288: error: expected ')' before numeric constant /home/vagrant/nginx-custom/nginx-1.4.1/debian/modules/ngx_pagespeed-release-1.5.27.3-beta/src/ngx_pagespeed.cc:1289: error: expected ')' before ';' token /home/vagrant/nginx-custom/nginx-1.4.1/debian/modules/ngx_pagespeed-release-1.5.27.3-beta/src/ngx_pagespeed.cc:1289: error: expected ')' before ';' token make[2]: *\ [objs/addon/src/ngx_pagespeed.o] Error 1

Regards

M

jeffkaufman commented 11 years ago

@Tombar has your workaround from last time stopped working? https://github.com/pagespeed/ngx_pagespeed/issues/197#issuecomment-17364537

Tombar commented 11 years ago

Hello @jeffkaufman

If i comment out lines 287 & 288 of ngx_pagespeed-release-1.5.27.3-beta/psol/include/third_party/chromium/src/base/logging.h

const LogSeverity LOG_INFO = 0;

const LogSeverity LOG_WARNING = 1;

When trying to compile nginx it complains it can't find the PSOL library

configuring additional modules adding module in /home/vagrant/nginx-custom/nginx-1.4.1x/debian/modules/nginx-upstream-fair

Let me know if you need any other test or info.

Regards

M

jeffkaufman commented 11 years ago

If instead of adding a '#' you add

#ifdef LOG_INFO
#undef LOG_INFO
#endif

#ifdef LOG_WARNING
#undef LOG_WARNING
#endif

to the top of chromium/src/base/logging.h, does that work?

Tombar commented 11 years ago

Jeff i've test it and it compiles and work :)

Thank you sir!

Regards

M

luntzel commented 11 years ago

The fix above is not working for me. Using dpkg-buildpackage -b, I get the output as described above. When using straight "make", I get the following output on Ubuntu 12.04, gcc version 4.6.3, nginx 1.4.1, pagespeed 1.5.27.3-beta:

cc -c -pipe -O -W -Wall -Wpointer-arith -Wno-unused-parameter -Werror -g -I src/core -I src/event -I src/event/modules -I src/os/unix -I ../ngx_pagespeed-release-1.5.27.3-beta/psol/include -I ../ngx_pagespeed-release-1.5.27.3-beta/psol/include/third_party/chromium/src -I ../ngx_pagespeed-release-1.5.27.3-beta/psol/include/third_party/google-sparsehash/src -I ../ngx_pagespeed-release-1.5.27.3-beta/psol/include/third_party/google-sparsehash/gen/arch/linux/x64/include -I ../ngx_pagespeed-release-1.5.27.3-beta/psol/include/third_party/protobuf/src -I ../ngx_pagespeed-release-1.5.27.3-beta/psol/include/third_party/re2/src -I ../ngx_pagespeed-release-1.5.27.3-beta/psol/include/out/Release/obj/gen -I ../ngx_pagespeed-release-1.5.27.3-beta/psol/include/third_party/apr/src/include -I ../ngx_pagespeed-release-1.5.27.3-beta/psol/include/third_party/aprutil/src/include -I ../ngx_pagespeed-release-1.5.27.3-beta/psol/include/third_party/apr/gen/arch/linux/x64/include -I ../ngx_pagespeed-release-1.5.27.3-beta/psol/include/third_party/aprutil/gen/arch/linux/x64/include -I objs -I src/http -I src/http/modules -I src/mail \ -o objs/addon/src/ngx_pagespeed.o \ ../ngx_pagespeed-release-1.5.27.3-beta/src/ngx_pagespeed.cc {standard input}: Assembler messages: {standard input}:14830: Warning: end of file not at end of a line; newline inserted {standard input}:16332: Error: unknown pseudo-op: .' {standard input}: Error: open CFI at the end of file; missing .cfi_endproc directive cc: internal compiler error: Killed (program cc1plus) Please submit a full bug report, with preprocessed source if appropriate. See <file:///usr/share/doc/gcc-4.6/README.Bugs> for instructions. make[1]: *** [objs/addon/src/ngx_pagespeed.o] Error 4 make[1]: Leaving directory/home/ubuntu/nginx-pagespeed/nginx-1.4.1' make: *\ [build] Error 2

janreges commented 6 years ago

Hi, is there any plan to fix this issue?

It blocks also implementation of PageSpeed into Gentoo, see https://bugs.gentoo.org/661482

Thank you.