Skip to contentSkip to conversation

apr-util/apache fixes for per-package folders

PATCH New Thomas Petazzoni 2 participants 5 messages Nov 20, 2018 – Nov 21, 2018

lore
Share & tools
lore
#0 lore raw

v2 cover apr-util/apache fixes for per-package folders

Hello,

This is a v2 of the apr-util/apache fixes for per-package folders. There is a changelog in each of the two patches.

Thanks,

Thomas

Thomas Petazzoni (2):
package/apr-util: fix build with per-package folders package/apache: add patch to fix per-package folder build

 ...in-handle-separate-APR_INCLUDE_DIR-A.patch | 48 +++++++++++++++++++
 package/apr-util/apr-util.mk                  |  7 +++
 2 files changed, 55 insertions(+)
 create mode 100644 package/apache/0003-server-Makefile.in-handle-separate-APR_INCLUDE_DIR-A.patch
Signature · 1 line
2.19.1
#1 lore raw

PATCH v2 1/2 package/apr-util: fix build with per-package folders

With per-package folder support enable, the build of apr-util fails, for two reasons:

- The rules.mk file is generated by the 'apr' package, and then copied into the 'apr-util' source directory. This is done by the 'apr-util' build process. Unfortunately, this rules.mk file has a number of hardcoded paths: to the compiler and to the libtool script.

Due to this, the compiler from the 'apr' per-package folder gets used. But this compiler uses the 'apr' package sysroot, which does not have all the dependencies of the 'apr-util' package, causing the build to fail because <expat.h> is not found.

- Similarly, the libtool script itself has some hardcoded paths, which make it use the compiler/linker from the 'apr' per-package folder, so it does not find the expat library.

We fix both issues by doing the necessary replacement in both rules.mk and libtool.

---
Changes since v1:
- Drop the useless logic to copy the libtool script before doing the
replacement: as noted by Yann E. Morin, sed is properly breaking hardlinks when doing an in-place replacement.

---
 package/apr-util/apr-util.mk | 7 +++++++
 1 file changed, 7 insertions(+)
package/apr-util/apr-util.mk +7 -0
index b006964ccb..379dfa692e 100644
@@ -15,6 +15,13 @@ APR_UTIL_CONF_OPTS = \
 	--with-apr=$(STAGING_DIR)/usr/bin/apr-1-config
 APR_UTIL_CONFIG_SCRIPTS = apu-1-config
 
+define APR_UTIL_FIX_RULES_MK_LIBTOOL
+	$(SED) 's,$(PER_PACKAGE_DIR)/apr/,$(PER_PACKAGE_DIR)/apr-util/,g' $(@D)/build/rules.mk
+	$(SED) 's,$(PER_PACKAGE_DIR)/apr/,$(PER_PACKAGE_DIR)/apr-util/,g' \
+		$(STAGING_DIR)/usr/build-1/libtool
+endef
+APR_UTIL_POST_CONFIGURE_HOOKS += APR_UTIL_FIX_RULES_MK_LIBTOOL
+
 # When iconv is available, then use it to provide charset conversion
 # features.
 APR_UTIL_DEPENDENCIES += $(if $(BR2_PACKAGE_LIBICONV),libiconv)
Signature · 1 line
2.19.1
#2 lore raw

PATCH v2 1/2 package/apr-util: fix build with per-package folders

Thomas, All,

On 2018-11-20 17:38 +0100, Thomas Petazzoni spake thusly:

Quoted 36 lines

With per-package folder support enable, the build of apr-util fails, for two reasons:

- The rules.mk file is generated by the 'apr' package, and then copied into the 'apr-util' source directory. This is done by the 'apr-util' build process. Unfortunately, this rules.mk file has a number of hardcoded paths: to the compiler and to the libtool script.

Due to this, the compiler from the 'apr' per-package folder gets used. But this compiler uses the 'apr' package sysroot, which does not have all the dependencies of the 'apr-util' package, causing the build to fail because <expat.h> is not found.

- Similarly, the libtool script itself has some hardcoded paths, which make it use the compiler/linker from the 'apr' per-package folder, so it does not find the expat library.

