Sdk: Capitalization of build folders names in case sensitive OS's

Created on 10 Nov 2016  路  14Comments  路  Source: dotnet/sdk

Moved from https://github.com/dotnet/cli/issues/2010 on behalf of @dmportella and @joshfree


Moved from https://github.com/dotnet/corefx/issues/7173 on behalf of @dmportella

HI everyone

Not sure if this is the right place to post this request but I wanted to ask for a change in the build folder setup for dotnet applications, I would like bin/Debug, bin/Release to be by default lower case. I know sounds lazy and all changing this would have no effect on a windows platform but would go be so cool not having to keep reminding yourself that the debug folder name is {caps}Debug and not debug.

image

Anyway

peace

Bug up-for-grabs

Most helpful comment

Change defaults is annoying. was like that in ages. maybe can we wait next big sdk change to cleanup/improve?

I think we're in the middle of the "next big sdk change", so now seems a good time to consider changing this.

MSBuild will treat /p:Configuration=Release and /p:Configuration=RElease the same, except now with the SDK the default output paths will be different on case-sensitive file systems. We'd run into the same problem even if the default configurations were "debug" and "release" and someone spelled them Debug and Release on the command line.

Because of this, I'm now leaning towards saying we should leave the Configurations capitalized, but when we construct the output and intermediate paths in the .targets files, we should convert the Configuration to lowercase before using it as a path element. This would only apply if someone hasn't already set the OutputPath and IntermediateOutputPath properties, so it would still be possible to use a different capitalization if you wanted.

All 14 comments

oh goodies... up for grabs it will be nice to get this :+1:

:-1: for two reason:

  • the configuration name passed is with upper case (like Debug), in not debug. So if i want to compose some path who contains the configuration name, i need to add a special ${Configuration}.ToLower() in script/tool/targets, instead of just ${Configuration}.
  • some tools already works xplat, expecially with xbuild/scripts and doesnt do .ToLower() atm, so is going to give annoying migration issues in case sensitive file system.

@enricosada
I know what you mean. I was looking at the code and it is really all based on the configuration name (which can be anything like "BanAnas"). The code tasks themselves dont need to be changed at all so no need to do the ${Configuration}.ToLower() as you said. It just the default bin locations where they are specified for the project that need to be set to debug and release. So no code changes.

Example of lines that need changing are:

https://github.com/dotnet/sdk/blob/master/src/Templates/CSharpTemplatesSetup/CSharpTemplatesSetup.csproj#L37
https://github.com/dotnet/sdk/blob/master/src/Templates/CSharpTemplatesSetup/CSharpTemplatesSetup.csproj#L43

That is just one example there are other proj files in this solution.

Its not even the configuration names actually its just the outputpath as they are all hardcoded in the project files.

@rainersigwald @cdmihai @AndyGerlicher @nguerrera @davkean What are you folks's thoughts on this? Should we make the default configurations lowercase? Should we call ToLower() whenever we construct a path based on the configuration in case someone miscapitalizes it (for example /p:Configuration=RElease).

I wanted really just change the default values to lowercase I don't mind if people want it as uppercase on its just annoying that in linux environment which is case sensitive you get used to write everything in lowercase as you are typing less keys to get where you want but when you have mixed capitalization on folders structures you end up having to ls just to see the correct spelling of the folder so you can cd into it.

This is the reason why I raised this issue in the first place I appreciate that changing the targets to call ToLower() on the output would make a few people nervous because well someone out there must have used a string comparison that is case sensitive. I would like to change just the defaults if that is possible.

To the example raised /p:Configuration=RELease where someone just misspelled the config name they are using I would expect MSBuild to do string comparisons that are case insensitive - always.

I mean stands to reason who would have 2 configurations in the same solution called Debug and debug

May be where we have comparisons we could do something like this.

[System.String]::Compare($(Configuration), 'Release', true) == 0

You couldn't compare Folder names like this as in Linux debug and Debug can be two different folders in the same location but I am suggesting that the default configuration is set to default lowercase so no comparisons would be affected.

Change defaults is annoying. was like that in ages. maybe can we wait next big sdk change to cleanup/improve?
Lots of little changes make things annoying to upgrade.

An example, someone changed to lowercase the directory names in nuget cache (like C:\Users\%USERNAME%\.nuget\packages\mstest.testadapter) after dotnet restore why?
I didnt checked because using windows atm, but i was sure was same case of package name in osx.
Now to reference that directory instead of just concat the package name, i need to explicit do lowercase it? annoying little change.

Personally, I'm a "your shell should do case-insensitive tab completion even on a case-sensitive system" person.

Professionally, I don't like changing a very longstanding default that fits with the (admittedly Microsoft-y) naming schemes of .NET and VS. I'm also not a fan of the likely bug tail of chasing down all the places we didn't realize we needed to call ToLower.

A possible compromise would be to change the default in the new SDK that lives in this repo, since those configurations don't live in the project file any more. But it would still feel a little unpolished for this to be all lower case:
image

Change defaults is annoying. was like that in ages. maybe can we wait next big sdk change to cleanup/improve?

I think we're in the middle of the "next big sdk change", so now seems a good time to consider changing this.

MSBuild will treat /p:Configuration=Release and /p:Configuration=RElease the same, except now with the SDK the default output paths will be different on case-sensitive file systems. We'd run into the same problem even if the default configurations were "debug" and "release" and someone spelled them Debug and Release on the command line.

Because of this, I'm now leaning towards saying we should leave the Configurations capitalized, but when we construct the output and intermediate paths in the .targets files, we should convert the Configuration to lowercase before using it as a path element. This would only apply if someone hasn't already set the OutputPath and IntermediateOutputPath properties, so it would still be possible to use a different capitalization if you wanted.

I agree with @dsplaisted and this is where we landed for https://github.com/dotnet/sdk/issues/293 too.

I am cool with this

@dmportella I think we have enough agreement to go ahead with leaving the Configurations themselves capitalized, but using a lowercased version of them when creating the default output path. So feel free to update your PR accordingly.

@davkean @nguerrera Should we use ToLower(), ToLowerInvariant(), or something else? How do we correctly handle the Turkish "i"?

Use ToLowerInvvariant.

@dsplaisted awesome I will make the changes to the default output path file and use the ToLowerInvariant as suggested

Should we do something like this? https://gist.github.com/dmportella/9eea84e48ee8db6ffbc0671712632b7e

Just wondering is we should just lower the configuration when used on the default path or whether we should lower the entire finished output path. I think the answer is probably a no.

Since there was some back and forth of opinions on this and we're about to ship without changing it, I think it should be optional or just not done at all. Something like UseLowerCaseDefaultPaths=true could be the mechanism if we really need it.

Was this page helpful?
0 / 5 - 0 ratings