Skip to content

Conversation

@gmbeard
Copy link
Contributor

@gmbeard gmbeard commented Mar 24, 2023

Testing the changes

  • I tested the changes in this PR: YES

Local build testing

  • I built this PR locally for my native architecture, x86_64-glibc
  • I built this PR locally for these architectures:
    • aarch64

This also addresses #42441

  • apr

Tests to fix

  • testatomic: consistency failure on atomic (i686)
  • testsockets: Socket bind failure (x86_64, i686, x86_64-musl)
  • testdso failure (x86_64-musl)

@gmbeard gmbeard marked this pull request as draft March 24, 2023 07:04
@gmbeard gmbeard force-pushed the apr/update-to-1.7.2 branch 7 times, most recently from 3711619 to f7ee083 Compare March 25, 2023 12:00
@gmbeard gmbeard marked this pull request as ready for review March 25, 2023 12:08
@classabbyamp classabbyamp added the needs-testing Testing a PR or reproducing an issue needed label Mar 28, 2023
@gmbeard
Copy link
Contributor Author

gmbeard commented Mar 28, 2023

I've managed to somewhat test this with apache in a x86_64-glibc chroot. I manually bumped apr-util to 1.6.3, and apache to 2.4.56 (looks like the existing apache template in the source tree has a dead distfile BTW. Maybe one for #42403).

Maybe not the most thorough of tests, but I can successfully start httpd with a default httpd.conf and get a dir listing page from http://127.0.0.1:8080

Output from apachectl -V
Server version: Apache/2.4.56 (Unix)
Server built:   Mar 25 2023 11:59:59
Server's Module Magic Number: 20120211:126
Server loaded:  APR 1.7.2, APR-UTIL 1.6.3, PCRE 10.39 2021-10-29
Compiled using: APR 1.7.2, APR-UTIL 1.6.3, PCRE 10.39 2021-10-29
Architecture:   64-bit
Server MPM:     event
  threaded:     yes (fixed thread count)
    forked:     yes (variable process count)
Server compiled with....
 -D APR_HAS_SENDFILE
 -D APR_HAS_MMAP
 -D APR_HAVE_IPV6 (IPv4-mapped addresses enabled)
 -D APR_USE_PROC_PTHREAD_SERIALIZE
 -D APR_USE_PTHREAD_SERIALIZE
 -D SINGLE_LISTEN_UNSERIALIZED_ACCEPT
 -D APR_HAS_OTHER_CHILD
 -D AP_HAVE_RELIABLE_PIPED_LOGS
 -D DYNAMIC_MODULE_LIMIT=256
 -D HTTPD_ROOT=""
 -D SUEXEC_BIN="/bin/suexec"
 -D DEFAULT_PIDLOG="/var/run/httpd/httpd.pid"
 -D DEFAULT_SCOREBOARD="logs/apache_runtime_status"
 -D DEFAULT_ERRORLOG="logs/error_log"
 -D AP_TYPES_CONFIG_FILE="/etc/apache/mime.types"
 -D SERVER_CONFIG_FILE="/etc/apache/httpd.conf"

@gmbeard
Copy link
Contributor Author

gmbeard commented Apr 5, 2023

@classabbyamp is there anything else I can do to help get this merged?

@classabbyamp classabbyamp removed the needs-testing Testing a PR or reproducing an issue needed label Apr 5, 2023
post_extract() {
if [ "${XBPS_TARGET_LIBC}" = "musl" ]; then
patch -d "${wrksrc}" -Np1 <"${FILESDIR}/musl-dso.patch"
fi
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does this patch also work on glibc? If yes, we should put it to patches/ and not apply it manually here

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this patch isn't really correct, the reason the test fails is because dlclose is a no-op and doesn't actually unload anything: https://wiki.musl-libc.org/functional-differences-from-glibc.html#Unloading-libraries

the correct thing to do is skip the test on musl

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nekopsykose That's very useful info about musl's dlclose. I couldn't wrap my head around why it didn't appear to unload the lib. I've now patched out the tests instead. Thank you.

Copy link
Contributor Author

@gmbeard gmbeard Apr 21, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@paper42 The (now updated) patch is only applicable to musl, but I guess it would work on glibc too. Would it be preferable to apply the patch unconditionally?

@gmbeard gmbeard force-pushed the apr/update-to-1.7.2 branch from f7ee083 to 78632db Compare April 21, 2023 06:21
@gmbeard gmbeard changed the title apr: update to 1.7.2. apr: update to 1.7.4 Apr 21, 2023
@gmbeard
Copy link
Contributor Author

gmbeard commented Apr 21, 2023

Updated the version to 1.7.4. There was a further update since opening the PR, which made some of the patches unnecessary.

@github-actions
Copy link

Pull Requests become stale 90 days after last activity and are closed 14 days after that. If this pull request is still relevant bump it or assign it.

@github-actions github-actions bot added the Stale label Jul 21, 2023
@github-actions github-actions bot closed this Aug 5, 2023
@gmbeard gmbeard deleted the apr/update-to-1.7.2 branch November 28, 2025 14:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants