Powershell plugin breaks path separators on Linux

[Flova]

As Powershell is theoretically also available on Linux it might be loaded by colcon to run commands due to its high internal priority. This is for example the case in GitHub actions, which come with Powershell installed. This auto selection is a bit strange, but not an issue per se.

Powershell and Windows use a semicolon as the path separator in e.g. the CMAKE_PREFIX_PATH but if Powershell is used on Linux this results in a missmatch as colcon and most other software use os.pathsep, which is a : on Linux. This results in e.g. the CMAKE_PREFIX_PATH containing both separator types, which breaks e.g. cmake which only expects : to be present on Linux. This then results in the paths around the ; not being discovered, as they are treated as one malformed/non existent path, resulting in package discovery errors.

The wrong semicolon in the CMAKE_PREFIX_PATH is inserted between the already build local packages and previous contents of the environment variable. This results in the issue being most pronounced if CMAKE_PREFIX_PATH already contains something, which is the case if e.g. pixi is used to supply some cmake packages.

Possible fixes:

• Uninstall Powershell. Depending on the environment this is not possible, also uninstalling is not enough as the environment variable PSModulePath which this extension uses to check for Powershell might prevail. Note that GitHub actions come with both by default.
• Disable the Powershell extension for Linux. Add a check similar to the ones that are already present for other shell extensions.
• Do some hack which manually sanitizes the paths inside the Powershell script when a Linux platform is detected.

[cottsay]

I don't think it will be difficult to just use the platform's native separator in this extension. Please take a look at #40, which seems to make things work as expected under Linux in my tests.

[cottsay]

This auto selection is a bit strange, but not an issue per se.

It looks like the logic to enable PowerShell as a primary shell candidate is pretty simple. It's possible that an innocuous change in GitHub Actions is now triggering it:

colcon-powershell/colcon_powershell/shell/powershell.py

Lines 64 to 69 in 87d1ea2

	# HACK heuristics to determine if the parent shell is PowerShell
	if sys.platform == 'win32':
	pathexts = os.environ.get('PATHEXT', '').lower().split(os.pathsep)
	self._is_primary = '.cpl' in pathexts
	else:
	self._is_primary = bool(os.environ.get('PSModulePath'))

Maybe we should consider implementing a more robust check, particularly on non-win32 systems.

[Flova]

I don't think it will be difficult to just use the platform's native separator in this extension. Please take a look at #40, which seems to make things work as expected under Linux in my tests.

That's nice. I gave it a quick look and it seems to be fine. That being said, I just uninstalled/deactivated Powershell for our Actions and will probably not invest too much further effort from my side into using PS on Linux.

[Flova]

This auto selection is a bit strange, but not an issue per se.

It looks like the logic to enable PowerShell as a primary shell candidate is pretty simple. It's possible that an innocuous change in GitHub Actions is now triggering it:

colcon-powershell/colcon_powershell/shell/powershell.py

Lines 64 to 69 in 87d1ea2

	# HACK heuristics to determine if the parent shell is PowerShell
	if sys.platform == 'win32':
	pathexts = os.environ.get('PATHEXT', '').lower().split(os.pathsep)
	self._is_primary = '.cpl' in pathexts
	else:
	self._is_primary = bool(os.environ.get('PSModulePath'))

Maybe we should consider implementing a more robust check, particularly on non-win32 systems.

I already saw that check during debugging. My original comment was just stating, that I was a bit surprised to see Powershell having the highest priority of all shells on a Unix system, not that it is available at all.

I totally agree that GitHub changed their action setup leading to this issue being discovered. They probably thought installing an additional package doesn't count as a breaking change (I would normally agree with that).

[cottsay]

I was a bit surprised to see Powershell having the highest priority of all shells on a Unix system

The check here should be determining if PowerShell is currently in use, not just if PowerShell is available. The PATHEXT trick on Windows is pretty good at doing this but we don't have an equivalent on Unix systems.

I think that having a higher priority for PowerShell does make sense if the check was functioning properly. If I'm in sh and invoke pwsh, I probably want to use PowerShell instead of sh even though my system can still use sh and the SHELL environment variable is still set to /bin/sh. Relying on PSModulePath just isn't sufficient to determine that the parent shell is PowerShell.