apache / incubator-pagespeed-mod

Apache module for rewriting web pages to reduce latency and bandwidth.
http://modpagespeed.com
Apache License 2.0
696 stars 158 forks source link

PSOL with bazel #2069

Open acachy opened 3 years ago

acachy commented 3 years ago

Trying to build nginx-module with current master for very long time...

Since no manuals or PSOL-related internal scripts know about bazel:

Finally I get pagespeed_automatic.a with:

/pagespeed/opt/http/libproperty_cache_proto.a
/pagespeed/opt/http/libhttp.a
/pagespeed/opt/ads/libads.a
/pagespeed/opt/logging/liblogging_proto.a
/pagespeed/opt/logging/liblogging.a
/pagespeed/apache/libapache.a
/pagespeed/automatic/libstatic_rewriter_lib.a
/pagespeed/automatic/libautomatic.a
/pagespeed/kernel/js/libjs.a
/pagespeed/kernel/image/libimage.a
/pagespeed/kernel/image/libimage_optimizer_proto.a
/pagespeed/kernel/util/libutil.a
/pagespeed/kernel/html/libhtml.a
/pagespeed/kernel/sharedmem/libshared_mem_cache_shapshot_proto.a
/pagespeed/kernel/sharedmem/libsharedmem.a
/pagespeed/kernel/http/libhttp_proto.a
/pagespeed/kernel/http/libhttp.a
/pagespeed/kernel/cache/libcache.a
/pagespeed/kernel/thread/libthread.a
/pagespeed/kernel/base/libpagespeed_base.a
/pagespeed/kernel/base/libpagespeed_base_core.a
/pagespeed/system/libsystem.a
/pagespeed/controller/libcontroller_proto.a
/pagespeed/controller/libcontroller_proto_grpc.a
/pagespeed/controller/libcontroller.a
/net/instaweb/js/libdata2c_lib.a
/net/instaweb/spriter/libspriter.a
/net/instaweb/spriter/libspriter_protos.a
/net/instaweb/http/libhttp.a
/net/instaweb/htmlparse/libhtmlparse.a
/net/instaweb/rewriter/librewriter_protos.a
/net/instaweb/rewriter/librewriter.a
/third_party/css_parser/libcss_parser.a
/third_party/css_parser/libutf.a
/base/liblog_shim.a

Module build fails with errors about google::LogMessage like

/usr/bin/ld: /incubator-pagespeed-mod/bazel-incubator-pagespeed-mod/pagespeed/automatic/pagespeed_automatic.a(13.html_parse.pic.o.o): in function `net_instaweb::HtmlParse::CheckParentFromAddEvent(net_instaweb::HtmlEvent*)':
html_parse.cc:(.text+0x61a): undefined reference to `google::LogMessageFatal::LogMessageFatal(char const*, int, google::CheckOpString const&)'

Any suggestions? Probably it was completely wrong way to try

oschaaf commented 3 years ago

As far as I know nobody has tried to build ngx_pagespeed yet. So we're in uncharted territory. It should be possible though.

To obtain pagespeed_automatic.a this should suffice:

bazel build -c opt //pagespeed/automatic:automatic

If that yields the undefined reference you mention, then I think we're missing a dependency in a BUILD file on glog. Maybe adding it in https://github.com/apache/incubator-pagespeed-mod/blob/master/pagespeed/automatic/BUILD#L20 helps.

acachy commented 3 years ago

bazel build -c opt //pagespeed/automatic:automatic

As far as I understand - it just generates libautomatic.a (that I already listed as a part of my merged pagespeed_automatic.a). And implies to use broken bazel_merge.sh after

To be sure, I've tried to use just libautomatic.a as pagespeed_automatic.a and ngx show errors like

