Skip to content

dotnet workload install should normalize pack directory name to lower case  #17438

Closed

Description

@grendello

dotnet workload install should normalize pack directory name to lower case

Original issue:

I discovered that even though the Xamarin.Android templates package (named Microsoft.Android.Templates.11.0.200-ci.main.216.nupkg) was installed in dotnet/template-packs, it wasn't considered by dotnet new. After running dotnet under strace, I found that it checks for the presence of the following packages in template-packs (apparently hardcoded?):

$ grep template-packs dotnet.log.381805 | grep lstat
lstat("dotnet/template-packs/microsoft.macos.templates.11.1.100-preview.3.1379.nupkg", 0x7f6864ce3350) = -1 ENOENT (No such file or directory)
lstat("dotnet/template-packs/microsoft.macos.templates.11.1.100-preview.3.1379.nupkg", 0x7f6864ce33d0) = -1 ENOENT (No such file or directory)
lstat("dotnet/template-packs/microsoft.android.templates.11.0.200-ci.main.216.nupkg", 0x7f6864ce3350) = -1 ENOENT (No such file or directory)
lstat("dotnet/template-packs/microsoft.android.templates.11.0.200-ci.main.216.nupkg", 0x7f6864ce33d0) = -1 ENOENT (No such file or directory)
lstat("dotnet/template-packs/microsoft.ios.templates.14.4.100-preview.3.1326.nupkg", 0x7f6864ce3350) = -1 ENOENT (No such file or directory)
lstat("dotnet/template-packs/microsoft.ios.templates.14.4.100-preview.3.1326.nupkg", 0x7f6864ce33d0) = -1 ENOENT (No such file or directory)
lstat("dotnet/template-packs/microsoft.maccatalyst.templates.14.3.100-preview.3.471.nupkg", 0x7f6864ce3350) = -1 ENOENT (No such file or directory)
lstat("dotnet/template-packs/microsoft.maccatalyst.templates.14.3.100-preview.3.471.nupkg", 0x7f6864ce33d0) = -1 ENOENT (No such file or directory)
lstat("dotnet/template-packs/microsoft.tvos.templates.14.3.100-preview.3.1379.nupkg", 0x7f6864ce3350) = -1 ENOENT (No such file or directory)
lstat("dotnet/template-packs/microsoft.tvos.templates.14.3.100-preview.3.1379.nupkg", 0x7f6864ce33d0) = -1 ENOENT (No such file or directory)

Copying our package to the dotnet/templates/ directory or renaming it to all lower case in dotnet/template-packs made dotnet new show the templates defined there.

I think for the best cross-platform support, dotnet new should perform a case-insensitive check for the presence of template packages.

Activity

  1. transferred this issue fromdotnet/sdkon Apr 28, 2021
  2. wli3 commented on Apr 29, 2021

    @wli3

    @DavidKarlas could you look into this? This impacts net6.0 workload work

  3. DavidKarlas commented on May 3, 2021

    @DavidKarlas
    Contributor

    Sorry I didn’t start looking into this yet, but I plan this week so its ready for Preview 5.

  4. DavidKarlas commented on May 5, 2021

    @DavidKarlas
    Contributor

    Root cause of problem is that we do NuGet PackageId.ToLower here: https://github.com/dotnet/sdk/blob/cb5b3ff/src/Resolvers/Microsoft.NET.Sdk.WorkloadManifestReader/WorkloadResolver.cs#L215

    This should be fixed one of this two ways:
    A) Stop doing ToLower(but what if original file has ".NUPKG" extension or missmatch between .json and .nupkg file name)
    B) Do "path resolution"/"find" logic on case sensitive file systems this will happen in https://github.com/dotnet/sdk/blob/main/src/Cli/dotnet/commands/dotnet-new/OptionalWorkloadProvider.cs#L47

    Since fix will be in SDK repo, transferring issue there, but keeping assignment on myself since I plan to fix this...

    @mhutch since you wrote original .ToLower code, and since I assume you are familiar with .nupkg file names casing missmatches, what do you think is preferred way to fix this?

  5. ghost added
    untriagedRequest triage from a team member
    on May 5, 2021
  6. mhutch commented on May 14, 2021

    @mhutch
    Contributor

    The ID in the on-disk path of the nupkg and extracted nupkg is expected to be lowercased to match what NuGet does (see ~/.nuget/.packages). This applies to all pack folders (packs, template-packs, library-packs) and OSes.

    It's expected that whatever installs the pack files (e.g dotnet workload install) will do this transformation. Any pack that does not follow this has not been installed correctly.

  7. DavidKarlas commented on May 14, 2021

    @DavidKarlas
    Contributor

    @dsplaisted @wli3 Is it OK if I close this issue? Do you want to open new one for dotnet workload install?

  8. wli3 commented on May 18, 2021

    @wli3

    @DavidKarlas let me just rename this issue

  9. 2 remaining items

  10. added this to the 6.0.1xx milestone on May 18, 2021
  11. mhutch commented on May 19, 2021

    @mhutch
    Contributor

    FWIW I'm not dead set on the lowercasing. If folks have strong opinions otherwise we could consider preserving original case. But either way there should be a consistent policy so that the casing of the paths returned from the manifest reader always matches the casing of the paths installed on disk.

  12. sfoslund commented on May 19, 2021

    @sfoslund
    Member

    @grendello how did install the package to this directory, was it using the dotnet workload install command? I believe the current implementation takes this into account and uses the path provided by the resolver when laying down the packs. I'm not able to repro this with the workload install command.

  13. dsplaisted commented on May 19, 2021

    @dsplaisted
    Member

    Maybe it was maui-check?

  14. grendello commented on May 19, 2021

    @grendello
    Author

    @grendello how did install the package to this directory, was it using the dotnet workload install command? I believe the current implementation takes this into account and uses the path provided by the resolver when laying down the packs. I'm not able to repro this with the workload install command.

    It was installed by our internal code in Xamarin.Android. Until NET6 is released, we build and "install" packages with our runtime in the copy of the current dotnet6 version we're using (also installed by us). The nuget in question was created with CamelCase simply following the convention used for all of our packages.

  15. sfoslund commented on May 24, 2021

    @sfoslund
    Member

    It sounds like the issue was in temporary code and shouldn't repro with the real workload installer then. Is there any action needed here?

  16. wli3 commented on May 24, 2021

    @wli3

    If it is a temporary issue, we can close.

  17. grendello commented on May 25, 2021

    @grendello
    Author

    It sounds like the issue was in temporary code and shouldn't repro with the real workload installer then. Is there any action needed here?

    I think there's a need to fix this. While I noticed it using temporary code, anyone can grab a template pack from the Internet and put it in the template-packs folder without changing its name case. That will result in the templates not being visible.

  18. sfoslund commented on May 25, 2021

    @sfoslund
    Member

    anyone can grab a template pack from the Internet and put it in the template-packs folder without changing its name case

    I imagine we don't want to encourage that scenario anyway.

  19. wli3 commented on May 25, 2021

    @wli3

    Agree. That is not a supported scenario. Especially considering dotnet folder could be in program files.

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

Metadata

Metadata

Assignees

Type

No type

Projects

No projects

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions