Skip to content

Add ppc64le cross-compilation support#9

Merged
JonathanLennox merged 6 commits intojitsi:mainfrom
fitzsim:ppc64le-support-2
Feb 18, 2025
Merged

Add ppc64le cross-compilation support#9
JonathanLennox merged 6 commits intojitsi:mainfrom
fitzsim:ppc64le-support-2

Conversation

@fitzsim
Copy link
Contributor

@fitzsim fitzsim commented Feb 11, 2025

I tested:

export JAVA_HOME=/usr/lib/jvm/java-17-openjdk-amd64
resources/ubuntu-build-all.sh ~/depot_tools ~/WebRTC

on x86-64 Debian 12. x86-64 and aarch64 libraries built as before, and the ppc64le cross-compilation succeeded. I tested the new library:

dcsctp4j/src/main/resources/linux-ppc64le/./libdcsctp4j.so

on my Debian 12 ppc64le Jitsi installation and it worked when I initiated a two-party meeting:

JVB 2025-02-10 22:02:53.029 INFO: [50] DcSctp4j.<clinit>#38: DcSctp4j lib loaded

I made ninja verbose so that its compiler arguments are available in continuous integration logs for comparison with those specified by the new Makefile.

The other two ubuntu-build.sh changes reorder some lines in preparation for the addition of the ppc64le logic.

echo " DEPOT_TOOLS_DIR: Directory containing Google depot tools"
echo " WEBRTC_DIR: Directory containing WebRTC source"
echo " ARCH: Architecture to build for (x86_64 or arm64)"
echo " ARCH: Architecture to build for (x86_64 or arm64 or ppc64le)"
Copy link
Member

Choose a reason for hiding this comment

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

Should be a comma-separated list: (x86_64, arm64, or ppc64le). Your choice whether to use the Oxford comma.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@@ -0,0 +1,55 @@
# Build libdcsctp.a on non-"gn" supported architectures
Copy link
Member

Choose a reason for hiding this comment

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

You seem to have also checked in a Makefile.2 -- was this a mistake?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, removed.

CXXFLAGS += -I $(S)/third_party/crc32c/src/include
# Assume all non-test .cc files under net/dcsctp are needed by libdcsctp4j.so.
# "gn/ninja" includes 43 extra objects but they are not needed by
# libdcsctp4j.so.
Copy link
Member

Choose a reason for hiding this comment

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

I'd like to verify that that dcsctp4j still links at -O0 (which may be needed for debugging) with just this list of files? It's possible that some dependencies from those 43 extra objects get optimized out.

Copy link
Contributor Author

@fitzsim fitzsim Feb 14, 2025

Choose a reason for hiding this comment

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

I found a few missing symbols with:

nm --demangle -D dcsctp4j/src/main/resources/linux-ppc64le/libdcsctp4j.so | grep ' U ' | grep -v @

I added six extra objects required to define them.

I added a debug argument to ubuntu-build.sh to test this. libdcsctp4j.so built with -O0 works on my ppc64le Jitsi setup.

# Assume all non-test .cc files under net/dcsctp are needed by libdcsctp4j.so.
# "gn/ninja" includes 43 extra objects but they are not needed by
# libdcsctp4j.so.
TESTS = $(wildcard $(S)/net/dcsctp/*/*_test.cc $(S)/net/dcsctp/*/*/*_test.cc)
Copy link
Member

Choose a reason for hiding this comment

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

There's also common/handover_testing.cc which doesn't match this pattern. I think if you make the pattern *_test*.cc you'll match that file as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

rm -rf $WEBRTC_BUILD
gn gen $WEBRTC_BUILD --args="use_custom_libcxx=false target_cpu=\"$GN_ARCH\" is_debug=false symbol_level=2"
ninja -C $WEBRTC_BUILD dcsctp
if test -z "$GNU_ARCH"; then
Copy link
Member

Choose a reason for hiding this comment

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

I'm not crazy about having this choice be implicit based on the presence or absence of GNU_ARCH; can you have something like an explicit BUILD_DCSCTP_WITH_MAKEFILE argument instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done, as an argument to ubuntu-build.sh.

@fitzsim
Copy link
Contributor Author

fitzsim commented Feb 14, 2025

Thank you for reviewing. I addressed all your comments, and added optional verbose and debug support as extra commits. To keep the history clean but keep discussion on the same pull request I force-pushed the branch, so you will have to check it out again.

@JonathanLennox JonathanLennox merged commit 3f09928 into jitsi:main Feb 18, 2025
JonathanLennox added a commit that referenced this pull request Feb 18, 2025
JonathanLennox added a commit that referenced this pull request Feb 18, 2025
@JonathanLennox
Copy link
Member

JonathanLennox commented Feb 19, 2025

Unfortunately this PR didn't work after I merged it, so I've reverted it.

There were a few problems:

  1. The command line for making the objdirs resulted in
make: execvp: /bin/sh: Argument list too long

because all the object directories were on a single command line and our paths on our build machine are fairly deep.

  1. The wildcard pattern
$(wildcard $(S)/*/ $(S)/*/*/ $(S)/*/*/*/ $(S)/*/*/*/*/ $(S)/*/*/*/*/*/)

matched things that weren't directories -- this is possibly a bug in the version of glibc's glob on our build machine's Ubuntu Focal release, and possibly a cause for problem 1 (because OBJDIRS ended up much larger).

  1. As mentioned our build machine is Ubuntu Focal, so it has GCC 9, which doesn't have two of the command-line arguments you included: -ftrivial-auto-var-init=pattern and -Wctad-maybe-unsupported. I think both of these are safely omittable.

I can do PRs against your branch to fix all three of those; however you'll need to open a new PR against dcsctp4j, because it doesn't look like GitHub has any way to re-open a merged PR.

@fitzsim
Copy link
Contributor Author

fitzsim commented Feb 19, 2025

OK, I pushed a rebased redo branch, https://github.com/fitzsim/dcsctp4j/tree/ppc64le-support-4. Do you want to send me pull requests against that before I submit it as a new pull request?

@JonathanLennox
Copy link
Member

I have my pull requests against your branch at fitzsim#2

@fitzsim
Copy link
Contributor Author

fitzsim commented Feb 19, 2025

Resubmitted as #11, thank you.

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.

2 participants