From d569e7880d50246f91e1ea9c55352f0416b2171e Mon Sep 17 00:00:00 2001 From: Marc Durdin Date: Mon, 20 Jul 2020 09:23:57 +1000 Subject: [PATCH 1/3] feat(windows): setup select tier from filename or parameter --- .../setup/Keyman.Setup.System.InstallInfo.pas | 27 +++++++-- ...eyman.Setup.System.OnlineResourceCheck.pas | 2 +- windows/src/desktop/setup/bootstrapmain.pas | 23 ++++++-- .../Keyman.System.Test.InstallInfoTest.pas | 56 ++++++++++++++++--- 4 files changed, 87 insertions(+), 21 deletions(-) diff --git a/windows/src/desktop/setup/Keyman.Setup.System.InstallInfo.pas b/windows/src/desktop/setup/Keyman.Setup.System.InstallInfo.pas index d351cb2f2b..79d5676e2a 100644 --- a/windows/src/desktop/setup/Keyman.Setup.System.InstallInfo.pas +++ b/windows/src/desktop/setup/Keyman.Setup.System.InstallInfo.pas @@ -117,6 +117,7 @@ type FIsNewerAvailable: Boolean; FTempPath: string; FShouldInstallKeyman: Boolean; + FTier: string; function GetBestMsi: TInstallInfoFileLocation; function GetPackageMetadata(const KmpFilename: string; p: TPackage): Boolean; public @@ -124,7 +125,7 @@ type destructor Destroy; override; procedure LoadSetupInf(const SetupInfPath: string); - procedure LocatePackagesFromFilename(Filename: string); + procedure LocatePackagesAndTierFromFilename(Filename: string); procedure LocatePackagesFromParameter(const Param: string); procedure LocatePackagesInPath(const path: string); @@ -152,6 +153,8 @@ type property StartDisabled: Boolean read FStartDisabled; property StartWithConfiguration: Boolean read FStartWithConfiguration; + property Tier: string read FTier write FTier; + property ShouldInstallKeyman: Boolean read FShouldInstallKeyman write FShouldInstallKeyman; end; @@ -181,6 +184,7 @@ begin FMsiLocations := TInstallInfoFileLocations.Create; FPackages := TInstallInfoPackages.Create; FStrings := TStringList.Create; + FTier := KeymanVersion.CKeymanVersionInfo.Tier; FShouldInstallKeyman := True; end; @@ -328,27 +332,38 @@ begin end; end; -procedure TInstallInfo.LocatePackagesFromFilename(Filename: string); +procedure TInstallInfo.LocatePackagesAndTierFromFilename(Filename: string); +const + SKeymanSetupPrefix = 'keyman-setup'; + SKeymanSetup_Alpha = SKeymanSetupPrefix+'-'+TIER_ALPHA; + SKeymanSetup_Beta = SKeymanSetupPrefix+'-'+TIER_BETA; + SKeymanSetup_Stable = SKeymanSetupPrefix+'-'+TIER_STABLE; var n: Integer; res: TArray; - id, FBCP47: string; + p, id, FBCP47: string; m: TMatch; begin // Get just the base filename Filename := ExtractFileName(ChangeFileExt(Filename, '')); // Strip " (1)" appended for multiple downloads of same file by most browsers - m := TRegEx.Match(Filename, '^(keyman-setup.+) \(\d+\)$'); + m := TRegEx.Match(Filename, '^('+SKeymanSetupPrefix+'.+) \(\d+\)$'); if m.Success then Filename := m.Groups[1].Value; // Look for our recognised pattern of keyman-setup.package_id.bcp47... res := TRegEx.Split(Filename, '\.'); - if (Length(res) < 2) or (res[0].ToLower <> 'keyman-setup') then - // No packages embedded in filename + if (Length(res) < 1) or not res[0].ToLower.StartsWith(SKeymanSetupPrefix) then + // No packages embedded in filename, or not a recognised filename pattern Exit; + // Look for an embedded tier in the filename, if not set, use default + p := res[0].ToLower; + if p.Equals(SKeymanSetup_Stable) then FTier := TIER_STABLE + else if p.Equals(SKeymanSetup_Beta) then FTier := TIER_BETA + else if p.Equals(SKeymanSetup_Alpha) then FTier := TIER_ALPHA; + n := 1; while n < Length(res) do begin diff --git a/windows/src/desktop/setup/Keyman.Setup.System.OnlineResourceCheck.pas b/windows/src/desktop/setup/Keyman.Setup.System.OnlineResourceCheck.pas index 8f82c4a4d2..599563b27c 100644 --- a/windows/src/desktop/setup/Keyman.Setup.System.OnlineResourceCheck.pas +++ b/windows/src/desktop/setup/Keyman.Setup.System.OnlineResourceCheck.pas @@ -46,7 +46,7 @@ begin try http.Request.SetURL(MakeAPIURL(API_Path_UpdateCheck_Windows)); http.Fields.Add('version', AnsiString(currentVersion)); - http.Fields.Add('tier', AnsiString(KeymanVersion.CKeymanVersionInfo.Tier)); + http.Fields.Add('tier', AnsiString(AInstallInfo.Tier)); http.Fields.Add('update', '0'); // This is probably a fresh install of a package, not an update for pack in AInstallInfo.Packages do http.Fields.Add(AnsiString('package_'+pack.ID), AnsiString(pack.Locations.LatestVersion)); diff --git a/windows/src/desktop/setup/bootstrapmain.pas b/windows/src/desktop/setup/bootstrapmain.pas index be5714d4b6..f79e373a37 100644 --- a/windows/src/desktop/setup/bootstrapmain.pas +++ b/windows/src/desktop/setup/bootstrapmain.pas @@ -92,7 +92,7 @@ procedure InstallKeyboardsInOldVersion(const ShellPath: string); forward; procedure DoExtractOnly(FSilent: Boolean; const FExtractOnly_Path: string); forward; function CreateTempDir: string; forward; procedure RemoveTempDir(const path: string); forward; -procedure ProcessCommandLine(var FPromptForReboot, FSilent, FForceOffline, FExtractOnly, FContinueSetup, FStartAfterInstall, FDisableUpgradeFrom6Or7Or8: Boolean; var FPackages, FExtractPath: string); forward; +procedure ProcessCommandLine(var FPromptForReboot, FSilent, FForceOffline, FExtractOnly, FContinueSetup, FStartAfterInstall, FDisableUpgradeFrom6Or7Or8: Boolean; var FPackages, FExtractPath, FTier: string); forward; procedure SetExitVal(c: Integer); forward; function IsKeymanDesktop7Installed: string; forward; function IsKeymanDesktop8Installed: string; forward; @@ -116,7 +116,7 @@ var FPromptForReboot: Boolean; // I3355 // I3500 FSilent: Boolean; FForceOffline: Boolean; - FPackages, FExtractOnly_Path: string; + FTier, FPackages, FExtractOnly_Path: string; BEGIN CoInitializeEx(nil, COINIT_APARTMENTTHREADED); try @@ -132,7 +132,7 @@ BEGIN { Display the dialog } - ProcessCommandLine(FPromptForReboot, FSilent, FForceOffline, FExtractOnly, FContinueSetup, FStartAfterInstall, FDisableUpgradeFrom6Or7Or8, FPackages, FExtractOnly_Path); // I2738, I2847 // I3355 // I3500 // I4293 + ProcessCommandLine(FPromptForReboot, FSilent, FForceOffline, FExtractOnly, FContinueSetup, FStartAfterInstall, FDisableUpgradeFrom6Or7Or8, FPackages, FExtractOnly_Path, FTier); // I2738, I2847 // I3355 // I3500 // I4293 GetRunTools.Silent := FSilent; if FExtractOnly then @@ -162,7 +162,7 @@ BEGIN // it to download khmer_angkor from the Keyman cloud and install it // for bcp47 tag km. See the setup documentation for more // examples. - FInstallInfo.LocatePackagesFromFilename(ParamStr(0)); + FInstallInfo.LocatePackagesAndTierFromFilename(ParamStr(0)); // Additionally, packages can be specified on the command line, with // the -p parameter, e.g. -p khmer_angkor=km,sil_euro_latin=fr @@ -176,6 +176,10 @@ BEGIN // this executable FInstallInfo.LocatePackagesInPath(ProgramPath); + // Lookup a tier from command line parameter + if FTier <> '' then + FInstallInfo.Tier := FTier; + GetRunTools.CheckInternetConnectedState; if not FForceOffline and GetRunTools.Online then @@ -381,7 +385,7 @@ begin DeletePath(ExcludeTrailingPathDelimiter(path)); // I3476 end; -procedure ProcessCommandLine(var FPromptForReboot, FSilent, FForceOffline, FExtractOnly, FContinueSetup, FStartAfterInstall, FDisableUpgradeFrom6Or7Or8: Boolean; var FPackages, FExtractPath: string); // I2847 // I3355 // I3500 // I4293 +procedure ProcessCommandLine(var FPromptForReboot, FSilent, FForceOffline, FExtractOnly, FContinueSetup, FStartAfterInstall, FDisableUpgradeFrom6Or7Or8: Boolean; var FPackages, FExtractPath, FTier: string); // I2847 // I3355 // I3500 // I4293 var i: Integer; begin @@ -428,6 +432,15 @@ begin // e.g. -p khmer_angkor=km,sil_euro_latin=fr Inc(i); FPackages := ParamStr(i); + end + else if SameText(ParamStr(i), '-t') then + begin + Inc(i); + FTier := ParamStr(i).ToLower.Trim; + if not FTier.Equals(TIER_ALPHA) and not FTier.Equals(TIER_BETA) and not FTier.Equals(TIER_STABLE) then + begin + FTier := ''; + end; end; Inc(i); end; diff --git a/windows/src/unit-tests/windows-setup/Keyman.System.Test.InstallInfoTest.pas b/windows/src/unit-tests/windows-setup/Keyman.System.Test.InstallInfoTest.pas index 3ac68ab1c5..da41760af3 100644 --- a/windows/src/unit-tests/windows-setup/Keyman.System.Test.InstallInfoTest.pas +++ b/windows/src/unit-tests/windows-setup/Keyman.System.Test.InstallInfoTest.pas @@ -12,7 +12,7 @@ type TInstallInfoTest = class(TObject) public [Test] - procedure TestLocatePackagesFromFilename; + procedure TestLocatePackagesAndTierFromFilename; [Test] procedure TestLocatePackagesFromParameter; @@ -21,18 +21,56 @@ type implementation uses + KeymanVersion, Keyman.Setup.System.InstallInfo; { TInstallInfoTest } -procedure TInstallInfoTest.TestLocatePackagesFromFilename; +procedure TInstallInfoTest.TestLocatePackagesAndTierFromFilename; var ii: TInstallInfo; begin ii := TInstallInfo.Create(''); try // It should match a standard pattern - ii.LocatePackagesFromFilename('c:\foo\keyman-setup.khmer_angkor.km.exe'); + ii.LocatePackagesAndTierFromFilename('c:\foo\keyman-setup.khmer_angkor.km.exe'); + Assert.AreEqual(CKeymanVersionInfo.Tier, ii.Tier); + Assert.AreEqual(1, ii.Packages.Count); + Assert.AreEqual('khmer_angkor', ii.Packages[0].ID); + Assert.AreEqual('km', ii.Packages[0].BCP47); + finally + ii.Free; + end; + + ii := TInstallInfo.Create(''); + try + // It should match a standard pattern with a tier + ii.LocatePackagesAndTierFromFilename('c:\foo\keyman-setup-alpha.khmer_angkor.km.exe'); + Assert.AreEqual(TIER_ALPHA, ii.Tier); + Assert.AreEqual(1, ii.Packages.Count); + Assert.AreEqual('khmer_angkor', ii.Packages[0].ID); + Assert.AreEqual('km', ii.Packages[0].BCP47); + finally + ii.Free; + end; + + ii := TInstallInfo.Create(''); + try + // It should match a standard pattern with a tier + ii.LocatePackagesAndTierFromFilename('c:\foo\keyman-setup-beta.khmer_angkor.km.exe'); + Assert.AreEqual(TIER_BETA, ii.Tier); + Assert.AreEqual(1, ii.Packages.Count); + Assert.AreEqual('khmer_angkor', ii.Packages[0].ID); + Assert.AreEqual('km', ii.Packages[0].BCP47); + finally + ii.Free; + end; + + ii := TInstallInfo.Create(''); + try + // It should match a standard pattern with a tier + ii.LocatePackagesAndTierFromFilename('c:\foo\keyman-setup-stable.khmer_angkor.km.exe'); + Assert.AreEqual(TIER_STABLE, ii.Tier); Assert.AreEqual(1, ii.Packages.Count); Assert.AreEqual('khmer_angkor', ii.Packages[0].ID); Assert.AreEqual('km', ii.Packages[0].BCP47); @@ -43,7 +81,7 @@ begin ii := TInstallInfo.Create(''); try // It should match a standard pattern - ii.LocatePackagesFromFilename('c:\foo\keyman-setup.khmer_angkor.km.sil_euro_latin.fr.exe'); + ii.LocatePackagesAndTierFromFilename('c:\foo\keyman-setup.khmer_angkor.km.sil_euro_latin.fr.exe'); Assert.AreEqual(2, ii.Packages.Count); Assert.AreEqual('khmer_angkor', ii.Packages[0].ID); Assert.AreEqual('km', ii.Packages[0].BCP47); @@ -56,7 +94,7 @@ begin ii := TInstallInfo.Create(''); try // It should strip off " (1)" suffixes when these are added by web browser - ii.LocatePackagesFromFilename('c:\foo\keyman-setup.khmer_angkor.km (1).exe'); + ii.LocatePackagesAndTierFromFilename('c:\foo\keyman-setup.khmer_angkor.km (1).exe'); Assert.AreEqual(1, ii.Packages.Count); Assert.AreEqual('khmer_angkor', ii.Packages[0].ID); Assert.AreEqual('km', ii.Packages[0].BCP47); @@ -67,7 +105,7 @@ begin ii := TInstallInfo.Create(''); try // It should give an empty BCP 47 tag if one is not provided - ii.LocatePackagesFromFilename('c:\foo\keyman-setup.khmer_angkor.exe'); + ii.LocatePackagesAndTierFromFilename('c:\foo\keyman-setup.khmer_angkor.exe'); Assert.AreEqual(1, ii.Packages.Count); Assert.AreEqual('khmer_angkor', ii.Packages[0].ID); Assert.IsEmpty(ii.Packages[0].BCP47); @@ -78,7 +116,7 @@ begin ii := TInstallInfo.Create(''); try // It should only match on keyman-setup - ii.LocatePackagesFromFilename('c:\foo\setup.khmer_angkor.km.exe'); + ii.LocatePackagesAndTierFromFilename('c:\foo\setup.khmer_angkor.km.exe'); Assert.AreEqual(0, ii.Packages.Count, 'setup.khmer_angkor.km.exe'); finally ii.Free; @@ -87,7 +125,7 @@ begin ii := TInstallInfo.Create(''); try // It should match packages with less common characters in filename - ii.LocatePackagesFromFilename('c:\foo\keyman-setup.khmer angkor.km.exe'); + ii.LocatePackagesAndTierFromFilename('c:\foo\keyman-setup.khmer angkor.km.exe'); Assert.AreEqual(1, ii.Packages.Count); Assert.AreEqual('khmer angkor', ii.Packages[0].ID); Assert.AreEqual('km', ii.Packages[0].BCP47); @@ -98,7 +136,7 @@ begin ii := TInstallInfo.Create(''); try // It should match packages with less common characters in filename - ii.LocatePackagesFromFilename('c:\foo\keyman-setup.khmer-angkor.km.exe'); + ii.LocatePackagesAndTierFromFilename('c:\foo\keyman-setup.khmer-angkor.km.exe'); Assert.AreEqual(1, ii.Packages.Count); Assert.AreEqual('khmer-angkor', ii.Packages[0].ID); Assert.AreEqual('km', ii.Packages[0].BCP47); From 2dc7f02431df5b0b902c9109cf1556d45b034e5c Mon Sep 17 00:00:00 2001 From: Marc Durdin Date: Mon, 20 Jul 2020 10:39:50 +1000 Subject: [PATCH 2/3] feat(windows): setup will retry if offline during initial install steps --- windows/src/desktop/setup/SetupStrings.pas | 10 +++- windows/src/desktop/setup/TntDialogHelp.pas | 8 ++- windows/src/desktop/setup/bootstrapmain.pas | 61 +++++++++++++++++---- 3 files changed, 64 insertions(+), 15 deletions(-) diff --git a/windows/src/desktop/setup/SetupStrings.pas b/windows/src/desktop/setup/SetupStrings.pas index df40de90c8..17161f7d90 100644 --- a/windows/src/desktop/setup/SetupStrings.pas +++ b/windows/src/desktop/setup/SetupStrings.pas @@ -87,7 +87,9 @@ type { Download dialog } ssDownloadingTitle, - ssDownloadingText + ssDownloadingText, + + ssOffline ); const @@ -157,7 +159,11 @@ const {ssOptionsDefaultLanguage} 'Default language', {ssDownloadingTitle} 'Downloading %0:s', // %0:s: filename - {ssDownloadingText} 'Downloading %0:s' // %0:s: filename + {ssDownloadingText} 'Downloading %0:s', // %0:s: filename + + {ssOffline} 'Keyman Setup could not connect to keyman.com to download additional resources.'#13#10#13#10+ + 'Please check that you are online, and give Keyman Setup permission to access the Internet in your firewall settings.'#13#10#13#10+ + 'Click Abort to exit Setup, Retry to try and download resources again, or Ignore to continue offline.' ); diff --git a/windows/src/desktop/setup/TntDialogHelp.pas b/windows/src/desktop/setup/TntDialogHelp.pas index 1dc05a4765..e414f9f1a8 100644 --- a/windows/src/desktop/setup/TntDialogHelp.pas +++ b/windows/src/desktop/setup/TntDialogHelp.pas @@ -91,13 +91,19 @@ begin IDCANCEL: Result := mrCancel; IDYES: Result := mrYes; IDNO: Result := mrNo; + IDABORT: Result := mrAbort; + IDIGNORE: Result := mrIgnore; + IDRETRY: Result := mrRetry; else Result := mrOk; end; end; procedure ShowMessageW(const Message: WideString); begin - Tnt_MessageBoxW(GetActiveWindow, PWideChar(Message), PChar(FInstallInfo.Text(ssMessageBoxTitle)), MB_OK); + if Assigned(FInstallInfo) then + Tnt_MessageBoxW(GetActiveWindow, PWideChar(Message), PChar(FInstallInfo.Text(ssMessageBoxTitle)), MB_OK) + else + Tnt_MessageBoxW(GetActiveWindow, PWideChar(Message), PChar('Setup'), MB_OK); end; end. diff --git a/windows/src/desktop/setup/bootstrapmain.pas b/windows/src/desktop/setup/bootstrapmain.pas index f79e373a37..98906b61b3 100644 --- a/windows/src/desktop/setup/bootstrapmain.pas +++ b/windows/src/desktop/setup/bootstrapmain.pas @@ -96,6 +96,7 @@ procedure ProcessCommandLine(var FPromptForReboot, FSilent, FForceOffline, FExtr procedure SetExitVal(c: Integer); forward; function IsKeymanDesktop7Installed: string; forward; function IsKeymanDesktop8Installed: string; forward; +function GetResourcesFromOnline(FSilent: Boolean; var FForceOffline: Boolean): Boolean; forward; var FNiceExitCodes: Boolean = True; // always, now @@ -122,14 +123,12 @@ BEGIN try try Vcl.Forms.Application.Icon.LoadFromResourceID(hInstance, 1); // I2611 + InitCommonControl(ICC_PROGRESS_CLASS); + FTempPath := CreateTempDir; try - FTempPath := CreateTempDir; + FInstallInfo := TInstallInfo.Create(FTempPath); try - InitCommonControl(ICC_PROGRESS_CLASS); - - FInstallInfo := TInstallInfo.Create(FTempPath); - { Display the dialog } ProcessCommandLine(FPromptForReboot, FSilent, FForceOffline, FExtractOnly, FContinueSetup, FStartAfterInstall, FDisableUpgradeFrom6Or7Or8, FPackages, FExtractOnly_Path, FTier); // I2738, I2847 // I3355 // I3500 // I4293 @@ -180,11 +179,12 @@ BEGIN if FTier <> '' then FInstallInfo.Tier := FTier; - GetRunTools.CheckInternetConnectedState; - - if not FForceOffline and GetRunTools.Online then - // TODO: retry strategies (and prompt around firewall etc) - TOnlineResourceCheck.QueryServer(FSilent, FInstallInfo); + // Try and get information from online + if not GetResourcesFromOnline(FSilent, FForceOffline) then + begin + SetExitVal(ERROR_FILE_NOT_FOUND); + Exit; + end; // This loads setup.inf, if present, for various additional strings and settings // The bundled installer usually contains a setup.inf. @@ -230,13 +230,13 @@ BEGIN Free; end; finally - RemoveTempDir(FTempPath); + FreeAndNil(FInstallInfo); end; SetExitVal(ERROR_SUCCESS); finally - FInstallInfo.Free; + RemoveTempDir(FTempPath); end; except on e:Exception do @@ -254,6 +254,43 @@ BEGIN end; end; +function GetResourcesFromOnline(FSilent: Boolean; var FForceOffline: Boolean): Boolean; +begin + if FForceOffline then + Exit(True); + repeat + try + GetRunTools.CheckInternetConnectedState; + + if GetRunTools.Online then + TOnlineResourceCheck.QueryServer(FSilent, FInstallInfo); + + // We've succeeded. + Exit(True); + except + on E:Exception do + begin + GetRunTools.LogInfo('Could not connect to site: '+E.Message); + if FSilent then + begin + // We log and attempt to continue + FForceOffline := True; + end + else + begin + case MessageDlgW(FInstallInfo.Text(ssOffline), mtError, mbAbortRetryIgnore, 0) of + mrAbort: Exit(False); + mrRetry: Continue; + mrIgnore: FForceOffline := True; + end; + end; + end; + end; + until FForceOffline; + + Result := True; +end; + function CheckForOldVersionScenario: Boolean; // I4460 var OldKMShellPath: string; From 77933c7abad6f45ce69f882d5f1ca9f43e60ba2a Mon Sep 17 00:00:00 2001 From: Marc Durdin Date: Mon, 20 Jul 2020 11:14:13 +1000 Subject: [PATCH 3/3] feat(windows): disable defaults options when Keyman already installed --- windows/src/desktop/setup/UfrmInstallOptions.pas | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/windows/src/desktop/setup/UfrmInstallOptions.pas b/windows/src/desktop/setup/UfrmInstallOptions.pas index afe0d7104b..396b5393a2 100644 --- a/windows/src/desktop/setup/UfrmInstallOptions.pas +++ b/windows/src/desktop/setup/UfrmInstallOptions.pas @@ -102,6 +102,8 @@ uses { TfrmInstallOptions } procedure TfrmInstallOptions.FormCreate(Sender: TObject); +var + FAllowOptions: Boolean; begin Caption := FInstallInfo.Text(ssOptionsTitle); chkStartWithWindows.Caption := FInstallInfo.Text(ssOptionsStartWithWindows); @@ -117,6 +119,12 @@ begin lblSelectModulesToInstall.Caption := FInstallInfo.Text(ssOptionsTitleSelectModulesToInstall); lblAssociatedKeyboardLanguage.Caption := FInstallInfo.Text(ssOptionsTitleAssociatedKeyboardLanguage); + FAllowOptions := not FInstallInfo.IsInstalled and FInstallInfo.IsNewerAvailable; + lblDefaultKeymanSettings.Visible := FAllowOptions; + chkAutomaticallyReportUsage.Visible := FAllowOptions; + chkCheckForUpdates.Visible := FAllowOptions; + chkStartWithWindows.Visible := FAllowOptions; + SetupDynamicOptions; end; @@ -241,8 +249,8 @@ begin end else case FInstallInfo.BestMsi.LocationType of - iilLocal: Text := FInstallInfo.Text(ssOptionsInstallKeyman, [FInstallInfo.BestMsi.Version]); - iilOnline: Text := FInstallInfo.Text(ssOptionsDownloadInstallKeyman, [FInstallInfo.BestMsi.Version, FormatFileSize(FInstallInfo.BestMsi.Size)]); + iilLocal: Text := FInstallInfo.Text(ssOptionsUpgradeKeyman, [FInstallInfo.BestMsi.Version]); + iilOnline: Text := FInstallInfo.Text(ssOptionsDownloadUpgradeKeyman, [FInstallInfo.BestMsi.Version, FormatFileSize(FInstallInfo.BestMsi.Size)]); end; end else if FInstallInfo.IsInstalled then