Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added logic to handle extraneous double-quotes around value for --msbuild-parameters that could be passed in .NET argument for certain execution environments. #360

Merged

Conversation

ashishdhingra
Copy link
Contributor

Issue #, if available: #358

Description of changes:
Added logic to handle extraneous double-quotes around value for --msbuild-parameters that could be passed in .NET argument for certain execution environments.

Customer is invoking dotnet lambda tooling via JavaScript running using exec.exec() from actions/github-script@v6 package in a GitHub action. For this environment, somehow value for --msbuild-parameters enclosed in double quotes has leading and trailing double quotes. Did below testing as well:

  • Removed use of double quotes all together, it only takes first value for --msbuild-parameters, ignoring other values if any.
  • Tried enclosing value in single quotes ', it tries to parse only first value beginning with single quote (e.g. '----no-restore).

NOTE(s):

  • CommandOptions also has separate property named MSBuildParameters used for storing parameters that start with /p: based on logic here.
    • These MS Build parameters are appended to existing values that might have been supplied using --msbuild-parameters switch (e.g., refer code here).
    • Hence, we would need to process trimming of enclosing double quotes " before we append CommandOptions.MSBuildParameters.
    • Values for consolidated MSBuild parameters are also used at different places while determining project information, e.g. TargetFramework using Utilitites.LookupTargetFrameworkFromProjectFile(), where it uses Utilities.LookupProjectProperties() and invokes msbuild command in a separate Process. This command would always fail if we don't handle this issue (assuming compatible parameter for msbuild command is used. We do have a fallback mechanism to inspect .csproj project file though.
  • We should only handle enclosing double-quote scenario only if --msbuild-parameters value start and end with a double-quote.
  • --msbuild-parameters is currently only supported by Amazon.Lambda.Tools.
    • Based on discussion with @normj, we could only version bump Amazon.Lambda.Tools.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@@ -81,5 +81,22 @@ public void ParseMSBuildParameters()
Assert.NotNull(param);
Assert.Equal("us-west-2", param.Item2.StringValue);
}

[Fact]
public void ParseMSBuildSwitchDoubleQuotedValue()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you add a test case for when -msbuild-parameters does not have double quotes

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added. I also added additional test in test/Amazon.Lambda.Tools.Test/CommandLineParserTest.cs that has scenario where both --msbuild-parameters enclosed in double quote and /p: MSBuild parameters are present.

…uild-parameters that could be passed in .NET argument for certain execution environments.
@ashishdhingra ashishdhingra force-pushed the user/ashdhin/MSBuildParamsDoubleQuoteFix-Issue358 branch from 3cac411 to 3ce8925 Compare January 9, 2025 22:27
@ashishdhingra ashishdhingra merged commit b17ea62 into dev Jan 10, 2025
3 checks passed
@ashishdhingra ashishdhingra deleted the user/ashdhin/MSBuildParamsDoubleQuoteFix-Issue358 branch January 14, 2025 02:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants