Skip to content

Remove Non-APT PowerShell Installs Before Installing From the Microsoft Feed - #789

Merged
ptr727 merged 7 commits into
developfrom
fix/pwsh-non-apt-cleanup
Aug 17, 2026
Merged

Remove Non-APT PowerShell Installs Before Installing From the Microsoft Feed#789
ptr727 merged 7 commits into
developfrom
fix/pwsh-non-apt-cleanup

Conversation

@ptr727

Copy link
Copy Markdown
Owner

Summary

When install-tools.sh --upgrade powershell installs from Microsoft's apt feed, a prior direct-download install (the portable tar.gz build that PowerShell's install.sh or a repo's install-powershell.sh places at /opt/microsoft/powershell/7 with a /usr/local/bin/pwsh symlink) would remain on the host, sharing the same tree the apt package installs into.

This PR makes powershell_install detect and remove non-APT copies before apt installs, using dpkg ownership as the discriminator (the same test tool_unshadow applies): a path the powershell package owns is the APT install and stays; anything unowned is cleared. This is the mirror of docker_install, which removes conflicting APT packages before a native install.

Changes

  • powershell_install now calls powershell_remove_non_apt after microsoft_feed and before apt_install powershell.
  • powershell_non_apt_paths lists non-APT pwsh copies:
    • the /usr/local/bin/pwsh symlink
    • the whole /opt/microsoft/powershell/7 tree (removed whole, not just the binary, so leftover files an APT install does not carry cannot survive)
    • any other pwsh on PATH no package owns (absolute path only, same guard as tool_shadow_path)

Verification

  • shellcheck clean
  • bash -n clean
  • Full pytest suite passes (686 passed, 866 subtests)
  • ruff check and format clean

CopilotAI lite review requested due to automatic review settings August 17, 2026 03:22

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the Linux host setup tooling to prevent PowerShell installs from Microsoft's APT feed from colliding with prior direct-download (non-APT) PowerShell installs that place files under /opt/microsoft/powershell/7 and/or /usr/local/bin/pwsh.

Changes:

  • Add a pre-install cleanup step to detect and remove non-APT pwsh installs before running apt_install powershell.
  • Introduce powershell_non_apt_paths to enumerate unowned pwsh paths (as determined by dpkg-query -S) and powershell_remove_non_apt to remove them.
Suppressed comments (1)

host-setup/linux/install-tools.sh:749

  • The header comment for powershell_non_apt_paths describes "a pwsh that the powershell package does not own", but the implementation actually treats any dpkg-owned path as "owned" (same approach as tool_unshadow). Please reword to reflect that the discriminator is dpkg ownership, not the specific powershell package name.
# A pwsh that the powershell package does not own, one per line.
# The direct-download layout installs /opt/microsoft/powershell/7 (the same tree the apt package
# owns once installed) plus a /usr/local/bin/pwsh symlink, where the apt package instead owns
# /usr/bin/pwsh, so dpkg ownership is the discriminator between the two, the same test
# tool_unshadow applies to a shadowing copy: a path the powershell package owns is the apt install

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment threadhost-setup/linux/install-tools.sh Outdated
CopilotAI review requested due to automatic review settings August 17, 2026 03:26
@ptr727

ptr727 commented Aug 17, 2026

Copy link
Copy Markdown
OwnerAuthor

Suppressed finding (1), host-setup/linux/install-tools.sh:749: the comment described ownership by the powershell package, while the code checks ownership by any dpkg package. Fixed in 3a9e2e3: the comments now describe dpkg ownership as the discriminator and pass the repository prose lint. Review: #789 (review)

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Suppressed comments (2)

host-setup/linux/install-tools.sh:773

  • Same issue for the PATH-resolved pwsh: ! dpkg-query -S "$resolved" treats dpkg-query errors as "unowned" and could delete /usr/bin/pwsh (or other dpkg-managed paths) if dpkg-query fails unexpectedly. Treat exit 0/1 separately and fail hard on other exit codes.
 local resolved
