Make sure that SettingFile arg is parsed before we load the settings #7449
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -200,6 +200,7 @@ internal CommandLineParameterParser(PSHostUserInterface hostUI, string bannerTex | |
| _helpText = helpText; | ||
| } | ||
|
|
||
| #region Internal properties | ||
| internal bool AbortStartup | ||
| { | ||
| get | ||
|
|
@@ -393,6 +394,205 @@ internal string WorkingDirectory | |
| get { return _workingDirectory; } | ||
| } | ||
|
|
||
| #endregion Internal properties | ||
|
|
||
| #region static methods | ||
| /// <summary> | ||
| /// Processes the -SettingFile Argument. | ||
| /// </summary> | ||
| /// <param name="args"> | ||
| /// The command line parameters to be processed. | ||
| /// </param> | ||
| /// <param name="settingFileArgIndex"> | ||
| /// The index in args to the argument following '-SettingFile'. | ||
| /// </param> | ||
| /// <param name="parser"> | ||
| /// Used to allow the helper to write errors to the console. If not supplied, no errors will be written. | ||
| /// </param> | ||
| /// <returns> | ||
| /// Returns true if the argument was parsed successfully and false if not. | ||
| /// </returns> | ||
| private static bool TryParseSettingFileHelper(string[] args, int settingFileArgIndex, CommandLineParameterParser parser) | ||
| { | ||
| if (settingFileArgIndex >= args.Length) | ||
| { | ||
| if (parser != null) | ||
| { | ||
| parser.WriteCommandLineError( | ||
| CommandLineParameterParserStrings.MissingSettingsFileArgument); | ||
| } | ||
|
|
||
| return false; | ||
| } | ||
|
|
||
| string configFile = null; | ||
| try | ||
| { | ||
| configFile = NormalizeFilePath(args[settingFileArgIndex]); | ||
| } | ||
| catch (Exception ex) | ||
| { | ||
| if (parser != null) | ||
| { | ||
| string error = string.Format(CultureInfo.CurrentCulture, CommandLineParameterParserStrings.InvalidSettingsFileArgument, args[settingFileArgIndex], ex.Message); | ||
| parser.WriteCommandLineError(error); | ||
| } | ||
|
|
||
| return false; | ||
| } | ||
|
|
||
| if (!System.IO.File.Exists(configFile)) | ||
| { | ||
| if (parser != null) | ||
| { | ||
| string error = string.Format(CultureInfo.CurrentCulture, CommandLineParameterParserStrings.SettingsFileNotExists, configFile); | ||
| parser.WriteCommandLineError(error); | ||
| } | ||
|
|
||
| return false; | ||
| } | ||
| PowerShellConfig.Instance.SetSystemConfigFilePath(configFile); | ||
| return true; | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Processes the command line parameters to ConsoleHost which must be parsed before the Host is created. | ||
| /// Success to indicate that the program should continue running. | ||
| /// </summary> | ||
| /// <param name="args"> | ||
| /// The command line parameters to be processed. | ||
| /// </param> | ||
| internal static void EarlyParse(string[] args) | ||
| { | ||
| // indicates that we've called this method on this instance, and that when it's done, the state variables | ||
| // will reflect the parse. | ||
|
|
||
| EarlyParseHelper(args); | ||
| } | ||
|
|
||
| private static string GetConfigurationNameFromGroupPolicy() | ||
| { | ||
| // Current user policy takes precedence. | ||
| var consoleSessionSetting = Utils.GetPolicySetting<ConsoleSessionConfiguration>(Utils.CurrentUserThenSystemWideConfig); | ||
|
|
||
| return (consoleSessionSetting?.EnableConsoleSessionConfiguration == true && !string.IsNullOrEmpty(consoleSessionSetting?.ConsoleSessionConfigurationName)) ? | ||
| consoleSessionSetting.ConsoleSessionConfigurationName : string.Empty; | ||
|
Collaborator
There was a problem hiding this comment. Maybe use more short name like
Member
Author
There was a problem hiding this comment. this is an existing variable name. Please file an issues |
||
| } | ||
|
|
||
| /// <summary> | ||
| /// Processes the command line parameters to ConsoleHost which must be parsed before the Host is created. | ||
| /// Success to indicate that the program should continue running. | ||
| /// </summary> | ||
| /// <param name="args"> | ||
| /// The command line parameters to be processed. | ||
| /// </param> | ||
| private static void EarlyParseHelper(string[] args) | ||
| { | ||
| if(args == null) | ||
| { | ||
| Dbg.Assert(args != null, "Argument 'args' to EarlyParseHelper should never be null"); | ||
| return; | ||
| } | ||
|
|
||
| bool noexitSeen = false; | ||
| for (int i = 0; i < args.Length; ++i) | ||
| { | ||
| (string SwitchKey, bool ShouldBreak) switchKeyResults = GetSwitchKey(args, ref i, parser: null, ref noexitSeen); | ||
| if (switchKeyResults.ShouldBreak) | ||
| { | ||
| break; | ||
| } | ||
|
|
||
| string switchKey = switchKeyResults.SwitchKey; | ||
|
|
||
| if (MatchSwitch(switchKey, match: "settingsfile", smallestUnambiguousMatch: "settings")) | ||
| { | ||
| // parse setting file arg and don't write error as there is no host yet. | ||
| if (!TryParseSettingFileHelper(args, ++i, parser: null)) | ||
| { | ||
| break; | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Gets the word in a switch from the current argument or parses a file. | ||
| /// For example -foo, /foo, or --foo would return 'foo'. | ||
| /// </summary> | ||
| /// <param name="args"> | ||
| /// The command line parameters to be processed. | ||
| /// </param> | ||
| /// <param name="argIndex"> | ||
| /// The index in args to the argument to process. | ||
| /// </param> | ||
| /// <param name="parser"> | ||
| /// Used to parse files in the args. If not supplied, Files will not be parsed. | ||
| /// </param> | ||
| /// <param name="noexitSeen"> | ||
| /// Used during parsing files. | ||
| /// </param> | ||
| /// <returns> | ||
| /// Returns a Tuple: | ||
| /// The first value is a String called SwitchKey with the word in a switch from the current argument or null. | ||
| /// The second value is a bool called ShouldBreak, indicating if the parsing look should break. | ||
| /// </returns> | ||
| private static (string SwitchKey, bool ShouldBreak) GetSwitchKey(string[] args, ref int argIndex, CommandLineParameterParser parser, ref bool noexitSeen) | ||
| { | ||
| string switchKey = args[argIndex].Trim().ToLowerInvariant(); | ||
| if (string.IsNullOrEmpty(switchKey)) | ||
| { | ||
| return (SwitchKey: null, ShouldBreak: false); | ||
| } | ||
|
|
||
| if (!SpecialCharacters.IsDash(switchKey[0]) && switchKey[0] != '/') | ||
| { | ||
| // then its a file | ||
| if (parser != null) | ||
| { | ||
| --argIndex; | ||
| parser.ParseFile(args, ref argIndex, noexitSeen); | ||
| } | ||
|
|
||
| return (SwitchKey: null, ShouldBreak: true); | ||
| } | ||
|
|
||
| // chop off the first character so that we're agnostic wrt specifying / or - | ||
| // in front of the switch name. | ||
| switchKey = switchKey.Substring(1); | ||
|
|
||
| // chop off the second dash so we're agnostic wrt specifying - or -- | ||
| if (!string.IsNullOrEmpty(switchKey) && SpecialCharacters.IsDash(switchKey[0])) | ||
| { | ||
| switchKey = switchKey.Substring(1); | ||
| } | ||
|
|
||
| return (SwitchKey: switchKey, ShouldBreak: false); | ||
| } | ||
|
|
||
| private static string NormalizeFilePath(string path) | ||
| { | ||
| // Normalize slashes | ||
| path = path.Replace(StringLiterals.AlternatePathSeparator, | ||
| StringLiterals.DefaultPathSeparator); | ||
|
|
||
| return Path.GetFullPath(path); | ||
|
Collaborator
There was a problem hiding this comment. The method is used once and then we call File.Exist() where Path.GetFullPath() is called again.
Member
Author
There was a problem hiding this comment. existing code, please file an issue, the method was just changed from an instance to static |
||
| } | ||
|
|
||
| private static bool MatchSwitch(string switchKey, string match, string smallestUnambiguousMatch) | ||
| { | ||
| Dbg.Assert(switchKey != null, "need a value"); | ||
| Dbg.Assert(!String.IsNullOrEmpty(match), "need a value"); | ||
| Dbg.Assert(match.Trim().ToLowerInvariant() == match, "match should be normalized to lowercase w/ no outside whitespace"); | ||
| Dbg.Assert(smallestUnambiguousMatch.Trim().ToLowerInvariant() == smallestUnambiguousMatch, "match should be normalized to lowercase w/ no outside whitespace"); | ||
| Dbg.Assert(match.Contains(smallestUnambiguousMatch), "sUM should be a substring of match"); | ||
|
|
||
| return (match.Trim().ToLowerInvariant().IndexOf(switchKey, StringComparison.Ordinal) == 0 && | ||
| switchKey.Length >= smallestUnambiguousMatch.Length); | ||
| } | ||
|
|
||
| #endregion | ||
|
|
||
| private void ShowHelp() | ||
| { | ||
| Dbg.Assert(_helpText != null, "_helpText should not be null"); | ||
|
|
@@ -463,57 +663,20 @@ internal void Parse(string[] args) | |
| } | ||
| } | ||
|
|
||
| private void ParseHelper(string[] args) | ||
| { | ||
| Dbg.Assert(args != null, "Argument 'args' to ParseHelper should never be null"); | ||
| bool noexitSeen = false; | ||
|
|
||
| for (int i = 0; i < args.Length; ++i) | ||
| { | ||
| (string SwitchKey, bool ShouldBreak) switchKeyResults = GetSwitchKey(args, ref i, this, ref noexitSeen); | ||
| if (switchKeyResults.ShouldBreak) | ||
| { | ||
| break; | ||
| } | ||
|
|
||
| string switchKey = switchKeyResults.SwitchKey; | ||
|
|
||
| // If version is in the commandline, don't continue to look at any other parameters | ||
| if (MatchSwitch(switchKey, "version", "v")) | ||
|
|
@@ -711,34 +874,13 @@ private void ParseHelper(string[] args) | |
| } | ||
| } | ||
|
|
||
| else if (MatchSwitch(switchKey, "settingsfile", "settings")) | ||
| { | ||
| // Parse setting file arg and write error | ||
| if (!TryParseSettingFileHelper(args, ++i, this)) | ||
| { | ||
| break; | ||
| } | ||
| } | ||
| #if STAMODE | ||
| // explicit setting of the ApartmentState Not supported on NanoServer | ||
|
|
@@ -818,25 +960,6 @@ private void WriteCommandLineError(string msg, bool showHelp = false, bool showB | |
| _exitCode = ConsoleHost.ExitCodeBadCommandLineParameter; | ||
| } | ||
|
|
||
| private void ParseFormat(string[] args, ref int i, ref Serialization.DataFormat format, string resourceStr) | ||
| { | ||
| StringBuilder sb = new StringBuilder(); | ||
|
|
@@ -892,15 +1015,6 @@ private void ParseExecutionPolicy(string[] args, ref int i, ref string execution | |
| executionPolicy = args[i]; | ||
| } | ||
|
|
||
| private bool ParseFile(string[] args, ref int i, bool noexitSeen) | ||
| { | ||
| // Process file execution. We don't need to worry about checking -command | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -148,6 +148,12 @@ internal static int Start( | |
|
|
||
| try | ||
| { | ||
| // We need to read the settings file before we create the console host | ||
| string[] tempArgs = new string[args.GetLength(0)]; | ||
| args.CopyTo(tempArgs, 0); | ||
|
Collaborator
There was a problem hiding this comment. Could you please clarify why we need clone
Member
Author
There was a problem hiding this comment. The existing code cloned the args. I don't want to break something by not cloning.
Member
Author
There was a problem hiding this comment. Reviewing the code. I don't think it has to be. I especially don't think it has to be twice. compromise, I'll remove the second clone, you file an issue to remove the first one. |
||
|
|
||
| CommandLineParameterParser.EarlyParse(tempArgs); | ||
|
Contributor
There was a problem hiding this comment. I've created issue #7460 to move this call to UnmanagedPSEntry.Start prior to PSEtwLog.LogConsoleStartup(). |
||
|
|
||
| // We might be able to ignore console host creation error if we are running in | ||
| // server mode, which does not require a console. | ||
| HostException hostException = null; | ||
|
|
@@ -168,8 +174,6 @@ internal static int Start( | |
| s_cpp = new CommandLineParameterParser( | ||
| (s_theConsoleHost != null) ? s_theConsoleHost.UI : (new NullHostUserInterface()), | ||
| bannerText, helpText); | ||
|
|
||
| s_cpp.Parse(tempArgs); | ||
|
|
||
|
|
||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I assume this means we won't be running tests against the prior OS version, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I found no differences in the log format, but yes correct