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

ARROW-5899: [Python][Packaging] Build and link uriparser statically in Windows wheel builds #4833

Closed
wants to merge 4 commits into from

Conversation

kszucs
Copy link
Member

@kszucs kszucs commented Jul 9, 2019

The windows nightly wheel builds are failing: https://ci.appveyor.com/project/Ursa-Labs/crossbow/builds/25688922 probably caused by 88fcb09, but it's hard to tell because of the error message "ImportError: DLL load failed: The specified module could not be found." is not very descriptive.

@kszucs
Copy link
Member Author

kszucs commented Jul 9, 2019

@ursabot crossbow submit wheel-win-cp37m

@ursabot
Copy link

ursabot commented Jul 9, 2019

No such command "submit".

@kszucs
Copy link
Member Author

kszucs commented Jul 9, 2019

@ursabot crossbow package wheel-win-cp37m

@ursabot
Copy link

ursabot commented Jul 9, 2019

AMD64 Conda Crossbow (#33681) builder has been succeeded.

Revision: 6b87333

Submitted crossbow builds: ursa-labs/crossbow @ ursabot-103

Task Status
wheel-win-cp37m Appveyor

@kszucs
Copy link
Member Author

kszucs commented Jul 9, 2019

@ursabot crossbow package wheel-win-cp37m

@ursabot
Copy link

ursabot commented Jul 9, 2019

AMD64 Conda Crossbow (#33732) builder has been succeeded.

Revision: a4ff13a

Submitted crossbow builds: ursa-labs/crossbow @ ursabot-106

Task Status
wheel-win-cp37m Appveyor

@codecov-io
Copy link

Codecov Report

Merging #4833 into master will decrease coverage by 17.44%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff             @@
##           master    #4833       +/-   ##
===========================================
- Coverage   82.62%   65.17%   -17.45%     
===========================================
  Files         335      487      +152     
  Lines       43377    64296    +20919     
  Branches     1418        0     -1418     
===========================================
+ Hits        35841    41907     +6066     
- Misses       7174    22389    +15215     
+ Partials      362        0      -362
Impacted Files Coverage Δ
r/src/recordbatch.cpp
r/R/Table.R
js/src/util/fn.ts
r/src/symbols.cpp
rust/datafusion/src/execution/projection.rs
r/R/Column.R
rust/arrow/src/csv/writer.rs
rust/datafusion/src/bin/main.rs
go/arrow/ipc/file_reader.go
js/src/builder/index.ts
... and 812 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7838886...a4ff13a. Read the comment docs.

@kszucs
Copy link
Member Author

kszucs commented Jul 9, 2019

@ursabot crossbow package wheel-win-cp35m wheel-win-cp36m wheel-win-cp37m

@ursabot
Copy link

ursabot commented Jul 9, 2019

AMD64 Conda Crossbow (#33769) builder has been succeeded.

Revision: a4ff13a

Submitted crossbow builds: ursa-labs/crossbow @ ursabot-108

Task Status
wheel-win-cp35m Appveyor
wheel-win-cp36m Appveyor
wheel-win-cp37m Appveyor

@kszucs kszucs changed the title [Python][Packaging] Bundle uriparser.dll in windows wheels ARROW-5899: [Python][Packaging] Bundle uriparser.dll in windows wheels Jul 10, 2019
@wesm
Copy link
Member

wesm commented Jul 10, 2019

Hmm, so using -Duriparser_SOURCE=BUNDLED should use static linking, so bundling should not be necessary. Can you try that?

@kszucs
Copy link
Member Author

kszucs commented Jul 11, 2019

@wesm Sure, but shouldn't we add uriparser's copyright notice to the LICENSE then? Also shouldn't we distribute arrow's LICENSE file with the wheels?

@wesm
Copy link
Member

wesm commented Jul 11, 2019

Yes, we should. Do you know how licenses get distributed with wheels in general?

@kszucs
Copy link
Member Author

kszucs commented Jul 11, 2019

Based on what we link statically additional entries are required in the LICENSE files (top level and the one under pyarrow).
I'll switch to static linking, and create a jira for the license distribution.

@kszucs
Copy link
Member Author

kszucs commented Jul 11, 2019

@ursabot crossbow package wheel-win-cp35m wheel-win-cp36m wheel-win-cp37m

@ursabot
Copy link

ursabot commented Jul 11, 2019

AMD64 Conda Crossbow (#35180) builder has been succeeded.

Revision: 52616ec

Submitted crossbow builds: ursa-labs/crossbow @ ursabot-118

Task Status
wheel-win-cp35m Appveyor
wheel-win-cp36m Appveyor
wheel-win-cp37m Appveyor

@kszucs
Copy link
Member Author

kszucs commented Jul 11, 2019

@wesm PTAL

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 changed the title ARROW-5899: [Python][Packaging] Bundle uriparser.dll in windows wheels ARROW-5899: [Python][Packaging] Build and link uriparser statically in Windows wheel builds Jul 12, 2019
@wesm wesm closed this in 4221db9 Jul 12, 2019
wesm pushed a commit that referenced this pull request Jul 13, 2019
…n Windows wheel builds

The windows nightly wheel builds are failing: https://ci.appveyor.com/project/Ursa-Labs/crossbow/builds/25688922 probably caused by 88fcb09, but it's hard to tell because of the error message  "ImportError: DLL load failed: The specified module could not be found." is not very descriptive.

Author: Krisztián Szűcs <[email protected]>

Closes #4833 from kszucs/win-wheel-uriparser and squashes the following commits:

52616ec <Krisztián Szűcs> missed from the previous commit
19fca63 <Krisztián Szűcs> bundled uriparser
a4ff13a <Krisztián Szűcs> cmake
6b87333 <Krisztián Szűcs> bundle uriparser.dll in windows wheels
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.

4 participants