resolved=$(type -P pwsh 2> /dev/null || true)
if [[ $resolved == /* ]] && ! dpkg-query -S "$resolved" > /dev/null 2>&1; then
paths+=("$resolved")
fi

host-setup/linux/install-tools.sh:760

  • The ownership check uses ! dpkg-query -S ... which treats any dpkg-query error (exit 2) as "unowned" and would then delete paths like /opt/microsoft/powershell/7 or /usr/local/bin/pwsh. With set -e disabled in if conditions, transient dpkg issues could cause removal of package-managed files. Handle dpkg-query exit codes explicitly (0=owned, 1=unowned, other=hard error) before deciding to delete.

This issue also appears on line 769 of the same file.

 if [[ -e /usr/local/bin/pwsh ]] && ! dpkg-query -S /usr/local/bin/pwsh > /dev/null 2>&1; then
paths+=(/usr/local/bin/pwsh)
fi
if [[ -e /opt/microsoft/powershell/7/pwsh ]] && ! dpkg-query -S /opt/microsoft/powershell/7/pwsh > /dev/null 2>&1; then
paths+=(/opt/microsoft/powershell/7)
fi

CopilotAI review requested due to automatic review settings August 17, 2026 03:36
@ptr727

Copy link
Copy Markdown
OwnerAuthor

Suppressed findings (2), review round: #789 (review)

Fixed in f8ad2aa: dpkg-query status 0 keeps package-owned paths, status 1 identifies unowned paths for removal, and any other status aborts cleanup. This covers the PATH-resolved check and both direct-download path checks.

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Suppressed comments (2)

host-setup/linux/install-tools.sh:772

  • This comment says "anywhere else on PATH", but the code only checks the single pwsh path that currently resolves via type -P. Consider rewording to match the actual behavior so readers do not assume we scan all PATH entries.
 # A package-unowned pwsh anywhere else on PATH is also a non-apt copy.

host-setup/linux/install-tools.sh:743

  • The comment above powershell_path_is_unowned reads like it describes output ("one per line"), but this is a predicate that returns 0/1. Updating the first line to describe the return semantics will prevent confusion for future readers.

This issue also appears on line 772 of the same file.

# A pwsh path that no dpkg package owns, one per line.

CopilotAI review requested due to automatic review settings August 17, 2026 03:39

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

Suppressed comments (2)

host-setup/linux/install-tools.sh:770

  • The /opt/microsoft/powershell/7 cleanup is gated on -e /opt/microsoft/powershell/7/pwsh. If the portable install is partially removed (tree still present but pwsh missing), the conflicting tree will be left behind. Since dpkg-query -S does not require the file to exist, it should be enough to gate on the directory existing and still query ownership of the expected pwsh path.
 if [[ -e /opt/microsoft/powershell/7/pwsh ]] && powershell_path_is_unowned /opt/microsoft/powershell/7/pwsh; then
paths+=(/opt/microsoft/powershell/7)
fi

host-setup/linux/install-tools.sh:767

  • [[ -e /usr/local/bin/pwsh ]] misses a dangling symlink, so a broken portable-install symlink can remain on PATH (and keep pwsh failing) even after the apt install, since the apt package does not replace /usr/local/bin/pwsh. Consider checking -L as well so dangling symlinks are cleaned up.
 if [[ -e /usr/local/bin/pwsh ]] && powershell_path_is_unowned /usr/local/bin/pwsh; then
paths+=(/usr/local/bin/pwsh)
fi

Comment threadhost-setup/linux/install-tools.sh
@ptr727

Copy link
Copy Markdown
OwnerAuthor

Suppressed findings (2), review round: #789 (review)

Fixed in 4ad94ec: the PATH comment now describes the single resolved path returned by , and the ownership predicate comment now describes its boolean return semantics.

CopilotAI review requested due to automatic review settings August 17, 2026 03:43
@ptr727

ptr727 commented Aug 17, 2026

Copy link
Copy Markdown
OwnerAuthor

Suppressed findings (2), review round: #789 (review)

Fixed in 7a65418: the cleanup now checks the installation directory itself, so partial trees are removed, and accepts [[ -L ]] so dangling /usr/local/bin/pwsh symlinks are removed too.

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Suppressed comments (1)

host-setup/linux/install-tools.sh:738

  • The comment implies that any pwsh path not owned by dpkg is specifically a "direct-download" install, but the logic also targets any non-dpkg-managed pwsh resolved from PATH (which could be snap/manual/etc). Rewording avoids implying a narrower invariant than the code enforces.
 # A pwsh path that no dpkg package owns is a direct-download install.
# The portable tar.gz build and the apt package share /opt/microsoft/powershell/7.
# Remove the direct-download copy before installing the apt package.
# This is the reverse of docker_install removing conflicting apt packages before a native install.
powershell_remove_non_apt

CopilotAI review requested due to automatic review settings August 17, 2026 03:46
@ptr727

Copy link
Copy Markdown
OwnerAuthor

Suppressed finding (1), review round: #789 (review)

Fixed in e6ddcac: the comment now describes all non-apt pwsh installs, including manual, snap, and other PATH-resolved sources, instead of claiming every unowned path is a direct-download install.

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Suppressed comments (1)

host-setup/linux/install-tools.sh:783

  • powershell_non_apt_paths can add an arbitrary absolute pwsh resolved from PATH and then powershell_remove_non_apt deletes it with rm -rf. Because the discriminator is dpkg ownership only, this can delete non-dpkg managed installs (for example snap-managed paths like /snap/bin/pwsh), potentially desynchronizing that package manager. It is safer to refuse and ask the user to uninstall/adjust PATH for known non-dpkg managers rather than deleting their files.
 local resolved
resolved=$(type -P pwsh 2> /dev/null || true)
if [[ $resolved == /* ]] && powershell_path_is_unowned "$resolved"; then
paths+=("$resolved")
fi

CopilotAI review requested due to automatic review settings August 17, 2026 03:50
@ptr727

Copy link
Copy Markdown
OwnerAuthor

Suppressed finding (1), review round: #789 (review)

Fixed in 6183e13: known PowerShell layouts remain removable, while an unowned pwsh resolved from any other absolute PATH location now stops with an instruction to remove or reprioritize it instead of deleting another package manager's files.

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

@ptr727
ptr727 merged commit c58fff1 into developAug 17, 2026
8 checks passed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@ptr727