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

vscode,vscode-insiders: fix insiders build #49716

Merged
merged 1 commit into from Dec 4, 2018
Merged

vscode,vscode-insiders: fix insiders build #49716

merged 1 commit into from Dec 4, 2018

Conversation

eadwu
Copy link
Member

@eadwu eadwu commented Nov 4, 2018 •
edited

Motivation for this change

Channel and version are basically just static so they were moved into the derivation and added isInsiders for the specific build fix.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option 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/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Fits CONTRIBUTING.md.

@eadwu eadwu force-pushed the vscode/insiders branch 3 times, most recently from fa0fd1a to ad064c5 Compare November 4, 2018 19:23
@worldofpeace
Copy link
Contributor

@eadwu
I'm not seeing the ability to use vscode insiders within nixpkgs prior to this change.
So the commit msg fix insiders build is confusing.

Also if I were to override vscode with isInsiders it's still fetching the stable .deb source.

@eadwu
Copy link
Member Author

eadwu commented Nov 5, 2018 •
edited

The version and channel variables made me think it was probably at least thought of beforehand, and if you fixed that getting the source would work at least. isInsiders is really just suppose to a patch in case you ever use the insidiers version, so it's meant for when you override the src as well, since I don't really think nightly releases are meant for nixpkgs.

vscodeInsiders = (super.vscode.override {
    isInsiders = true;
  }).overrideAttrs(oldAttrs: rec {
    name = "vscode-insiders-${version}";
    version = "1541336565";

    src = self.pkgs.fetchurl {
      name = "VSCode_latest_linux-x64.tar.gz";
      url = "https://vscode-update.azurewebsites.net/latest/linux-x64/insider";
      sha256 = "0angszy5rp397w3nxa3bqlpylyi9aqdlixzchlg6wlpq9rrzqspy";
    };
  });

@veprbl veprbl mentioned this pull request Nov 28, 2018
Closed
9 tasks
@eadwu eadwu changed the title vscode: fix insiders build vscode,vscode-insiders: fix insiders build Nov 28, 2018
veprbl
veprbl approved these changes Dec 4, 2018
View reviewed changes
Copy link
Member

@veprbl veprbl left a comment

Choose a reason for hiding this comment

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

Looks good to me

@veprbl veprbl merged commit aa85911 into NixOS:master Dec 4, 2018
@eadwu eadwu mentioned this pull request Dec 16, 2018
Closed
eddyb
eddyb reviewed Aug 10, 2019
View reviewed changes
pkgs/applications/editors/vscode/default.nix
@@ -1,9 +1,9 @@
{ stdenv, lib, fetchurl, unzip, atomEnv, makeDesktopItem,
gtk2, wrapGAppsHook, libXScrnSaver, libxkbfile, libsecret }:
gtk2, wrapGAppsHook, libXScrnSaver, libxkbfile, libsecret,
isInsiders ? false }:
Copy link
Contributor

Choose a reason for hiding this comment

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

How does one actually set this? I was hoping there would be vscode and vscode-insiders packages, but no suck luck.

Copy link
Member Author

Choose a reason for hiding this comment

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

Here's my overlay, not really deterministic but gets me my nightly releases.

vscode = (super.vscode.override {
    isInsiders = true;
  }).overrideAttrs(_: rec {
    name = "${pname}-${version}";
    pname = "vscode-insiders";
    version = "nightly";

    src = builtins.fetchurl {
      name = "VSCode_latest_linux-x64.tar.gz";
      url = "https://vscode-update.azurewebsites.net/latest/linux-x64/insider";
    };
  });

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, I see, I'll do the same, thanks!

@eadwu eadwu mentioned this pull request Jul 10, 2020
Closed
@eadwu eadwu deleted the vscode/insiders branch November 17, 2020 23:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@eddyb eddyb eddyb left review comments

@veprbl veprbl veprbl approved these changes

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
5 participants
@eadwu @worldofpeace @eddyb @veprbl @GrahamcOfBorg