ScoopInstaller / Scoop

A command-line installer for Windows.
https://scoop.sh
Other
21.36k stars 1.41k forks source link

[Bug] Scoop's internal functions conflict with user-defined PowerShell functions/aliases #5196

Open ev-dev opened 2 years ago

ev-dev commented 2 years ago

Bug Report

Current Behavior

Occasionally, I'll create some PowerShell function/alias which happens to have the same name as one of Scoop's internal functions, and it can cause assorted problems within normal scoop operations.

A simple example is the one I ran into: I have my own 'warn' alias for a personal PowerShell function defined in my $PROFILE (meaning it's available in the global scope for all sessions), and although its functionality is similar to Scoop's internal warn function (definied in lib/core.ps1:151), it still caused errors whenever a scoop command called warn internally, like when a shim is being overwritten.

Expected Behavior

Scoop's internal functions used throughout various scoop commands, should avoid clobbering with external function/alias/cmdlet/executable/etc names whenever possible.

Additional context/output

Let me preface my comments with this: I'm a big fan of scoop and its ecosystem/community. I believe the simplicity of how it approaches system package management, is a core strength that makes it a preferrable solution for myself and many others, when compared to other Windows tools like chocolatey/winget.

That said, the source code (this repo) has a lot of aspects that seemed to cater more towards the development of scoop itself than to the end-user. The naming of internal functions, especially those defined in core.ps1 like warn,info,env,fullpath,etc. is an example where just a bit more attention to end-user could benefit the overall maintainability of the source code and longevity of scoop overall.

Possible Solutions

  1. Rename several of scoop's internal functions in some standardized way, even a prefix like __warn for these function names is far less likely to conflict with external names. Ideally, some agreed upon function name standard for internal functions like OriginalFuncName-ScoopInternal, or ScoopInternal-OrigName kind of technique, which could be validated with PSScriptAnalyzer during development or via tests (alternatively, tests hooked into PRs could instead check new code for valid namings)

    • PRO: Would improve readability of the source code
    • CON: It's a chore to update all the calls to all of these renamed functions throughout the codebase, and then continue to use these new names in future development
  2. Migrate scoop to use PowerShell Module features, which enable more control over scoping/prefixing

    • PRO: Aligns with scoop's Why PowerShell? wiki and embraces the slow demise of cmd.exe
    • CON: Assumes there's an insignificant amount of scoop users who do not use PowerShell at all
  3. Other ideas?

Notes

I'd be happy to work at this if maintainers agree it'd be worth the effort. I've already made a branch on my own machine which has prefixes all of scoop's internal functions with __ just as a self-fix for the issue I described. Thanks again to all those who contribute!

niheaven commented 2 years ago

That's great! And please submit the PR, the simple renaming should be a temporal solution before we have time to refactor all the code to Verb-Noun...

aetonsi commented 1 year ago

Hi i just stumbled upon this problem.. i can't use scoop from a pwsh with my $PROFILE file loaded since it contains an alias to env like the following:

function fn{write-error SMT; exit 1}
set-alias -Name env -Value fn

Scoop is practically unusable if called from a session where certain alias/functions have been defined, i always have to run a -noprofile session just to update apps