nixpkgs icon indicating copy to clipboard operation
nixpkgs copied to clipboard

novnc: correctly install and point to `websockify`

Open dwf opened this issue 3 years ago • 2 comments

utils/novnc_proxy tries to download and run the websockify package in the directory where the script is located, which doesn't work because the nix store is read-only. This patches the script to point to nix-installed websockify.

As it's logically a separate change I separated this and ##188897 into separate PRs. I'll need to rebase this once #188897 is submitted.

Description of changes
Things done
  • Built on platform(s)
    • [x] x86_64-linux
    • [ ] aarch64-linux
    • [ ] x86_64-darwin
    • [ ] aarch64-darwin
  • [ ] For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • [ ] Tested, as applicable:
  • [x] Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • [ ] Tested basic functionality of all binary files (usually in ./result/bin/)
  • 22.11 Release Notes (or backporting 22.05 Release notes)
    • [ ] (Package updates) Added a release notes entry if the change is major or breaking
    • [ ] (Module updates) Added a release notes entry if the change is significant
    • [ ] (Module addition) Added a release notes entry if adding a new NixOS module
    • [ ] (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
  • [x] Fits CONTRIBUTING.md.

dwf avatar Aug 30 '22 01:08 dwf

@NeverBehave Any thoughts on this one?

dwf avatar Sep 26 '22 01:09 dwf

@AndersonTorres Similarly, you might be an appropriate second reviewer for this PR.

dwf avatar Sep 26 '22 01:09 dwf

@applePrincess Likewise, mind giving this a look?

dwf avatar Sep 29 '22 00:09 dwf

As it's logically a separate change I separated this and #https://github.com/NixOS/nixpkgs/pull/188897 into separate PRs. I'll need to rebase this once https://github.com/NixOS/nixpkgs/pull/188897 is submitted.

Wait, are these changes somehow dependent?

AndersonTorres avatar Sep 29 '22 01:09 AndersonTorres

Wait, are these changes somehow dependent?

They aren't, they're just so close by that I'm betting on a merge conflict. Maybe git will pleasantly surprise me.

dwf avatar Sep 29 '22 23:09 dwf

Rebase

AndersonTorres avatar Sep 30 '22 00:09 AndersonTorres

Done. I concatenated to the list rather than simply added an element to make it clear that the with statement is scoped/meant for to the first part, not sure if this is considered good style or not.

dwf avatar Sep 30 '22 00:09 dwf

Next time you can concentrate related commits in a single PR.

AndersonTorres avatar Oct 01 '22 01:10 AndersonTorres