Opened 6 years ago

Closed 6 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 when configure fails to find any libprofiler.[dylib|a] in /opt/local/lib. I can rollback my portindex-ed Portfile and re-run port -v configure nginx +google_perftools if you'd like the full log, but here are the essential parts of the configure 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 to pre-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)

patch-nginx_upload_module.tmp-ngx_http_upload_module.c.diff (50.9 KB) - added by anthropologoi@… 6 years ago.
patch-nginx-Portfile.combined.diff (5.9 KB) - added by anthropologoi@… 6 years ago.
patch-nginx-Portfile.google_perftools.diff (2.5 KB) - added by anthropologoi@… 6 years ago.
patch-nginx-Portfile.upload.diff (4.0 KB) - added by anthropologoi@… 6 years ago.
patch-nginx-Portfile.upload--sans-upload_progress-fiddling.diff (3.0 KB) - added by anthropologoi@… 6 years ago.

Download all attachments as: .zip

Change History (8)

Changed 6 years ago by anthropologoi@…

Changed 6 years ago by anthropologoi@…

Changed 6 years ago by anthropologoi@…

Changed 6 years ago by anthropologoi@…

comment:1 Changed 6 years ago by anthropologoi@…

( First note to self as "I'm new around here" guy: use bold and italics now and again. )

comment:2 Changed 6 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 6 years ago by neverpanic (Clemens Lang)

Resolution: fixed
Status: newclosed
  • 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. The google_perftools might not be installed before configure, causing the check to fail.

Thanks for the patches.

Note: See TracTickets for help on using tickets.