Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

apr-util: update to 1.6.3 #42636

Merged
merged 1 commit into from
Aug 22, 2023
Merged

Conversation

klarasm
Copy link
Contributor

@klarasm klarasm commented Mar 6, 2023

Failed to download distfiles when revbumping in #41948

  • remove upstreamed patch
  • switch to vsed
  • cross-compilation fix

Testing the changes

  • I tested the changes in this PR: I don't use this package directly

Local build testing

  • I built this PR locally for my native architecture, (x86_64-glibc)
  • I built this PR locally for my native architecture, (x86_64-musl)

@klarasm
Copy link
Contributor Author

klarasm commented Mar 6, 2023

The test suite seems to fail randomly even on glibc (cannot reproduce that on my local machine though) so may have to disable it completely again.

@klarasm
Copy link
Contributor Author

klarasm commented Mar 7, 2023

I feel like I'm in over my head on this one.

/usr/bin/ld: dbm/.libs/apr_dbm_gdbm.o: Relocations in generic ELF (EM: 183)
/usr/bin/ld: dbm/.libs/apr_dbm_gdbm.o: error adding symbols: file in wrong format

Is it really correct that the host linker is used rather than the one from the crosscompiler?

@klarasm klarasm force-pushed the apr-util-update branch 2 times, most recently from 0676b4b to a638a45 Compare March 7, 2023 01:07
@klarasm
Copy link
Contributor Author

klarasm commented Mar 9, 2023

configure script complains of:

configure: WARNING: unrecognized options: --with-sysroot, --with-libtool-sysroot

which may be related.

The options are set as:

"--with-sysroot=/usr/aarch64-linux-gnu" \
"--with-libtool-sysroot=/usr/aarch64-linux-gnu" \

@klarasm
Copy link
Contributor Author

klarasm commented Mar 9, 2023

I think I've found the issue. If I revert dd789b9 from #40606 it crosscompiles fine.

@paper42
Copy link
Member

paper42 commented Mar 9, 2023

I think I've found the issue. If I revert dd789b9 from #40606 it crosscompiles fine.

cc @oreo639

@klarasm
Copy link
Contributor Author

klarasm commented Mar 9, 2023

If I look into the libtool script without the commit i get:

# The linker used to build libraries.
LD=${LD:=ld}

which allows the linker to be overridden by the environment. With the commit i get:

# The linker used to build libraries.
LD="ld"

which sets the linker unconditionally.

So the problematic part of the commit is probably:

-               # e.g. AR="armv7l-linux-gnueabihf-ar" becomes AR="${AR:=ar}"
-               vsed -i -e "s,\([A-Z]\+\)=\"${XBPS_CROSS_TRIPLET}\-\(.*\)\",\1=\$\{\1:=\2\},g" \
+               # e.g. AR="armv7l-linux-gnueabihf-ar" becomes AR="ar"
+               vsed -i -e "s,\([A-Z]\+\)=\"${XBPS_CROSS_TRIPLET}\-\(.*\)\",\1=\"\2\",g" \

I'll revert that part only and see if it still builds ok.

@klarasm klarasm force-pushed the apr-util-update branch 3 times, most recently from 5e22e58 to ae53fdf Compare March 9, 2023 19:37
@oreo639
Copy link
Member

oreo639 commented Mar 9, 2023

The arm libtool package should be consistent with the x86_64 libtool package, I'm not sure why this change should be made for one but not the other (especially if that patch breaks building C++ applications like ntl).

If you need LD=${LD:=ld}, maybe you can sed libtool to XBPS_WRAPPERDIR and use that?

@klarasm
Copy link
Contributor Author

klarasm commented Mar 9, 2023

If you need LD=${LD:=ld}, maybe you can sed libtool to XBPS_WRAPPERDIR and use that?

That worked, thanks!

This should be mostly ready now. Though I don't use this package directly so additional testing would be appreciated.

@klarasm klarasm marked this pull request as ready for review March 9, 2023 22:06
@klarasm
Copy link
Contributor Author

