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.

Sign up for GitHub

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

Jump to bottom

PARQUET-1382: [C++] Prepare for arrow::test namespace removal #487

Closed
wants to merge 9 commits into from
Closed

PARQUET-1382: [C++] Prepare for arrow::test namespace removal #487

wants to merge 9 commits into from

Conversation

pitrou
Copy link
Member

@pitrou pitrou commented Aug 16, 2018 •
edited

Also tried to make Travis-CI builds faster.

@pitrou pitrou mentioned this pull request Aug 16, 2018
Closed
@pitrou pitrou force-pushed the PARQUET-1382-arrow-test-namespace-shim branch from 048fe39 to 3ede47c Compare August 16, 2018 08:02
@pitrou
Copy link
Member Author

pitrou commented Aug 16, 2018

There are a lot of warnings now in the OS X build, such as:

ld: warning: direct access in apache::thrift::protocol::TCompactProtocolFactoryT<apache::thrift::transport::TMemoryBuffer>::getProtocol(std::__1::shared_ptr<apache::thrift::transport::TTransport>) to global weak symbol typeinfo for apache::thrift::transport::TTransport means the weak symbol cannot be overridden at runtime. This was likely caused by different translation units being compiled with different visibility settings.

Also, ListLargeRecords failed here @wesm :
https://travis-ci.org/apache/parquet-cpp/jobs/416711723#L5106

@pitrou pitrou force-pushed the PARQUET-1382-arrow-test-namespace-shim branch from 71943fd to a6a2198 Compare August 16, 2018 09:15
@pitrou
Copy link
Member Author

pitrou commented Aug 16, 2018

apt is very slow on Travis-CI today :/

@pitrou pitrou force-pushed the PARQUET-1382-arrow-test-namespace-shim branch from d7cffd4 to 0fe54f3 Compare August 16, 2018 11:42
This was referenced Aug 16, 2018
Closed
Closed
@pitrou
Copy link
Member Author

pitrou commented Aug 16, 2018

The AppVeyor toolchain build apparently failed linking with brotli:
https://ci.appveyor.com/project/ApacheSoftwareFoundation/parquet-cpp/build/1.0.762

(is brotli actually useful? There are better compression options nowadays...)

wesm pushed a commit to apache/arrow that referenced this pull request Aug 16, 2018
@pitrou @wesm
ARROW-3059: [C++] Remove namespace arrow::test
719c954 
Depends on PR apache/parquet-cpp#487

Author: Antoine Pitrou <antoine@python.org>

Closes #2436 from pitrou/ARROW-3059-remove-test-namespace and squashes the following commits:

47bf2e3 <Antoine Pitrou> ARROW-3059:  Remove namespace arrow::test
@wesm
Copy link
Member

wesm commented Aug 16, 2018

Brotli is in the format. We might drop it at some point or make it optional in the build

@pitrou pitrou force-pushed the PARQUET-1382-arrow-test-namespace-shim branch from 5ea26bf to 71b4b26 Compare August 16, 2018 14:36
@pitrou
Copy link
Member Author

pitrou commented Aug 16, 2018

Ok, our AppVeyor CI script was forcing an old Brotli version, so I'm trying to bump it up to see whether that fixes things.

@pitrou pitrou force-pushed the PARQUET-1382-arrow-test-namespace-shim branch from bcfa740 to 323e47f Compare August 16, 2018 16:16
@pitrou
Copy link
Member Author

pitrou commented Aug 16, 2018

Ok, so the AppVeyor build is fixed now. The Travis-CI build still has the OS X failure. What should we do? Do you know how to investigate that bug?

@wesm
Copy link
Member

wesm commented Aug 16, 2018

I have about 2 hours to spend on it today, I'll write in PARQUET-1308 as I make progress figuring it out

@wesm wesm force-pushed the PARQUET-1382-arrow-test-namespace-shim branch from 55708f4 to 2c169ec Compare August 17, 2018 06:30
@wesm
Copy link
Member

wesm commented Aug 17, 2018

Just rebased this (there were some small conflicts, hopefully I fixed them correctly). Should hopefully get a green build now

@pitrou pitrou mentioned this pull request Aug 17, 2018
Closed
wesm
wesm approved these changes Aug 17, 2018
View reviewed changes
Copy link
Member

@wesm wesm left a comment

Choose a reason for hiding this comment

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

+1

@wesm wesm closed this in ebb4b4b Aug 17, 2018
@pitrou pitrou deleted the PARQUET-1382-arrow-test-namespace-shim branch August 17, 2018 15:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wesm wesm wesm approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
2 participants
@pitrou @wesm