-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Enabled shimming of external applications #2072
Enabled shimming of external applications #2072
Conversation
Do you have an example for which this could be useful? |
It could be useful for virtually any case where script is being shipped without interpreter. For example,
Of course one can make support of every scripting language (and every version, yep) 'natively' in shims but that seems to be questionable complication from my point of view. |
Basically my point is that every programmer who writes scripts and wants to ship it to Windows systems needs to make some |
excitoon, just to let you know, two links (for wakeonlan and swaddle) are incorrect |
I've also found this patch useful for a tool I've been using (which is actually a php script). |
lib/install.ps1
Outdated
$bin = "$dir\$target" | ||
if(!(is_in_dir $dir $bin)) { | ||
abort "Error in manifest: bin '$target' is outside the app directory." | ||
if(test-path "$dir\$target") { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
test-path
should probably have -pathType leaf
to be safe
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. Actually I did not use Powershell before.
lib/install.ps1
Outdated
abort "Error in manifest: bin '$target' is outside the app directory." | ||
if(test-path "$dir\$target") { | ||
$bin = "$dir\$target" | ||
} elseif(test-path $target) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure what this check is for (if $target is absolute?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, just because absolute path is better than invalid path.
lib/install.ps1
Outdated
} elseif(test-path $target) { | ||
$bin = $target | ||
} else { | ||
$bin = search_in_path $target | ||
} | ||
if(!(test-path $bin)) { abort "Can't shim '$target': File doesn't exist."} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This needs to be something like if(!$bin) { ... }
instead. Currently, if we don't find the file, $bin
is null and this check blows up, causing lots of errors as the null value propogates.
Also there's no need to do the test-path
again.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, you're right.
da15cc3
to
b9850fa
Compare
Rebased first commit onto current master. |
One more thing about
If you write it like that, function will return a list consisting of joined return values from each element. That's totally counter-intuitive. If one replace that with regular |
I guess it is ready to merge/review again. Tested all mentioned cases. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM (but untested)
Hi.
Now one can make shims for external applications (system, other applications installed by scoop and so on).
First,
$bin
is to be searched in$dir
, then globally, then in user and systemPATH
.Examples: