-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Add #:project
directive
#49311
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
Add #:project
directive
#49311
Conversation
var resolvedProjectPath = Path.Combine(sourceDirectory, directiveText.Replace('\\', '/')); | ||
if (Directory.Exists(resolvedProjectPath)) | ||
{ | ||
var fullFilePath = MsbuildProject.GetProjectFileFromDirectory(resolvedProjectPath).FullName; |
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.
A few lines above we normalize \\
to /
but FullName
on windows will still have \\
in the path. Should we re-normalize here?
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.
Consider user writing #:project ..\lib
on Windows and another user executing that on Linux. The normalization above makes sure that's possible.
Normalization here isn't necessary for that to work, but I guess we could normalize for other reasons. But the current behavior (without normalization) should be consistent with what dotnet reference add
is doing (since we use the same utility MsbuildProject.GetProjectFileFromDirectory
as that command).
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.
It's not obvious to me that #:project ..\lib
should work on Linux. Do you happen to know the behavior of dotnet run --project ..\lib
?
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.
dotnet run --project ..\lib
probably wouldn't work. But <ProjectReference Include="..\lib\lib.csproj" />
would. A general rule I take from that is that using \
on command line won't work but in a source code (as #:project
or <ProjectReference/>
) works. I guess the reason it works in <ProjectReference/>
is that msbuild started as Windows-only and they wanted it to be portable. We don't need to do that for #:project
I guess. So I'm not against not normalizing at all and in effect forcing users to use forward slashes if they want to write cross-platform code. Let me know what you think.
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.
Yep MSBuild "solves" this for us as paths there are normalized. We can decide to simply pass through what the user types in the directive directly I think.
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.
We can decide to simply pass through what the user types in the directive directly I think.
It's not so simple since we also want to support users specifying directory paths which <ProjectReference/>
elements don't support, therefore we might need to search for the project file in the directory and for that to work on linux, we need to normalize slashes.
Resolves #48746.