vscode: Fix "Save as Admin" #49643 #102010
Conversation
| @@ -69,6 +69,16 @@ in | |||
| dontBuild = true; | |||
| dontConfigure = true; | |||
|
|
|||
| patchPhase = | |||
| if system != "x86_64-darwin" then '' | |||
There was a problem hiding this comment.
Can you elaborate why this is not required on darwin?
There was a problem hiding this comment.
Does the darwin flavor of sudo-prompt even use the same scripts to achieve administrator access? How does nix even handle sandboxing shell commands on an OS like that?
There was a problem hiding this comment.
As I suspected:
https://github.com/jorangreef/sudo-prompt/blob/master/index.js
sudo-prompt uses a completely different code path on macOS/Darwin to achieve the sudo prompt, going as far as bundling a special program in a zip file, unpacking it, and running it, to elevate the requested command.
The above patch for /bin/bash and /usr/bin/pkexec processes only applies to NixOS, and any other Linux system adhering to Nix file paths for these two utilities.
I wasn't sure if this was the correct way to limit it, though. I took the hint from the install process, which gates Darwin specific install process from everything else, since only Darwin will be installing a single .app bundle.
I'm just going to guess Nix doesn't target native Windows? If it does, then this check should specifically apply only to Linux versions.
There was a problem hiding this comment.
I would prefer if we apply this unconditionally to prevent bit rot as long as it does not break something not already broken on darwin.
There was a problem hiding this comment.
Sure, you can apply it unconditionally, but upstream only supplies binary packages for x86_64 anyway.
|
Result of 3 packages built:
|
|
Friendly bump 😃 |
|
Lgtm, not able to test it atm tho
…On Wed, 13 Jan 2021, 16:48 Nicolas Berbiche, ***@***.***> wrote:
Friendly bump 😃
—
You are receiving this because your review was requested.
Reply to this email directly, view it on GitHub
<#102010 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABV7PJYMRDBKQLFO4RSTIYLSZW6GBANCNFSM4TDNVB6Q>
.
|
| @@ -69,6 +69,16 @@ in | |||
| dontBuild = true; | |||
| dontConfigure = true; | |||
|
|
|||
| patchPhase = | |||
There was a problem hiding this comment.
| patchPhase = | |
| postPatch = |
| @@ -69,6 +69,16 @@ in | |||
| dontBuild = true; | |||
| dontConfigure = true; | |||
|
|
|||
| patchPhase = | |||
| if system != "x86_64-darwin" then '' | |||
There was a problem hiding this comment.
| if system != "x86_64-darwin" then '' | |
| lib.optionalString (system != "x86_64-darwin") '' |
| sed -i 's|/bin/bash|${bash}/bin/bash|' tmp/sudo-prompt/index.js | ||
| ${nodePackages.asar}/bin/asar pack tmp ./resources/app/node_modules.asar | ||
| rm -rf tmp | ||
| '' else ''''; |
There was a problem hiding this comment.
| '' else ''''; | |
| ''; |
|
I'm just gonna close this, as I don't intend to use Nix again any time in the future anyway. |
|
@kode54 sorry for the cc, the situation is unfortunate, thanks for the groundwork in any event 👍 . |
Janik-Haag
added
the
12. first-time contribution
Motivation for this change
I decided to fix something that I found was broken, and saw in the relevant issue that a fix was already applied to another package in a similar fashion, but had not made its way to this package yet.
Things done
sandboxinnix.confon non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"./result/bin/)nix path-info -Sbefore and after)