Skip to content

uutils-coreutils-lean@0.10.0: Improve pre_install script - #8393

Merged
z-Fng merged 2 commits into
ScoopInstaller:masterfrom
God-damnit-all:patch-8
Aug 16, 2026
Merged

uutils-coreutils-lean@0.10.0: Improve pre_install script#8393
z-Fng merged 2 commits into
ScoopInstaller:masterfrom
God-damnit-all:patch-8

Conversation

@God-damnit-all

@God-damnit-all God-damnit-all commented Aug 16, 2026

Copy link
Copy Markdown
Contributor

Relates to #8363

I'd included an install script in my last PR that the reviewer must've decided was too elaborate and it was trimmed down to:

Get-ChildItem "$dir\*" -Include *.exe | Where-Object Name -ne 'coreutils.exe' | Remove-Item

But I have some issues with this. The main one being, that if for any reason $dir should ever be $null (like if there's ever a bug in a future Scoop update), this would (instead of what it was meant to do) silently delete any exe file not named coreutils.exe in the current working directory, which could be disasterous.

Furthermore, should [ or ] be in the path for any reason, those would be interpreted as wildcard characters and the command would fail.

It's also redundant, if you're going to use a wildcard for -Path anyway, you might as well use do Get-ChildItem "$dir\*.exe" and skip -Include/-Filter altogether.

Also, -Filter is considered superior to -Include in the official documentation: "Filters are more efficient than other parameters. The provider applies the filter when the cmdlet gets the objects rather than having PowerShell filter the objects after they're retrieved." (The caveat is that -Filter can only be used for the FileSystem provider, which is why it doesn't fully supersede -Include.)

This is the new line that prevents strange edge-cases and is more performant:

Get-ChildItem -LiteralPath $dir -Filter *.exe | Where-Object Name -ne 'coreutils.exe' | Remove-Item
coderabbitai suggested I add the -File parameter, and while I was at it, I also single-quoted *.exe for consistency's sake.

Get-ChildItem -LiteralPath $dir -Filter '*.exe' -File | Where-Object Name -ne 'coreutils.exe' | Remove-Item

I understand I'm being picky, but I'm hoping that Scoop eventually defaults to always using the much safer -LiteralPath parameter for its operations.

  • Use conventional PR title: <manifest-name[@version]|chore>: <general summary of the pull request>
  • I have read the Contributing Guide

@coderabbitai

coderabbitai Bot commented Aug 16, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 636d70f2-a147-41d2-bfe7-94237f839b9d

📥 Commits

Reviewing files that changed from the base of the PR and between 3c0f859 and 19ad1b6.

📒 Files selected for processing (1)
  • bucket/uutils-coreutils-lean.json
🚧 Files skipped from review as they are similar to previous changes (1)
  • bucket/uutils-coreutils-lean.json

Included review availability: Your plan includes up to 4 reviews per rolling hour; 2 remain after this review.


Summary by CodeRabbit

  • Bug Fixes
    • Improved installation cleanup to reliably identify executables in the target directory.
    • Ensured coreutils.exe is preserved while other executables are removed.

Walkthrough

The pre_install command now enumerates .exe files directly under $dir with -LiteralPath and -Filter. It still preserves coreutils.exe and removes other executables.

Changes

Coreutils installation cleanup

Layer / File(s) Summary
Literal executable enumeration
bucket/uutils-coreutils-lean.json
The pre_install command uses -LiteralPath $dir -Filter '*.exe' and retains the existing executable exclusion and removal logic.

Possibly related PRs

  • ScoopInstaller/Main#8363: Both PRs modify the pre_install executable cleanup command in bucket/uutils-coreutils-lean.json. PR #8393 refines the script introduced by this PR.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description check ✅ Passed The description explains the safety and performance changes, references an issue, and completes both required checklist items.
Title check ✅ Passed The title identifies the manifest and version and clearly states that the pre_install script improves.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1


ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 5371b851-e1e5-48ff-9c6d-791792d69318

📥 Commits

Reviewing files that changed from the base of the PR and between 1c73596 and 3c0f859.

📒 Files selected for processing (1)
  • bucket/uutils-coreutils-lean.json

Included review availability: Your plan includes up to 4 reviews per rolling hour; 3 remain after this review.

Comment thread bucket/uutils-coreutils-lean.json Outdated
@God-damnit-all

Copy link
Copy Markdown
Contributor Author

/verify

@github-actions

Copy link
Copy Markdown
Contributor

All changes look good.

Wait for review from human collaborators.

uutils-coreutils-lean

  • Lint
  • Description
  • License
  • Hashes
  • Checkver
  • Autoupdate

Check the full log for details.

@z-Fng z-Fng changed the title uutils-coreutils-lean@0.10.0: path safety revision uutils-coreutils-lean@0.10.0: Improve pre_install script Aug 16, 2026
@z-Fng
z-Fng merged commit 6f02b40 into ScoopInstaller:master Aug 16, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants