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

grafana: made phantomjs2 optional #80666

Merged
merged 1 commit into from Feb 25, 2020
Merged

grafana: made phantomjs2 optional #80666

merged 1 commit into from Feb 25, 2020

Conversation

netixx
Copy link
Contributor

@netixx netixx commented Feb 20, 2020

On servers especially, phantomjs2 pulls graphical dependencies which is unecessary.
This pathes enable the package to be linked/installed without
phantomjs2.

Motivation for this change

On systems with environment.noXlibs = true;, grafana pulls a lot of dependencies with the current package because phantomjs2 depends on X libraries.

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
Ma27 reviewed Feb 21, 2020
View reviewed changes
Copy link
Member

@Ma27 Ma27 left a comment

Choose a reason for hiding this comment

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

Just talked to @WilliButz about this change. It's probably the best solution for now to make it optional just as you did (but make it disabled by default).
We shouldn't backport the change then and it would be great if you could write release notes about this in rl-2009 :)

@netixx
Copy link
Contributor Author

netixx commented Feb 21, 2020

I pushed the default to false.

Where can I find the info to write a release note ?

@Ma27
Copy link
Member

Ma27 commented Feb 21, 2020

You can find it in nixos/doc/manual/release-notes/rl-2009.xml (probably add it to Backward Incompatibilities). You can test the change by running nix build --print-build-logs -f nixos/release.nix --arg supportedSystems '[ builtins.currentSystem ]' --option builders '' manual and then $BROWSER result/share/doc/nixos/release-notes.html.

@netixx
Copy link
Contributor Author

netixx commented Feb 21, 2020

Done!

@ofborg ofborg bot requested a review from Ma27 February 23, 2020 13:59
@netixx @Ma27
grafana: made phantomjs2 optional
9f0014b 
On servers especially, phantomjs2 pulls graphical dependencies which is unecessary.
This pathes enable the package to be linked/installed without
phantomjs2. Phantomjs2 is disabled by default since it has been deprecated in grafana https://grafana.com/docs/grafana/latest/guides/whats-new-in-v6-4/
@Ma27 Ma27 force-pushed the grafana-phantomhs2-optional branch from e21c904 to 9f0014b Compare February 25, 2020 19:42
@Ma27 Ma27 changed the title grafana: made phantomjs2 optionnal grafana: made phantomjs2 optional Feb 25, 2020
@Ma27 Ma27 merged commit e661d07 into NixOS:master Feb 25, 2020
@netixx
Copy link
Contributor Author

netixx commented Feb 26, 2020

Thank you for your help, I couldn't have found it with the logs...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Ma27 Ma27 Ma27 approved these changes

@fpletz fpletz Awaiting requested review from fpletz

@globin globin Awaiting requested review from globin

@WilliButz WilliButz Awaiting requested review from WilliButz

@ehmry ehmry Awaiting requested review from ehmry

@offlinehacker offlinehacker Awaiting requested review from offlinehacker

Assignees
No one assigned
Labels
6.topic: nixos 8.has: changelog 8.has: documentation 10.rebuild-darwin: 0 10.rebuild-linux: 1-10
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@netixx @Ma27