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 Resolver API changes break the ngx_pagespeed build #578

Closed
jprostko opened this Issue Dec 17, 2013 · 7 comments

Comments

Projects
None yet
4 participants
@jprostko

jprostko commented Dec 17, 2013

Changes present in Nginx 1.5.8 break the build of the Pagespeed module (v1.7.30.1-beta).

Below is the build error I encountered while building this morning:

modules/nginx-pagespeed-module/src/ngx_fetch.cc
modules/nginx-pagespeed-module/src/ngx_fetch.cc: In member function bool net_instaweb::NgxFetch::Init():
modules/nginx-pagespeed-module/src/ngx_fetch.cc:167: error: struct ngx_resolver_ctx_s has no member named type
modules/nginx-pagespeed-module/src/ngx_fetch.cc: In static member function static void net_instaweb::NgxFetch::NgxFetchResolveDone(ngx_resolver_ctx_t_):
modules/nginx-pagespeed-module/src/ngx_fetch.cc:304: error: cannot convert ngx_addr_t to in_addr_t in assignment
make[1]: *_* [objs/addon/src/ngx_fetch.o] Error 1
make[1]: Leaving directory `/home/jprostko/nginx-1.5.8'
make: *** [build] Error 2

I discovered this forum post about the API changes, and it helps shed some light on the matter:

http://forum.nginx.org/read.php?2,245532

This is likely the commit that is causing the issue, I believe, although other ones made around the same time could be related, I suppose:

http://mailman.nginx.org/pipermail/nginx-devel/2013-December/004682.html

@jeffkaufman

This comment has been minimized.

Show comment
Hide comment
@jeffkaufman

jeffkaufman Dec 17, 2013

Contributor

@yaoweibin @dinic This is with the native fetcher.

Contributor

jeffkaufman commented Dec 17, 2013

@yaoweibin @dinic This is with the native fetcher.

@yaoweibin

This comment has been minimized.

Show comment
Hide comment
@yaoweibin

yaoweibin Dec 17, 2013

Contributor

OK. We will track this issue.

Contributor

yaoweibin commented Dec 17, 2013

OK. We will track this issue.

@jeffkaufman

This comment has been minimized.

Show comment
Hide comment
@jeffkaufman

jeffkaufman Dec 17, 2013

Contributor

Thanks!

Contributor

jeffkaufman commented Dec 17, 2013

Thanks!

@jprostko

This comment has been minimized.

Show comment
Hide comment
@jprostko

jprostko Dec 17, 2013

Indeed, thank you for looking into this! None of the other Nginx modules I use are affected, but I can imagine there's plenty more that will get bitten by this API change.

jprostko commented Dec 17, 2013

Indeed, thank you for looking into this! None of the other Nginx modules I use are affected, but I can imagine there's plenty more that will get bitten by this API change.

@jeffkaufman

This comment has been minimized.

Show comment
Hide comment
@jeffkaufman

jeffkaufman Dec 19, 2013

Contributor

Building from master should now fix this, and it will be fixed for everyone in our next release.

Contributor

jeffkaufman commented Dec 19, 2013

Building from master should now fix this, and it will be fixed for everyone in our next release.

@pixelchutes

This comment has been minimized.

Show comment
Hide comment
@pixelchutes

pixelchutes Dec 20, 2013

Thanks for posting this, wasn't sure what was up with my build before seeing this. Appreciate it! 🍻

pixelchutes commented Dec 20, 2013

Thanks for posting this, wasn't sure what was up with my build before seeing this. Appreciate it! 🍻

jeffkaufman added a commit that referenced this issue Jan 7, 2014

native-fetcher: fix to work with nginx 1.5.8+
nginx 1.5.8 changed the resolver api, which the native fetcher uses.

Fixes #578.

Squash-merge of @dinic's #581.
@jeffkaufman

This comment has been minimized.

Show comment
Hide comment
@jeffkaufman

jeffkaufman Jan 8, 2014

Contributor

This is now released with 1.7.30.2

Contributor

jeffkaufman commented Jan 8, 2014

This is now released with 1.7.30.2

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