Skip to content
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

Modernize administrator and Mono version checks #3933

Merged
merged 1 commit into from
Dec 1, 2023

Conversation

HebaruSan
Copy link
Member

@HebaruSan HebaruSan commented Nov 30, 2023

Motivations

  • CKAN has a check for whether the user is privileged and an --asroot flag to bypass this, but it's Linux-only. Windows users are just as likely to benefit from this guardrail. It's also implemented entirely within CmdLine, even though it could in principle apply more generally.
  • There's a check that prints a warning if the user is on a version of Mono before 3.1.0, which duplicates platform-specific logic from Core in CmdLine, and I'm pretty sure you need a much newer version of Mono to run current CKAN
  • Platform.IsMonoFourOrLater and Platform.IsOnMonoFourOrLater have been unused since Master search bar and misc GUI clean-up #3041

Changes

  • Now the root user check is moved to Platform.IsAdministrator()
    • Now it has an IsWindows section that checks for administrator or system roles
    • Now the messages for the root user check are platform agnostic
    • I wanted to add an --asadmin alias for the --asroot flag, but I could not find a mechanism for that in the command line parser library, and I don't want to break existing scripts or shortcuts by changing it
  • Now the duplicative Mono versioning logic from Platform.IsOnMonoFourOrLater and CmdLine.Options.CheckMonoVersion is moved to a Platform.MonoVersion field
    • Now the Mono warning is printed if they're on a version before 5.0.0 (stored in Platform.RecommendedMonoVersion), which is still pretty old but not as guaranteed ancient as 3.1.0
    • This uses a new RegexExtensions.TryMatch function to check a regex for a match and use its output within a functional-style expression

@HebaruSan HebaruSan added Enhancement New features or functionality Easy This is easy to fix Cmdline Issues affecting the command line Core (ckan.dll) Issues affecting the core part of CKAN Windows Issues specific for Windows Mono Issues specific for Mono labels Nov 30, 2023
@HebaruSan HebaruSan requested a review from techman83 November 30, 2023 18:47
Copy link
Member

@techman83 techman83 left a comment

Choose a reason for hiding this comment

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

This is super clean, and removing unused code is always a win 🎉 thanks @HebaruSan !

@HebaruSan HebaruSan merged commit d6ecc87 into KSP-CKAN:master Dec 1, 2023
8 checks passed
@HebaruSan HebaruSan deleted the feature/windows-admin-check branch December 1, 2023 05:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Cmdline Issues affecting the command line Core (ckan.dll) Issues affecting the core part of CKAN Easy This is easy to fix Enhancement New features or functionality Mono Issues specific for Mono Windows Issues specific for Windows
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants