Skip to content

Make nuget packages path use forward slashes. - #977

Merged
VSadov merged 1 commit into
dotnet:masterfrom
VSadov:nugetConfig
Mar 4, 2015
Merged

Make nuget packages path use forward slashes.#977
VSadov merged 1 commit into
dotnet:masterfrom
VSadov:nugetConfig

Conversation

@VSadov

@VSadov VSadov commented Mar 3, 2015

Copy link
Copy Markdown
Member

Need to use forward slashes to be compatible with Unix-like platforms.

Need to use forward slashes to be compatible with Unix-like platforms.
@VSadov VSadov changed the title Removed file locking when opening resource files. Make nuget packages path use forward slashes. Mar 3, 2015
@gafter gafter added Area-Infrastructure Concept-Portability The issue deals with portable code (portable libraries, etc.). labels Mar 4, 2015
@gafter

gafter commented Mar 4, 2015

Copy link
Copy Markdown
Member

👍

@agocke

agocke commented Mar 4, 2015

Copy link
Copy Markdown
Member

:shipit:

@jaredpar

jaredpar commented Mar 4, 2015

Copy link
Copy Markdown
Member

Why is NuGet special here? Nowhere else are we using forward slashes, the expectation is the tools on top need to be smart about slashes when crossing platforms.

@VSadov

VSadov commented Mar 4, 2015

Copy link
Copy Markdown
Member Author

My view on this is that indeed, the tools on the top that perform path resolution and pass paths to other tools downlevel (like MSBuild) must be smart with slashes. However in this case there is no top level tool. The path is simply stored in a config file and NuGet does not do any resolution, just simply passes it to the file system. From this view it seems that we just need to fix the path in the config.

VSadov added a commit that referenced this pull request Mar 4, 2015
Make nuget packages path use forward slashes.
@VSadov
VSadov merged commit c0fa6e9 into dotnet:master Mar 4, 2015
@VSadov
VSadov deleted the nugetConfig branch March 4, 2015 18:36
@jaredpar

jaredpar commented Mar 4, 2015

Copy link
Copy Markdown
Member

@VSadov how is this not broken for Windows?

@jaredpar

jaredpar commented Mar 4, 2015

Copy link
Copy Markdown
Member

You're saying there is no top level tool but we're also making a change to un-break something. So what's broken? And why is that item not dealing with the path like every other .Net tool.

@VSadov

VSadov commented Mar 4, 2015

Copy link
Copy Markdown
Member Author

The path in the config file is is not correct on Unix-like platforms. If the path was created by an automated tool/process we would need to fix that, but the path is simply directly editable data. Essentially the top level tool that controls this path is us and we should just fix the path.
It is the same as if we had a path in a project file in wrong case. Assuming support for case-sensitive environments, that would be a human error and the proj file would need to be fixed.

@jaredpar

jaredpar commented Mar 4, 2015

Copy link
Copy Markdown
Member

I'm still not understanding what broke? What prompted this change?

@VSadov

VSadov commented Mar 4, 2015

Copy link
Copy Markdown
Member Author

Without this change, "NuGet.exe restore src/Roslyn.sln " restores packages literally into ./....\packages
Note that "....\packages" is a valid directory name on Unix and on the part of NuGet this is technically a correct behavior.

@jaredpar

jaredpar commented Mar 4, 2015

Copy link
Copy Markdown
Member

Why don't we think this is a bug in NuGet? We are expecting all of our other tools like MSBuild to do path normalization on Linux. I don't see how NuGet is special here.

@VSadov

VSadov commented Mar 4, 2015

Copy link
Copy Markdown
Member Author

I actually expect that most tools do not need to get involved with paths. MSBuild is special here since it is already heavily involved in the business of path resolution/canonization.
It is good question to the NuGet owners whether they see NuGet as a neutral consumer of paths (like csc.exe) or as a tool that wants to deal with paths in some platform-aware fashion (like MSBuild). At the moment it clearly just passes paths through to IO, therefore the fix is needed at least to get unblocked.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area-Infrastructure Concept-Portability The issue deals with portable code (portable libraries, etc.).

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants