Opened 11 years ago
Closed 11 years ago
#39773 closed defect (fixed)
nginx 1.4.1 build failures with +upload, +google_perftools
Reported by: | anthropologoi@… | Owned by: | neverpanic (Clemens Lang) |
---|---|---|---|
Priority: | Normal | Milestone: | |
Component: | ports | Version: | 2.1.3 |
Keywords: | haspatch | Cc: | jonasjonas (Frank Hellenkamp), cooljeanius (Eric Gallager) |
Port: | nginx |
Description
Filing a new ticket, since this 1) pertains to nginx v1.4.1 and 2) also concerns the +google_perftools variant and includes a "fix" for it. Since it might be preferable to break the 2 variants / build-failures up, I've also included Portfile diffs for each variant's fix alone, and this ticket can be switched to just the gperftools issue, and the upload_module fixes + discussion + what-have-you can be tacked on to #38968 for just that issue.
- Ok, regarding the +upload variant build failure, the attached diffs allow successful patching and building of the upload_module's lone c-file. Bear with me here: The diff was generated using [a] the most recent version of the file ( updated for this very issue ) from the "master" branch ( renamed from "2.2" ) of [b] ( username ) TimothyKlim's fork of [c] the "2.2" branch -- equivalent to what the port uses now ( i.e. NOT the "master" branch ) -- of [d] Valery Kholodkov's github repo for this module ( vkholodkov/nginx-upload-module ). [ exhale... ]
The new file used can be accessed at ( caveat: [...]/blob/[...] links have a way of disappearing over time. ):
http://github.com/TimothyKlim/nginx-upload-module/blob/b8c1ce8f4f27f51093261d35e3a0376013930c2c/ngx_http_upload_module.c
- Next, regarding the +google_perftools build failure, my
ui_warn
msg might be too verbose for others' liking, but I was aiming for something more enlightening than what's reported whenconfigure
fails to find any libprofiler.[dylib|a] in /opt/local/lib. I can rollback myportindex
-ed Portfile and re-runport -v configure nginx +google_perftools
if you'd like the full log, but here are the essential parts of theconfigure
test that fails ( from the file [$worksrcpath]/auto/lib/google-perftools/conf -- indentation altered for clarity ):
... if [ $NGX_RPATH = YES ]; then ngx_feature_libs="-R/opt/local/lib -L/opt/local/lib -lprofiler" else ngx_feature_libs="-L/opt/local/lib -lprofiler" fi ...[snip]... if [ $ngx_found = yes ]; then CORE_LIBS="$CORE_LIBS $ngx_feature_libs" else cat << END $0: error: the Google perftool module requires the Google perftools library. You can either do not enable the module or install the library. END ...
The google_perftools port does not in fact install any profiler components on my OSX 10.7.5 system, only the ( probably multiply-broken )
pprof
binary. No error is reported when installing that port -- its configure script just shrugs and skips building libprofiler when it can't find a needed header. I'll be opening a separate bug + working on fixes for the google_perftools port, and I'll report here if the issue proves to be my error.
Anyway, it's *kind of* a moot point, since my patch to nginx's +google_perftools variant-routine is written so it only errors out if whoever else's system, like mine, ended up with an "installed" google_perftools port, but no profiler lib for the module to link to.
Last bits:
- There are 2 versions of the Portfile diff for fixing the +upload variant. I used a simple
proc
for Portfile-wide scope-access, in order to pass the upload_module's $worksrcpath topre-patch
+post-patch
. Using this approach, you only need to set the module's ( actual or desired ) $distname/$version/$worksrcdir vars in *1* place, and they just get copied around where needed. Since the upload_progress_module's variant-routine is substantially the same, I also jimmied in the same logic there. If ( cal! ) you'd prefer to leave well enough alone, the Portfile diff with "sans-upload_progress-fiddling" in its name omits those changes. - Speaking of which, I'm new around here. If the scope-jumping logic, in and of itself, is not "Portfile-appropriate", let me know. I'll cut it and hard-code the appropriate $distname/$version/$worksrcdir strings wherever referenced ( and sigh, sigh, sigh ).
I think that's everything...
Attachments (5)
Change History (8)
Changed 11 years ago by anthropologoi@…
Attachment: | patch-nginx_upload_module.tmp-ngx_http_upload_module.c.diff added |
---|
Changed 11 years ago by anthropologoi@…
Attachment: | patch-nginx-Portfile.combined.diff added |
---|
Changed 11 years ago by anthropologoi@…
Attachment: | patch-nginx-Portfile.google_perftools.diff added |
---|
Changed 11 years ago by anthropologoi@…
Attachment: | patch-nginx-Portfile.upload.diff added |
---|
Changed 11 years ago by anthropologoi@…
Attachment: | patch-nginx-Portfile.upload--sans-upload_progress-fiddling.diff added |
---|
comment:1 Changed 11 years ago by anthropologoi@…
comment:2 Changed 11 years ago by larryv (Lawrence Velázquez)
Cc: | cal@… removed |
---|---|
Keywords: | haspatch added |
Owner: | changed from macports-tickets@… to cal@… |
Summary: | nginx 1.4.1 build failures with +upload, +google_perftools -- WITH proposed fixes ( ALSO RE: #38968, nginx 1.4.0 update/build fails ) → nginx 1.4.1 build failures with +upload, +google_perftools |
comment:3 Changed 11 years ago by neverpanic (Clemens Lang)
Resolution: | → fixed |
---|---|
Status: | new → closed |
- Committed the fix for the upload module in r108280 with the following change: Avoided the rev-bump, I'll update to 1.4.2 soon anyway
- The
upvar
code is fine and actually a nice solution to a problem we often see. We're using Tcl anyway, so why not make use of its features to solve problems? :) - Committed the cleanup for
+upload_progress
in r108281. - Committed the warning regarding
google_perftools
in r108283 with the following changes:- Abort with an error message instead of printing a warning and continuing without the configure flag. A user should consciously disable the variant if he knows it won't work and wants to continue installing nginx without it.
- Move the check to a
pre-configure
phase. Thegoogle_perftools
might not be installed before configure, causing the check to fail.
Thanks for the patches.
( First note to self as "I'm new around here" guy: use bold and italics now and again. )