We fix both issues by doing the necessary replacement in both rules.mk and libtool.

---
Changes since v1:
- Drop the useless logic to copy the libtool script before doing the
replacement: as noted by Yann E. Morin, sed is properly breaking hardlinks when doing an in-place replacement.

---
 package/apr-util/apr-util.mk | 7 +++++++
 1 file changed, 7 insertions(+)
package/apr-util/apr-util.mk +0 -0
index b006964ccb..379dfa692e 100644

package/apr-util/apr-util.mk APR_UTIL_CONF_OPTS = \ · line 20

Quoted 6 lines
package/apr-util/apr-util.mk +3 -0
@@ -15,6 +15,13 @@ APR_UTIL_CONF_OPTS = \
 	--with-apr=$(STAGING_DIR)/usr/bin/apr-1-config
 APR_UTIL_CONFIG_SCRIPTS = apu-1-config
 
+define APR_UTIL_FIX_RULES_MK_LIBTOOL
+	$(SED) 's,$(PER_PACKAGE_DIR)/apr/,$(PER_PACKAGE_DIR)/apr-util/,g' $(@D)/build/rules.mk
+	$(SED) 's,$(PER_PACKAGE_DIR)/apr/,$(PER_PACKAGE_DIR)/apr-util/,g' \

I know we do not have a strong policy on this, but can you use '@' as the separator, instead of the comma ',' as we are sure that the build directory will not contain any '@':

    https://git.buildroot.org/buildroot/commit/Makefile?id=7007dc2bc99ad191c418c468707cdc3980273cda

package/apr-util/apr-util.mk APR_UTIL_CONF_OPTS = \ · line 21

Quoted 1 line
package/apr-util/apr-util.mk +1 -0
+		$(STAGING_DIR)/usr/build-1/libtool

So, you're patching a file in STAGING_DIR in a post-configure hook?

Also, PER_PACKAGE_DIR does yet exist in master (or next, for that matters), so this patch will have to leave in your branch for a little while still, I'm afraid.. :-/

Quoted 9 lines
package/apr-util/apr-util.mk +3 -0
+endef
+APR_UTIL_POST_CONFIGURE_HOOKS += APR_UTIL_FIX_RULES_MK_LIBTOOL
+
 # When iconv is available, then use it to provide charset conversion
 # features.
 APR_UTIL_DEPENDENCIES += $(if $(BR2_PACKAGE_LIBICONV),libiconv)
Signature · 1 line
2.19.1
Signature · 6 lines
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
#3 lore raw

PATCH v2 1/2 package/apr-util: fix build with per-package folders

Hello,

Quoted 8 lines · Yann E. MORIN

package/apr-util/apr-util.mk APR_UTIL_CONF_OPTS = \ · line 20

Quoted 3 lines
package/apr-util/apr-util.mk +3 -0
+define APR_UTIL_FIX_RULES_MK_LIBTOOL
+	$(SED) 's,$(PER_PACKAGE_DIR)/apr/,$(PER_PACKAGE_DIR)/apr-util/,g' $(@D)/build/rules.mk
+	$(SED) 's,$(PER_PACKAGE_DIR)/apr/,$(PER_PACKAGE_DIR)/apr-util/,g' \  

I know we do not have a strong policy on this, but can you use '@' as the separator, instead of the comma ',' as we are sure that the build directory will not contain any '@':

    https://git.buildroot.org/buildroot/commit/Makefile?id=7007dc2bc99ad191c418c468707cdc3980273cda

ACK.

Quoted 3 lines

package/apr-util/apr-util.mk APR_UTIL_CONF_OPTS = \ · line 21

Quoted 1 line
package/apr-util/apr-util.mk +1 -0
+		$(STAGING_DIR)/usr/build-1/libtool  

So, you're patching a file in STAGING_DIR in a post-configure hook?

Yes. With per-package folder, we are preparing the target and host (including staging) folders with the package dependencies during the configure step. It is what we copy that contains references to other per-package folders that should be fixed up before the build. Hence, this needs to be done during the configure step.

Quoted 3 lines

