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

MsBuild should 'unescape' properties from the command line on all places #4718

Closed
matthid opened this issue Sep 11, 2019 · 3 comments
Closed
Labels

Comments

@matthid
Copy link

matthid commented Sep 11, 2019

Related

#4086
#3468
#2178

Steps to reproduce

Originally reported here:
fsprojects/FAKE#2392

Project file

<Project xmlns="http://schemas.microsoft.com/developer/msbuild/2003" DefaultTargets="Build">
    <UsingTask TaskName="WixAssignCulture" AssemblyFile="$(WixTasksPath)" />
    <Target Name="Build">
        <Message Text="__BEFORE__$(Arg)__AFTER__" />
        <WixAssignCulture />
    </Target>
</Project>

Command line

msbuild /p:WixTargetsPath=c:%5CCode%5Cpackages%5Cbuild%5CWiX%5Ctools%5Cwix.targets

Expected behavior

$(WixTasksPath) is properly unescaped, according to https://docs.microsoft.com/en-us/visualstudio/msbuild/msbuild-special-characters?view=vs-2017

Actual behavior

$(WixTasksPath) is not unescaped in this position. Others (like printing a message) work.

Description

The problem is that we don't know a workaround which works "properly" in all situations we have cross platform unit tests and they start failing as soon as we disable escaping for / or \, I guess this is due to the related issues.

What is the proper way to get a string into a property over the command line which works for all special and non-special characters?

Environment data

msbuild /version output:

$ dotnet msbuild /version
Microsoft (R) Build Engine version 15.9.20+g88f5fadfbe for .NET Core
Copyright (C) Microsoft Corporation. All rights reserved.

15.9.20.62856

OS info: Windows 10

If applicable, version of the tool that invokes MSBuild (Visual Studio, dotnet CLI, etc):
FAKE

/cc @vbfox

@vbfox
Copy link

vbfox commented Sep 11, 2019

Seem like the using task case has already been ruled out as by design as it's legacy and won't be changed in #885 (comment) :(

@rainersigwald
Copy link
Member

Team triage: we think the reasoning from the previous won't-fix decision stands, unfortunately:

The evaluation of UsingTask elements does not unescape the attribute under AssemblyFile: https://github.com/Microsoft/msbuild/blob/master/src/Build/Instance/TaskRegistry.cs#L276

Since this behaviour has been there forever, changing it could break existing code. The workaround would be to manually unescape the string in the UsingTask via the [MSBuild]::Unescape intrinsic property function.

@rainersigwald
Copy link
Member

Duplicate of #885

@rainersigwald rainersigwald marked this as a duplicate of #885 Sep 23, 2019
@AR-May AR-May added the triaged label Feb 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants