Skip to content
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

nginx 1.25.2 rpm build (Fedora/COPR) + brotli/master as dynamic module FAIL: error: Brotli library is missing ... #158

Closed
pgnd opened this issue Oct 9, 2023 · 6 comments

Comments

@pgnd
Copy link

pgnd commented Oct 9, 2023

building, as usual here, nginx 1.25.2, with brotli master added as a dynamic module

rpm build pulls from master as usual

...
INFO: Downloading ngx_brotli-master.tar.gz
INFO: Calling: curl -H Pragma: -o ngx_brotli-master.tar.gz --location --connect-timeout 60 --retry 3 --retry-delay 10 --remote-time --show-error --fail --retry-all-errors https://copr-dist-git.fedorainfracloud.org/repo/pkgs/pgfed/nginx-mainline/nginx/ngx_brotli-master.tar.gz/md5/4c6b0e825807af3d9c2ce1404b8e0d9e/ngx_brotli-master.tar.gz
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100 16365  100 16365    0     0   276k      0 --:--:-- --:--:-- --:--:--  280k
INFO: Reading stdout from command: md5sum ngx_brotli-master.tar.gz
...

but fails (new/recent bevavior...)

adding module in ../ngx_brotli-master

./auto/configure: error: Brotli library is missing from the ../ngx_brotli-master/deps/brotli/c directory.

Please make sure that the git submodule has been checked out:

    cd ../ngx_brotli-master && git submodule update --init && cd /builddir/build/BUILD/nginx-release-1.25.2

full build spec & FAIL log output here:

https://download.copr.fedorainfracloud.org/results/pgfed/nginx-mainline/fedora-38-x86_64/06509057-nginx/nginx.spec

&

https://download.copr.fedorainfracloud.org/results/pgfed/nginx-mainline/fedora-38-x86_64/06509057-nginx/builder-live.log.gz

last successful build output (same spec), from ~ a month ago,

https://download.copr.fedorainfracloud.org/results/pgfed/nginx-mainline/fedora-38-x86_64/06299856-nginx/builder-live.log.gz

cc: @u5surf
related? #157

@pgnd
Copy link
Author

pgnd commented Oct 9, 2023

Looks like Sep 7 commit 63ca02a is the culprit

Builds,

Oct  9, 2023  a71f9312c2deb28875acc7bacfdd5695a111aa53  FAIL

Sep  7, 2023  63ca02abdcf79c9e788d2eedcc388d2335902e52  FAIL
	log ->
		https://download.copr.fedorainfracloud.org/results/pgfed/nginx-mainline/fedora-38-x86_64/06509212-nginx/builder-live.log.gz

Sep  1, 2023  8f726ea6614eb5541812647a673abd8f66eb4660  GOOD
	log ->
		https://download.copr.fedorainfracloud.org/results/pgfed/nginx-mainline/fedora-38-x86_64/06509221-nginx/builder-live.log.gz

Apr 29, 2022  6e975bcb015f62e1f303054897783355e2a877dc  GOOD
	log ->
		https://download.copr.fedorainfracloud.org/results/pgfed/nginx-mainline/fedora-38-x86_64/06509194-nginx/builder-live.log.gz

cc: @wyattoday

@cschug
Copy link

cschug commented Oct 9, 2023

@pgnd To me it looks like you did not follow the updated build instructed which tell you that you'll have to build Brotli library before running Nginx's configure script. That's a new extra step required.

Nevertheless there is room for improvement when it comes to the build instructions for the Brotli library as the build flags are highly opinionated (like "works on my desk") and might potentially break Nginx either during build or runtime.

-flto might or might not work correctly dependent how recent the compiler suit is, -march=native -mtune=native might build fine but can cause signal 4 core dumps (illegal instruction) during runtime if executed on a different machine with a different CPU not supporting the same instructions as the build platform. The C++ flags defined per CMAKE_CXX_FLAGS aren't IMHO required at all as ther is no C++ code involved as far as I can see.

@pgnd
Copy link
Author

pgnd commented Oct 9, 2023

@cschug

you'll have to build Brotli library before running Nginx's configure script. That's a new extra step required.

can you point to that ?

this,

https://github.com/google/ngx_brotli/blob/master/README.md#dynamically-loaded

doesn't appear to mention it

@u5surf
Copy link
Contributor

u5surf commented Oct 9, 2023

@pgnd Hi, thanks reporting.

related? #157

I guess it is nothing to be related with #157.
Because nginx gzip module has been same care of #157 below.

https://github.com/nginx/nginx/blob/c37fdcdd1e1527d2c98cc68a978cf928589c7330/src/http/modules/ngx_http_gzip_filter_module.c#L284

https://github.com/nginx/nginx/blob/c37fdcdd1e1527d2c98cc68a978cf928589c7330/src/http/modules/ngx_http_gzip_static_module.c#L245

If you said it failed the build for that, nginx gzip module would had already been build failed a few month ago when the below commit came into.
nginx/nginx@e59c209

@cschug
Copy link

cschug commented Oct 10, 2023

@pgnd Indeed. Actually I cannot tell for 100 % because I always build Nginx statically, but I guess you'll need to replicate the first code block of the "Statically compiled" instructions, because the check you're failing ("Brotli library is missing from ...") is very simple logic just checking the presence of a header file.

It should be noted that in an RPM package build you'll have to mimic of the git clone --recurse-submodules. I'm not super familiar with COPR, but it looks to me you are using the source code of https://github.com/google/ngx_brotli only, but that one uses the https://github.com/google/brotli as a Git sub-module (see https://github.com/google/ngx_brotli/blob/a71f9312c2deb28875acc7bacfdd5695a111aa53/.gitmodules). Hence you'll need to download and extract those sources too, at the named patch deps/brotli, and ideally also at the revision ed737e8 (see https://github.com/google/ngx_brotli/tree/a71f9312c2deb28875acc7bacfdd5695a111aa53/deps).

@pgnd
Copy link
Author

pgnd commented Oct 10, 2023

@cschug

replicate the first code block
...
very simple logic just checking the presence of a header file
...
in an RPM package build you'll have to mimic

yup.

there was at one time (~2015) discussion about 'git submodule' support in COPR rpm builds,

https://lists.fedoraproject.org/archives/list/devel@lists.fedoraproject.org/thread/24IXWXUDHT3OFCXABNSQ6V4DYUSADA7I/

a draft PR was here

https://fedoraproject.org/wiki/User:Gbcox/PackagingDrafts/SourceURL#Git_Submodules

afaict, it was never adopted,

https://docs.fedoraproject.org/en-US/packaging-guidelines/SourceURL/

unclear as to why not ... haven't found subsequent discuss

so, instead, standalone/manual prep in the build

https://download.copr.fedorainfracloud.org/results/pgfed/nginx-mainline/fedora-38-x86_64/06513099-nginx/nginx.spec

does the trick.

build's successful, and exec's good.

thx! o/

@pgnd pgnd closed this as completed Oct 10, 2023
@pgnd pgnd changed the title nginx 1.25.2 build with latest brotli/master as dynamic module FAIL: error: Brotli library is missing from the ../ngx_brotli-master/deps/brotli/c directory nginx 1.25.2 rpm build (Fedora/COPR) + brotli/master as dynamic module FAIL: error: Brotli library is missing ... Oct 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants