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

grafana: 6.7.3 -> 7.0.0 #88242

Merged
merged 1 commit into from May 23, 2020
Merged

grafana: 6.7.3 -> 7.0.0 #88242

merged 1 commit into from May 23, 2020

Conversation

@JJJollyjim
Copy link
Contributor

JJJollyjim commented May 20, 2020

This version removes PhantomJS support.

Upstream also stopped vendoring dependencies, so I switched to buildGoModule. I do not have the go packaging experience to fully understand the implications of this -- would appreciate review from someone who does.

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.
Copy link
Member

mdlayher left a comment

LGTM on NixOS 20.03:

[matt@servnerr-3:~/src/nixpkgs]$ nix-build -A grafana                                                                                                                                              
these derivations will be built:                                                                                                                                                                   
  /nix/store/3w099kjh1m33zxfxmsxg8kn8mcry2px6-remove-references-to.drv                                                                                                                             
  /nix/store/jd0qyr47s5c0p1mzz2m08rphyskz7013-source.drv                                                                                                                                           
  /nix/store/gf1ck2a6pq9pbzmj1wi1l77q5ysdy6ji-grafana-7.0.0-go-modules.drv                                                                                                                         
  /nix/store/lda1cwfim1hsz0pd0wqn8c7fcig30486-grafana-7.0.0.linux-amd64.tar.gz.drv                                                                                                                 
  /nix/store/4s5js9kkhcvqnlcbl4kzai9fwkqdy85l-grafana-7.0.0.drv 

[...]

post-installation fixup
shrinking RPATHs of ELF executables and libraries in /nix/store/53a65a2gapdyagc5vpp6k6kh99saqqy5-grafana-7.0.0
shrinking /nix/store/53a65a2gapdyagc5vpp6k6kh99saqqy5-grafana-7.0.0/bin/slow_proxy_mac
shrinking /nix/store/53a65a2gapdyagc5vpp6k6kh99saqqy5-grafana-7.0.0/bin/slow_proxy
shrinking /nix/store/53a65a2gapdyagc5vpp6k6kh99saqqy5-grafana-7.0.0/bin/grafana-server
shrinking /nix/store/53a65a2gapdyagc5vpp6k6kh99saqqy5-grafana-7.0.0/bin/grafana-cli
shrinking /nix/store/53a65a2gapdyagc5vpp6k6kh99saqqy5-grafana-7.0.0/bin/alert_webhook_listener
strip is /nix/store/a57856fs4m8ir6vlv14h3gq3sv9aq2lb-binutils-2.31.1/bin/strip
stripping (with command strip and flags -S) in /nix/store/53a65a2gapdyagc5vpp6k6kh99saqqy5-grafana-7.0.0/bin
patching script interpreter paths in /nix/store/53a65a2gapdyagc5vpp6k6kh99saqqy5-grafana-7.0.0
checking for references to /build/ in /nix/store/53a65a2gapdyagc5vpp6k6kh99saqqy5-grafana-7.0.0...
/nix/store/53a65a2gapdyagc5vpp6k6kh99saqqy5-grafana-7.0.0

[matt@servnerr-3:~/src/nixpkgs]$ ./result/bin/grafana-server -v
Version 7.0.0 (commit: NA, branch: master)
@mdlayher
Copy link
Member

mdlayher commented May 20, 2020

@GrahamcOfBorg test grafana

@Ma27
Copy link
Member

Ma27 commented May 20, 2020

Before merging this, I'd like to wait for a review of @WilliButz. We should discuss if we actually want to use buildGoModule here.

@mdlayher
Copy link
Member

mdlayher commented May 20, 2020

Although I am not the maintainer of this particular package, I am experienced with Go and would support moving to buildGoModule.

The Grafana repository uses Go modules (note go.mod and go.sum in the repo root) and has removed the vendor folder, so buildGoModule seems appropriate. I use it in the CoreRAD NixOS package as well in the same scenario. (Edit: just realized I haven't bumped CoreRAD yet since removing vendor a few days ago, so this data point is less useful)

@WilliButz
Copy link
Member

WilliButz commented May 20, 2020

@JJJollyjim first of all, thanks for taking care of the update :)

My opinion on buildGoModule (same applies to buildRustPackage) has changed over time:
Though at first it seemed to me like a very convenient way to handle the language-specific build dependencies, while only having to specify a single hash to pin them all, it takes away the control Nix had over said dependencies.
I was forced a few times to update the modSha256 as well as cargoSha256 of some packages, because some not strictly pinned dependency changed.