/usr/bin/ld: /tmp/ccBVH139.o: warning: relocation against `_ZTVN12net_instaweb18NullMessageHandlerE' in read-only section `.text.startup'
/usr/bin/ld: /tmp/ccBVH139.o: in function `main':
autotest.cc:(.text.startup+0x37): undefined reference to `vtable for net_instaweb::StringWriter'
/usr/bin/ld: autotest.cc:(.text.startup+0x83): undefined reference to `net_instaweb::MessageHandler::MessageHandler()'

I've checked a bit how pagespeed_automatic is made. In "before-bazel" way it uses merge_libraries.sh to combine these libraries:

/build/libwebp_dsp.a
/build/libwebp_enc.a
/build/libwebp_mux.a
/build/libwebp_utils.a
/net/instaweb/libautomatic_util.a
/net/instaweb/libinstaweb_add_instrumentation_data2c.a
/net/instaweb/libinstaweb_add_instrumentation_opt_data2c.a
/net/instaweb/libinstaweb_admin_site_css_data2c.a
/net/instaweb/libinstaweb_apr.a
/net/instaweb/libinstaweb_automatic.a
/net/instaweb/libinstaweb_caches_css_data2c.a
/net/instaweb/libinstaweb_caches_js_data2c.a
/net/instaweb/libinstaweb_caches_js_opt_data2c.a
/net/instaweb/libinstaweb_client_domain_rewriter_data2c.a
/net/instaweb/libinstaweb_client_domain_rewriter_opt_data2c.a
/net/instaweb/libinstaweb_console_css_data2c.a
/net/instaweb/libinstaweb_console_js_data2c.a
/net/instaweb/libinstaweb_console_js_opt_data2c.a
/net/instaweb/libinstaweb_critical_css_beacon_data2c.a
/net/instaweb/libinstaweb_critical_css_beacon_opt_data2c.a
/net/instaweb/libinstaweb_critical_css_loader_data2c.a
/net/instaweb/libinstaweb_critical_css_loader_opt_data2c.a
/net/instaweb/libinstaweb_critical_images_beacon_data2c.a
/net/instaweb/libinstaweb_critical_images_beacon_opt_data2c.a
/net/instaweb/libinstaweb_critical_images_pb.a
/net/instaweb/libinstaweb_critical_keys_pb.a
/net/instaweb/libinstaweb_dedup_inlined_images_data2c.a
/net/instaweb/libinstaweb_dedup_inlined_images_opt_data2c.a
/net/instaweb/libinstaweb_defer_iframe_data2c.a
/net/instaweb/libinstaweb_defer_iframe_opt_data2c.a
/net/instaweb/libinstaweb_delay_images_data2c.a
/net/instaweb/libinstaweb_delay_images_inline_data2c.a
/net/instaweb/libinstaweb_delay_images_inline_opt_data2c.a
/net/instaweb/libinstaweb_delay_images_opt_data2c.a
/net/instaweb/libinstaweb_dependencies_pb.a
/net/instaweb/libinstaweb_deterministic_data2c.a
/net/instaweb/libinstaweb_deterministic_opt_data2c.a
/net/instaweb/libinstaweb_extended_instrumentation_data2c.a
/net/instaweb/libinstaweb_extended_instrumentation_opt_data2c.a
/net/instaweb/libinstaweb_flush_early_pb.a
/net/instaweb/libinstaweb_graphs_css_data2c.a
/net/instaweb/libinstaweb_graphs_js_data2c.a
/net/instaweb/libinstaweb_graphs_js_opt_data2c.a
/net/instaweb/libinstaweb_input_info_pb.a
/net/instaweb/libinstaweb_js_defer_data2c.a
/net/instaweb/libinstaweb_js_defer_opt_data2c.a
/net/instaweb/libinstaweb_lazyload_images_data2c.a
/net/instaweb/libinstaweb_lazyload_images_opt_data2c.a
/net/instaweb/libinstaweb_local_storage_cache_data2c.a
/net/instaweb/libinstaweb_local_storage_cache_opt_data2c.a
/net/instaweb/libinstaweb_messages_js_data2c.a
/net/instaweb/libinstaweb_messages_js_opt_data2c.a
/net/instaweb/libinstaweb_rendered_image_pb.a
/net/instaweb/libinstaweb_responsive_js_data2c.a
/net/instaweb/libinstaweb_responsive_js_opt_data2c.a
/net/instaweb/libinstaweb_rewriter.a
/net/instaweb/libinstaweb_rewriter_base.a
/net/instaweb/libinstaweb_rewriter_css.a
/net/instaweb/libinstaweb_rewriter_html.a
/net/instaweb/libinstaweb_rewriter_html_gperf.a
/net/instaweb/libinstaweb_rewriter_csp_gperf.a
/net/instaweb/libinstaweb_rewriter_image.a
/net/instaweb/libinstaweb_rewriter_javascript.a
/net/instaweb/libinstaweb_rewriter_pb.a
/net/instaweb/libinstaweb_spriter.a
/net/instaweb/libinstaweb_spriter_pb.a
/net/instaweb/libinstaweb_static_asset_config_pb.a
/net/instaweb/libinstaweb_statistics_css_data2c.a
/net/instaweb/libinstaweb_statistics_js_data2c.a
/net/instaweb/libinstaweb_statistics_js_opt_data2c.a
/net/instaweb/libinstaweb_system.a
/net/instaweb/libinstaweb_util.a
/net/instaweb/libprocess_context.a
/net/instaweb/libtest_infrastructure.a
/pagespeed/libjs_tokenizer.a
/pagespeed/libjsminify.a
/pagespeed/libmem_lock.a
/pagespeed/libpagespeed_ads_util.a
/pagespeed/libpagespeed_base.a
/pagespeed/libpagespeed_base_core.a
/pagespeed/libpagespeed_base_test_infrastructure.a
/pagespeed/libpagespeed_cache.a
/pagespeed/libpagespeed_controller.a
/pagespeed/libpagespeed_controller_proto.a
/pagespeed/libpagespeed_html.a
/pagespeed/libpagespeed_html_gperf.a
/pagespeed/libpagespeed_http.a
/pagespeed/libpagespeed_http_core.a
/pagespeed/libpagespeed_http_gperf.a
/pagespeed/libpagespeed_http_pb.a
/pagespeed/libpagespeed_image_processing.a
/pagespeed/libpagespeed_image_types_pb.a
/pagespeed/libpagespeed_javascript_gperf.a
/pagespeed/libpagespeed_logging.a
/pagespeed/libpagespeed_logging_enums_pb.a
/pagespeed/libpagespeed_logging_pb.a
/pagespeed/libpagespeed_opt_http.a
/pagespeed/libpagespeed_property_cache_pb.a
/pagespeed/libpagespeed_sharedmem.a
/pagespeed/libpagespeed_sharedmem_pb.a
/pagespeed/libpagespeed_thread.a
/pagespeed/libpthread_system.a
/pagespeed/libutil.a
/third_party/apr/libapr.a
/third_party/aprutil/libaprutil.a
/third_party/base64/libbase64.a
/third_party/chromium/src/base/third_party/dynamic_annotations/libdynamic_annotations.a
/third_party/css_parser/libcss_parser.a
/third_party/css_parser/libcss_parser_gperf.a
/third_party/css_parser/libutf.a
/third_party/domain_registry_provider/src/domain_registry/libassert_lib.a
/third_party/domain_registry_provider/src/domain_registry/libdomain_registry_lib.a
/third_party/domain_registry_provider/src/domain_registry/libinit_registry_tables_lib.a
/third_party/gflags/libgflags.a
/third_party/giflib/libdgiflib.a
/third_party/giflib/libgiflib_core.a
/third_party/grpc/libgpr.a
/third_party/grpc/libgrpc_core.a
/third_party/grpc/libgrpc_cpp.a
/third_party/hiredis/libhiredis.a
/third_party/icu/libicudata.a
/third_party/icu/libicuuc.a
/third_party/jsoncpp/libjsoncpp.a
/third_party/libjpeg_turbo/libjpeg.a
/third_party/libjpeg_turbo/src/libjpeg_turbo.a
/third_party/libpng/libpng.a
/third_party/optipng/libopngreduc.a
/third_party/protobuf/libprotobuf_full_do_not_use.a
/third_party/re2/libre2.a
/third_party/serf/libopenssl.a
/third_party/serf/libserf.a
/third_party/zlib/libzlib.a
/url/liburl_lib.a

But now there are almost totally different names, so I've just get all "*.a" libs except few copies and ones that related to envoy and tests. At least, errors are different after

If that yields the undefined reference you mention, then I think we're missing a dependency in a BUILD file on glog. Maybe adding it in https://github.com/apache/incubator-pagespeed-mod/blob/master/pagespeed/automatic/BUILD#L20 helps.

At the moment I didn't succeed with search for correct dependency name

However

So, maybe it needs to include some extra ".a" library?

eilandert commented 3 years ago

@acachy how did you manage to build?

I get a lot of errors like this: pagespeed/kernel/base/string_util.cc: In function 'GoogleString net_instaweb::internal::StrCatNineOrMore(const StringPiece*, ...)': pagespeed/kernel/base/string_util.cc:110:3: error: 'va_start' was not declared in this scope

acachy commented 3 years ago

@acachy how did you manage to build?

I get a lot of errors like this: pagespeed/kernel/base/string_util.cc: In function 'GoogleString net_instaweb::internal::StrCatNineOrMore(const StringPiece*, ...)': pagespeed/kernel/base/string_util.cc:110:3: error: 'va_start' was not declared in this scope

@eilandert, looks like you've already moved far enough in #2084

My previous attempts were forgotten because of fail at the end... So I tried again on latest commit and here is a part of my deploy script for Debian, if you wanna try to repeate. Sorry for a lot of trash ways inside

  1. Prepare dir with sources
    
    echo "deb [arch=amd64] https://storage.googleapis.com/bazel-apt stable jdk1.8" >> /etc/apt/sources.list
    wget --no-check-certificate -qO - https://bazel.build/bazel-release.pub.gpg | apt-key add - 2>&1
    apt-get install -y bazel-4.1.0
    apt-get install -y ninja-build 
    mkdir -p /usr/share/man/man1
    apt-get install -y openjdk-11-jre-headless

mkdir /ngx cd /ngx wget https://github.com/nginx/nginx/archive/release-1.19.10.tar.gz tar xzf gz rm gz

git clone https://github.com/apache/incubator-pagespeed-ngx.git cd incubator-pagespeed-ngx git reset --hard 9e70f6d git config --global url.https://github.com/apache/.insteadOf git://git.apache.org/ git submodule update --init --recursive --jobs=16 --force cd ..

git clone --recursive https://github.com/apache/incubator-pagespeed-mod.git cd incubator-pagespeed-mod git reset --hard 6199d78


2. Bazel-build with few hacks 

sed -i '1s/^/#include \n/' pagespeed/kernel/base/string_util.cc bazel build -c fastbuild //pagespeed/kernel/... //pagespeed/automatic/... //pagespeed/system/... //pagespeed/controller/... //pagespeed/opt/... //base/... //net/instaweb/... //third_party/... mod_pagespeed sed -i -r 's/sys_siglist[signum]/strsignal(signum)/g' ./bazel-bin/../../../../../external/apr/threadproc/unix/signals.c bazel build -c fastbuild //pagespeed/kernel/... //pagespeed/automatic/... //pagespeed/system/... //pagespeed/controller/... //pagespeed/opt/... //base/... //net/instaweb/... //third_party/... mod_pagespeed


3. Update pagespeed/automatic/bazel_merge.sh
- Skip Envoy 
- Change final destination path for lib

!/bin/bash

ADIR=$(bazel info bazel-bin) ALIST=$(find $ADIR | grep \.a$ | grep -v main | grep -v copy | grep -v envoy | xargs echo)

for item in ${ALIST[*]} do echo $item done

find $ADIR | grep \.a$ | grep -v main | grep -v copy

set -e echo "merging libs" ./merge_libraries.sh ~/pagespeed_automatic.a.dirty $ALIST > merge.log

echo "renaming symbols"

XXX(oschaaf): objcopy 2.31 doesn't like clang-7's output unless we pass in -fno-addrsig

https://github.com/travitch/whole-program-llvm/issues/75

not passing this in will make the scripts that rename symbols fail

./rename_c_symbols.sh ~/pagespeed_automatic.a.dirty ~/pagespeed_automatic.a > symbol-rename.log rm ~/pagespeed_automatic.a.dirty cp ~/pagespeed_automatic.a ./../../bazel-out/k8-fastbuild/bin/pagespeed/automatic/ mv ~/pagespeed_automatic.a ./ echo "done"


4. Create pagespeed_automatic.a

cd pagespeed/automatic bash bazel_merge.sh


5. Try to build nginx

cd /ngx/nginx-release-1.19.10 MOD_PAGESPEED_DIR="/ngx/incubator-pagespeed-mod" ./auto/configure --prefix=/usr/share/nginx --sbin-path=/usr/sbin/nginx --conf-path=/etc/nginx/nginx.conf --http-log-path=/var/log/nginx/access.log --error-log-path=/var/log/nginx/error.log --lock-path=/var/lock/nginx.lock --pid-path=/run/nginx.pid --modules-path=/usr/lib/nginx/modules --http-client-body-temp-path=/var/lib/nginx/body --http-fastcgi-temp-path=/var/lib/nginx/fastcgi --http-proxy-temp-path=/var/lib/nginx/proxy --http-scgi-temp-path=/var/lib/nginx/scgi --http-uwsgi-temp-path=/var/lib/nginx/uwsgi --with-pcre-jit --with-http_ssl_module --with-http_stub_status_module --with-http_realip_module --with-http_auth_request_module --with-http_v2_module --with-http_dav_module --with-http_slice_module --with-threads --with-http_addition_module --with-http_flv_module --with-http_gunzip_module --with-http_gzip_static_module --with-http_mp4_module --with-http_random_index_module --with-http_secure_link_module --with-http_sub_module --with-mail=dynamic --with-mail_ssl_module --with-stream=dynamic --with-stream_ssl_module --with-stream_ssl_preread_module --with-http_perl_module=dynamic --add-module=/ngx/incubator-pagespeed-ngx


6. There are include-errors that can be seen in details at the end of ./objs/autoconf.err 
I fix it step-by-step by searching paths with missing files and adding extra include-dirs to /ngx/incubator-pagespeed-ngx/config
After few steps I have include rule like this:

pagespeed_include="\ $mod_pagespeed_dir \ $mod_pagespeed_dir/third_party/chromium/src \ $mod_pagespeed_dir/third_party/google-sparsehash/src/src \ $mod_pagespeed_dir/third_party/google-sparsehash/gen/arch/$os_name/$arch_name/include \ $mod_pagespeed_dir/third_party/grpc/src/include \ $mod_pagespeed_dir/third_party/protobuf/src/src \ $mod_pagespeed_dir/third_party/re2/src \ $mod_pagespeed_dir/out/$buildtype/obj/gen \ $mod_pagespeed_dir/out/$buildtype/obj/gen/protoc_out/instaweb \ $mod_pagespeed_dir/third_party/apr/src/include \ $mod_pagespeed_dir/third_party/aprutil/src/include \ $mod_pagespeed_dir/third_party/apr/gen/arch/$os_name/$arch_name/include \ $mod_pagespeed_dir/third_party/aprutil/gen/arch/$os_name/$arch_name/include \ $mod_pagespeed_dir/bazel-out/../../../external/com_google_absl \ $mod_pagespeed_dir/bazel-out/k8-fastbuild/bin/external/com_github_gflags_gflags/_virtual_includes/gflags \ $mod_pagespeed_dir/bazel-out/k8-fastbuild/bin/external/glog/_virtual_includes/default_glog_headers \ $mod_pagespeed_dir/bazel-out/k8-fastbuild/bin/external/google_sparsehash/_virtual_includes/google_sparsehash \ $mod_pagespeed_dir/url"


And my last error on attempt to use auto/configure:

... /usr/bin/ld: doctype.cc:(.text+0x430): undefined reference to `google::LogMessageFatal::~LogMessageFatal()' collect2: error: ld returned 1 exit status



