Merge pull request #222 from intersystems/better-initialization

Improvements to configuration and initialization

Author: isc-tleavitt

CHANGELOG.md

@@ -5,21 +5,22 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
-## [2.0.6] - Unreleased
+## [2.1.0] - Unreleased
 
 ### Added
-- Support for Caché/Ensemble 2016.2.3, 2017.1.2, 2017.2.1, 2018.1.0 and later. (Treated as patch version bump because no compatibility impact for existing users.)
+- Support for Caché/Ensemble 2016.2.3, 2017.1.2, 2017.2.1, 2018.1.0 and later.
+- Installation adds a Management Portal favorite for all users (#209)
+- Improved configuration and defaults to simplify initial configuration (#213)
+  - If no name/email is specified, defaults to `$username` and `$username@<hostname>`
 
 ### Fixed
 - "Import All" will properly recognize new files
 - "Import All" and "Export All" apply only to the current package manager context and disregard items outside that context
 - "Import All" treats "Other" document types (DFI, LUT, etc.) properly
 
-### Changed
-- Various minor things under the hood to support use without the package manager and/or on older platforms
-
 ### Internal
 - Added CI script and tweaked unit tests to run properly in a container and bootstrap their own extension configuration
+- Tweaked various minor things under the hood to support use without the package manager and/or on older platforms
 
 ## [2.0.5] - 2022-12-02
 

README.md

@@ -50,8 +50,26 @@ This might look like:
 ![Example of mapping configuration](docs/images/settings.PNG "Example of mapping configuration")
 
 ### Security
+
+#### Dubious Ownership
+Newer git versions may produce output like:
+```
+fatal: detected dubious ownership in repository at 'C:/Your/Repo/Root'
+To add an exception for this directory, call:
+
+   git config --global --add safe.directory C:/Your/Repo/Root
+
+Set the environment variable GIT_TEST_DEBUG_UNSAFE_DIRECTORIES=true and run
+again for more information.
+```
+It is important for the namespace temp folder to be owned by the user IRIS runs as. (On Unix, commonly irisusr; on Windows, generally a designated service account or SYSTEM.) Setting this config flag is unlikely to actually help; just make sure the ownership is correct.
+
+#### Interacting with Remotes
 If you want to interact with remotes from VSCode/Studio directly (e.g., to push/pull), you must use ssh (rather than https), create a public/private key pair to identify the instance (not yourself), configure the private key file for use in Settings, and configure the public key as a deploy key in the remote(s).
 
+#### IRIS Privileges
+For developers to be able to run this extension, they'll need the USE privilege on %System_Callout.
+
 ### Setting up multiple GitHub deploy keys on one machine
 
 Assuming you have the local and remote repositories created,

cls/SourceControl/Git/Settings.cls

@@ -6,25 +6,25 @@ Class SourceControl.Git.Settings Extends %RegisteredObject
 {
 
 /// Path to git executable
-Property gitBinPath As %String;
+Property gitBinPath As %String(MAXLEN = "");
 
 /// Local git repo root folder
-Property namespaceTemp As %String [ InitialExpression = {##class(SourceControl.Git.Utils).TempFolder()}, Required ];
+Property namespaceTemp As %String(MAXLEN = "") [ InitialExpression = {##class(SourceControl.Git.Utils).TempFolder()}, Required ];
 
 /// Path to private key file (for ssh remotes)
-Property privateKeyFile As %String [ InitialExpression = {##class(SourceControl.Git.Utils).PrivateKeyFile()} ];
+Property privateKeyFile As %String(MAXLEN = "") [ InitialExpression = {##class(SourceControl.Git.Utils).PrivateKeyFile()} ];
 
 /// Event handler class for git pull
-Property pullEventClass As %String [ InitialExpression = {##class(SourceControl.Git.Utils).PullEventClass()}, Required ];
+Property pullEventClass As %String(MAXLEN = 255) [ InitialExpression = {##class(SourceControl.Git.Utils).PullEventClass()}, Required ];
 
 /// Character to replace % symbol when importing %-classes into the file systems
 Property percentClassReplace As %String [ InitialExpression = {##class(SourceControl.Git.Utils).PercentClassReplace()} ];
 
 /// Attribution: Git username for user ${username}
-Property gitUserName As %String [ InitialExpression = {##class(SourceControl.Git.Utils).GitUserName()}, Required ];
+Property gitUserName As %String(MAXLEN = 255) [ InitialExpression = {##class(SourceControl.Git.Utils).GitUserName()} ];
 
 /// Attribution: Email address for user ${username}
-Property gitUserEmail As %String [ InitialExpression = {##class(SourceControl.Git.Utils).GitUserEmail()}, Required ];
+Property gitUserEmail As %String(MAXLEN = 255) [ InitialExpression = {##class(SourceControl.Git.Utils).GitUserEmail()} ];
 
 Property Mappings [ MultiDimensional ];
 
@@ -40,6 +40,11 @@ Method %OnNew() As %Status
 
 Method %Save() As %Status
 {
+    set sc = ..%ValidateObject()
+    if $$$ISERR(sc) {
+        quit sc
+    }
+
     set sysStorage = ##class(SourceControl.Git.Utils).InstallNamespaceStorage()
     set storage = ##class(SourceControl.Git.Utils).#Storage
     kill @sysStorage@("%gitBinPath")
@@ -48,7 +53,8 @@ Method %Save() As %Status
     }
     kill ^||GitVersion
 
-    set @storage@("settings","namespaceTemp") = ##class(SourceControl.Git.Utils).AddSlash(..namespaceTemp)
+    set ..namespaceTemp = ##class(%Library.File).NormalizeDirectory(..namespaceTemp)
+    set @storage@("settings","namespaceTemp") = ..namespaceTemp
     if ('##class(%File).DirectoryExists(@storage@("settings","namespaceTemp"))){
         do ##class(%Library.File).CreateDirectoryChain(@storage@("settings","namespaceTemp"))
     }
@@ -94,3 +100,4 @@ ClassMethod Configure() As %Boolean [ CodeMode = objectgenerator ]
 }
 
 }
+

cls/SourceControl/Git/Utils.cls

@@ -1,4 +1,4 @@
-Include (%occStatus, %occErrors, SourceControl.Git)
+Include (%occStatus, %occErrors, SourceControl.Git, %sySecurity)
 
 Class SourceControl.Git.Utils [ Abstract, ProcedureBlock ]
 {
@@ -99,12 +99,12 @@ ClassMethod GitBinPath(Output isDefault) As %String
 
 ClassMethod GitUserName() As %String
 {
-    quit $get(@..#Storage@("settings","user",$username,"gitUserName"))
+    quit $get(@..#Storage@("settings","user",$username,"gitUserName"),$username)
 }
 
 ClassMethod GitUserEmail() As %String
 {
-    quit $get(@..#Storage@("settings","user",$username,"gitUserEmail"))
+    quit $get(@..#Storage@("settings","user",$username,"gitUserEmail"),$username_"@"_$zconvert(##class(%SYS.System).GetNodeName(),"L"))
 }
 
 ClassMethod PrivateKeyFile() As %String
@@ -122,14 +122,6 @@ ClassMethod InstallNamespace() As %String [ CodeMode = expression ]
 ..#InstallNamespace
 }
 
-ClassMethod AddSlash(path As %String) As %String
-{
-    if path'="" && ($extract(path,*)'=..#Slash) {
-        set path = path_..#Slash
-    }
-    quit path
-}
-
 ClassMethod IsMenuGitCommand(menuItemName As %String) As %Boolean [ CodeMode = expression ]
 {
 $Find(..#GitMenuItems, ","_menuItemName_",") > 0
@@ -1373,27 +1365,30 @@ ClassMethod RunGitCommand(command As %String, Output errStream, Output outStream
 
 ClassMethod RunGitCommandWithInput(command As %String, inFile As %String = "", Output errStream, Output outStream, args...) As %Integer
 {
-    set newArgs($increment(newArgs)) = "-C"
-    set newArgs($increment(newArgs)) = ..TempFolder()
+    // Special case: git --version is used internally even when the settings incorporated here may be invalid/unspecified.
+    if (command '= "--version") {
+        set newArgs($increment(newArgs)) = "-C"
+        set newArgs($increment(newArgs)) = ..TempFolder()
 
-    set privateKeyFile = ..PrivateKeyFile()
-    if (privateKeyFile '= "") {
-        if $$$isWINDOWS {
-            // Escape slashes
-            set privateKeyFile = $replace(privateKeyFile,"\","\\")
+        set privateKeyFile = ..PrivateKeyFile()
+        if (privateKeyFile '= "") {
+            if $$$isWINDOWS {
+                // Escape slashes
+                set privateKeyFile = $replace(privateKeyFile,"\","\\")
+            }
+            set newArgs($increment(newArgs)) = "-c"
+            // StrictHostKeyChecking=accept-new for good behavior on first connection
+            set newArgs($increment(newArgs)) = "core.sshCommand=ssh -F /dev/null -o StrictHostKeyChecking=accept-new -i "_privateKeyFile
         }
-        set newArgs($increment(newArgs)) = "-c"
-        // StrictHostKeyChecking=accept-new for good behavior on first connection
-        set newArgs($increment(newArgs)) = "core.sshCommand=ssh -F /dev/null -o StrictHostKeyChecking=accept-new -i "_privateKeyFile
-    }
 
-    set username = ..GitUserName()
-    set email = ..GitUserEmail()
+        set username = ..GitUserName()
+        set email = ..GitUserEmail()
 
-    set newArgs($increment(newArgs)) = "-c"
-    set newArgs($increment(newArgs)) = "user.name="_username
-    set newArgs($increment(newArgs)) = "-c"
-    set newArgs($increment(newArgs)) = "user.email="_email
+        set newArgs($increment(newArgs)) = "-c"
+        set newArgs($increment(newArgs)) = "user.name="_username
+        set newArgs($increment(newArgs)) = "-c"
+        set newArgs($increment(newArgs)) = "user.email="_email
+    }
 
     set newArgs($increment(newArgs)) = command
 
@@ -1470,7 +1465,7 @@ ClassMethod Name(InternalName As %String, ByRef MappingExists As %Boolean) As %S
         }
     }
 
-    if $$CheckProtect^%qccServer(InternalName) {
+    if '##class(%Library.RoutineMgr).UserType(InternalName) && $$CheckProtect^%qccServer(InternalName) {
         quit ""
     }
 
@@ -1751,6 +1746,45 @@ ClassMethod Localize()
     }
 }
 
+ClassMethod ConfigureWeb()
+{
+    set installNamespace = $Namespace
+    new $Namespace
+    set $Namespace = "%SYS"
+    write !,"Adding favorite for all users... "
+    set sql = "insert or update into %SYS_Portal.Users (Username, Page, Data) "_
+        "select ID,?,? from Security.Users"
+    set caption = "Git: "_installNamespace
+    set link = "/isc/studio/usertemplates/gitsourcecontrol/webuidriver.csp/"_installNamespace_"/"
+    do ##class(%SQL.Statement).%ExecDirect(,sql,caption,link).%Display()
+    write !,"Setting GroupById to %ISCMgtPortal for /isc/studio/usertemplates... "
+    set sql = "update Security.Applications set GroupById='%ISCMgtPortal' where ID = '/isc/studio/usertemplates'"
+    do ##class(%SQL.Statement).%ExecDirect(,sql).%Display()
+}
+
+ClassMethod CheckInitialization()
+{
+    if ##class(SourceControl.Git.Utils).GitBinExists(.version) {
+        // Note: version includes "git"
+        write !,"Will use "_version_" (already installed and on the PATH)."
+    } else {
+        set path = ##class(SourceControl.Git.Utils).GitBinPath(.isDefault)
+        if isDefault {
+            write !,"WARNING: Could not find git on the PATH. Confirm that git is installed, then add it to the PATH or point to its path by running: "
+            write !,?5,"do ##class(SourceControl.Git.API).Configure()"
+            write !,"and answering the prompts."
+        } else {
+            write !,path," is not a valid path to a Git executable. Confirm that git is installed, then update the path to it by running: "
+            write !,?5,"do ##class(SourceControl.Git.API).Configure()"
+            write !,"and answering the prompts."
+        }
+    }
+    If '$System.Security.Check("%System_Callout","USE") {
+        write !,"WARNING: You do not have USE privilege on %System_Callout, which is needed to invoke git.",
+            "Unless this privilege is granted to users of the extension, it won't work."
+    }
+}
+
 ClassMethod GetSourceControlInclude() As %String
 {
     quit $select(##class(%Library.EnsembleMgr).IsEnsembleInstalled():
@@ -1808,6 +1842,10 @@ ClassMethod BuildCEInstallationPackage(ByRef destination As %String) As %Status
         set code($i(code)) = " Do ##class(SourceControl.Git.Utils).Localize()"
         set code($i(code)) = " Write !!"
         set code($i(code)) = " Do ##class(SourceControl.Git.Utils).OutputConfigureMessage()"
+        set code($i(code)) = " Write !!"
+        set code($i(code)) = " Do ##class(SourceControl.Git.Utils).ConfigureWeb()"
+        set code($i(code)) = " Write !!"
+        set code($i(code)) = " Do ##class(SourceControl.Git.Utils).CheckInitialization()"
 
         // Put installer automation in class
         do $System.OBJ.Delete("SourceControl.Git.Installer.CLS","-d")

module.xml

@@ -3,7 +3,7 @@
   <Document name="git-source-control.ZPM">
     <Module>
       <Name>git-source-control</Name>
-      <Version>2.0.6</Version>
+      <Version>2.1.0</Version>
       <Description>Server-side source control extension for use of Git on InterSystems platforms</Description>
       <Keywords>git source control studio vscode</Keywords>
       <Packaging>module</Packaging>
@@ -23,6 +23,8 @@
 
       <Invoke Class="SourceControl.Git.Utils" Method="OutputConfigureMessage" />
       <Invoke Class="SourceControl.Git.Utils" Method="Localize" />
+      <Invoke Class="SourceControl.Git.Utils" Method="ConfigureWeb" />
+      <Invoke Class="SourceControl.Git.Utils" Method="CheckInitialization" />
     </Module>
   </Document>
 </Export>

test/UnitTest/SourceControl/Git/AddRemove.cls

@@ -50,9 +50,10 @@ Method %OnNew(initvalue) As %Status
 {
 	Merge ..SourceControlGlobal = ^SYS("SourceControl")
 	Kill ^SYS("SourceControl")
-	Set folder = ##class(%Library.File).NormalizeFilename(##class(%Library.File).TempFilename()_"dir/")
-	Set ^SYS("SourceControl","Git","settings","namespaceTemp") = folder
-	Set ^SYS("SourceControl","Git","settings","mappings","MAC","*")="rtn/"
+	Set settings = ##class(SourceControl.Git.Settings).%New()
+	Set settings.namespaceTemp = ##class(%Library.File).TempFilename()_"dir"
+	Set settings.Mappings("MAC","*")="rtn/"
+	Do settings.%Save()
 	Do ##class(%Studio.SourceControl.Interface).SourceControlClassSet("SourceControl.Git.Extension")
 	Quit ##super(initvalue)
 }

test/UnitTest/SourceControl/Git/Initialization.cls

@@ -0,0 +1,42 @@
+Include %sySecurity
+
+Class UnitTest.SourceControl.Git.Initialization Extends %UnitTest.TestCase
+{
+
+Method TestSetupFavorites()
+{
+    Set page = "Git: "_$Namespace
+    Set username = $Username
+    &sql(delete from %SYS_Portal.Users where Page = :page and Username = :username)
+    // Intentionally called twice!
+    Do ##class(SourceControl.Git.Utils).ConfigureWeb()
+    Do ##class(SourceControl.Git.Utils).ConfigureWeb()
+    &sql(select count(*) into :favoriteCount from %SYS_Portal.Users where Page = :page and Username = :username)
+    Do $$$AssertEquals(favoriteCount,1)
+    Do $$$AssertTrue($$$SecurityApplicationsExists("/isc/studio/usertemplates",record))
+    Do $$$AssertEquals($$$GetSecurityApplicationsGroupById(record),"%ISCMgtPortal")
+}
+
+Method TestRunGitInGarbageContext()
+{
+    Set settings = ##class(SourceControl.Git.Settings).%New()
+    Set oldTemp = settings.namespaceTemp
+    Set settings.namespaceTemp = ##class(%Library.File).TempFilename()_"nonexistentdir"
+    Do $$$AssertStatusOK(settings.%Save())
+    Try {
+        Do ##class(%Library.File).RemoveDirectory(settings.namespaceTemp)
+        // This is a prerequisite in any testing environment.
+        Write ##class(SourceControl.Git.Utils).TempFolder()
+        Do $$$AssertTrue(##class(SourceControl.Git.Utils).GitBinExists())
+    } Catch e {
+        Do $$$AssertFailure("Error occurred: "_$System.Status.GetErrorText(e.AsStatus()))
+    }
+    // OK for unit test to leak this if it was empty to start
+    If (oldTemp '= "") {
+        Set settings.namespaceTemp = oldTemp
+        Do $$$AssertStatusOK(settings.%Save())
+    }
+}
+
+}
+

test/UnitTest/SourceControl/Git/NameToInternalNameTest.cls

@@ -96,12 +96,12 @@ Method OnBeforeAllTests() As %Status
 	set settings = ##class(SourceControl.Git.Settings).%New()
 	set ..OldNamespaceTemp = settings.namespaceTemp
 	set ..OldPercentClassReplace = settings.percentClassReplace
+	merge ..Mappings = @##class(SourceControl.Git.Utils).MappingsNode()
 	set folder = ##class(%File).NormalizeDirectory(##class(%Library.File).TempFilename()_"dir/")
 	set settings.namespaceTemp = folder
 	set settings.percentClassReplace = "_"
 	$$$ThrowOnError(settings.%Save())
 	do ##class(%Library.File).CopyDir($Piece(..Manager.CurrentDir,"test"),folder,1)
-	merge ..Mappings = @##class(SourceControl.Git.Utils).MappingsNode()
 	kill @##class(SourceControl.Git.Utils).MappingsNode()
 	Set app = $System.CSP.GetDefaultApp($Namespace)
 	If (app '= "") {