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

arrow-cpp: 0.17.1 -> 1.0.0 #94849

Merged
merged 2 commits into from Aug 21, 2020
Merged

arrow-cpp: 0.17.1 -> 1.0.0 #94849

merged 2 commits into from Aug 21, 2020

Conversation

tobim
Copy link
Contributor

@tobim tobim commented Aug 7, 2020

Motivation for this change

https://arrow.apache.org/blog/2020/07/24/1.0.0-release/

Things done

Fixed the static build with:

Checks
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@tobim tobim requested a review from veprbl August 7, 2020 06:08
@ofborg ofborg bot requested a review from ftrvxmtrx August 7, 2020 06:19
veprbl
veprbl reviewed Aug 7, 2020
View reviewed changes
Copy link
Member

@veprbl veprbl left a comment

Choose a reason for hiding this comment

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

I can't review this properly until the week after next, maybe someone else could pick this up. The diff looks good to me, there are few minor comments.

pkgs/development/libraries/arrow-cpp/default.nix Outdated Show resolved Hide resolved
pkgs/development/libraries/arrow-cpp/default.nix Outdated Show resolved Hide resolved
pkgs/development/libraries/arrow-cpp/default.nix Outdated Show resolved Hide resolved
pkgs/development/libraries/arrow-cpp/default.nix Show resolved Hide resolved
@ofborg ofborg bot requested a review from veprbl August 7, 2020 19:58
@tobim tobim force-pushed the pkgs/arrow-cpp-1.0 branch 3 times, most recently from a10a9cb to 212e68a Compare August 9, 2020 05:33
@ludovicc
Copy link

ludovicc commented Aug 9, 2020

I'm trying to review this PR on Darwin, but I'm having trouble from some upstream dependencies:

nix-shell -p nixpkgs-review --run "nixpkgs-review pr 94849"

[...]

$ nix build --no-link --keep-going --option build-use-sandbox relaxed -f /Users/lclaude/.cache/nixpkgs-review/pr-94849-1/build.nix
builder for '/nix/store/cayzbjkbcfpa8sic0vrnjz7v5xajbmb3-arpack-3.7.0.drv' failed with exit code 2; last 10 log lines:
           1 - dnsimp_tst (SEGFAULT)
      2 - bug_1315_single_tst (Child aborted)
        3 - bug_1315_double_tst (Child aborted)
        4 - bug_1323_tst (SEGFAULT)
            5 - bug_58_double_tst (SEGFAULT)
       6 - bug_79_double_complex_tst (SEGFAULT)
       7 - bug_142_tst (SEGFAULT)
     8 - bug_142_gen_tst (SEGFAULT)
  Errors while running CTest
  make: *** [Makefile:114: test] Error 8
cannot build derivation '/nix/store/mc08m835f15a8b8hsq0fwhvhvcqsg9c1-julia-0.7.0.drv': 1 dependencies couldn't be built
cannot build derivation '/nix/store/bb8yvm3zshizccxnc4d1ymahq998mqi8-julia-1.0.4.drv': 1 dependencies couldn't be built
cannot build derivation '/nix/store/0xam4h8xzd7qgig0qvxmmw2dccgysz34-julia-1.1.1.drv': 1 dependencies couldn't be built
cannot build derivation '/nix/store/6jv7ayjf2ywg4g82pv4bw6xppvia678b-julia-1.3.1.drv': 1 dependencies couldn't be built
builder for '/nix/store/dg7x4qi09kw0mfd3ijn0rkmcx9xq6dc2-chronos-maven-deps.drv' failed with exit code 22; last 10 log lines:
    0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0
  curl: (22) The requested URL returned error: 501 HTTPS Required
    % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                   Dload  Upload   Total   Spent    Left  Speed
    0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0
  curl: (22) The requested URL returned error: 404 Not Found
    % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                   Dload  Upload   Total   Spent    Left  Speed
    0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0
  curl: (22) The requested URL returned error: 403 Forbidden

@tobim
Copy link
Contributor Author

tobim commented Aug 9, 2020

@ludovicc the final output of nixpkgs-review on a macOS 10.14 I tested on is:

1 package marked as broken and skipped:
spark

9 packages failed to build:
chronos julia julia_07 julia_11 julia_13 mesos python27Packages.pysvn python37Packages.google_cloud_bigquery python38Packages.google_cloud_bigquery

25 packages built:
arrow-cpp cabal2nix dep2nix git-doc gitFull gitSVN gitAndTools.svn-all-fast-export svn2git nix-prefetch-scripts nix-prefetch-svn nix-update-source perl528Packages.SVNSimple perl530Packages.SVNSimple python37Packages.ibis-framework python37Packages.intake python37Packages.pyarrow python38Packages.intake python38Packages.pyarrow reposurgeon subversion subversionClient subversion_1_10 utf8proc vcstool ydiff

The failing packages aren't dependencies of arrow. They rather need to be rebuilt because they are directly or transitively affected by the changes of this PR. My assessments after taking a brief look:

  • google_cloud_bigquery has some tests failing, probably needs to be updated.
  • the julias depend on arpack which is broken on darwin. That seems to be an independent issue.
  • mesos requires protobuf but that is missing from the inputs -> seems unrelated.
  • Not really sure why chronos is being built, probably needs a fix to fetch from https.
  • subversion and related packages are built because of the change to utf8proc. pysvn exits because it can't find g++, I don't think this change is responsible.
    `

@veprbl veprbl mentioned this pull request Aug 17, 2020
Closed
10 tasks
@veprbl
Copy link
Member

veprbl commented Aug 19, 2020

@tobim The changes look great! Could you, please, reorganize them into two commits with messages according to the convention (e.g. "arrow-cpp: 0.17.1 -> 1.0.0" and "utf8proc: switch to cmake").

@tobim
Copy link
Contributor Author

tobim commented Aug 19, 2020

@veprbl done.

@ofborg ofborg bot requested a review from veprbl August 20, 2020 09:04
@tobim
Copy link
Contributor Author

tobim commented Aug 21, 2020

@veprbl are you waiting for a review from someone else? If not may I politely ask you to merge this?

@veprbl veprbl merged commit cf7a15a into NixOS:master Aug 21, 2020
@veprbl
Copy link
Member

veprbl commented Aug 21, 2020

@tobim I apologize for the delay. This somehow was marked as already merged in my to do list.

@tobim tobim deleted the pkgs/arrow-cpp-1.0 branch August 21, 2020 20:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ftrvxmtrx ftrvxmtrx Awaiting requested review from ftrvxmtrx

@veprbl veprbl Awaiting requested review from veprbl

Assignees
No one assigned
Labels
10.rebuild-darwin: 11-100 10.rebuild-linux: 11-100
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@tobim @ludovicc @veprbl