-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
[Bug]: TaskItem normalises path to build platform, not target platform #10121
Comments
Oh that's a very interesting failure mode for this. I'd say it's an MSBuild bug, but it's a hard one to fix, so we might need to figure out how to work around for the SDK. |
Thanks for recognizing it as a bug 😄 It's probably an edge case, as almost every property that contains a path should be correct on the build server. However, there might be some others that would make sense to resolve on the target platform as well. Maybe dotnet tools have the same issue? But, I'm not sure. We'll have to think a bit on how to solve this. I made a first attempt in my fork of the SDK repo at "fixing" it by swapping out the Do you have suggestions on the best way of fixing this? Is there anywhere in the build pipeline we can decide which implementation of |
One possible hacky option: renormalize inside the task, which should know the target OS's preferred slash direction :-/
This sounds directionally correct to me, but I'm afraid I haven't been in that portion of the code for a while and don't remember offhand where there might be some good hooks to change things. Another possible option would be an attribute on the
|
… of TaskItem using metadata To be able to have TaskItems that you don't want normalised, or you want then normalised to another OS, e.g. when using dotnet publish to containers to another OS than you are building on, I suggest using "magic" metadata names to be able to tell in an MSBuild file that you want a property on your build item of type TaskItem to either: * _not_ have the paths normalised at all, by setting the FixFilePath metadata to "false" (defaults to true, of course), or * normalise the paths to a different platform by specifying the TargetOs metadata with one of the following values: - windows - unix - current (is also used if the property is specified, but an empty or invalid value is supplied)
I haven't looked at the details of the PR but I am broadly in favor of having a way for items, item types, or tasks to opt out of the path "correction"--since it's heuristic it's caused problems before. |
I want to keep this open and see if it bites more people - It feels like the kind of thing that could be handled on a Task-by-Task basis, perhaps with an annotation on the Task's Input itself? |
I think keeping this one open is a good idea. I'm not sure whether the "build for an OS that has a different directory separator char than the one you are building on, AND, having to resolve paths at build-time, that are to be used runtime" is a very, very corner-case (only applicable to container builds), or if it's a broader issue. That said, having worked a bit more on the PR, I'm not so much a fan of my own suggestion to solve the problem. It got a bit ugly, since the evaluated (already path-replaced) string is stored on the Additionally, there are many implementations of the I fear "fixing" this in the One possible way is, of course, to just go all the way, and accept that Windows works well with forward-slash paths, and has done for decades, but I think people will feel this is obtrusive. Sorry, just a lot of loose, random thoughts here, but I think the "idea bubbling phase" is important here, both to hammer out:
In my head, the short-term solution to choose, is depending on the answers to the questions above too. |
Issue Description
I've been working a bit on a bug in the dotnet/sdk#559, and stumbled upon something that might be considered a bug in either Microsoft.Build.Utilities.TaskItem, or Microsoft.Build.Shared.FileUtilities.FixFilePath. One might also argue that it's not a bug, and that it's the usage of it in Microsoft.NET.Build.Containers.Tasks.CreateNewImage that is wrong.
Anyway, the problem is, when publishing dotnet projects to containers, some properties are set on the CreateNewImage MSBuild task, that are needed runtime in the built container. So, the paths need to be compatible with the container target platform, not the platform it's built on. The properties in question are (at least) Entrypoint, EntrypointArgs, AppCommand, AppCommandArgs.
However, as the properties are of type
ITaskItem
, implemented byTaskItem
, the paths are normalised when initialised. And, the normalisation uses the Path.DirectorySeparatorChar of the build platform, not the target.Would you consider this a bug/shortcoming in the
TaskItem
, or are we/they using it in the wrong way in the MSBuild task?Steps to Reproduce
Use a
TaskItem
in an MSBuild task, where you need the directory separator char of a different platform than the one you are building on (e.g. use C:\app\foo.exe when building on *nix)Expected Behavior
The item should stay as C:\app\foo.exe
Actual Behavior
The path is changed to C:/app/foo.exe
Analysis
msbuild/src/Utilities/TaskItem.cs
Line 80 in 8632094
There is no way to tell either: 1) That you don't want normalisation of directory separator characters, or 2) which style of normalisation you want
Versions & Configurations
dotnet msbuild --version
17.11.0.25706
The text was updated successfully, but these errors were encountered: