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

GH-40964: [CI][Archery] Archery linking should also check for undefined symbols Linux #40520

Open
wants to merge 29 commits into
base: main
Choose a base branch
from

Conversation

vibhatha
Copy link
Collaborator

@vibhatha vibhatha commented Mar 14, 2024

Rationale for this change

This PR includes a check to find out if there are undefined symbols associated with the libraries.

What changes are included in this PR?

Includes a util functions which uses nm and ldconfig tools to find out libraries and their symbols and determine
if there are undefined symbols excluding the symbols in the allowed dependencies.

Are these changes tested?

Locally tested with a faulty shared library, but this needs validation on versions as there is a version mismatch in the local libraries.

Are there any user-facing changes?

No

Copy link

Thanks for opening a pull request!

If this is not a minor PR. Could you open an issue for this pull request on GitHub? https://github.com/apache/arrow/issues/new/choose

Opening GitHub issues ahead of time contributes to the Openness of the Apache Arrow project.

Then could you also rename the pull request title in the following format?

GH-${GITHUB_ISSUE_ID}: [${COMPONENT}] ${SUMMARY}

or

MINOR: [${COMPONENT}] ${SUMMARY}

In the case of PARQUET issues on JIRA the title also supports:

PARQUET-${JIRA_ISSUE_ID}: [${COMPONENT}] ${SUMMARY}

See also:

@kou
Copy link
Member

kou commented Mar 14, 2024

@github-actions crossbow submit java-jars

Copy link

Revision: 090275e

Submitted crossbow builds: ursacomputing/crossbow @ actions-7ff12b2218

Task Status
java-jars GitHub Actions

@kou kou changed the title [CI][Archery] Archery linking should also check for undefined symbols GH-40018: [CI][Archery] Archery linking should also check for undefined symbols Mar 14, 2024
Copy link

⚠️ GitHub issue #40018 has been automatically assigned in GitHub to PR creator.

@vibhatha
Copy link
Collaborator Author

@github-actions crossbow submit java-jars

Copy link

Revision: 090275e

Submitted crossbow builds: ursacomputing/crossbow @ actions-e8a43da78d

Task Status
java-jars GitHub Actions

@vibhatha
Copy link
Collaborator Author

@vibhatha
Copy link
Collaborator Author

@github-actions crossbow submit java-jars

Copy link

Revision: 803932a

Submitted crossbow builds: ursacomputing/crossbow @ actions-cf41726ea3

Task Status
java-jars GitHub Actions

@vibhatha
Copy link
Collaborator Author

@github-actions crossbow submit java-jars

Copy link

Revision: b9fb4f2

Submitted crossbow builds: ursacomputing/crossbow @ actions-2164940a0a

Task Status
java-jars GitHub Actions

@vibhatha
Copy link
Collaborator Author

@github-actions crossbow submit java-jars

This comment was marked as outdated.

@vibhatha
Copy link
Collaborator Author

@github-actions crossbow submit java-jars

This comment was marked as outdated.

@vibhatha
Copy link
Collaborator Author

Locally I have the following nm

nm --version
GNU nm (GNU Binutils) 2.40
Copyright (C) 2023 Free Software Foundation, Inc.
This program is free software; you may redistribute it under the terms of
the GNU General Public License version 3 or (at your option) any later version.
This program has absolutely no warranty.

but in the CI we have

nm: just-symbols: invalid output format
GNU nm version 2.35-5.el7.4
Copyright (C) 2020 Free Software Foundation, Inc.
This program is free software; you may redistribute it under the terms of
the GNU General Public License version 3 or (at your option) any later version.
This program has absolutely no warranty.

Would it be possible to upgrade the nm version in the CI containers?

cc @raulcd

@kou
Copy link
Member

kou commented Mar 14, 2024

If we run archery linking check-dependencies in package-jars job not build-cpp-ubuntu, we can use newer nm.
But we can't change nm version for java-jni-manylinux-2014. We need to use CentOS 7 based image for java-jni-manylinux-2014.
If we use manylinux_2_28 not manylinux-2014, we can use newer nm. (Can we drop support for CentOS 7?)

@vibhatha
Copy link
Collaborator Author

I see, it seems a bit complicated approach to update nm. Then should we target on more work on Python rather than nm update?

@kou
Copy link
Member

kou commented Mar 15, 2024

Yes. Could you try the approach?

@vibhatha
Copy link
Collaborator Author

@kou I will try this.

@vibhatha
Copy link
Collaborator Author

@github-actions crossbow submit java-jars

@vibhatha
Copy link
Collaborator Author

@github-actions crossbow submit -g java

This comment was marked as outdated.

This comment was marked as outdated.

@vibhatha
Copy link
Collaborator Author

@kou, I just checked with the https://repo1.maven.org/maven2/org/apache/arrow/arrow-dataset/15.0.2/arrow-dataset-15.0.2.jar

Error: Undefined symbols found in /home/vibhatha/Documents/Work/Apache_Arrow/issues/gh-40018/arrow-dataset-15.0.2/x86_64/libarrow_dataset_jni.so:
EVP_MD_CTX_create
EVP_MD_CTX_destroy
HMAC_CTX_cleanup
HMAC_CTX_init

This PR fixes the protobuf issue, but it seems like we still do have openssl related stuff here, which is not in the allowed list right?

@vibhatha vibhatha marked this pull request as ready for review March 20, 2024 01:05
@vibhatha
Copy link
Collaborator Author

vibhatha commented Jun 4, 2024

@pitrou I updated the PR, but I have one question and noted here

@vibhatha
Copy link
Collaborator Author

vibhatha commented Aug 7, 2024

@pitrou appreciate your feedback for this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting change review Awaiting change review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants