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

redshift, xarchiver: add hicolor-icon-theme to buildInputs #36137

Merged
merged 2 commits into from Feb 28, 2018
Merged

redshift, xarchiver: add hicolor-icon-theme to buildInputs #36137

merged 2 commits into from Feb 28, 2018

Conversation

rardiol
Copy link
Contributor

@rardiol rardiol commented Feb 28, 2018

Motivation for this change

#10370

maintainers @domenkozar @yegortimoshenko

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option build-use-sandbox in nix.conf on non-NixOS)
  • 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 nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Fits CONTRIBUTING.md.

jtojnar
jtojnar requested changes Feb 28, 2018
View reviewed changes
Copy link
Contributor

@jtojnar jtojnar left a comment

Choose a reason for hiding this comment

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

hicolor-icon-theme is now spelt with dashes.

@lukateras lukateras changed the title Hicolor redshift, xarchiver: add hicolor-icon-theme to buildInputs Feb 28, 2018
lukateras
lukateras previously requested changes Feb 28, 2018
View reviewed changes
Copy link
Member

@lukateras lukateras left a comment

Choose a reason for hiding this comment

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

find $(nix-build '<nixpkgs>' -A redshift --no-out-link) -name \*.cache | wc -l returns 0, so I don't think #10370 applies here.

@rardiol
Copy link
Contributor Author

rardiol commented Feb 28, 2018 •
edited

really? I got a conflict between those two today. And i have this in my nix store:

/nix/store/mp6331kwvvd2yl6z4ax2vp4y9rpgi60y-redshift-1.11/share/icons/hicolor/icon-theme.cache

Also changed name to using dashes.

@lukateras
Copy link
Member

  • system: "x86_64-linux"
  • host os: Linux 4.14.21, NixOS, 18.03pre129218.36b0efc12f4 (Impala)
  • multi-user?: yes
  • sandbox: yes
  • version: nix-env (Nix) 1.11.16
  • channels(root): "nixos-hardware, nixos-18.03pre130151.268d99b1fe4, nixos-overlay"
  • nixpkgs: /nix/var/nix/profiles/per-user/root/channels/nixos/nixpkgs

I have /nix/store/xxdnjsc6vhqw6wb8zk9v6663grr4n623-redshift-1.11 and /nix/store/y0j5m8lcrs81870sgmdijjn5ha86h5zm-redshift-1.11 in the store that do not have a cache file, and /nix/store/y0j5m8lcrs81870sgmdijjn5ha86h5zm-redshift-1.11 that does have a cache file. Perhaps someone has recently made a change elsewhere that affected this elsewhere, but I don't know of any.

I wouldn't mind a preemptive change in redshift.

@lukateras lukateras dismissed their stale review February 28, 2018 20:10

Incorrect judgement

@rardiol
Copy link
Contributor Author

rardiol commented Feb 28, 2018

Commit 47f25fa fixes this on redshift. Is adding the hicolor dependency or manually removing the better way to fix the issue?

@lukateras
Copy link
Member

lukateras commented Feb 28, 2018 •
edited

I think dependency is better (more declarative) than a manual deletion of the cache file. The hook is not unrelated to hicolor-icon-theme after all.

@lukateras lukateras merged commit 275bc74 into NixOS:master Feb 28, 2018
@lukateras
Copy link
Member

Thanks!

@rardiol rardiol deleted the hicolor branch February 28, 2018 20:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jtojnar jtojnar jtojnar requested changes

@lukateras lukateras lukateras left review comments

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

Successfully merging this pull request may close these issues.

None yet
4 participants
@rardiol @lukateras @jtojnar @GrahamcOfBorg