diff --git a/CHANGELOG.md b/CHANGELOG.md index 48efbb5e..44f8a1ca 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed - Fixed a bug where interop deployment manager would in some cases fail because it tried to load unrelated items from the repo (#977) +- Improved reporting of `` errors on the git executable, with an actionable message and diagnostics pointing to likely causes (#462) - Fixed bugs causing compilation errors when first adding an IRIS Interoperability BPL or BR (#984) ## [2.17.0] - 2026-06-22 diff --git a/Dockerfile b/Dockerfile index e4e31642..849325ca 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,4 +1,4 @@ -ARG BASE=containers.intersystems.com/intersystems/iris-community:2025.1 +ARG BASE=containers.intersystems.com/intersystems/iris-community:2026.1 FROM ${BASE} diff --git a/cls/SourceControl/Git/Utils.cls b/cls/SourceControl/Git/Utils.cls index 1d516c93..9c51ca42 100644 --- a/cls/SourceControl/Git/Utils.cls +++ b/cls/SourceControl/Git/Utils.cls @@ -2175,11 +2175,22 @@ ClassMethod RunGitCommandWithInput(command As %String, inFile As %String = "", O set env("XDG_CONFIG_HOME") = ##class(%File).ManagerDirectory() set returnCode = $zf(-100,"/ENV=env... "_baseArgs,gitCommand,newArgs...) } catch e { - if $$$isWINDOWS { - set returnCode = $zf(-100,baseArgs,gitCommand,newArgs...) - } else { - // If can't inject XDG_CONFIG_HOME (older IRIS version), need /SHELL on Linux to avoid permissions errors trying to use root's config - set returnCode = $zf(-100,"/SHELL "_baseArgs,gitCommand,newArgs...) + // /ENV injection failed (older IRIS); retry without it. + try { + if $$$isWINDOWS { + set returnCode = $zf(-100,baseArgs,gitCommand,newArgs...) + } else { + // If can't inject XDG_CONFIG_HOME (older IRIS version), need /SHELL on Linux to avoid permissions errors trying to use root's config + set returnCode = $zf(-100,"/SHELL "_baseArgs,gitCommand,newArgs...) + } + } catch retryErr { + throw ..GitLaunchErrorFromNotOpen(retryErr) + } + // On Linux, the /SHELL retry absorbs a failed git launch into a shell exit + // code (127 = command not found, 126 = found but not executable) instead of + // throwing , so the launch failure must be detected here instead. + if '$$$isWINDOWS && ((returnCode = 127) || (returnCode = 126)) { + throw ..GitLaunchErrorFromShellExit(returnCode) } } @@ -2248,6 +2259,58 @@ ClassMethod RunGitCommandWithInput(command As %String, inFile As %String = "", O quit returnCode } +/// Translates a <NOTOPEN> error from a failed git launch into an actionable +/// exception. The message includes the underlying OS error (from +/// $system.Process.OSError()) plus a hint targeted at the specific error code. +/// Non-<NOTOPEN> errors are returned unchanged. +ClassMethod GitLaunchErrorFromNotOpen(e As %Exception.AbstractException) As %Exception.AbstractException +{ + if (e.Name '= "") { + quit e + } + // Read the OS error immediately: it is a lingering value, but the failed launch + // that produced this has just refreshed it to the real cause. + // On Windows, OSError() has a trailing newline; strip trailing control chars so + // it doesn't break the message onto an extra line. + set os = $zstrip($system.Process.OSError(), ">C") + set errno = $piece($piece(os, "<", 2), ">", 1) + set prefix = "git could not be launched"_$select(os '= "": " ("_os_")", 1: "")_"."_$c(13,10) + if (errno = 2) { + quit ##class(%Exception.General).%New("", , , prefix_..GitNotFoundHint()) + } elseif (errno = 13) { + quit ##class(%Exception.General).%New("", , , prefix_..GitNotExecutableHint()) + } + quit ##class(%Exception.General).%New("", , , prefix_..GitLaunchGenericHint()) +} + +/// Translates a failed git launch on Linux, where the /SHELL retry absorbs the +/// failure into a shell exit code (127 = command not found, 126 = found but not +/// executable) instead of throwing <NOTOPEN>, into the same actionable exception +/// that GitLaunchErrorFromNotOpen produces for the <NOTOPEN> case. +ClassMethod GitLaunchErrorFromShellExit(returnCode As %Integer) As %Exception.AbstractException +{ + set prefix = "git could not be launched (shell exit "_returnCode_")."_$c(13,10) + if (returnCode = 127) { + quit ##class(%Exception.General).%New("", , , prefix_..GitNotFoundHint()) + } + quit ##class(%Exception.General).%New("", , , prefix_..GitNotExecutableHint()) +} + +ClassMethod GitNotFoundHint() As %String [ CodeMode = expression ] +{ +"git was not found. Make sure git is installed and on the IRIS user's PATH, or set an absolute path to the git executable on the Settings page." +} + +ClassMethod GitNotExecutableHint() As %String [ CodeMode = expression ] +{ +"git could not be executed. Check the permissions on the git executable"_$select($$$isWINDOWS: ", and ensure the IRIS user has the ""Replace a process level token"" privilege.", 1: ".") +} + +ClassMethod GitLaunchGenericHint() As %String [ CodeMode = expression ] +{ +"Check that git is installed, on the IRIS user's PATH (or set an absolute path on the Settings page), and that the IRIS user has permission to run it." +} + ClassMethod SyncIrisWithRepoThroughCommand(ByRef outStream) As %Status { set deletedFiles = "" diff --git a/csp/gitprojectsettings.csp b/csp/gitprojectsettings.csp index 1f51ab2a..3ba4d29b 100644 --- a/csp/gitprojectsettings.csp +++ b/csp/gitprojectsettings.csp @@ -213,7 +213,7 @@ body { } } catch err { do err.Log() - &html<
An error occurred and has been logged to the application error log.
> + &html<
An error occurred and has been logged to the application error log: #(..EscapeHTML(err.DisplayString()))#
> }
diff --git a/test/UnitTest/SourceControl/Git/Utils/GitLaunchErrors.cls b/test/UnitTest/SourceControl/Git/Utils/GitLaunchErrors.cls new file mode 100644 index 00000000..0aceb573 --- /dev/null +++ b/test/UnitTest/SourceControl/Git/Utils/GitLaunchErrors.cls @@ -0,0 +1,125 @@ +Class UnitTest.SourceControl.Git.Utils.GitLaunchErrors Extends UnitTest.SourceControl.Git.AbstractTest +{ + +/// Triggers a real by launching a nonexistent executable, passes the caught +/// exception through GitLaunchErrorFromNotOpen, and verifies the message reports the actual OS +/// error (ENOENT) plus the "not found" hint, with no docs link. +Method TestTranslatesNotOpenForMissingBinary() +{ + set outLog = ##class(%Library.File).TempFilename() + set errLog = ##class(%Library.File).TempFilename() + set baseArgs = "/STDOUT="_""""_outLog_""""_" /STDERR="_""""_errLog_"""" + set translated = "" + try { + set rc = $zf(-100, baseArgs, "/nonexistent/path/to/git-binary-xyz", "--version") + } catch e { + set translated = ##class(SourceControl.Git.Utils).GitLaunchErrorFromNotOpen(e) + } + do ##class(%File).Delete(outLog) + do ##class(%File).Delete(errLog) + + do $$$AssertTrue($isobject(translated), "a was raised and translated") + if '$isobject(translated) { + quit + } + set msg = translated.DisplayString() + do $$$AssertEquals(translated.Name, "", "translated exception keeps the name") + do $$$AssertTrue(msg [ "git could not be launched", "message explains git launch failed") + do $$$AssertTrue(msg [ "git was not found", "errno 2 maps to the not-found hint") + // The OS error text is embedded; on POSIX this is "No such file or directory". + do $$$AssertTrue(msg [ "No such file or directory", "message includes the underlying OS error text") + do $$$AssertTrue('(msg [ "http"), "runtime message contains no docs link") +} + +/// A whose OS error code is neither ENOENT nor EACCES falls back to the +/// generic hint. OSError() is a lingering value, so this is verified indirectly: any +/// translation must still produce an actionable, link-free message. +Method TestTranslatesNotOpenIsActionable() +{ + set src = ##class(%Exception.General).%New("", "", "", "") + set translated = ##class(SourceControl.Git.Utils).GitLaunchErrorFromNotOpen(src) + set msg = translated.DisplayString() + do $$$AssertEquals(translated.Name, "", "translated exception keeps the name") + do $$$AssertTrue(msg [ "git could not be launched", "message explains git launch failed") + do $$$AssertTrue(msg [ "git", "message references git and how to fix it") + do $$$AssertTrue('(msg [ "http"), "runtime message contains no docs link") +} + +Method TestPassesThroughOtherErrors() +{ + set src = ##class(%Exception.General).%New("", "", "", "") + set src.Name = "" + set translated = ##class(SourceControl.Git.Utils).GitLaunchErrorFromNotOpen(src) + do $$$AssertEquals(translated.Name, "", "non-NOTOPEN error passes through unchanged") +} + +/// On Linux, a launch failure inside the /SHELL retry does NOT throw -- it +/// returns shell exit code 127 (command not found). RunGitCommand must still raise an +/// actionable error rather than silently returning 127. +Method TestRunGitCommandThrowsForMissingConfiguredPath() +{ + set storage = ##class(SourceControl.Git.Utils).%SYSNamespaceStorage() + set saved = $get(@storage@("%gitBinPath")) + set threw = 0 + try { + set @storage@("%gitBinPath") = "/nonexistent/path/to/git-binary-xyz" + kill ^||GitVersion + try { + do ##class(SourceControl.Git.Utils).RunGitCommand("status", .err, .out) + } catch e { + set threw = 1 + do $$$AssertEquals(e.Name, "", "missing configured git path raises ") + set msg = e.DisplayString() + do $$$AssertTrue(msg [ "git was not found", "message identifies the missing git executable") + } + do $$$AssertTrue(threw, "RunGitCommand raised an error instead of returning a bare shell exit code") + } catch ex { + do $$$AssertStatusOK(ex.AsStatus()) + } + if (saved = "") { + kill @storage@("%gitBinPath") + } else { + set @storage@("%gitBinPath") = saved + } + kill ^||GitVersion +} + +/// Same shell-retry gap, for a configured path that exists but is not executable +/// (shell exit 126). +Method TestRunGitCommandThrowsForNonExecutableConfiguredPath() +{ + set fake = "/tmp/eg-test-fakegit-"_$job + set stream = ##class(%Stream.FileCharacter).%New() + set stream.Filename = fake + do stream.Write("#!/bin/sh"_$c(10)) + do stream.%Save() + do $zf(-100, "/SHELL", "chmod", "600", fake) + + set storage = ##class(SourceControl.Git.Utils).%SYSNamespaceStorage() + set saved = $get(@storage@("%gitBinPath")) + set threw = 0 + try { + set @storage@("%gitBinPath") = fake + kill ^||GitVersion + try { + do ##class(SourceControl.Git.Utils).RunGitCommand("status", .err, .out) + } catch e { + set threw = 1 + do $$$AssertEquals(e.Name, "", "non-executable configured git path raises ") + set msg = e.DisplayString() + do $$$AssertTrue(msg [ "git could not be executed", "message identifies the non-executable git path") + } + do $$$AssertTrue(threw, "RunGitCommand raised an error instead of returning a bare shell exit code") + } catch ex { + do $$$AssertStatusOK(ex.AsStatus()) + } + if (saved = "") { + kill @storage@("%gitBinPath") + } else { + set @storage@("%gitBinPath") = saved + } + kill ^||GitVersion + do ##class(%File).Delete(fake) +} + +}