klarasm commented May 15, 2023

Use archive.apache.org instead of downloads.apache.org as the former does not delete outdated versions.
I've also verified that the cross-compilation fix also works on 1.6.1, and if it's preferred for that to be fixed separately I can create a new PR.

@dkwo
Copy link
Contributor

dkwo commented Aug 4, 2023

I'm seeing the same configure: WARNING: unrecognized options: --with-sysroot, --with-libtool-sysroot when cross compiling coreutils in #45305 Can you remind me how to deal with it?

@oreo639
Copy link
Member

oreo639 commented Aug 4, 2023

That just means that those options aren't being used.
For example, --with-sysroot is used by gmp because the aclocal.m4 script was generated on a distro where libtool uses the --with-sysroot variable, whereas if it was generated on Void it would use the --with-libtool-sysroot variable since we patched libtool.

Both apr-util and coreutils the package doesn't perform LT_INIT and don't check --with-sysroot manually so neither of the variables are used.

In other words, you shouldn't need to do anything.

@klarasm
Copy link
Contributor Author

klarasm commented Aug 4, 2023

You could try autoreconf -fi to replace it with Void's version but I don't think it's what causes the test failure (the crossbuilds seem to complete, which would be the most likely to fail as a result of the flags not being used correctly. I'm no expert at autoconf/autotools, though).

It's probably more likely the failure is a result of the CI restricting the inotify feature if it doesn't happen to you locally.

@oreo639
Copy link
Member

oreo639 commented Aug 4, 2023

It is not a failure, it is a warning (one that can be safely ignored in this case).

@klarasm
Copy link
Contributor Author

klarasm commented Aug 4, 2023

It is not a failure, it is a warning (one that can be safely ignored in this case).

I also referred to the test failure in my first sentence, sorry if I was unclear.

@oreo639
Copy link
Member

oreo639 commented Aug 4, 2023

Ah, my bad.

@dkwo
Copy link
Contributor

dkwo commented Aug 4, 2023

Thank you, then I'll ignore the warning.
Right, the test failure is only on CI (locally it passes), likely unrelated.

@oreo639
Copy link
Member

oreo639 commented Aug 8, 2023

Also, @klarasm here is a better solution without the libtool wrapdir thing:
master...oreo639:void-packages:apr-util

@klarasm
Copy link
Contributor Author

klarasm commented Aug 8, 2023

I may have overlooked some vseds when applying...

@oreo639
Copy link
Member

oreo639 commented Aug 8, 2023

The vsed should be if [ "$CROSS_BUILD" ]; then, sorry.

@klarasm
Copy link
Contributor Author

klarasm commented Aug 8, 2023

The vsed should be if [ "$CROSS_BUILD" ]; then, sorry.

No worries. The 32bit arm crossbuilds seems to fail unrelated to this, though. There seems to be a -mtune=generic inserted in the CFLAGS somehow which I guess does not exist on 32 bit arm?

@oreo639
Copy link
Member

oreo639 commented Aug 8, 2023

You can fix that by using -e "/^CFLAGS=/s:=.*:=${CFLAGS}:" in the vsed.

@klarasm
Copy link
Contributor Author

klarasm commented Aug 8, 2023

You can fix that by using -e "/^CFLAGS=/s:=.*:=${CFLAGS}:" in the vsed.

That did it, thanks!

initially failed to download distfiles when revbumping, though that has
been fixed separately.

- remove upstreamed patch
- switch to vsed
- cross-compilation fix (thanks @oreo639)
@Emru1
Copy link
Contributor

Emru1 commented Aug 19, 2023

Is anything preventing merge in this PR?

@oreo639
Copy link
Member

oreo639 commented Aug 20, 2023

That is up to the maintainers. You can try asking for a review on the IRC (politely ofc)

@classabbyamp classabbyamp merged commit 4681278 into void-linux:master Aug 22, 2023
8 checks passed
@klarasm klarasm deleted the apr-util-update branch August 22, 2023 21:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants