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

clickhouse: 19.17.9.60 -> 20.5.2.7 #92694

Merged
merged 2 commits into from Jul 8, 2020
Merged

clickhouse: 19.17.9.60 -> 20.5.2.7 #92694

merged 2 commits into from Jul 8, 2020

Conversation

RaghavSood
Copy link
Member

@RaghavSood RaghavSood commented Jul 8, 2020 •
edited

Motivation for this change

Implements #90354

I tried the nixpkgs-review command, but I'm not sure if it worked - I got No diff detected, stopping review... which seems wrong
nixpkgs-review builds only clickhouse and gives it a clean chit

llvm was bumped from 7 to 9 to match what Clickhouse recommends

Things done
  • 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.

@Ma27
Copy link
Member

Ma27 commented Jul 8, 2020

I tried the nixpkgs-review command, but I'm not sure if it worked - I got No diff detected, stopping review... which seems wrong

Which command did you run?

@RaghavSood
Copy link
Member Author

@Ma27 It's the command given in the PR template

[raghav@ch-misc:~/nixpkgs/nixpkgs]$ git status
On branch clickhouse/20.5.2.7
Your branch is up to date with 'origin/clickhouse/20.5.2.7'.

nothing to commit, working tree clean

[raghav@ch-misc:~/nixpkgs/nixpkgs]$ nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
$ git -c fetch.prune=false fetch --force https://github.com/NixOS/nixpkgs master:refs/nixpkgs-review/0
remote: Enumerating objects: 70, done.
remote: Counting objects: 100% (70/70), done.
remote: Compressing objects: 100% (21/21), done.
remote: Total 87 (delta 53), reused 59 (delta 49), pack-reused 17
Unpacking objects: 100% (87/87), 331.57 KiB | 1.05 MiB/s, done.
From https://github.com/NixOS/nixpkgs
   cefa2ad525a..143398d7da2  master     -> refs/nixpkgs-review/0
$ git worktree add /home/raghav/.cache/nixpkgs-review/rev-2761377682bdd30de0ceb94407b62e51f1368a4d-dirty-1/nixpkgs 143398d7da2af8c8c1db948e6211e1e1dec3d800
Preparing worktree (detached HEAD 143398d7da2)
Updating files: 100% (22075/22075), done.
HEAD is now at 143398d7da2 Merge pull request #92390 from fionera/add-fuzzel
$ nix-env -f /home/raghav/.cache/nixpkgs-review/rev-2761377682bdd30de0ceb94407b62e51f1368a4d-dirty-1/nixpkgs -qaP --xml --out-path --show-trace
No diff detected, stopping review...
$ git worktree prune

@Ma27
Copy link
Member

Ma27 commented Jul 8, 2020

Well, the wip-command tests your local, uncommitted changes. In your case you probably want to run nix-review rev <rev of your commit>.

@RaghavSood
Copy link
Member Author

Ah, right, that makes sense - running that now.

Seems a bit odd to have a WIP command in the pull request template, since by the time you're making a pull request you would have committed your changes.

@RaghavSood
Copy link
Member Author

nixpkgs-review builds only clickhouse, no errors

@Ma27 Ma27 merged commit 904aa5d into NixOS:master Jul 8, 2020
@Ma27 Ma27 mentioned this pull request Jul 8, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
10.rebuild-darwin: 0 10.rebuild-linux: 1-10 10.rebuild-linux: 1
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@RaghavSood @Ma27