On 18/09/2016 8:52 AM, Yann Ylavic wrote:
On Sat, Sep 17, 2016 at 1:30 PM, NormW <no...@gknw.net> wrote:
Hi,
Having little to watch on TV, have played about with httpd-trunk and
apr-1.6, to see where the hiccups are hiding... What follows are the issues
found, and in most cases, fairly simple solutions.


Thanks Norm, applied (mostly, see below).
Thankyou for taking the time to review; I will omit any commited in the following:


Index: modules/http2/NWGNUmod_http2
===================================================================
--- modules/http2/NWGNUmod_http2        (revision 1760988)
+++ modules/http2/NWGNUmod_http2        (working copy)
@@ -369,7 +369,7 @@
         @echo $(DL) h2_proxy_res_ignore_header,$(DL) >> $@
         @echo $(DL) h2_headers_add_h1,$(DL) >> $@
         @echo $(DL) h2_req_create,$(DL) >> $@
-       @echo $(DL) h2_req_createn,$(DL) >> $@
+#      @echo $(DL) h2_req_createn,$(DL) >> $@
         @echo $(DL) h2_util_camel_case_header,$(DL) >> $@
         @echo $(DL) h2_util_frame_print,$(DL) >> $@
         @echo $(DL) h2_util_ngheader_make_req,$(DL) >> $@


These are fairly obvious but subject to what vintage compiler you prefer and
what should be done about symbols that move about... For ease of review I've
just commented out the offending symbol.

The h2_req_createn removal/move seems disputed (not sure why since
this is a "private" function, even though its shared..), so I didn't
apply this one...
Stefan, something to be done about this?
Agree a procedure needs to agreed when symbols need to move; what ever the resolution, mod_http2 can't export something no longer there.


Over in APR-1.6...

D:\Projects\svn\apr-1.6.x>svn diff
Index: build/NWGNUmakefile
===================================================================
--- build/NWGNUmakefile (revision 1760898)
+++ build/NWGNUmakefile (working copy)
@@ -22,7 +22,7 @@
         $(APU)/include/apr_ldap.h \
         $(APU)/include/private/apu_config.h \
         $(APU)/include/private/apu_select_dbm.h \
-       $(APUXML)/expat/lib/expat_config.h \
+       $(APUXML)/expat_config.h \
         $(APR)/include/private/apr_escape_test_char.h \
         $(EOLIST)

First commited in http://svn.apache.org/r1761283, but finally reverted
(r1761292), trunk's xml/NWGNUmakefile seems to reference
$(EXPATSRC)/lib for expat_config.h.
So I'm not sure here, do we need a change in trunk too?
I presume here you refer to apr-trunk; never tried but if it builds aprxml.lib then it seems a better alternative to apr-util-1.6 as it creates the expat_config.h from the makefile rather than use an external .hnw. which would benefit apr-util-1.6. The .\xml structure in apr-util-1.5 is different to trunk and 1.6 so a common make file for all three trees is likely not possible.
Index: locks/netware/thread_cond.c
===================================================================
--- locks/netware/thread_cond.c (revision 1760898)
+++ locks/netware/thread_cond.c (working copy)
@@ -73,7 +73,7 @@
          rc = NXCondWait(cond->cond, mutex->mutex);
      }
      else {
-        timeout = timeout * 1000 / XGetSystemTick();
+        timeout = timeout * 1000 / NXGetSystemTick();
          rc = NXCondTimedWait(cond->cond, mutex->mutex, timeout);
          if (rc == NX_ETIMEDOUT) {
              return APR_TIMEUP;

Applied in http://svn.apache.org/r1761279 (trunk had the same issue),
and backported to 1.6.x (r1761280).
It also belongs apr-1.5 AFAICT. The '/N seems to confuse the compiler and drops the 'N', giving a missing symbol XGetSystemTick().


Mostly these are typo's in a non-released package so don't count as a
critique; the major issue comes with .\include\apr_proc_mutex.h:

Hard to propagate changes to every supported platforms w/o being able
to (at least) compile check on all of them, sorry about the
locks/netware ones (my bad)...
Did/Can you try "make check" (or alike) on Netware regarding these APR changes?
Not an option that I'm aware of; all I can do at this point is try to build and hope it goes to completion without complaint.


APR_DECLARE(apr_status_t) apr_proc_mutex_timedlock(apr_proc_mutex_t
*mutex,
                                                    apr_time_t timeout,
                                                    int absolute);

The NetWare proc_mutex.c requires it defined as:

APR_DECLARE(apr_status_t) apr_proc_mutex_timedlock(apr_thread_mutex_t
*mutex,
                                                    apr_time_t timeout,
                                                    int absolute)

I've just temporarily renamed it to see if it would compile and it did; how
to solve this I leave to experts.

That's the good fix I think, bad copy/paste on my side.
OK



APR-UTIL-1.6 shows:

D:\Projects\svn\apr-util-1.6.x>svn diff
Index: include/apu.hnw
===================================================================
--- include/apu.hnw     (revision 1760898)
+++ include/apu.hnw     (working copy)
@@ -108,7 +108,7 @@
  #define APU_HAVE_ODBC           0
  #endif

-#define APU_HAVE_CRYPTO         0
+#define APU_HAVE_CRYPTO         1

  #ifndef APU_DSO_MODULE_BUILD
  #define APU_HAVE_OPENSSL        0

If it works on Netware I guess we can enable it (not done yet).
Does "make check" work here too (and/or tests framework for mod_crypto
in httpd, if any)?
This was a new addition to httpd-trunk build system back in July this year; unsure why it was never enabled before - it was clearly disabled in apu.hnw up to that time... anyone care to comment? In either case, mod_crypto will only build with apr-util 1.6, so httpd-trunk needs to know if apr-util 1.6 is being used else skip it entirely.


Index: xml/NWGNUmakefile
===================================================================
--- xml/NWGNUmakefile   (revision 1760898)
+++ xml/NWGNUmakefile   (working copy)
@@ -30,7 +30,7 @@
                         $(APU)/uri \
                         $(APU)/dbm/sdbm \
                         $(APU)/include/private \
-                       $(APUXML)/expat/lib \
+                       $(EXPATSRC)/lib \
                         $(EOLIST)

  #
@@ -248,7 +248,7 @@
  # Any specialized rules here
  #

-vpath %.c expat/lib
+vpath %.c $(EXPATSRC)/lib

Same as above (related at least), I'd like to clarify trunk vs
apu-1.6.x/apu-1.5.x w.r.t. expat before applying this (though this
particular change looks good).
Maybe someone else could help with expat's build?
I will see what standardisation is achievable with .\xml using both expat 2.0.1 and the newer 2.2 release.


Anyway, thanks again for the patches Norm.

Regards,
Yann.

Norm

Reply via email to