Make install scripts more consistent over different operating systems

[Geweldig]

PR Summary

I noticed a couple of inconsistencies when reading through the install bash scripts.

• Make documentation for switches consistent over all files.
• Replace all sed implementations of lowercase with a more maintainable tr implementation.
• Set the OS variable in every install script, making it so previously unused checks are actually used.
• Exit with a non-zero exit code when the script reaches an illegal state.

PR Context

A lot of people, including myself, read the install scripts before executing them. While doing so I noticed they contained inconsistencies. For example, certain flags you can pass to the install script were either undocumented or had an incorrect description. This PR fixes some of these inconsistencies, which should make them easier to maintain and easier to read.

PR Checklist

• PR has a meaningful title
  • Use the present tense and imperative mood when describing your changes
• Summarized changes
• Change is not breaking
• Make sure all .h, .cpp, .cs, .ps1 and .psm1 files have the correct copyright header
• This PR is ready to merge and is not Work in Progress.
  • If the PR is work in progress, please add the prefix WIP: or [ WIP ] to the beginning of the title (the WIP bot will keep its status check at Pending while the prefix is present) and remove the prefix when the PR is ready.
• User-facing changes
  • Not Applicable
  • OR
  • Documentation needed
    • Issue filed:
• Testing - New and feature
  • N/A or can only be tested interactively
  • OR
  • Make sure you've added a new test if existing tests do not effectively test the code changed
    • Add [feature] to your commit messages if the change is significant or affects feature tests

[msftclas]

All CLA requirements met.

[Geweldig]

I used ShellCheck as a linter locally, as it seemed to create the same output as Codacy. I also noticed some of the files contained invalid bash (if then if instead of if then fi), which has also been resolved.

[RDIL]

[Geweldig]

@iSazonov thanks for the great suggestions! I was not entirely sure about the correct casing for Visual Studio Code, as I could find it as VSCode, vscode and VS Code. Because of this I decided to just leave it all lowercase. I have implemented your suggestions, thanks for the review!

[Geweldig]

It appears the connection to windowsitpro.com has timed out, which is why the static analysis has failed. The next connection to it however does finish successfully. Should the CI be restarted or is this not an issue?

[iSazonov]

Restart CI-static-analysis.

[TravisEz13]

@DarwinJS Would you mind giving this a first pass?

[TravisEz13]

Okay... I tried to add support to the CI for a representative portion of the distros.

