Skip to content

GH-46748: [C++] Initial port on AIX #46749

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

Merged
merged 4 commits into from
Jun 18, 2025
Merged

Conversation

ranjitshs
Copy link
Contributor

@ranjitshs ranjitshs commented Jun 9, 2025

Rationale for this change

Fix build errors on AIX.

What changes are included in this PR?

Compilation issue fixed as mentioned in the above issue.

Are these changes tested?

Yes.
Compiled with below settings for initial port.
cmake . -B build --preset ninja-debug-minimal -DCMAKE_INSTALL_PREFIX=/usr/local -DCMAKE_AIX_SHARED_LIBRARY_ARCHIVE=ON -DARROW_CSV=ON -DARROW_JSON=ON -DARROW_FILESYSTEM=ON -DARROW_COMPUTE=ON -DARROW_DATASET=ON

Are there any user-facing changes?

NA

Copy link

github-actions bot commented Jun 9, 2025

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}

See also:

@ranjitshs ranjitshs changed the title [AIX]Initial port on AIX GH-46748: [CPP] Initial port on AIX Jun 9, 2025
Copy link

github-actions bot commented Jun 9, 2025

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

@kou kou changed the title GH-46748: [CPP] Initial port on AIX GH-46748: [C++] Initial port on AIX Jun 9, 2025
@kou
Copy link
Member

kou commented Jun 10, 2025

Could you fix lint errors?

How to maintain AIX support? See also: #46002

@ranjitshs
Copy link
Contributor Author

Could you fix lint errors?

How to maintain AIX support? See also: #46002

Thanks for suggestion about lint. I have updated the change. Let me know if any comments.
Will go through #46002

@ranjitshs
Copy link
Contributor Author

Hi @kou
Lint is still failing without giving any details. Please let me know if anything is needed/missing from my side.

C++ Lint.................................................................Failed
- hook id: cpplint
- exit code: 1

@kou
Copy link
Member

kou commented Jun 12, 2025

https://github.com/apache/arrow/actions/runs/15561745991/job/43843132193?pr=46749#step:7:148

cpp/src/arrow/filesystem/localfs.cc:167:  Use int16/int64/etc, rather than the C type long  [runtime/int] [4]

@ranjitshs
Copy link
Contributor Author

https://github.com/apache/arrow/actions/runs/15561745991/job/43843132193?pr=46749#step:7:148

cpp/src/arrow/filesystem/localfs.cc:167:  Use int16/int64/etc, rather than the C type long  [runtime/int] [4]

I missed to see this in lint report. Thanks for pointing out.
Updated the code as suggested by lint.

@ranjitshs
Copy link
Contributor Author

@kou
What's next on this PR from my side. Please let me know.

@kou
Copy link
Member

kou commented Jun 17, 2025

Do you have any idea how to maintain AIX support?
We don't have any AIX CI jobs now. So AIX support may be broken in the feature. Is it acceptable?

@ranjitshs
Copy link
Contributor Author

Do you have any idea how to maintain AIX support? We don't have any AIX CI jobs now. So AIX support may be broken in the feature. Is it acceptable?

I will setup Jenkins CI job for AIX locally for this package and will build main branch on daily basis.
If community can support , then we can provide AIX system where community can setup the CI. But not sure about complexity on setting this .
So for time being, I am thinking to track AIX CI locally...
Let me know your thoughts.

Copy link
Member

@kou kou left a comment

Choose a reason for hiding this comment

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

+1

OK. Let's start with the approach.

If self-hosted GitHub Runner on AIX is provided, we can add it to our CI.

@kou kou merged commit bd31f83 into apache:main Jun 18, 2025
36 of 37 checks passed
@kou kou removed the awaiting review Awaiting review label Jun 18, 2025
@ranjitshs
Copy link
Contributor Author

Thanks @kou for reviewing/merging this.

Copy link

After merging your PR, Conbench analyzed the 3 benchmarking runs that have been run so far on merge-commit bd31f83.

There were 119 benchmark results with an error:

There were no benchmark performance regressions. 🎉

The full Conbench report has more details.

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

Successfully merging this pull request may close these issues.

2 participants