-
-
Notifications
You must be signed in to change notification settings - Fork 543
Allow Executable Selection #3703
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
base: main
Are you sure you want to change the base?
Conversation
NPM, Scoop, Chocolatey, and PowerShell have not been implemented as they are awaiting clarification.
f3096e3
to
9d29cf6
Compare
@marticliment - I have a question about the way certain package managers run their commands. NPM and Scoop both seem to use PowerShell to run their commands instead of running them directly, whereas no other package manager does it that way. What was the reasoning behind this? Was it that whole thing with PowerShell where stuff adds "PowerShell Aliases" instead of actually properly installing it? Basically what I'm asking is is it necessary to keep that in, or does the Also, the way this PR is implemented would make it fairly simple for system chocolatey and bundled chocolatey to just be two parts of the same selection, removing the need for a "use system chocolatey" checkbox. Would you want that, or do you think it would be too confusing and the checkbox should stay? We can have it use bundled chocolatey if there's nothing else, since that's the only path, or use system chocolatey if one was found, which I think was something requested and does make sense (to have bundled chocolatey be only for users without chocolatey installed). |
Also, tests are failing due to missing language key |
Is there a way to show a restart required banner for the package manager settings? It doesn't seem like the function is still there... |
Yes, it is. This happens because scoop and npm are not executable files, but rather scripts, so on certain conditions they wouldn't work as expected, unless run from a shell such as cmd or powershell |
We can have it use bundled chocolatey if there's nothing else, since that's the only path, or use system chocolatey if one was found, which I think was something requested and does make sense (to have bundled chocolatey be only for users without chocolatey installed). I guess this makes sense. |
Don't worry about this, I'll be fixing this soon on main |
Yes, you should be able to invoke the |
…nd not the path-infused bundled chocolatey
I'm unable to coerce PowerShell into properly executing the custom script paths; I updated the executed command to effectively be I could have it only use the npm PS1 scripts, but I don't know if every npm installation provides one... |
It looks like the Perhaps, anothet possible solution could be running through an inner command prompt |
Does scoop have a ps1 script? I don't have it installed. I feel like cmd is probably more robust if the users want to change their npm location... I'll try it when I can and see if they work. |
Give it a try, but I have experienced issues with CMD and encoding, so please be careful there. At CoreData, there is a CodePage (or Encoding or sth property), which loads the user's codepage, to build a compatible encoding to pass to Process(). Then, special characters will work as expected. |
Should I change it to allow the selection of any executable, or leave it as a dropdown? |
I would leave it as a drop down |
@mrixner, the secure settings PR has been merged |
OK. I can't work on this next week but will finish it when I have time. |
don't worry about timing 🙂. |
For future reference- |
I would go with a switch that enables the user to change this setting (see how command-line arguments are handled) , and then let this setting be a regular dictionary setting. I think it would be too much of a hassle to have the user UAC each time the path is changed, and with only an initial toggle the point would be already achieved... |
Also, for the dictionary settings thing, you could go Setting[package manager] |
This works great, but requires an extra quotation mark to be added to the end of the command - not too big a deal until dealing with update / install commands and stuff. Using |
Any user suspected of farming GitHub activity with crypto purposes will get banned. Submitting broken code wastes the contributors' time, who have to spend their free time reviewing, fixing, and testing code that does not even compile breaks other features, or does not introduce any useful changes. I appreciate your understanding.
This PR allows the user a (limited, due to security concerns) ability to change the package manager executables used by the updater. It also makes it so the log page opens debug logs by default on debug builds.
TODO:
Chocolatey.cs:322
: I'm not sure what this is doing, and thus I didn't port this. I also did not remove the setting from the telemetry, as the setting stays how it was before it was ported and it is useful information.I would suggest mentioning that the system chocolatey stuff has changed in the release notes. I had it automatically port old "UseSystemChocolatey" configs to setting the default path.
Sorry for all the formatting-only changes, my editor started autoformatting and I don't really know how to turn it off...
I don't think it closes these issues, specifically the FNM ones, because those are really about being able to choose any path, since I don't know if the FNM install directory is located by UniGetUI - However, there is now a simple fix for people with similar issues, which is to add the standard install directories for their tools to the search path in
LoadAvailablePaths
.Relates to #1855
Relates to #3447
Relates to #2658
Relates to #3313 (not closing, because I want to fix this in a better way with custom destinations)
Possibly fixes #3655
Possibly fixes #3753