diff --git a/CHANGELOG.md b/CHANGELOG.md index 3dd5aba0..a4f50e19 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added - Record map classes and their generated data classes are now automatically added to source control (#955) +### Fixed +- Import All no longer deletes items mapped in from an IPM package in another namespace, which could delete Embedded Git itself where it is mapped instance-wide; Export All and baseline export skip such items too, including when the owning package cannot be determined (#997) + ## [2.17.1] - 2026-08-18 ### Fixed diff --git a/cls/SourceControl/Git/PackageManagerContext.cls b/cls/SourceControl/Git/PackageManagerContext.cls index 9438a47b..7c4cda02 100644 --- a/cls/SourceControl/Git/PackageManagerContext.cls +++ b/cls/SourceControl/Git/PackageManagerContext.cls @@ -1,6 +1,13 @@ +Include %occStatus + Class SourceControl.Git.PackageManagerContext Extends SourceControl.Git.Util.Singleton { +/// Everything this class reports is namespace-specific, so a process that visits more than one +/// namespace - as Embedded Git does when it is mapped instance-wide - needs one instance per namespace +/// rather than one shared instance holding whichever namespace's answers it saw first. +Parameter NAMESPACESCOPE As BOOLEAN = 1; + Property InternalName As %String; Property IsInDefaultPackage As %Boolean [ InitialExpression = 0 ]; @@ -10,6 +17,25 @@ Property IsInGitEnabledPackage As %Boolean [ InitialExpression = 0 ]; /// Really is a %ZPM.PackageManager.Developer.Module / %IPM.Storage.Module Property Package As %RegisteredObject [ InitialExpression = {$$$NULLOREF} ]; +/// Name of the IPM package that owns InternalName, or "" if no package does. +///

This is not always Package.Name: an item mapped in from another namespace +/// has no module record here, so Package reports it as unowned. That is how +/// Embedded Git came to delete its own mapped-in classes - see +/// https://github.com/intersystems/git-source-control/issues/997 . Only the name is established for +/// such items, deliberately: resolving Package would put this namespace in charge +/// of another namespace's repository.

+///

An empty name only means "no package owns this" when +/// HomePackageUnknown is false.

+Property HomePackageName As %String; + +/// Set when InternalName is mapped in from another database and the owning package +/// could not be determined - most often because the user running Embedded Git has no privileges on the +/// namespace the item is mapped from, or on the databases behind it. +///

Such an item must be treated as belonging to something else rather than as unowned: a user can +/// have enough privileges to delete a mapped-in item and not enough to find out who owns it, which is +/// how https://github.com/intersystems/git-source-control/issues/997 destroyed an installation.

+Property HomePackageUnknown As %Boolean [ InitialExpression = 0 ]; + /// Really is a %ZPM.PackageManager.Developer.ResourceReference / %IPM.Storage.ResourceReference Property ResourceReference As %RegisteredObject [ InitialExpression = {$$$NULLOREF} ]; @@ -19,15 +45,26 @@ Method InternalNameSet(InternalName As %String = "") As %Status if (InternalName '= i%InternalName) { set i%InternalName = InternalName set resourceReference = $$$NULLOREF + set ..HomePackageUnknown = 0 if (InternalName = ##class(SourceControl.Git.Settings.Document).#INTERNALNAME) { // Embedded Git settings document is never in an IPM context set ..Package = $$$NULLOREF - } elseif $$$comClassDefined("%IPM.ExtensionBase.Utils") { - set ..Package = ##class(%IPM.ExtensionBase.Utils).FindHomeModule(InternalName,,.resourceReference) - } elseif $$$comClassDefined("%ZPM.PackageManager.Developer.Extension.Utils") { - set ..Package = ##class(%ZPM.PackageManager.Developer.Extension.Utils).FindHomeModule(InternalName,,.resourceReference) + set ..HomePackageName = "" } else { - set ..Package = $$$NULLOREF + set sourceNamespace = ..SourceNamespace(InternalName, .resolved) + if 'resolved { + // mapped in from a database that could not be traced back to a namespace + set ..Package = $$$NULLOREF + set ..HomePackageName = "" + set ..HomePackageUnknown = 1 + } elseif (sourceNamespace '= "") { + set ..Package = $$$NULLOREF + set ..HomePackageName = ..FindHomePackageName(InternalName, .resolved) + set ..HomePackageUnknown = 'resolved + } else { + set ..Package = ..FindHomeModule(InternalName, .resourceReference) + set ..HomePackageName = $select($isobject(..Package):..Package.Name, 1:"") + } } set ..ResourceReference = resourceReference set ..IsInGitEnabledPackage = $isobject(..Package) && ##class(%Library.File).Exists(##class(%Library.File).NormalizeFilename(".git",..Package.Root)) @@ -44,6 +81,127 @@ ClassMethod ForInternalName(InternalName As %String = "") As SourceControl.Git.P quit instance } +/// Returns the module that owns internalName in the current namespace, or $$$NULLOREF if no +/// module does or IPM is not enabled here. +ClassMethod FindHomeModule(internalName As %String, Output resourceReference As %RegisteredObject) As %RegisteredObject [ Internal, Private ] +{ + set resourceReference = $$$NULLOREF + if $$$comClassDefined("%IPM.ExtensionBase.Utils") { + quit ##class(%IPM.ExtensionBase.Utils).FindHomeModule(internalName,,.resourceReference) + } + if $$$comClassDefined("%ZPM.PackageManager.Developer.Extension.Utils") { + quit ##class(%ZPM.PackageManager.Developer.Extension.Utils).FindHomeModule(internalName,,.resourceReference) + } + quit $$$NULLOREF +} + +/// Returns the name of the IPM package that owns internalName, looking in the namespace the +/// item is mapped from, or "" if no package does. resolved is false if the lookup could not +/// be made at all, in which case the empty name says nothing about ownership. +///

Only the name is returned because a swizzled %IPM.Storage.Module carries relationships that would +/// resolve against the wrong globals once $namespace is restored.

+ClassMethod FindHomePackageName(internalName As %String = "", Output resolved As %Boolean) As %String [ Internal ] +{ + set resolved = 1 + set packageName = "" + new $namespace + try { + set sourceNamespace = ..SourceNamespace(internalName, .resolved) + if resolved { + set:sourceNamespace'="" $namespace = sourceNamespace + set package = ..FindHomeModule(internalName) + set packageName = $select($isobject(package):package.Name, 1:"") + // Release the module before $namespace is restored. + set package = $$$NULLOREF + } + } catch e { + // if the user may not enter the namespace the item is mapped from, or may not read + // the database IPM's own data lives in there + set resolved = 0 + set packageName = "" + } + quit packageName +} + +/// Returns the namespace internalName is mapped in from, or "" if it lives in the current +/// namespace or in a database - IRISLIB, ENSLIB - that is no namespace's own. resolved is +/// false if the item is mapped in from somewhere but where could not be established. +ClassMethod SourceNamespace(internalName As %String, Output resolved As %Boolean) As %String [ Internal ] +{ + set resolved = 1 + if '##class(%Library.RoutineMgr).IsMapped(internalName, .sourceDB) { + quit "" + } + set sourceNamespace = ..NamespaceForDatabase(sourceDB, .resolved) + quit $select(sourceNamespace = $namespace:"", 1:sourceNamespace) +} + +/// Returns the namespace whose default routine database is database - in the "^^/dir/" or +/// "^server^/dir/" form reported by %Library.RoutineMgr.IsMapped - or "" if no +/// namespace has it, as is the case for the library databases every namespace maps code in from. +/// resolved is false if the question could not be answered. +///

%IPM.ExtensionBase.CompositeMethodOverrides:FindNamespaceForDatabase answers this by querying the +/// configuration in %SYS, which an ordinary developer has no privileges for; %SYS.Namespace answers it +/// from the namespace we are already in.

+ClassMethod NamespaceForDatabase(database As %String, Output resolved As %Boolean) As %String [ Internal ] +{ + set resolved = 1 + // The mapping is fixed for the life of the process, and bulk operations ask about the same + // handful of databases once per item. + set cache = $name(^||SourceControl.Git.PackageManagerContext("namespaceForDatabase", $namespace, database)) + if $data(@cache, namespace) { + quit namespace + } + + set namespace = "" + try { + set key = ..DatabaseKey(database) + do ##class(%SYS.Namespace).ListAll(.namespaces) + set candidate = "" + for { + set candidate = $order(namespaces(candidate)) + quit:candidate="" + // implied namespaces are database directories rather than namespaces to look for packages in + continue:$extract(candidate)="^" + // one unusable namespace - dismounted, misconfigured - must not stop the search + try { + set destination = ##class(%SYS.Namespace).GetRoutineDest(candidate) + } catch e { + set destination = "" + } + continue:destination="" + continue:..DatabaseKey(destination)'=key + // More than one namespace can have the same default routine database; any of them will + // resolve the same set of packages, so take the first. + set namespace = candidate + quit + } + } catch e { + set resolved = 0 + } + if 'resolved { + quit "" + } + set @cache = namespace + quit namespace +} + +/// Returns a comparable form of a routine database reference, which is reported as "^^/dir/" by +/// %Library.RoutineMgr.IsMapped and as "^/dir/" by +/// %SYS.Namespace.GetRoutineDest, and carries the server name for a remote database +/// in either case. +ClassMethod DatabaseKey(database As %String) As %String [ Internal, Private ] +{ + // drop the leading caret, then the server name if there is one + set reference = $extract(database, 2, *) + set server = "" + if (reference [ "^") { + set server = $piece(reference, "^") + set reference = $piece(reference, "^", 2, *) + } + quit $zconvert(server, "U")_"^"_$zconvert(##class(%Library.File).NormalizeDirectory(reference), "U") +} + Method Dump() { write !,"Package manager context: " diff --git a/cls/SourceControl/Git/Utils.cls b/cls/SourceControl/Git/Utils.cls index 240cfbad..c1b0d1b3 100644 --- a/cls/SourceControl/Git/Utils.cls +++ b/cls/SourceControl/Git/Utils.cls @@ -1648,7 +1648,7 @@ ClassMethod ListItemsInFiles(ByRef itemList, ByRef err) As %Status ClassMethod ImportRoutines(force As %Boolean = 0, pullEventClass As %String) As %Status { set refContext = ##class(SourceControl.Git.PackageManagerContext).%Get() - set refPackage = refContext.Package + set refPackageName = refContext.HomePackageName write !, "==import start==" @@ -1700,7 +1700,8 @@ ClassMethod ImportRoutines(force As %Boolean = 0, pullEventClass As %String) As continue:internalName=##class(SourceControl.Git.Settings.Document).#INTERNALNAME set context = ##class(SourceControl.Git.PackageManagerContext).ForInternalName(internalName) - continue:context.Package'=refPackage + continue:context.HomePackageUnknown + continue:context.HomePackageName'=refPackageName set doImport = force || ..IsRoutineOutdated(internalName) if '..IsInSourceControl(internalName) { set sc = ..AddToServerSideSourceControl(internalName) @@ -1747,11 +1748,18 @@ ClassMethod ImportRoutines(force As %Boolean = 0, pullEventClass As %String) As quit:item="" set context = ##class(SourceControl.Git.PackageManagerContext).ForInternalName(item) - continue:context.Package'=refPackage + set ownerUnknown = context.HomePackageUnknown + continue:'ownerUnknown&&(context.HomePackageName'=refPackageName) set externalName = ..ExternalName(item) set fullExternalName = ..FullExternalName(item) if '##class(%File).Exists(fullExternalName) { + // an item whose owner cannot be established is left alone: it is mapped in from a namespace + // this process cannot see into, and deleting it here deletes it there as well (#997) + if ownerUnknown { + write !,fullExternalName," does not exist, but ",item," is mapped in from a namespace whose IPM packages cannot be read - not deleting it" + continue + } write !,fullExternalName," does not exist - deleting ",item set modification = ##class(SourceControl.Git.Modification).%New() set modification.changeType = "D" @@ -1910,7 +1918,7 @@ ClassMethod ImportAll(force As %Boolean = 0, pullEventClass As %String) As %Stat ClassMethod ExportRoutines(force As %Boolean = 0) As %Status { set refContext = ##class(SourceControl.Git.PackageManagerContext).%Get() - set refPackage = refContext.Package + set refPackageName = refContext.HomePackageName #dim item as %String = "" #dim ec as %Status = $$$OK @@ -1918,7 +1926,8 @@ ClassMethod ExportRoutines(force As %Boolean = 0) As %Status set item = $order(@..#Storage@("items",item)) quit:item="" set context = ##class(SourceControl.Git.PackageManagerContext).ForInternalName(item) - continue:context.Package'=refPackage + continue:context.HomePackageUnknown + continue:context.HomePackageName'=refPackageName set ec = ..ExportItem(item, 1, force) quit:'ec } @@ -3289,7 +3298,11 @@ ClassMethod BaselineExport(pCommitMessage = "", pPushToRemote = "", pVerbose = 0 } // exclude items in a non-default IPM package set context = ##class(SourceControl.Git.PackageManagerContext).ForInternalName(internalName) - if ($isobject(context.Package) && 'context.IsInDefaultPackage) { + if context.HomePackageUnknown { + write:pVerbose !?5, "skipping item mapped in from a namespace whose IPM packages cannot be read" + continue + } + if ((context.HomePackageName '= "") && 'context.IsInDefaultPackage) { write:pVerbose !?5, "skipping item in non-default IPM package" continue } diff --git a/test/UnitTest/SourceControl/Git/MappedNamespace.cls b/test/UnitTest/SourceControl/Git/MappedNamespace.cls new file mode 100644 index 00000000..5f1f4f30 --- /dev/null +++ b/test/UnitTest/SourceControl/Git/MappedNamespace.cls @@ -0,0 +1,178 @@ +/// Regression test for https://github.com/intersystems/git-source-control/issues/997 - "Embedded Git +/// deletes itself on import". Embedded Git installed in one namespace and mapped into the rest of the +/// instance with SourceControl.Git.API.MapEverywhere has no IPM module record in the +/// other namespaces, so Import All there used to see its classes as unowned and delete them - out of +/// the database they are mapped from, breaking the installation instance-wide. +Class UnitTest.SourceControl.Git.MappedNamespace Extends UnitTest.SourceControl.Git.AbstractTest +{ + +/// Namespace and database created by this test and deleted again afterwards. +Parameter ScratchNamespace As STRING = "GSCUNITTESTMAP"; + +/// The item Embedded Git deleted out from under itself in #997. +Parameter MappedItem As STRING = "SourceControl.Git.Utils.CLS"; + +Method TestImportAllInMappedNamespace() +{ + set homePackage = ##class(SourceControl.Git.PackageManagerContext).ForInternalName(..#MappedItem).HomePackageName + do $$$AssertNotEquals(homePackage, "", "Embedded Git belongs to an IPM package in "_$namespace) + + kill result + set error = $$$NULLOREF + do ..RecordMapEverywhereMappings(.mappings) + try { + $$$ThrowOnError(..CreateScratchNamespace()) + $$$ThrowOnError(##class(SourceControl.Git.API).MapEverywhere()) + do ..ImportAllInScratchNamespace(.result) + } catch e { + set error = e + } + set teardown = ..RemoveAddedMappings(.mappings) + set teardown = $$$ADDSC(teardown, ..DeleteScratchNamespace()) + + // assertions run after the switch back, so that they are recorded in this namespace + if $isobject(error) { + do $$$AssertStatusOK(error.AsStatus(), "setup and import in "_..#ScratchNamespace) + } + + // the conditions that made #997 possible: the item is mapped in from the namespace Embedded Git + // is installed in, and has no module of its own where Import All is running + do $$$AssertEquals($get(result("sourceNamespace")), $namespace) + do $$$AssertNotTrue($get(result("localPackage"))) + do $$$AssertEquals($get(result("homePackage")), homePackage) + do $$$AssertNotTrue(##class(%File).Exists($get(result("fullExternalName")))) + + // ...and Import All leaves it alone anyway + do $$$AssertTrue($get(result("importRan")), "Import All ran in "_..#ScratchNamespace) + do $$$AssertStatusOK($get(result("importStatus"), $$$OK)) + do $$$AssertTrue($get(result("survived"))) + do $$$AssertTrue(##class(%Dictionary.ClassDefinition).%ExistsId($piece(..#MappedItem, ".", 1, *-1))) + + do $$$AssertStatusOK(teardown, "cleaned up "_..#ScratchNamespace) +} + +/// Configures Embedded Git in the scratch namespace and runs Import All there with a stale timestamp +/// for MappedItem and no file for it on disk - the state that triggered #997. +/// Observations are returned in result rather than asserted, because %UnitTest records +/// assertions in the namespace they are made in. +Method ImportAllInScratchNamespace(Output result) [ Private ] +{ + kill result + new $namespace + set $namespace = ..#ScratchNamespace + + set repository = ##class(%File).NormalizeDirectory(##class(%File).TempFilename()_"dir") + do ##class(%File).CreateDirectoryChain(repository) + kill ^SYS("SourceControl") + set settings = ##class(SourceControl.Git.Settings).%New() + set settings.namespaceTemp = repository + set settings.Mappings("CLS","*") = "cls/" + $$$ThrowOnError(settings.%Save()) + do ##class(%Studio.SourceControl.Interface).SourceControlClassSet("SourceControl.Git.Extension") + + // Import All from the web UI runs with no current document, so the ambient context is empty + set result("ambientPackage") = ##class(SourceControl.Git.PackageManagerContext).%Get().HomePackageName + set result("sourceNamespace") = ##class(SourceControl.Git.PackageManagerContext).SourceNamespace(..#MappedItem) + set context = ##class(SourceControl.Git.PackageManagerContext).ForInternalName(..#MappedItem) + set result("localPackage") = $isobject(context.Package) + set result("homePackage") = context.HomePackageName + // reading the above made the mapped item the current document; put the context back the way + // Import All would have found it + set context.InternalName = "" + + $$$ThrowOnError(##class(SourceControl.Git.Utils).UpdateRoutineTSH(..#MappedItem)) + set result("fullExternalName") = ##class(SourceControl.Git.Utils).FullExternalName(..#MappedItem) + + // Don't import at all if the context says Import All would delete Embedded Git out of the + // namespace it is installed in - the observations above report why. + if (result("ambientPackage") = "") && (result("homePackage") '= "") { + set result("importRan") = 1 + set result("importStatus") = ##class(SourceControl.Git.API).ImportAll(1) + set result("survived") = ##class(%Dictionary.ClassDefinition).%ExistsId($piece(..#MappedItem, ".", 1, *-1)) + } + + kill ^SYS("SourceControl") + do ##class(%File).RemoveDirectoryTree(repository) +} + +/// Creates the scratch database and namespace, removing any left over from an earlier run. +ClassMethod CreateScratchNamespace() As %Status [ Private ] +{ + new $namespace + $$$QuitOnError(..DeleteScratchNamespace()) + set $namespace = "%SYS" + set directory = ##class(%File).NormalizeDirectory(..#ScratchNamespace, $system.Util.ManagerDirectory()) + if '##class(%File).CreateDirectoryChain(directory, .return) { + quit $$$ERROR($$$GeneralError, "failed to create directory "_directory_": "_return) + } + $$$QuitOnError(##class(SYS.Database).CreateDatabase(directory)) + kill properties + set properties("Directory") = directory + $$$QuitOnError(##class(Config.Databases).Create(..#ScratchNamespace, .properties)) + kill properties + set properties("Globals") = ..#ScratchNamespace + set properties("Routines") = ..#ScratchNamespace + quit ##class(Config.Namespaces).Create(..#ScratchNamespace, .properties) +} + +/// Deletes the scratch namespace, its database, and the database's files. +ClassMethod DeleteScratchNamespace() As %Status [ Private ] +{ + new $namespace + set sc = $$$OK + set $namespace = "%SYS" + if ##class(Config.Namespaces).Exists(..#ScratchNamespace) { + set sc = $$$ADDSC(sc, ##class(Config.Namespaces).Delete(..#ScratchNamespace)) + } + set directory = "" + if ##class(Config.Databases).Exists(..#ScratchNamespace) { + set sc = $$$ADDSC(sc, ##class(Config.Databases).Get(..#ScratchNamespace, .properties)) + set directory = $get(properties("Directory")) + set sc = $$$ADDSC(sc, ##class(Config.Databases).Delete(..#ScratchNamespace)) + } + if (directory '= "") { + do ##class(SYS.Database).DismountDatabase(directory) + do ##class(%File).RemoveDirectoryTree(directory) + } + quit sc +} + +/// Records which of the %ALL mappings SourceControl.Git.API.MapEverywhere makes are +/// already configured, so that only the ones this test adds are removed again. +ClassMethod RecordMapEverywhereMappings(Output mappings) [ Private ] +{ + new $namespace + kill mappings + set $namespace = "%SYS" + set mappings("package", "SourceControl.Git") = ##class(Config.MapPackages).Exists("%ALL", "SourceControl.Git") + set mappings("global", "IRIS.Msg") = ##class(Config.MapGlobals).Exists("%ALL", "IRIS.Msg") + set mappings("global", "IRIS.Msg(""Studio"")") = ##class(Config.MapGlobals).Exists("%ALL", "IRIS.Msg(""Studio"")") +} + +/// Removes the %ALL mappings that were not configured when RecordMapEverywhereMappings +/// last ran. +ClassMethod RemoveAddedMappings(ByRef mappings) As %Status [ Private ] +{ + new $namespace + set sc = $$$OK + set $namespace = "%SYS" + set kind = "" + for { + set kind = $order(mappings(kind)) + quit:kind="" + set name = "" + for { + set name = $order(mappings(kind, name), 1, existed) + quit:name="" + continue:existed + if (kind = "package") { + set sc = $$$ADDSC(sc, ##class(Config.MapPackages).Delete("%ALL", name)) + } else { + set sc = $$$ADDSC(sc, ##class(Config.MapGlobals).Delete("%ALL", name)) + } + } + } + quit sc +} + +}