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

Abseil test failure on AIX due to some linking error(terminate getting called) #1608

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

krushna100
Copy link

Thank you for your contribution to Abseil!

Before submitting this PR, please be sure to read our contributing
guidelines
.

If you are a Googler, please also note that it is required that you send us a
Piper CL instead of using the GitHub pull-request process. The code propagation
process will deliver the change to GitHub.

@krushna100 krushna100 changed the title Abseil test failure on AIX Abseil test failure on AIX due to some linking error(terminate getting called) Feb 5, 2024
@krushna100
Copy link
Author

Hi Team,
This PR is raised from long time.
Can someone review the change

@derekmauro
Copy link
Member

I don't understand the purpose of this or why it fixes an issue or even what the issue is. Since AIX is not a supported platform (https://opensource.google/documentation/policies/cplusplus-support) and this complicates the code, I'd rather not accept this as is.

@ranjitshs
Copy link

ranjitshs commented Apr 25, 2024

@derekmauro
Even AIX is not supported platform as per link you shared, I see some closed PR related to AIX
https://github.com/abseil/abseil-cpp/pulls?q=AIX

Regarding complexities, can we have separate section for AIX rules like below,
#if AIX
< existing rules + AIX changes>
#else
<existing rules>

@derekmauro
Copy link
Member

We sometimes accept PRs for unsupported platforms if they do not complicate the code. This PR introduces unexplained platform specific build flags. It also provides no explanation why it is necessary since everything seemed to be working before. I suspect there is a more generic way of changing the CMake configuration anyway to achieve whatever this PR is meant to achieve.

@ranjitshs
Copy link

@derekmauro
Thank you for following up on this. I agree that problem/solution is not described properly.

@krushna100
Could you please create a issue and provide the details of the problem/how to reproduce and the proposed solution .
Mention this PR in that issue.

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