vscode: Fix vscode-with-extensions with #70486 #71264
Conversation
| @@ -71,6 +71,8 @@ in | |||
| if system == "x86_64-darwin" then '' | |||
| mkdir -p "$out/Applications/${longName}.app" $out/bin | |||
| cp -r ./* "$out/Applications/${longName}.app" | |||
|
|
|||
| # vscodium's macOS release zip dosn't rename the code exectuable properly. | |||
There was a problem hiding this comment.
this applies to vscode also right?
There was a problem hiding this comment.
vscode also has the executable named code yes.
There was a problem hiding this comment.
I was just asking because it says vscodium
| # vscodium's macOS release zip dosn't rename the code exectuable properly. | |
| # macOS release zip dosn't rename the code exectuable properly. |
There was a problem hiding this comment.
The rename only applies to vscodium, and only for the macOS release zip. The vscode package doesn't need the rename, nor does the linux package of vscodium, so I felt it better to err on the side of verbosity.
|
Conflicts will arise from #71251, otherwise, the changes for x86 look fine, can't really test the darwin changes. |
| mkdir -p "$out/Applications/${longName}.app/Contents/MacOS" | ||
|
|
||
| for path in PkgInfo Frameworks Resources _CodeSignature Info.plist; do | ||
| cp -r "${vscode}/Applications/${longName}.app/Contents/$path" "$out/Applications/${longName}.app/Contents/" |
There was a problem hiding this comment.
let's just link these paths, otherwise it would end up duplicating the package closure size
| cp -r "${vscode}/Applications/${longName}.app/Contents/$path" "$out/Applications/${longName}.app/Contents/" | |
| ln -s "${vscode}/Applications/${longName}.app/Contents/$path" "$out/Applications/${longName}.app/Contents/" |
btw, not blocking for this PR, can we replace this runCommand with buildEnv?
There was a problem hiding this comment.
I'm having some trouble testing this with nix-review wip -- not sure if I'm not understanding the tool properly, or what. I'm not sure if macOS will handle those directories being a symlink though.
nix-review wip run (with the suggested change applied):
There was a problem hiding this comment.
it's an error unrelated to this PR. I've created another issue #71544.
There was a problem hiding this comment.
@AmandaCameron are you still hitting the same error on nix-review wip ?
There was a problem hiding this comment.
I tried committing the change instead of just using it from a dirty tree, but now it's just saying "No diffs detected"? I'm confused, and not sure if I'm just overly tired today or if nix-review wip isn't behaving how I'd think it should?
There was a problem hiding this comment.
[
nix-review wipis] just saying "No diffs detected"?
If you staged your changes, you need to add --staged flag
|
Thank you for your contributions.
|
stale
bot
added
the
2.status: stale
|
i was resolving the conflict and the feedback but couldn't push straight to it so made a new pr #133407 |
stale
bot
removed
the
2.status: stale
Motivation for this change
Make it so that under nix-darwin the
vscode-with-extensionspackage appears inside~/Applications/Nix AppsThings done
This appliea #70486 to the
vscode-with-extensionspackage as well.In addition, I added a suggested comment from the original MR that didn't . make it through before it got merged.
sandboxinnix.confon non-NixOS)nix-shell -p nix-review --run "nix-review wip"./result/bin/)nix path-info -Sbefore and after)Notify maintainers
cc @eadwu @Synthetica