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

[wip] Turn on cppyy for Ubuntu 22.04 arm jobs ci #126

Conversation

mcbarton
Copy link
Collaborator

This PR will make it so you can build cppyy on Ubuntu 22.04 arm.

Copy link
Collaborator

@Vipul-Cariappa Vipul-Cariappa left a comment

Choose a reason for hiding this comment

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

This looks good.
I assume it took quite some time figuring out which tests crash.

test/support.py Show resolved Hide resolved
test/test_api.py Outdated Show resolved Hide resolved
@mcbarton mcbarton force-pushed the Add-Ubuntu-22.04-arm-support-to-cppyy branch from 7d17195 to 5db93d9 Compare January 27, 2025 15:52
@mcbarton mcbarton requested a review from aaronj0 January 27, 2025 15:53
@mcbarton
Copy link
Collaborator Author

@aaronj0 Thank you for your work adding the Valgrind suppressions to the Ubuntu 24.04 x86 PR. Can you do the same for this Ubuntu 22.04 arm PR?

@mcbarton mcbarton marked this pull request as ready for review January 27, 2025 22:44
@mcbarton
Copy link
Collaborator Author

@aaronj0 I've fixed the merge conflict so you can carry on with your Valgrind fix.

@aaronj0 aaronj0 force-pushed the Add-Ubuntu-22.04-arm-support-to-cppyy branch from cb1d63f to c75c204 Compare January 28, 2025 04:53
@aaronj0 aaronj0 force-pushed the Add-Ubuntu-22.04-arm-support-to-cppyy branch 4 times, most recently from 6aacb76 to 68abd65 Compare January 28, 2025 09:07
This stems from a mismatched free() context causing 3 errors with all LLVM versions (13, 17, 18, 19). The suppression files are made seperate so we do not regress mismatched free() memory errors on non-ARM builds
@aaronj0 aaronj0 force-pushed the Add-Ubuntu-22.04-arm-support-to-cppyy branch from aa256c7 to 466885b Compare January 28, 2025 11:00
Copy link
Collaborator

@aaronj0 aaronj0 left a comment

Choose a reason for hiding this comment

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

LGTM! This PR is ready to be merged now

@aaronj0 aaronj0 merged commit 01be0e0 into compiler-research:master Jan 28, 2025
50 checks passed
Copy link
Collaborator

@Vipul-Cariappa Vipul-Cariappa left a comment

Choose a reason for hiding this comment

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

As this PR is already merged. I will investigate the comments myself, in a new PR.
Hopefully someone can remind me, if I forget.

@@ -818,6 +825,7 @@ def z(self):
assert a.m_2 == 42
assert a.m_3 == 67

@mark.xfail(run=not IS_CLANG_DEBUG, reason="Crashes with ClangRepl with 'toString not implemented'")
Copy link
Collaborator

Choose a reason for hiding this comment

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

I suspect the xfail tag here is wrong.
It should be

Suggested change
@mark.xfail(run=not IS_CLANG_DEBUG, reason="Crashes with ClangRepl with 'toString not implemented'")
@mark.xfail(run=False, condition=not IS_CLANG_DEBUG, reason="Crashes with ClangRepl with 'toString not implemented'")

Also, if this function passes in all other platforms, then the reason might not be related to toString.

@@ -905,6 +913,7 @@ def z(self):
assert a.m_2 == 88
assert a.m_3 == -11

@mark.xfail(run=not IS_CLANG_DEBUG, reason="Crashes with ClangRepl with 'toString not implemented'")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same as above.

@@ -1276,6 +1285,7 @@ class D(B):
assert inst.fun1() == val1
assert inst.fun2() == inst.fun1()

@mark.xfail(run=not IS_CLANG_DEBUG, reason="Crashes with ClangRepl with 'toString not implemented'")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same as above.

@@ -1517,6 +1527,7 @@ def getValue(self):
gc.collect()
assert ns.Component.get_count() == 0

@mark.xfail(run=not IS_CLANG_DEBUG, reason="Crashes with ClangRepl with 'toString not implemented'")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same as above.

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.

3 participants