-
Notifications
You must be signed in to change notification settings - Fork 35
Enable ARM jobs for Connext RMW #853
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -25,8 +25,8 @@ echo "done." | |
| . /etc/os-release | ||
| VERSION_ID_MAJOR=$(echo $VERSION_ID | sed 's/\..*//') | ||
|
|
||
| # We only attempt to install Connext on Ubuntu amd64 | ||
| if [ "${ARCH}" = "x86_64" -a "${ID}" = "ubuntu" ]; then | ||
| # We only attempt to install Connext on Ubuntu amd64 or aarch64 | ||
| if [ "${ID}" = "ubuntu" ]; then | ||
| IGNORE_CONNEXTDDS="" | ||
| ignore_rwm_seen="false" | ||
| for arg in ${CI_ARGS} ; do | ||
|
|
@@ -50,13 +50,22 @@ if [ "${ARCH}" = "x86_64" -a "${ID}" = "ubuntu" ]; then | |
|
|
||
| case "${CI_ARGS}" in | ||
| *--connext-debs*) | ||
| # Installing Connext through Debian packages is supported on both x86_64 and aarch64 architectures. If we're on an unsupported architecture, skip the installation and exit with an error. | ||
| echo "Using Debian package of Connext" | ||
| if test -r /opt/rti.com/rti_connext_dds-${CONNEXT_DISPLAY_VERSION}/resource/scripts/rtisetenv_x64Linux4gcc7.3.0.sh; then | ||
| echo "Sourcing RTI setenv script /opt/rti.com/rti_connext_dds-${CONNEXT_DISPLAY_VERSION}/resource/scripts/rtisetenv_x64Linux4gcc7.3.0.sh" | ||
| . /opt/rti.com/rti_connext_dds-${CONNEXT_DISPLAY_VERSION}/resource/scripts/rtisetenv_x64Linux4gcc7.3.0.sh | ||
| fi | ||
| ;; | ||
| *) | ||
| # Support for installing Connext through the RTI website installers is only supported on | ||
| # x86_64 architecture. If we're on a different architecture, skip the installation and | ||
| # exit with an error. | ||
| if [ "${ARCH}" != "x86_64" ]; then | ||
| echo "Connext is only supported on amd64 architecture. Skipping Connext installation." >&2 | ||
| exit 1 | ||
|
Comment on lines
+65
to
+66
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Existing behavior is to quietly skip connext installation on aarch64 even if requested. While this PR enables the deb installation scenario, I think we should continue the existing behavior and not fail the build if non-deb installation is requested. The main scenario here is that someone might trigger a job from the launcher that uses the non-deb Connext installation. I don't think it would be a good experience if the aarch64 builds suddenly start failing and require re-running with connext disabled.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There are a couple of places where we do |
||
| fi | ||
|
|
||
| echo "Installing Connext binaries off RTI website..." | ||
| if test -x /tmp/rticonnextdds-src/rti_connext_dds-${CONNEXT_DISPLAY_VERSION}-pro-host-x64Linux.run; then | ||
| rtipkg_list="\ | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A lot of these changes don't seem related to aarch64. Possibly prep for a new version of Connext in Rolling soon?
Can we keep this PR scoped to the changes necessary for aarch64?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for that, we are working in parallel on #844, and some changes slipped. I'll remove those here