Also, PER_PACKAGE_DIR does yet exist in master (or next, for that matters), so this patch will have to leave in your branch for a little while still, I'm afraid.. :-/

Perhaps I should include this logic inside a BR2_PER_PACKAGE_FOLDERS condition.

Best regards,

Thomas

Signature · 3 lines
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
#4 lore raw

PATCH v2 2/2 package/apache: add patch to fix per-package folder build

In reply to Thomas Petazzoni

When APR_INCLUDEDIR and APU_INCLUDEDIR point to the same directory, Apache builds properly. However, with per-package folder support, they point to different directories, and APU_INCLUDEDIR contains both the APR headers and the APU headers.

Due to this, the Apache Makefile logic to generate its exports.c file leads to duplicate definitions, because the APR headers are considered
twice: once from APR_INCLUDEDIR, once from APU_INCLUDEDIR.

We fix this by introducing a patch to the Apache build system.

---
Changes since v1:
- As suggested by Yann E. morin, the patch has been submitted upstream, upstream information have been added to the patch itself.
- Reviewed-by from Yann E. Morin added.

---
 ...in-handle-separate-APR_INCLUDE_DIR-A.patch | 48 +++++++++++++++++++
 1 file changed, 48 insertions(+)
 create mode 100644 package/apache/0003-server-Makefile.in-handle-separate-APR_INCLUDE_DIR-A.patch
package/apache/0003-server-Makefile.in-handle-separate-APR_INCLUDE_DIR-A.patch new file +48 -0
new file mode 100644
index 0000000000..b4ad128677
@@ -0,0 +1,48 @@
+From 00281390e82db18fe0de4033be4045f9391a8ee5 Mon Sep 17 00:00:00 2001
+From: Thomas Petazzoni <[email protected]>
+Date: Thu, 15 Nov 2018 14:04:29 +0100
+Subject: [PATCH] server/Makefile.in: handle separate
+ APR_INCLUDE_DIR/APU_INCLUDE_DIR
+
+If APR_INCLUDEDIR and APU_INCLUDEDIR point to different directories,
+but for example APU_INCLUDEDIR contains both the apr headers and apu
+headers, the "export_files" file will contain duplicate header files,
+causing the exports.c file to contain duplicate definitions, making
+the build fail.
+
+This commit fixes that by making sure we only use the apr headers from
+APR_INCLUDEDIR and the apu headers from the APU_INCLUDEDIR.
+
+Signed-off-by: Thomas Petazzoni <[email protected]>
+Upstream: https://bz.apache.org/bugzilla/show_bug.cgi?id=62930
+---
+ server/Makefile.in | 6 ++----
+ 1 file changed, 2 insertions(+), 4 deletions(-)
+
+diff --git a/server/Makefile.in b/server/Makefile.in
+index 1fa334467d..2258f0bdf2 100644
+--- a/server/Makefile.in
++++ b/server/Makefile.in
+@@ -34,7 +34,6 @@ test_char.h: gen_test_char
+ util.lo: test_char.h
+ 
+ EXPORT_DIRS = $(top_srcdir)/include $(top_srcdir)/os/$(OS_DIR)
+-EXPORT_DIRS_APR = $(APR_INCLUDEDIR) $(APU_INCLUDEDIR)
+ 
+ # If export_files is a dependency here, but we remove it during this stage,
+ # when exports.c is generated, make will not detect that export_files is no
+@@ -60,9 +59,8 @@ export_files:
+ 	      ls $$dir/*.h ; \
+ 	  done; \
+ 	  echo "$(top_srcdir)/server/mpm_fdqueue.h"; \
+-	  for dir in $(EXPORT_DIRS_APR); do \
+-	      ls $$dir/ap[ru].h $$dir/ap[ru]_*.h 2>/dev/null; \
+-	  done; \
++	  ls $(APR_INCLUDE_DIR)/{apr.h,apr_*.h} 2>/dev/null; \
++	  ls $(APU_INCLUDE_DIR)/{apu.h,apu_*.h} 2>/dev/null; \
+ 	) | sed -e s,//,/,g | sort -u > $@
+ 
+ exports.c: export_files
+-- 
+2.19.1
+
Signature · 1 line
2.19.1