AmazonLinux image failed because it was missing adduser. (I'll ask in the docker repo if we want to add this to get this scenario working.)

OpenSUSE failed because it Azure DevOps could not find pwsh (I can fix this later)

[TravisEz13]

So CentOS testing was the only distro added. cc @adityapatwardhan

[DarwinJS]

Travis - Amazon optimizes every purpose specific use of Amazon Linux and of course specifically the one for containers.

Since the size of the Amazon built Amazon Linux containers affects their server density globally across all customers, it will likely be necessary to add back what you need to get CI running.

PS For my local testing of Amazon Linux 2 - I have actually created some antipattern containers that have the entire EC2 build version: https://cloudywindows.io/post/three-amazon-linux-2-containers-for-testing/

[DarwinJS]

@Geweldig - did you test your changes on the major distros?
@TravisEz13 - how do we know what distros these install scripts are tested on - is there a manifest somewhere?

[Geweldig]

@DarwinJS I have verified it to be working for the following operating systems:

• Centos7
• Ubuntu 18.04
• Debian 9 (Assuming lsb-release has been shipped with it, might be worthwhile to look at an alternative check as it is not always included)
• Fedora 29
• macOS 12

Which should cover the major distros. I have tried the installer on openSUSE Leap 15 (as 42.3 is reaching EOL in July) where I have not been able to get it to work. However, when trying to run the script which is currently on master I couldn't get it to work either. This problem is twofold, as the OS is not recognized by the main script and the openSUSE installer which is currently provided contains invalid bash. Unfortunately I am not familiar with openSUSE and would need some time to figure out why it currently is not recognized by the script on master and find a robust solution to recognize the operating system.

[TravisEz13]

We have not produced an appimage in a long time. This in not blocking, but I think this code can either go, or be replaced with a check for snapd and then use snapd

[Geweldig]

Adding a check for snapd sounds like a good replacement, but might not be very robust as snapd is not yet standard in a lot of distributions (I believe it's currently only shipped with Ubuntu). Because of this I would argue that deleting the code for appimage would make more sense, and implementing a snap install when Snappy is more mature/widely used.

[TravisEz13]

@DarwinJS The CI YML for testing these scripts is here:

PowerShell/.vsts-ci/install-ps.yml

Lines 45 to 64 in 7031954

	pool: Hosted Ubuntu 1604
	# amazonlinux was missing adduser
	- template: templates/install-ps-phase.yml
	parameters:
	scriptName: ./tools/install-powershell.sh
	jobName: InstallPowerShellCentOS
	pool: Hosted Ubuntu 1604
	container: mcr.microsoft.com/powershell:centos-7
	# VSTS could not find pwsh in:
	# mcr.microsoft.com/powershell:opensuse-42.3
	# could not repo locally
	- template: templates/install-ps-phase.yml
	parameters:
	scriptName: ./tools/install-powershell.sh
	jobName: InstallPowerShellMacOS
	pool: Hosted macOS

NOTE: where a container is listed, it overrides the pool. Like on line 54, to switch to CentOS.

Currently Ubuntu 16.04, CentOS 7, and macOS Mojave are in the list.

[DarwinJS]

Is it ok if I reverse these changes?
Amazon Linux 2 install is not working whether using the stand alone script or the main one. I'm not sure how many more are doing that?

[iSazonov]

Why revert? Do you know the issue root cause? Is it possible to fix?

[DarwinJS]

I don't know the root cause, but I let this slip through without realizing the distro fingerprinting had been altered to DEPEND on the parent script being called for each distro specific script. I had specifically designed it to not depend - the parent script was a convenience when wanting to target distros homogenously.

But then I wanted to get off using my original merge branch for linuxmint support which still works for Amazon Linux - and this one does not. (I use it here to create Amazon LInux 2 containers that have PowerShell: https://gitlab.com/DarwinJS/amazon-linux-2-container-with-powershell/blob/master/Dockerfile#L4)

So this all points to the changes not being comprehensively tested - something I was doing whenever I made broad-based changes.

Since these changes seemed to be mainly making the code prettier - not more functional - I'd rather revert to a known working state that I had tested across all distros - than try to fix and retest on all distros. I really don't have the time for the latter.

[iSazonov]

@Geweldig @TravisEz13 Thoughts?

[DarwinJS]

@DarwinJS I have verified it to be working for the following operating systems:

• Centos7
• Ubuntu 18.04
• Debian 9 (Assuming lsb-release has been shipped with it, might be worthwhile to look at an alternative check as it is not always included)
• Fedora 29
• macOS 12

Which should cover the major distros. I have tried the installer on openSUSE Leap 15 (as 42.3 is reaching EOL in July) where I have not been able to get it to work. However, when trying to run the script which is currently on master I couldn't get it to work either. This problem is twofold, as the OS is not recognized by the main script and the openSUSE installer which is currently provided contains invalid bash. Unfortunately I am not familiar with openSUSE and would need some time to figure out why it currently is not recognized by the script on master and find a robust solution to recognize the operating system.

OK, then maybe the solution is to consider Amazon Linux 2 a major distro and test and debug for it. Not only is Amazon treating it this way, but it was the first distro to have prebuilt cloud image template (AMIs) from the cloud provider with .NET Core and PowerShell Core preinstalled. That is a full Ec2 image, I want to use amazon linux 2 w/ PowerShell Core under Gitlab CI on Linux containers - so still need installation support.

[Geweldig]

@DarwinJS I indeed didn't test the install script on Amazon Linux 2, as I did not have access to a build environment for it. However, I believe the changes made to the install script shouldn't have broken anything as they only cleaned up unused and broken variables, but I'm obviously wrong. All changes which I believe could break the build on Linux, should also break all other scripts which we have tested to be working.

Do you have a specific error so we can pinpoint where the script goes wrong?

[iSazonov]

I have concerns about the rollback since the PR has been merged for a long time (we have a release after that).

I did not have access to a build environment for it.

Maybe you could cooperate with @DarwinJS directly?

[DarwinJS]

The following Docker commands will let you easily test:

# This Works:
docker run -v -it amazonlinux /bin/bash -c "curl -s https://raw.githubusercontent.com/DarwinJS/PowerShell/installpsh-debian-support-for-linuxmint/tools/installpsh-amazonlinux.sh | bash ; pwsh"

# This Does Not:
docker run -v -it amazonlinux /bin/bash -c "curl -s https://raw.githubusercontent.com/PowerShell/PowerShell/master/tools/installpsh-amazonlinux.sh | bash ; pwsh"

[Geweldig]

Ah I see what the problem is, I rewrote the OS checks in a single script and copied them over all other scripts. However, the script I originally based it on seems to not have included the check for amazonlinux. Not sure how I missed that, but it shouldn't be difficult to fix.

[Geweldig]

@DarwinJS I have opened #9967 to fix this issue, I'm currently verifying the install scripts again on all different operating systems. This has resolved the problem for amazon linux.

[TravisEz13]

@iSazonov @DarwinJS @Geweldig We have started to move the packaging/installation tools into separate repos that are maintained by the people that care about that tool. Honestly, long term I think we should do the same thing here. We could keep the main script with a warning to migrate.

cc @SteveL-MSFT

[iSazonov]

@TravisEz13 Could you please reference the new repo?

[TravisEz13]

@iSazonov This is a concept. There is no current. But Docker and Snap have been separated, if that is what you are talking about.