From c08f151a9c44e424a5a82049e676ef2cc1d66c2b Mon Sep 17 00:00:00 2001 From: Robert Holt Date: Wed, 20 Jun 2018 18:05:20 -0700 Subject: [PATCH 1/6] [Feature] Refactor module version comparison logic so it all goes through one method --- .../engine/Modules/ImportModuleCommand.cs | 9 +- .../engine/Modules/ModuleCmdletBase.cs | 326 +++++++----------- .../engine/Modules/ModuleIntrinsics.cs | 282 ++++++++++++++- 3 files changed, 392 insertions(+), 225 deletions(-) diff --git a/src/System.Management.Automation/engine/Modules/ImportModuleCommand.cs b/src/System.Management.Automation/engine/Modules/ImportModuleCommand.cs index da7926f927e..d64fe21fef3 100644 --- a/src/System.Management.Automation/engine/Modules/ImportModuleCommand.cs +++ b/src/System.Management.Automation/engine/Modules/ImportModuleCommand.cs @@ -586,13 +586,8 @@ private PSModuleInfo ImportModule_LocallyViaName(ImportModuleOptions importModul PSModuleInfo module; if (!BaseForce && Context.Modules.ModuleTable.TryGetValue(rootedPath, out module)) { - if (RequiredVersion == null - || module.Version.Equals(RequiredVersion) - || (BaseMinimumVersion == null && BaseMaximumVersion == null) - || module.ModuleType != ModuleType.Manifest - || (BaseMinimumVersion == null && BaseMaximumVersion != null && module.Version <= BaseMaximumVersion) - || (BaseMinimumVersion != null && BaseMaximumVersion == null && module.Version >= BaseMinimumVersion) - || (BaseMinimumVersion != null && BaseMaximumVersion != null && module.Version >= BaseMinimumVersion && module.Version <= BaseMaximumVersion)) + if (module.ModuleType != ModuleType.Manifest + || ModuleIntrinsics.IsVersionMatchingConstraints(module.Version, RequiredVersion, BaseMinimumVersion, BaseMaximumVersion)) { alreadyLoaded = true; AddModuleToModuleTables(this.Context, this.TargetSessionState.Internal, module); diff --git a/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs b/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs index e3c85d9d31a..2fc61cc3b7a 100644 --- a/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs +++ b/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs @@ -361,9 +361,7 @@ internal PSModuleInfo LoadUsingMultiVersionModuleBase(string moduleBase, Manifes foreach (var version in ModuleUtils.GetModuleVersionSubfolders(moduleBase)) { // Skip the version folder if it is not equal to the required version or does not satisfy the minimum/maximum version criteria - if ((BaseRequiredVersion != null && !BaseRequiredVersion.Equals(version)) - || (BaseMinimumVersion != null && BaseRequiredVersion == null && version < BaseMinimumVersion) - || (BaseMaximumVersion != null && BaseRequiredVersion == null && version > BaseMaximumVersion)) + if (ModuleIntrinsics.IsVersionMatchingConstraints(version, BaseRequiredVersion, BaseMinimumVersion, BaseMaximumVersion)) { continue; } @@ -1585,22 +1583,7 @@ internal PSModuleInfo LoadModuleManifest( } } - // TODO/FIXME: use IsModuleAlreadyLoaded to get consistent behavior - // if (IsModuleAlreadyLoaded(loadedModule) && - // ((manifestProcessingFlags & ManifestProcessingFlags.LoadElements) == ManifestProcessingFlags.LoadElements)) - if (loadedModule != null && - (BaseRequiredVersion == null || loadedModule.Version.Equals(BaseRequiredVersion)) && - ((BaseMinimumVersion == null && BaseMaximumVersion == null) - || - (BaseMaximumVersion != null && BaseMinimumVersion == null && - loadedModule.Version <= BaseMaximumVersion) - || - (BaseMaximumVersion == null && BaseMinimumVersion != null && - loadedModule.Version >= BaseMinimumVersion) - || - (BaseMaximumVersion != null && BaseMinimumVersion != null && - loadedModule.Version >= BaseMinimumVersion && loadedModule.Version <= BaseMaximumVersion)) && - (BaseGuid == null || loadedModule.Guid.Equals(BaseGuid)) && importingModule) + if (importingModule && IsModuleAlreadyLoaded(loadedModule)) { if (!BaseForce) { @@ -1672,23 +1655,20 @@ internal PSModuleInfo LoadModuleManifest( } if (bailOnFirstError) return null; } - else if (requiredVersion != null && !moduleVersion.Equals(requiredVersion)) + else if (ModuleIntrinsics.AreModuleFieldsMatchingConstraints( + moduleGuid: manifestGuid, + moduleVersion: moduleVersion, + requiredGuid: requiredModuleGuid, + requiredVersion: requiredVersion, + minimumRequiredVersion: minimumVersion, + maximumRequiredVersion: maximumVersion)) { - if (bailOnFirstError) return null; - } - else - { - if (moduleVersion < minimumVersion || (maximumVersion != null && moduleVersion > maximumVersion)) + if (bailOnFirstError) { - if (bailOnFirstError) return null; + return null; } } - if (requiredModuleGuid != null && !requiredModuleGuid.Equals(manifestGuid)) - { - if (bailOnFirstError) return null; - } - // Verify that the module version from the module manifest is equal to module version folder. DirectoryInfo parent = null; try @@ -3644,91 +3624,27 @@ private static ErrorRecord GenerateInvalidModuleMemberErrorRecord(string manifes /// /// Execution Context. /// Either a string or a hash of ModuleName, optional Guid, and ModuleVersion. - /// Sets if the module cannot be found due to incorrect version. - /// Sets if the module cannot be found due to incorrect guid. + /// The reason the module failed to load, or null on success. /// Sets if the module/snapin is already present. /// null if the module is not loaded or loadElements is false, the loaded module otherwise. - internal static object IsModuleLoaded(ExecutionContext context, ModuleSpecification requiredModule, out bool wrongVersion, out bool wrongGuid, out bool loaded) + internal static object IsModuleLoaded(ExecutionContext context, ModuleSpecification requiredModule, out ModuleMatchFailure matchFailureReason, out bool loaded) { loaded = false; Dbg.Assert(requiredModule != null, "Caller should verify requiredModuleSpecification != null"); // Assume the module is not loaded. object result = null; - wrongVersion = false; - wrongGuid = false; - - string moduleName = requiredModule.Name; - Guid? moduleGuid = requiredModule.Guid; - Dbg.Assert(moduleName != null, "GetModuleSpecification should guarantee that moduleName != null"); + Dbg.Assert(requiredModule.Name != null, "GetModuleSpecification should guarantee that moduleName != null"); + ModuleMatchFailure matchFailure = ModuleMatchFailure.None; foreach (PSModuleInfo module in context.Modules.GetModules(new string[] { "*" }, false)) { - if (moduleName.Equals(module.Name, StringComparison.OrdinalIgnoreCase)) + // Check that the module meets the module constraints give + if (ModuleIntrinsics.IsModuleMatchingModuleSpec(out matchFailure, module, requiredModule)) { - if (!moduleGuid.HasValue || moduleGuid.Value.Equals(module.Guid)) - { - if (requiredModule.RequiredVersion != null) - { - if (requiredModule.RequiredVersion.Equals(module.Version)) - { - result = module; - loaded = true; - break; - } - - wrongVersion = true; - } - else if (requiredModule.Version != null) - { - if (requiredModule.MaximumVersion != null) - { - if (GetMaximumVersion(requiredModule.MaximumVersion) >= module.Version && requiredModule.Version <= module.Version) - { - result = module; - loaded = true; - break; - } - else - { - wrongVersion = true; - } - } - else if (requiredModule.Version <= module.Version) - { - result = module; - loaded = true; - break; - } - else - { - wrongVersion = true; - } - } - else if (requiredModule.MaximumVersion != null) - { - if (GetMaximumVersion(requiredModule.MaximumVersion) >= module.Version) - { - result = module; - loaded = true; - break; - } - else - { - wrongVersion = true; - } - } - else - { - result = module; - loaded = true; - break; - } - } - else - { - wrongGuid = true; - } + result = module; + loaded = true; + break; } } @@ -3742,6 +3658,7 @@ internal static object IsModuleLoaded(ExecutionContext context, ModuleSpecificat } } + matchFailureReason = matchFailure; return result; } @@ -3795,10 +3712,9 @@ internal static PSModuleInfo LoadRequiredModule(ExecutionContext context, Guid? moduleGuid = requiredModuleSpecification.Guid; PSModuleInfo result = null; - bool wrongVersion = false; - bool wrongGuid = false; + ModuleMatchFailure loadFailureReason = ModuleMatchFailure.None; bool loaded = false; - object loadedModule = IsModuleLoaded(context, requiredModuleSpecification, out wrongVersion, out wrongGuid, out loaded); + object loadedModule = IsModuleLoaded(context, requiredModuleSpecification, out loadFailureReason, out loaded); if (loadedModule == null) { @@ -3855,33 +3771,73 @@ internal static PSModuleInfo LoadRequiredModule(ExecutionContext context, { if (0 != (manifestProcessingFlags & ManifestProcessingFlags.WriteErrors)) { - if (wrongVersion) - { - if (requiredModuleSpecification.RequiredVersion != null) - { - message = StringUtil.Format(Modules.RequiredModuleNotLoadedWrongVersion, moduleManifestPath, moduleName, requiredModuleSpecification.RequiredVersion); - } - else if (requiredModuleSpecification.Version != null && requiredModuleSpecification.MaximumVersion == null) - { - message = StringUtil.Format(Modules.RequiredModuleNotLoadedWrongVersion, moduleManifestPath, moduleName, requiredModuleSpecification.Version); - } - else if (requiredModuleSpecification.Version == null && requiredModuleSpecification.MaximumVersion != null) - { - message = StringUtil.Format(Modules.RequiredModuleNotLoadedWrongMaximumVersion, moduleManifestPath, moduleName, requiredModuleSpecification.MaximumVersion); - } - else - { - message = StringUtil.Format(Modules.RequiredModuleNotLoadedWrongMinimumVersionAndMaximumVersion, moduleManifestPath, moduleName, requiredModuleSpecification.Version, requiredModuleSpecification.MaximumVersion); - } - } - else if (wrongGuid) - { - message = StringUtil.Format(Modules.RequiredModuleNotLoadedWrongGuid, moduleManifestPath, moduleName, moduleGuid.Value); - } - else + switch (loadFailureReason) { - message = StringUtil.Format(Modules.RequiredModuleNotLoaded, moduleManifestPath, moduleName); + case ModuleMatchFailure.RequiredVersion: + message = StringUtil.Format( + Modules.RequiredModuleNotLoadedWrongVersion, + moduleManifestPath, + moduleName, + requiredModuleSpecification.RequiredVersion); + break; + + case ModuleMatchFailure.MinimumVersion: + // If both max and min versions were specified, use a different error message + if (requiredModuleSpecification.MaximumVersion == null) + { + message = StringUtil.Format( + Modules.RequiredModuleNotLoadedWrongVersion, + moduleManifestPath, moduleName, + requiredModuleSpecification.Version); + } + else + { + message = StringUtil.Format( + Modules.RequiredModuleNotLoadedWrongMinimumVersionAndMaximumVersion, + moduleManifestPath, + moduleName, + requiredModuleSpecification.Version, + requiredModuleSpecification.MaximumVersion); + } + break; + + case ModuleMatchFailure.MaximumVersion: + // If both max and min versions were specified, use a different error message + if (requiredModuleSpecification.Version == null) + { + message = StringUtil.Format( + Modules.RequiredModuleNotLoadedWrongMaximumVersion, + moduleManifestPath, + moduleName, + requiredModuleSpecification.MaximumVersion); + } + else + { + message = StringUtil.Format( + Modules.RequiredModuleNotLoadedWrongMinimumVersionAndMaximumVersion, + moduleManifestPath, + moduleName, + requiredModuleSpecification.Version, + requiredModuleSpecification.MaximumVersion); + } + break; + + case ModuleMatchFailure.Guid: + message = StringUtil.Format( + Modules.RequiredModuleNotLoadedWrongGuid, + moduleManifestPath, + moduleName, + moduleGuid.Value); + break; + + default: + message = StringUtil.Format( + Modules.RequiredModuleNotLoaded, + moduleManifestPath, + moduleName); + break; } + MissingMemberException mm = new MissingMemberException(message); error = new ErrorRecord(mm, "Modules_InvalidManifest", ErrorCategory.ResourceUnavailable, moduleManifestPath); @@ -3943,8 +3899,6 @@ private static PSModuleInfo ImportRequiredModule(ExecutionContext context, Modul } else { - bool wrongVersion = false; - bool wrongGuid = false; // Check if the correct module is loaded using Version , Guid Information. string moduleNameToCheckAgainst = requiredModule.Name; string manifestPath = string.Empty; @@ -3953,6 +3907,7 @@ private static PSModuleInfo ImportRequiredModule(ExecutionContext context, Modul manifestPath = moduleNameToCheckAgainst; moduleNameToCheckAgainst = Path.GetFileNameWithoutExtension(moduleNameToCheckAgainst); } + ModuleSpecification ms = new ModuleSpecification(moduleNameToCheckAgainst); if (requiredModule.Guid != null) { @@ -3970,9 +3925,13 @@ private static PSModuleInfo ImportRequiredModule(ExecutionContext context, Modul { ms.MaximumVersion = requiredModule.MaximumVersion; } + + ModuleMatchFailure loadFailureReason; bool loaded = false; - object r = IsModuleLoaded(context, ms, out wrongVersion, out wrongGuid, out loaded); + object r = IsModuleLoaded(context, ms, out loadFailureReason, out loaded); + Dbg.Assert(r is PSModuleInfo, "The returned value should be PSModuleInfo"); + result = r as PSModuleInfo; if (result == null) { @@ -4099,43 +4058,13 @@ internal static Collection GetModuleIfAvailable(ModuleSpecificatio tempResult = powerShell.Invoke(); } + // Check if the available module is of the correct version and GUID - foreach (var m in tempResult) + foreach (var module in tempResult) { - if (!requiredModule.Guid.HasValue || requiredModule.Guid.Value.Equals(m.Guid)) + if (ModuleIntrinsics.IsModuleMatchingModuleSpec(module, requiredModule)) { - if (requiredModule.RequiredVersion != null) - { - if (requiredModule.RequiredVersion.Equals(m.Version)) - { - result.Add(m); - } - } - else if (requiredModule.Version != null) - { - if (requiredModule.MaximumVersion != null) - { - if (GetMaximumVersion(requiredModule.MaximumVersion) >= m.Version && requiredModule.Version <= m.Version) - { - result.Add(m); - } - } - else if (requiredModule.Version <= m.Version) - { - result.Add(m); - } - } - else if (requiredModule.MaximumVersion != null) - { - if (GetMaximumVersion(requiredModule.MaximumVersion) >= m.Version) - { - result.Add(m); - } - } - else - { - result.Add(m); - } + result.Add(module); } } return result; @@ -5059,34 +4988,19 @@ private bool ShouldModuleBeRemoved(PSModuleInfo module, string moduleNameInRemov return false; } + /// + /// Checks if an already loaded module meets the constraints passed to the module cmdlet by the user. + /// + /// The already loaded module that matched the name of the module to load. + /// True if the pre-loaded module matches all GUID and version constraints provided, false otherwise. internal bool IsModuleAlreadyLoaded(PSModuleInfo alreadyLoadedModule) { - if (alreadyLoadedModule == null) - { - return false; - } - if (this.BaseRequiredVersion != null && !alreadyLoadedModule.Version.Equals(this.BaseRequiredVersion)) - { - // "alreadyLoadedModule" is a different module (than the one we want to load) - return false; - } - if (this.BaseMinimumVersion != null && alreadyLoadedModule.Version < this.BaseMinimumVersion) - { - // "alreadyLoadedModule" is a different module (than the one we want to load) - return false; - } - if (this.BaseMaximumVersion != null && alreadyLoadedModule.Version > this.BaseMaximumVersion) - { - // "alreadyLoadedModule" is a different module (than the one we want to load) - return false; - } - if (BaseGuid != null && !alreadyLoadedModule.Guid.Equals(this.BaseGuid)) - { - // "alreadyLoadedModule" is a different module (than the one we want to load) - return false; - } - - return true; + return ModuleIntrinsics.IsModuleMatchingConstraints( + alreadyLoadedModule, + guid: BaseGuid, + requiredVersion: BaseRequiredVersion, + minimumVersion: BaseMinimumVersion, + maximumVersion: BaseMaximumVersion); } /// @@ -5241,6 +5155,7 @@ internal PSModuleInfo LoadUsingExtensions(PSModuleInfo parentModule, // If the module has already been loaded, just emit it and continue... Context.Modules.ModuleTable.TryGetValue(fileName, out module); +<<<<<<< HEAD // TODO/FIXME: use IsModuleAlreadyLoaded to get consistent behavior // if (!BaseForce && // IsModuleAlreadyLoaded(module) && @@ -5252,6 +5167,9 @@ internal PSModuleInfo LoadUsingExtensions(PSModuleInfo parentModule, || (BaseMaximumVersion == null && BaseMinimumVersion != null && module.Version >= BaseMinimumVersion) || (BaseMaximumVersion != null && BaseMinimumVersion != null && module.Version >= BaseMinimumVersion && module.Version <= BaseMaximumVersion)) && (BaseGuid == null || module.Guid.Equals(BaseGuid)) && importingModule) +======= + if (!BaseForce && importingModule && IsModuleAlreadyLoaded(module)) +>>>>>>> [Feature] Refactor module version comparison logic so it all goes through one method { moduleFileFound = true; module = Context.Modules.ModuleTable[fileName]; @@ -5296,12 +5214,7 @@ internal PSModuleInfo LoadUsingExtensions(PSModuleInfo parentModule, { moduleFileFound = true; // Win8: 325243 - Added the version check so that we do not unload modules with the same name but different version - if (BaseForce && module != null && - (this.BaseRequiredVersion == null || module.Version.Equals(this.BaseRequiredVersion)) && - (BaseGuid == null || module.Guid.Equals(BaseGuid))) - // TODO/FIXME: use IsModuleAlreadyLoaded to get consistent behavior - // TODO/FIXME: (for example the checks above are not lookint at this.BaseMinimumVersion) - // if (BaseForce && IsModuleAlreadyLoaded(module)) + if (BaseForce && IsModuleAlreadyLoaded(module)) { RemoveModule(module); } @@ -5478,14 +5391,11 @@ internal PSModuleInfo LoadModule(PSModuleInfo parentModule, string fileName, str // This will allow the search to continue and load a different version of the module. if (Context.Modules.ModuleTable.TryGetValue(fileName, out module)) { - if (BaseMinimumVersion != null && module.Version >= BaseMinimumVersion) + if (!ModuleIntrinsics.IsVersionMatchingConstraints(module.Version, minimumVersion: BaseMinimumVersion, maximumVersion: BaseMaximumVersion)) { - if (BaseMaximumVersion == null || module.Version <= BaseMaximumVersion) - { - found = false; - moduleFileFound = false; - return null; - } + found = false; + moduleFileFound = false; + return null; } } } diff --git a/src/System.Management.Automation/engine/Modules/ModuleIntrinsics.cs b/src/System.Management.Automation/engine/Modules/ModuleIntrinsics.cs index 8e008dc0ef2..967c8347e64 100644 --- a/src/System.Management.Automation/engine/Modules/ModuleIntrinsics.cs +++ b/src/System.Management.Automation/engine/Modules/ModuleIntrinsics.cs @@ -401,21 +401,253 @@ internal List GetModules(ModuleSpecification[] fullyQualifiedName, return modulesMatched.OrderBy(m => m.Name).ToList(); } + + /// + /// Check if a given module info object matches a given module specification. + /// + /// The module info object to check. + /// The module specification to match the module info object against. + /// True if the module info object meets all the constraints on the module specification, false otherwise. internal static bool IsModuleMatchingModuleSpec(PSModuleInfo moduleInfo, ModuleSpecification moduleSpec) { - if (moduleInfo != null && moduleSpec != null && - moduleInfo.Name.Equals(moduleSpec.Name, StringComparison.OrdinalIgnoreCase) && - (!moduleSpec.Guid.HasValue || moduleSpec.Guid.Equals(moduleInfo.Guid)) && - ((moduleSpec.Version == null && moduleSpec.RequiredVersion == null && moduleSpec.MaximumVersion == null) - || (moduleSpec.RequiredVersion != null && moduleSpec.RequiredVersion.Equals(moduleInfo.Version)) - || (moduleSpec.MaximumVersion == null && moduleSpec.Version != null && moduleSpec.RequiredVersion == null && moduleSpec.Version <= moduleInfo.Version) - || (moduleSpec.MaximumVersion != null && moduleSpec.Version == null && moduleSpec.RequiredVersion == null && ModuleCmdletBase.GetMaximumVersion(moduleSpec.MaximumVersion) >= moduleInfo.Version) - || (moduleSpec.MaximumVersion != null && moduleSpec.Version != null && moduleSpec.RequiredVersion == null && ModuleCmdletBase.GetMaximumVersion(moduleSpec.MaximumVersion) >= moduleInfo.Version && moduleSpec.Version <= moduleInfo.Version))) + return IsModuleMatchingModuleSpec(out ModuleMatchFailure matchFailureReason, moduleInfo, moduleSpec); + } + + /// + /// Check if a given module info object matches a given module specification. + /// + /// The constraint that caused the match failure, if any. + /// The module info object to check. + /// The module specification to match the module info object against. + /// True if the module info object meets all the constraints on the module specification, false otherwise. + internal static bool IsModuleMatchingModuleSpec(out ModuleMatchFailure matchFailureReason, PSModuleInfo moduleInfo, ModuleSpecification moduleSpec) + { + if (moduleSpec == null) { - return true; + matchFailureReason = ModuleMatchFailure.NullModuleSpecification; + return false; } - return false; + return IsModuleMatchingConstraints( + out matchFailureReason, + moduleInfo, + moduleSpec.Name, + moduleSpec.Guid, + moduleSpec.RequiredVersion, + moduleSpec.Version, + moduleSpec.MaximumVersion == null ? null : ModuleCmdletBase.GetMaximumVersion(moduleSpec.MaximumVersion)); + } + + /// + /// Check if a given module info object matches the given constraints. + /// Constraints given as null are ignored. + /// + /// The module info object to check. + /// The name of the expected module. + /// The guid of the expected module. + /// The required version of the expected module. + /// The minimum required version of the expected module. + /// The maximum required version of the expected module. + /// True if the module info object matches all given constraints, false otherwise. + internal static bool IsModuleMatchingConstraints( + PSModuleInfo moduleInfo, + string name = null, + Guid? guid = null, + Version requiredVersion = null, + Version minimumVersion = null, + Version maximumVersion = null) + { + return IsModuleMatchingConstraints( + out ModuleMatchFailure matchFailureReason, + moduleInfo, + name, + guid, + requiredVersion, + minimumVersion, + maximumVersion); + } + + /// + /// Check if a given module info object matches the given constraints. + /// Constraints given as null are ignored. + /// + /// The reason for the module constraint match failing. + /// The module info object to check. + /// The name of the expected module. + /// The guid of the expected module. + /// The required version of the expected module. + /// The minimum required version of the expected module. + /// The maximum required version of the expected module. + /// True if the module info object matches all given constraints, false otherwise. + internal static bool IsModuleMatchingConstraints( + out ModuleMatchFailure matchFailureReason, + PSModuleInfo moduleInfo, + string name = null, + Guid? guid = null, + Version requiredVersion = null, + Version minimumVersion = null, + Version maximumVersion = null) + { + // Define that a null module does not meet any constraints + if (moduleInfo == null) + { + matchFailureReason = ModuleMatchFailure.NullModule; + return false; + } + + return AreModuleFieldsMatchingConstraints( + out matchFailureReason, + moduleInfo.Name, + moduleInfo.Guid, + moduleInfo.Version, + name, + guid, + requiredVersion, + minimumVersion, + maximumVersion + ); + } + + /// + /// Check that given module fields meet any given constraints. + /// + /// The name of the module to check. + /// The GUID of the module to check. + /// The version of the module to check. + /// The name the module must have, if any. + /// The GUID the module must have, if any. + /// The exact version the module must have, if any. + /// The minimum version the module may have, if any. + /// The maximum version the module may have, if any. + /// True if the module parameters match all given constraints, false otherwise. + internal static bool AreModuleFieldsMatchingConstraints( + string moduleName = null, + Guid? moduleGuid = null, + Version moduleVersion = null, + string requiredName = null, + Guid? requiredGuid = null, + Version requiredVersion = null, + Version minimumRequiredVersion = null, + Version maximumRequiredVersion = null) + { + return AreModuleFieldsMatchingConstraints( + out ModuleMatchFailure matchFailureReason, + moduleName, + moduleGuid, + moduleVersion, + requiredName, + requiredGuid, + requiredVersion, + minimumRequiredVersion, + maximumRequiredVersion); + } + + /// + /// Check that given module fields meet any given constraints. + /// + /// The reason the match failed, if any. + /// The name of the module to check. + /// The GUID of the module to check. + /// The version of the module to check. + /// The name the module must have, if any. + /// The GUID the module must have, if any. + /// The exact version the module must have, if any. + /// The minimum version the module may have, if any. + /// The maximum version the module may have, if any. + /// True if the module parameters match all given constraints, false otherwise. + internal static bool AreModuleFieldsMatchingConstraints( + out ModuleMatchFailure matchFailureReason, + string moduleName = null, + Guid? moduleGuid = null, + Version moduleVersion = null, + string requiredName = null, + Guid? requiredGuid = null, + Version requiredVersion = null, + Version minimumRequiredVersion = null, + Version maximumRequiredVersion = null) + { + // If a name is required, check it matches + if (requiredName != null && !requiredName.Equals(moduleName, StringComparison.OrdinalIgnoreCase)) + { + matchFailureReason = ModuleMatchFailure.Name; + return false; + } + + // If a GUID is required, check it matches + if (requiredGuid != null && !requiredGuid.Equals(moduleGuid)) + { + matchFailureReason = ModuleMatchFailure.Guid; + return false; + } + + // Check the versions + return IsVersionMatchingConstraints(out matchFailureReason, moduleVersion, requiredVersion, minimumRequiredVersion, maximumRequiredVersion); + } + + /// + /// Check that a given module version matches the required or minimum/maximum version constraints. + /// Null constraints are not checked. + /// + /// The module version to check. Must not be null + /// The version that the given version must be, if not null. + /// The minimum version that the given version must be greater than or equal to, if not null. + /// The maximum version that the given version must be less then or equal to, if not null. + /// + /// True if the version matches the required version, or if it is absent, is between the minimum and maximum versions, and false otherwise. + /// + internal static bool IsVersionMatchingConstraints( + Version version, + Version requiredVersion = null, + Version minimumVersion = null, + Version maximumVersion = null) + { + return IsVersionMatchingConstraints(out ModuleMatchFailure matchFailureReason, version, requiredVersion, minimumVersion, maximumVersion); + } + + /// + /// Check that a given module version matches the required or minimum/maximum version constraints. + /// Null constraints are not checked. + /// + /// The reason why the match failed. + /// The module version to check. Must not be null + /// The version that the given version must be, if not null. + /// The minimum version that the given version must be greater than or equal to, if not null. + /// The maximum version that the given version must be less then or equal to, if not null. + /// + /// True if the version matches the required version, or if it is absent, is between the minimum and maximum versions, and false otherwise. + /// + internal static bool IsVersionMatchingConstraints( + out ModuleMatchFailure matchFailureReason, + Version version, + Version requiredVersion = null, + Version minimumVersion = null, + Version maximumVersion = null) + { + Dbg.Assert(version != null, $"Caller to verify that {nameof(version)} is not null"); + + // If a RequiredVersion is given it overrides other version settings + if (requiredVersion != null) + { + matchFailureReason = ModuleMatchFailure.RequiredVersion; + return requiredVersion.Equals(version); + } + + // Check the version is at least the minimum version + if (minimumVersion != null && version < minimumVersion) + { + matchFailureReason = ModuleMatchFailure.MinimumVersion; + return false; + } + + // Check the version is at most the maximum version + if (maximumVersion != null && version > maximumVersion) + { + matchFailureReason = ModuleMatchFailure.MaximumVersion; + return false; + } + + matchFailureReason = ModuleMatchFailure.None; + return true; } internal static Version GetManifestModuleVersion(string manifestPath) @@ -1297,6 +1529,36 @@ private static AliasInfo NewAliasInfo(AliasInfo alias, SessionStateInternal sess } } // ModuleIntrinsics + /// + /// Enumeration of reasons for a failure to match a module by constraints. + /// + internal enum ModuleMatchFailure + { + /// Match did not fail. + None, + + /// Match failed because the module was null. + NullModule, + + /// Module name did not match. + Name, + + /// Module GUID did not match. + Guid, + + /// Module version did not match the required version. + RequiredVersion, + + /// Module version was lower than the minimum version. + MinimumVersion, + + /// Module version was greater than the maximum version. + MaximumVersion, + + /// The module specifcation passed in was null. + NullModuleSpecification, + } + /// /// Used by Modules/Snapins to provide a hook to the engine for startup initialization /// w.r.t compiled assembly loading. From e56b1b4c7f073c9361715cb5fc8152760ea76d72 Mon Sep 17 00:00:00 2001 From: Robert Holt Date: Wed, 20 Jun 2018 19:27:55 -0700 Subject: [PATCH 2/6] Fix inverted if conditions --- .../engine/Modules/ModuleCmdletBase.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs b/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs index 2fc61cc3b7a..9ac17634d5e 100644 --- a/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs +++ b/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs @@ -361,7 +361,7 @@ internal PSModuleInfo LoadUsingMultiVersionModuleBase(string moduleBase, Manifes foreach (var version in ModuleUtils.GetModuleVersionSubfolders(moduleBase)) { // Skip the version folder if it is not equal to the required version or does not satisfy the minimum/maximum version criteria - if (ModuleIntrinsics.IsVersionMatchingConstraints(version, BaseRequiredVersion, BaseMinimumVersion, BaseMaximumVersion)) + if (!ModuleIntrinsics.IsVersionMatchingConstraints(version, BaseRequiredVersion, BaseMinimumVersion, BaseMaximumVersion)) { continue; } @@ -1655,7 +1655,7 @@ internal PSModuleInfo LoadModuleManifest( } if (bailOnFirstError) return null; } - else if (ModuleIntrinsics.AreModuleFieldsMatchingConstraints( + else if (!ModuleIntrinsics.AreModuleFieldsMatchingConstraints( moduleGuid: manifestGuid, moduleVersion: moduleVersion, requiredGuid: requiredModuleGuid, From a1d4707da6e285c613571996b6a3a87c1de8e02e Mon Sep 17 00:00:00 2001 From: Robert Holt Date: Fri, 28 Sep 2018 15:35:51 -0700 Subject: [PATCH 3/6] [Feature] Remove rebase artefacts --- .../engine/Modules/ModuleCmdletBase.cs | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs b/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs index 9ac17634d5e..1497941757c 100644 --- a/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs +++ b/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs @@ -5155,21 +5155,7 @@ internal PSModuleInfo LoadUsingExtensions(PSModuleInfo parentModule, // If the module has already been loaded, just emit it and continue... Context.Modules.ModuleTable.TryGetValue(fileName, out module); -<<<<<<< HEAD - // TODO/FIXME: use IsModuleAlreadyLoaded to get consistent behavior - // if (!BaseForce && - // IsModuleAlreadyLoaded(module) && - // ((manifestProcessingFlags & ManifestProcessingFlags.LoadElements) == ManifestProcessingFlags.LoadElements)) - if (!BaseForce && module != null && - (BaseRequiredVersion == null || module.Version.Equals(BaseRequiredVersion)) && - ((BaseMinimumVersion == null && BaseMaximumVersion == null) - || (BaseMaximumVersion != null && BaseMinimumVersion == null && module.Version <= BaseMaximumVersion) - || (BaseMaximumVersion == null && BaseMinimumVersion != null && module.Version >= BaseMinimumVersion) - || (BaseMaximumVersion != null && BaseMinimumVersion != null && module.Version >= BaseMinimumVersion && module.Version <= BaseMaximumVersion)) && - (BaseGuid == null || module.Guid.Equals(BaseGuid)) && importingModule) -======= if (!BaseForce && importingModule && IsModuleAlreadyLoaded(module)) ->>>>>>> [Feature] Refactor module version comparison logic so it all goes through one method { moduleFileFound = true; module = Context.Modules.ModuleTable[fileName]; From dc731a3913dcf2ded5af74b5f0eb9e4669c8d180 Mon Sep 17 00:00:00 2001 From: Robert Holt Date: Wed, 10 Oct 2018 16:02:56 -0700 Subject: [PATCH 4/6] [Feature] Run formerly pending tests to check loaded module versions --- .../Microsoft.PowerShell.Core/ModuleConstraint.Tests.ps1 | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/powershell/Modules/Microsoft.PowerShell.Core/ModuleConstraint.Tests.ps1 b/test/powershell/Modules/Microsoft.PowerShell.Core/ModuleConstraint.Tests.ps1 index cc2c202a14d..42b54d1aa9e 100644 --- a/test/powershell/Modules/Microsoft.PowerShell.Core/ModuleConstraint.Tests.ps1 +++ b/test/powershell/Modules/Microsoft.PowerShell.Core/ModuleConstraint.Tests.ps1 @@ -865,7 +865,7 @@ Describe "Preloaded module specification checking" -Tags "Feature" { { Import-Module -FullyQualifiedName $modSpec -ErrorAction Stop } | Should -Throw -ErrorId 'Modules_ModuleWithVersionNotFound,Microsoft.PowerShell.Commands.ImportModuleCommand' } - It "Does not load the module with FullyQualifiedName from the manifest when ModuleVersion=, MaximumVersion=, RequiredVersion=, Guid=" -TestCases $guidFailCases -Pending { + It "Does not load the module with FullyQualifiedName from the manifest when ModuleVersion=, MaximumVersion=, RequiredVersion=, Guid=" -TestCases $guidFailCases { param($ModuleVersion, $MaximumVersion, $RequiredVersion, $Guid) $modSpec = New-ModuleSpecification -ModuleName $manifestPath -ModuleVersion $ModuleVersion -MaximumVersion $MaximumVersion -RequiredVersion $RequiredVersion -Guid $Guid @@ -1088,7 +1088,7 @@ Describe "Preloaded modules with versioned directory version checking" -Tag "Fea { Import-Module -FullyQualifiedName $modSpec -ErrorAction Stop } | Should -Throw -ErrorId 'Modules_ModuleWithVersionNotFound,Microsoft.PowerShell.Commands.ImportModuleCommand' } - It "Does not load the module with FullyQualifiedName from the manifest when ModuleVersion=, MaximumVersion=, RequiredVersion=, Guid=" -TestCases $guidFailCases -Pending { + It "Does not load the module with FullyQualifiedName from the manifest when ModuleVersion=, MaximumVersion=, RequiredVersion=, Guid=" -TestCases $guidFailCases { param($ModuleVersion, $MaximumVersion, $RequiredVersion, $Guid) $modSpec = New-ModuleSpecification -ModuleName $manifestPath -ModuleVersion $ModuleVersion -MaximumVersion $MaximumVersion -RequiredVersion $RequiredVersion -Guid $Guid From c1141d97671734d3fc47951e0b47c2551e595879 Mon Sep 17 00:00:00 2001 From: Robert Holt Date: Fri, 12 Oct 2018 10:14:20 -0700 Subject: [PATCH 5/6] [Feature] Remove parameter defaults for local module constraint method calls --- .../engine/Modules/ModuleIntrinsics.cs | 26 +++++++++---------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/src/System.Management.Automation/engine/Modules/ModuleIntrinsics.cs b/src/System.Management.Automation/engine/Modules/ModuleIntrinsics.cs index 967c8347e64..c34595a6132 100644 --- a/src/System.Management.Automation/engine/Modules/ModuleIntrinsics.cs +++ b/src/System.Management.Automation/engine/Modules/ModuleIntrinsics.cs @@ -482,11 +482,11 @@ internal static bool IsModuleMatchingConstraints( internal static bool IsModuleMatchingConstraints( out ModuleMatchFailure matchFailureReason, PSModuleInfo moduleInfo, - string name = null, - Guid? guid = null, - Version requiredVersion = null, - Version minimumVersion = null, - Version maximumVersion = null) + string name, + Guid? guid, + Version requiredVersion, + Version minimumVersion, + Version maximumVersion) { // Define that a null module does not meet any constraints if (moduleInfo == null) @@ -557,14 +557,14 @@ internal static bool AreModuleFieldsMatchingConstraints( /// True if the module parameters match all given constraints, false otherwise. internal static bool AreModuleFieldsMatchingConstraints( out ModuleMatchFailure matchFailureReason, - string moduleName = null, - Guid? moduleGuid = null, - Version moduleVersion = null, - string requiredName = null, - Guid? requiredGuid = null, - Version requiredVersion = null, - Version minimumRequiredVersion = null, - Version maximumRequiredVersion = null) + string moduleName, + Guid? moduleGuid, + Version moduleVersion, + string requiredName, + Guid? requiredGuid, + Version requiredVersion, + Version minimumRequiredVersion, + Version maximumRequiredVersion) { // If a name is required, check it matches if (requiredName != null && !requiredName.Equals(moduleName, StringComparison.OrdinalIgnoreCase)) From bf88fd97cad4770fe2d4260de976888044676b43 Mon Sep 17 00:00:00 2001 From: Robert Holt Date: Tue, 30 Oct 2018 12:03:59 -0700 Subject: [PATCH 6/6] [Feature] Rename IsModuleAlreadyLoaded --- .../engine/Modules/ImportModuleCommand.cs | 2 +- .../engine/Modules/ModuleCmdletBase.cs | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/System.Management.Automation/engine/Modules/ImportModuleCommand.cs b/src/System.Management.Automation/engine/Modules/ImportModuleCommand.cs index d64fe21fef3..50d09e195db 100644 --- a/src/System.Management.Automation/engine/Modules/ImportModuleCommand.cs +++ b/src/System.Management.Automation/engine/Modules/ImportModuleCommand.cs @@ -388,7 +388,7 @@ private void ImportModule_ViaLocalModuleInfo(ImportModuleOptions importModuleOpt { PSModuleInfo alreadyLoadedModule = null; Context.Modules.ModuleTable.TryGetValue(module.Path, out alreadyLoadedModule); - if (!BaseForce && IsModuleAlreadyLoaded(alreadyLoadedModule)) + if (!BaseForce && DoesAlreadyLoadedModuleSatisfyConstraints(alreadyLoadedModule)) { AddModuleToModuleTables(this.Context, this.TargetSessionState.Internal, alreadyLoadedModule); diff --git a/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs b/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs index 1497941757c..7c75ad049b0 100644 --- a/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs +++ b/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs @@ -1583,7 +1583,7 @@ internal PSModuleInfo LoadModuleManifest( } } - if (importingModule && IsModuleAlreadyLoaded(loadedModule)) + if (importingModule && DoesAlreadyLoadedModuleSatisfyConstraints(loadedModule)) { if (!BaseForce) { @@ -4993,7 +4993,7 @@ private bool ShouldModuleBeRemoved(PSModuleInfo module, string moduleNameInRemov /// /// The already loaded module that matched the name of the module to load. /// True if the pre-loaded module matches all GUID and version constraints provided, false otherwise. - internal bool IsModuleAlreadyLoaded(PSModuleInfo alreadyLoadedModule) + internal bool DoesAlreadyLoadedModuleSatisfyConstraints(PSModuleInfo alreadyLoadedModule) { return ModuleIntrinsics.IsModuleMatchingConstraints( alreadyLoadedModule, @@ -5017,7 +5017,7 @@ internal PSModuleInfo IsModuleImportUnnecessaryBecauseModuleIsAlreadyLoaded(stri PSModuleInfo alreadyLoadedModule; if (this.Context.Modules.ModuleTable.TryGetValue(modulePath, out alreadyLoadedModule)) { - if (this.IsModuleAlreadyLoaded(alreadyLoadedModule)) + if (this.DoesAlreadyLoadedModuleSatisfyConstraints(alreadyLoadedModule)) { if (this.BaseForce) // remove the previously imported module + return null (null = please proceed with regular import) { @@ -5155,7 +5155,7 @@ internal PSModuleInfo LoadUsingExtensions(PSModuleInfo parentModule, // If the module has already been loaded, just emit it and continue... Context.Modules.ModuleTable.TryGetValue(fileName, out module); - if (!BaseForce && importingModule && IsModuleAlreadyLoaded(module)) + if (!BaseForce && importingModule && DoesAlreadyLoadedModuleSatisfyConstraints(module)) { moduleFileFound = true; module = Context.Modules.ModuleTable[fileName]; @@ -5200,7 +5200,7 @@ internal PSModuleInfo LoadUsingExtensions(PSModuleInfo parentModule, { moduleFileFound = true; // Win8: 325243 - Added the version check so that we do not unload modules with the same name but different version - if (BaseForce && IsModuleAlreadyLoaded(module)) + if (BaseForce && DoesAlreadyLoadedModuleSatisfyConstraints(module)) { RemoveModule(module); }