Probably we should use other include-paths (for most files there are different copies in different dirs) or better ways to fix include-errors
I have no significant knowledge here, just tried to fix everything that I've seen
eilandert commented 3 years ago

Thanks! There are a few things I can use / didn't know

When I have some time I'll give it another try. Same as you, trying to fix what I encounter. Never used bazel before ;-)

eilandert commented 3 years ago

@acachy

Thanks for your pointers, I used them in a slightly different approach but I am a lot further ahead than I was, but I am stuck on the same error now.

/usr/bin/ld: doctype.cc:(.text+0x430): undefined reference to `google::LogMessageFatal::~LogMessageFatal()' collect2: error: ld returned 1 exit status



Probably we should use other include-paths (for most files there are different copies in different dirs) or better ways to fix include-errors I have no significant knowledge here, just tried to fix everything that I've seen

After some digging I discovered that glog is build in objects but there is no .a library, so glog is not merged into the pagespeed library.

@oschaaf , perhaps we need a bazel/glog.bzl ?

Please note, I don't know what i am doing either ;-) Bazel is not my thing yet

eilandert commented 3 years ago

Or we need a @glog//:glog dependency

bazel build -c fastbuild @glog//:glog seems to do something ;-)

acachy commented 3 years ago

Or we need a @glog//:glog dependency

bazel build -c fastbuild @glog//:glog seems to do something ;-)

Looks like your way is the good one

I've build a bunch of extra .a-libs with

bazel build -c fastbuild @//...

Now I have different error (but that build was not completed)

eilandert commented 3 years ago

Or we need a @glog//:glog dependency bazel build -c fastbuild @glog//:glog seems to do something ;-)

Looks like your way is the good one

I've build a bunch of extra .a-libs with

bazel build -c fastbuild @//...

Now I have different error (but that build was not completed)

This is what I do now, I have some troubles with Google URL and ABSL. Can't seem to find the correct dependencies for now, but it's a step further ;-)

bazel build -c fastbuild \
  @glog//:glog \
  @com_google_absl//absl/algorithm:algorithm @com_google_absl//absl/base:raw_logging_internal \
  @com_google_absl//absl/base @com_google_absl//absl/strings @com_google_absl//absl/hash \
 @com_google_absl//absl/memory @com_google_absl//absl/status @com_google_absl//absl/synchronization \ 
 @com_google_absl//absl/utility \
  @com_github_gflags_gflags//:gflags @com_googlesource_googleurl//base @com_googlesource_googleurl//url \
  //pagespeed/kernel/... //pagespeed/automatic/... //pagespeed/system/... //pagespeed/controller/... \
  //pagespeed/opt/... //base/... //net/instaweb/... //third_party/... \
  mod_pagespeed
eilandert commented 3 years ago

ok, I do this: https://github.com/eilandert/psol-bazel/blob/master/docker/build.sh

bazel build -c fastbuild \
  @glog//:glog \
  @com_googlesource_googleurl//base/third_party/icu \
  @com_google_absl//... \
  @com_github_gflags_gflags//... \
  @com_googlesource_googleurl//... \
  @google_sparsehash//... \
  //pagespeed/kernel/... //pagespeed/automatic/... //pagespeed/system/... //pagespeed/controller/... \
  //pagespeed/opt/... //base/... //net/instaweb/... //third_party/... \
  mod_pagespeed

But whatever I do, I get the following error:

----------------------------------------
checking for psol

/usr/bin/ld: src/incubator-pagespeed-ngx//psol/lib/Release/linux/x64/pagespeed_automatic.a(94.url_idna_icu.pic.o.o): in function `url::(anonymous
namespace)::UIDNAWrapper::UIDNAWrapper()':
(.text+0x41): undefined reference to `uidna_openUTS46_66'
/usr/bin/ld: (.text+0x8c): undefined reference to `u_errorName_66'
/usr/bin/ld: src/incubator-pagespeed-ngx//psol/lib/Release/linux/x64/pagespeed_automatic.a(94.url_idna_icu.pic.o.o): in function `url::IDNToASCII(
unsigned short const*, int, url::CanonOutputT<unsigned short>*)':
(.text+0x232): undefined reference to `uidna_nameToASCII_66'
collect2: error: ld returned 1 exit status
----------

Getting close though, I suppose this is the last fence to jump.

Lofesa commented 3 years ago

Hi @eilandert I´m not a developer, so can´t help here. But I have a clue.... Greping in and old copy of the repo I have found that icu_whatever_things are in /third_party/icu. This is not in the master, but you can find this folder in version 36 the last before bazel. the folder is a pointer to https://github.com/apache/incubator-pagespeed-icu. Maybe this must be included.....

eilandert commented 3 years ago

Hi @Lofesa

I am not a dev either ;-) But I like to puzzle.

I have it working now with latest master, well at least it passes ./configure from nginx. So PSOL itself should be good to go.

I need headers from chromium (I think) to compile the ngx_pagespeed module, but there is no chromium anymore in /third_party. It will take some time but we'll get there sooner or later when I find the time ;-)

Lofesa commented 3 years ago

Hi If I´m not wrong, chromium was changed by envoy in the bazel brach. And I think nginx module is a wrapper around the psol lib to work with nginx, so if you have the pagespeed_automatic.a you can compile the nginx_pagespeed module. Maybe @oschaaf can clarify if chromium headers are needed.

eilandert commented 3 years ago

Having the pagespeed_automatic.a is not enough, we need the headerfiles to be copied, otherwise the NGINX module is not aware of the functions and how to use them. I think I took care of that in https://github.com/eilandert/psol-bazel/blob/master/docker/build.sh (see the rsync section)

Now we have to mess around in the nginx module itself.

For example: log_message_handler.cc:31:#include "base/debug/debugger.h"

That is a hard include to (I believe) chromium. So I am not sure to include chromium or to delete or change that line. And this is only the first item I come across, I think there are a lot more.

I am not a dev, but I am willing to take a dive into it, but I am limited in time at the moment. So please bear with me ;-)

Lofesa commented 3 years ago

I only intend to help as much as I can reading the code. The files in the include, in fact all files under "base/debug", don´t exist anymore. I think is better to delete it.

acachy commented 3 years ago

@eilandert

  1. I use your bazel-build command, but extra includes in pagespeed-ngx/config looks like still necessary

  2. Bazel-build with "-c opt" creates more libraries I didn't go deep. Got error on symbols renaming about duplicates. But probably it might be because of next point.

  3. My bazel-merge.sh had to be fixed against eating pagespeed_automatic.a or it became constantly growing with each build by including itself

  4. I had your error about _uidna_openUTS4666 but other suspicions ate time "find+grep" shows that it's related to libicu-lib

There should be bazel-way to include... And maybe this alternative way would bring problems. But don't care at the moment

Just add system's package libs to merge_libraries.sh call in bazel-merge.sh /usr/lib/x86_64-linux-gnu/libicuuc.a /usr/lib/x86_64-linux-gnu/libicudata.a

#!/bin/bash

ADIR=$(bazel info bazel-bin)
ALIST=$(find $ADIR | grep \\.a$ | grep -v main | grep -v copy | grep -v envoy | grep -v /pagespeed_automatic\\.a | xargs echo)

for item in ${ALIST[*]}
do
echo $item
done

#find $ADIR | grep \\.a$ | grep -v main | grep -v copy

set -e
echo "merging libs"
bash  ./merge_libraries.sh ~/pagespeed_automatic.a.dirty $ALIST /usr/lib/x86_64-linux-gnu/libicuuc.a /usr/lib/x86_64-linux-gnu/libicudata.a > merge.log

echo "renaming symbols"

# XXX(oschaaf): objcopy 2.31 doesn't like clang-7's output unless we pass in -fno-addrsig
# https://github.com/travitch/whole-program-llvm/issues/75
# not passing this in will make the scripts that rename symbols fail
./rename_c_symbols.sh ~/pagespeed_automatic.a.dirty ~/pagespeed_automatic.a > symbol-rename.log
rm ~/pagespeed_automatic.a.dirty
cp ~/pagespeed_automatic.a ./../../bazel-out/k8-fastbuild/bin/pagespeed/automatic/
mv ~/pagespeed_automatic.a ./
echo "done"
  1. Then we get "opendl" error - it is fixed in "pagespeed-ngx/config"

    pagespeed_libs="-lstdc++"
    ->
    pagespeed_libs="-lstdc++ -ldl"
  2. Now configure is successful

  3. Haha. Make-stage shows a lot of lost files They may be fixed with more extra includes (maybe good way is in some other place for includes)

pagespeed_include="\
  $mod_pagespeed_dir \
  $mod_pagespeed_dir/pagespeed/kernel \
  $mod_pagespeed_dir/bazel-out/../../../external/com_google_absl \
  $ngx_addon_dir/testing-dependencies/mod_pagespeed/third_party/chromium/src \
  $mod_pagespeed_dir/third_party/chromium/src \
  $mod_pagespeed_dir/third_party/google-sparsehash/src/src \
  $mod_pagespeed_dir/third_party/google-sparsehash/gen/arch/$os_name/$arch_name/include \
  $mod_pagespeed_dir/third_party/grpc/src/include \
  $mod_pagespeed_dir/third_party/protobuf/src/src \
  $mod_pagespeed_dir/third_party/re2/src \
  $mod_pagespeed_dir/out/$buildtype/obj/gen \
  $mod_pagespeed_dir/out/$buildtype/obj/gen/protoc_out/instaweb \
  $mod_pagespeed_dir/third_party/apr/src/include \
  $mod_pagespeed_dir/third_party/aprutil/src/include \
  $mod_pagespeed_dir/third_party/apr/gen/arch/$os_name/$arch_name/include \
  $mod_pagespeed_dir/third_party/aprutil/gen/arch/$os_name/$arch_name/include \
  $mod_pagespeed_dir/url \
  $mod_pagespeed_dir/bazel-bin/external/glog/_virtual_includes/default_glog_headers \
  $mod_pagespeed_dir/bazel-out/k8-fastbuild/bin/external/com_github_gflags_gflags/_virtual_includes/gflags \
  $mod_pagespeed_dir/bazel-out/k8-fastbuild/bin/external/google_sparsehash/_virtual_includes/google_sparsehash \
  $mod_pagespeed_dir/bazel-incubator-pagespeed-mod \
  $mod_pagespeed_dir/bazel-incubator-pagespeed-mod/external/com_google_protobuf/src \
  $mod_pagespeed_dir/bazel-incubator-pagespeed-mod/external/re2 \
  $mod_pagespeed_dir/bazel-bin \
  $mod_pagespeed_dir/bazel-incubator-pagespeed-mod/external/com_github_grpc_grpc/include \
  $mod_pagespeed_dir/bazel-incubator-pagespeed-mod/external/apr/include \
  $mod_pagespeed_dir/bazel-incubator-pagespeed-mod/external/aprutil/include"
  1. Then new funny things appear like
/ngx/incubator-pagespeed-ngx/src/ngx_pagespeed.cc: In function ‘char* net_instaweb::{anonymous}::ps_init_dir(const StringPiece&, const StringPiece&, ngx_conf_t*)’:
/ngx/incubator-pagespeed-ngx/src/ngx_pagespeed.cc:780:33: error: ‘StrCat’ is not a member of ‘net_instaweb’
  780 |         cf->pool, net_instaweb::StrCat(directive, " ", path,
/ngx/incubator-pagespeed-ngx/src/log_message_handler.cc:54:19: error: ‘LOG_ERROR_REPORT’ is not a member of ‘logging’
   54 |     case logging::LOG_ERROR_REPORT:
acachy commented 3 years ago

Same as last errors, but more clear example

/ngx/incubator-pagespeed-ngx/src/log_message_handler.cc: In function ‘void net_instaweb::log_message_handler::Install(ngx_log_t*)’:
/ngx/incubator-pagespeed-ngx/src/log_message_handler.cc:106:12: error: ‘SetLogMessageHandler’ is not a member of ‘logging’
  106 |   logging::SetLogMessageHandler(&LogMessageHandler);

Function "SetLogMessageHandler" not exists in modpagespeed at all, it can be seen only in /incubator-pagespeed-ngx/testing-dependencies/mod_pagespeed/thirdparty/chromium/src/base/logging.h

As far as I understand - it's not just build problem further...

  1. Or we restore chronium parts around I think it's "not meant to be" Even if we build it (can use before-bazel commit for test) it should be difficult enough since "logging" and other things are exists in new places\ways

  2. Or we update included-modules to make them fit current pagespeed-ngx

  3. Or we fix pagespeed-ngx As @Lofesa suggested I've deleted "base/debug" includes (together with deleting my funny chromium include). However, there are still this logging and other problems...

  4. Or worse

I think some hybrid of №2+№3 is the best way Not good feeling for non-dev approach thought...

Lofesa commented 3 years ago

But now there is base/loggin.h

I found SetLogMessageHandler in pagespeed/envoy/log_message_handler.cc, but in a comment in an epmty function https://github.com/apache/incubator-pagespeed-mod/blob/d8070cf1cd0dbfb70a05e28b739707c66b1cf635/pagespeed/envoy/log_message_handler.cc#L66

I think envoy is the sustitute for chromium and must be included

eilandert commented 3 years ago

@Lofesa I think we need to include the logging.h from the glog package.

@acachy:

My approach is not to include /root/.bazel/* but to create a psol.tar.gz package with the libautomatic.a and the needed headers so people can untar it in nginx-1.21.4/modules/ngx_pagespeed/psol without the need to build it with bazel. See the rsync commands in https://github.com/eilandert/psol-bazel/blob/master/docker/build.sh

My approach is: 1) create a functional psol (done, I suppose) 2) create a psol.tar.gz with lib+headers (done, I suppose) 3) clone the ngx pagespeed module and make some changes.

When 3 works we have a proof of concept. Then we continue to:

4) adjust master/pagespeed/automatic/BUILD and stuff so everything will be build by "bazel build -c fastbuild //pagespeed/automatic:automatic" or something. 5) find out the bazel way to build libicu. When I build ICU with bazel, the build fails because gURL wants to build a target that doesn;t exist. 6) optimize the build. not everything has to be in the lib I think. the lib is around 111MB now, while in the old way it was only half of that. 7) do a pull request

I think it is not that difficult, but since I am no dev I have to find everything out for myself with common sense, and currently I haven't got the time. I'm not home for a couple of weeks, i've been sent to a hospital far away due to covid to help out (i am actually an healthcare provider)

So, it will take time ;-)

oschaaf commented 3 years ago

Hey @eilandert & @acachy & @Lofesa awesome job; I think that with some persistence this will be resolved, as it should be viable. Earlier I had hoped that there would be a variant of nginx out there which would support the bazel build system. That would have made life much easier; but alas there are no (up-to-date) efforts in that direction that I know of. It looks like supporting a "bazelified" build of nginx ourselves would require an ongoing and significant effort; so this project will very probably be better off with what is being attempted here: package psol like we used to do earlier. I'm hoping I can find some time around the end of this year to contribute to this effort.

eilandert commented 3 years ago

@oschaaf That would be cool, I hope to have time in a couple of weeks too.

In the meantime I've created https://github.com/eilandert/psol-bazel , you can clone this, run ./build.sh and docker does all the magic until the first unfixed error in https://github.com/eilandert/pagespeed-ngx-bazel ;-)