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

Switch to C++17 for Arrow compilation #1858

Merged
merged 1 commit into from
Oct 26, 2022

Conversation

bmcdonald3
Copy link
Contributor

@bmcdonald3 bmcdonald3 commented Oct 26, 2022

The Arrow pre-release for 10.0.0 came out today and it now requires C++17 and our CI was getting that latest version, which apparently includes pre-releases.

After chatting with Ethan and Pierce, we decided it is probably best to have the CI be using the make install-arrow command that is in the Makefile so the CI is installing in the same way that a user would be on their machine, rather than the specific install of getting the latest that it is using today.

We want to keep up to date with the new releases, but figure we will just hardcode the release for now, since Arkouda is already doing that for a number of other dependencies.

Chapel requires GCC 7+, which supports the majority of C++17 features, so Elliot thought it wouldn't be the end of the world to use C++17 in the Arkouda build, so if we end up wanting to support the new Arrow versions, that probably won't cause any issues, but I think we are going to defer that for now.

Previously, we were building Arrow with -std=c++11, but
now Arrow is using features that require C++17, so this
PR updates that compilation flag.
@bmcdonald3
Copy link
Contributor Author

The Arrow pre-release for 10.0.0 came out today and it now requires C++17 and our CI was getting that latest version, which apparently includes pre-releases.

After chatting with Ethan, we decided it is probably best to have the CI be using the make install-arrow command that is in the Makefile so the CI is installing in the same way that a user would be on their machine, rather than the specific install of getting the latest that it is using today.

We want to keep up to date with the new releases, but figure we will just hardcode the release for now, since Arkouda is already doing that for a number of other dependencies.

Chapel requires GCC 7+, which supports the majority of C++17 features, so Elliot thought it wouldn't be the end of the world to use C++17 in the Arkouda build, so if we end up wanting to support the new Arrow versions, that probably won't cause any issues, but I think we are going to defer that for now.

@bmcdonald3 bmcdonald3 closed this Oct 26, 2022
@bmcdonald3 bmcdonald3 reopened this Oct 26, 2022
Copy link
Contributor

@Ethan-DeBandi99 Ethan-DeBandi99 left a comment

Choose a reason for hiding this comment

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

This is our best option to get CI running for the time being. We will look into the alignment of the Makefile and the CI at a later time.

@Ethan-DeBandi99 Ethan-DeBandi99 merged commit dcad734 into Bears-R-Us:master Oct 26, 2022
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