Because the recent update to buildGoModule using -mod=vendor seems somewhat promising and because the change needed to go from the vendored dependencies to buildGoModule is quite simple, I'd be ok with giving it a try to see how stable the grafana dependencies (and all of their dependencies) are.
However, if it should come up that vendorSha256 needs to be regularly updated, I'd suggest going for the deps.nix approach and trading some more generated Nix for a long term stable build.

On the major version update:
Could you add a short entry to the release notes for the upcoming release including a reference to the upgrade instructions?

pkgs/servers/monitoring/grafana/default.nix Outdated Show resolved Hide resolved
@JJJollyjim JJJollyjim force-pushed the JJJollyjim:grafana-7.0.0 branch 2 times, most recently from b85f710 to 04274b1 May 21, 2020
@JJJollyjim
Copy link
Contributor Author

JJJollyjim commented May 21, 2020

Release notes added.

Copy link
Member

Frostman left a comment

nixosTests.grafana passed and binaries seems to be working fine.

Copy link
Member

mdlayher left a comment

LGTM again, but left a release notes nit.

nixos/doc/manual/release-notes/rl-2009.xml Outdated Show resolved Hide resolved
@JJJollyjim JJJollyjim force-pushed the JJJollyjim:grafana-7.0.0 branch from 04274b1 to 07a30d6 May 22, 2020
This version removes PhantomJS support.

Upstream also stopped vendoring dependencies, so I switched to buildGoModule.
@JJJollyjim JJJollyjim force-pushed the JJJollyjim:grafana-7.0.0 branch from 07a30d6 to 3d2def3 May 23, 2020
@JJJollyjim
Copy link
Contributor Author

JJJollyjim commented May 23, 2020

Merge conflict resolved.

@ofborg ofborg bot requested a review from Frostman May 23, 2020
Copy link
Member

WilliButz left a comment

upgrade of my existing installation went smoothly, will merge after borg finished 👍

@WilliButz
Copy link
Member

WilliButz commented May 23, 2020

@GrahamcOfBorg test grafana

@WilliButz WilliButz merged commit 10b6f23 into NixOS:master May 23, 2020
18 checks passed
18 checks passed
Evaluation Performance Report Evaluator Performance Report
Details
grafana, grafana.passthru.tests on aarch64-linux Success
Details
grafana, grafana.passthru.tests on x86_64-linux Success
Details
grahamcofborg-eval ^.^!
Details
grahamcofborg-eval-check-maintainers matching changed paths to changed attrs...
Details
grahamcofborg-eval-check-meta config.nix: checkMeta = true
Details
grahamcofborg-eval-darwin nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="3d2def3"; rev="3d2def38ae8977e8355ee4ed951f2ff320e9b8e3"; } ./pkgs/t
Details
grahamcofborg-eval-lib-tests nix-build --arg pkgs import ./. {} ./lib/tests/release.nix
Details
grahamcofborg-eval-nixos nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="3d2def3"; rev="3d2def38ae8977e8355ee4ed951f2ff320e9b8e3"; } ./nixos/
Details
grahamcofborg-eval-nixos-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="3d2def3"; rev="3d2def38ae8977e8355ee4ed951f2ff320e9b8e3"; } ./nixos/
Details
grahamcofborg-eval-nixos-options nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="3d2def3"; rev="3d2def38ae8977e8355ee4ed951f2ff320e9b8e3"; } ./nixos/
Details
grahamcofborg-eval-nixpkgs-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="3d2def3"; rev="3d2def38ae8977e8355ee4ed951f2ff320e9b8e3"; } ./pkgs/t
Details
grahamcofborg-eval-nixpkgs-tarball nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="3d2def3"; rev="3d2def38ae8977e8355ee4ed951f2ff320e9b8e3"; } ./pkgs/t
Details
grahamcofborg-eval-nixpkgs-unstable-jobset nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="3d2def3"; rev="3d2def38ae8977e8355ee4ed951f2ff320e9b8e3"; } ./pkgs/t
Details
grahamcofborg-eval-package-list nix-env -qa --json --file .
Details
grahamcofborg-eval-package-list-no-aliases nix-env -qa --json --file . --arg config { allowAliases = false; }
Details
tests.grafana on aarch64-linux Success
Details
tests.grafana on x86_64-linux Success
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants
You can’t perform that action at this time.