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

Jump to bottom

spicetify-cli: init at 1.1.0 #96685

Merged
merged 1 commit into from Nov 10, 2020
Merged

spicetify-cli: init at 1.1.0 #96685

merged 1 commit into from Nov 10, 2020

Conversation

jonringer
Copy link
Contributor

@jonringer jonringer commented Aug 30, 2020 •
edited

Motivation for this change

was testing my nix-template tool, decided to try and package a go package

closes: #96419

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.

@ofborg ofborg bot requested a review from kalbasit August 30, 2020 19:06
@aaronjanse
Copy link
Member

Nice! I think spicetify-cli modifies some files in /nix/store/*spotify*/, so users will need a little glue to get this working. Thank you @jonringer!

@jonringer
Copy link
Contributor Author

jonringer commented Sep 4, 2020 •
edited

Nice! I think spicetify-cli modifies some files in /nix/store/*spotify*/, so users will need a little glue to get this working. Thank you @jonringer!

ah, that will probably need to be upstreamed

EDIT: very import words

@glittershark
Copy link
Member

When actually running this it looks for jsHelper inside of the bin directory - I think we need:

    postInstall = ''
      cp -r ${src}/jsHelper $out/bin/jsHelper
    '';

@jonringer
Copy link
Contributor Author

I don't use spotify anymore...

@le-mich
Copy link

le-mich commented Oct 26, 2020

Are there any updates on this package?

@aaronjanse
Copy link
Member

I've manually gotten this to modify Spotify, so I plan to package that process next. But I don't have time to do so until at least Thursday, possibly not until next Tuesday. Of course, if someone else is willing to package it, feel free to do so; no need to wait for me

@aaronjanse
Copy link
Member

I've got a working derivation. Next I'll clean it up for submission to Nixpkgs

https://gist.github.com/aaronjanse/1783a6197d28e0aeda0d9f402175daf6

aaronjanse
aaronjanse approved these changes Nov 10, 2020
View reviewed changes
Copy link
Member

@aaronjanse aaronjanse left a comment

Choose a reason for hiding this comment

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

Thank you @jonringer! This looks great, and I was able to successfully use it to patch Spotify.

Would it be appropriate to submit a Spotify theme wrapper using spicetify-cli [1] to nixpkgs, or should I publish it as a 3rd-party flake?

[1] jonringer/nixpkgs@add-spicetify-cli...aaronjanse:aj-add-spotify-spiced

@jonringer
Copy link
Contributor Author

@aaronjanse I would probably expose themes as something similar to:

spotify.theme.spiced

implementation being something like:

let
  spotify = mkDerivation {
    ...
    passthru = {
       theme = {
         spice = spotify.overide { spotify-unwrapped = spotify-spiced; }
       };
    };
in spotify

@kalbasit
Copy link
Member

Is this PR still in progress? @jonringer can you convert it to Draft if it requires further changes?

@aaronjanse
Copy link
Member

I think this PR is complete. The discussion above is planning for creating another PR that uses the package added here

@jonringer
Copy link
Contributor Author

I'm going to say that it's working as intended.

Additional improvements can be done in a later PR

@jonringer jonringer merged commit b2e857a into NixOS:master Nov 10, 2020
@jonringer jonringer deleted the add-spicetify-cli branch November 10, 2020 22:48
@svrana svrana mentioned this pull request Nov 24, 2020
Closed
@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/adding-proper-spicetify-support-to-nix/11202/1

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

@aaronjanse aaronjanse aaronjanse approved these changes

@kalbasit kalbasit Awaiting requested review from kalbasit

Assignees
No one assigned
Labels
8.has: package (new) 10.rebuild-darwin: 1-10 10.rebuild-darwin: 1 10.rebuild-linux: 1-10 10.rebuild-linux: 1 11.by: package-maintainer
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Packaging Request: spicetify-cli
6 participants
@jonringer @aaronjanse @glittershark @le-mich @kalbasit @nixos-discourse