Skip to content

[VMR] nuget-client build fails on case-preserving filesystems (Mac, Windows) #3149

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

Closed
directhex opened this issue Dec 2, 2022 · 9 comments · Fixed by NuGet/NuGet.Client#5414
Labels
area-upstream-fix Needs a change in a contributing repo

Comments

@directhex
Copy link

On Linux, NuGet.config and NuGet.Config are different files. Both can exist in the same folder, and attempts to read the "wrong" file will result in a file not found error. As a result, we have a patch in the NuGet repo, eng/source-build-patches/0001-Rename-NuGet.Config-to-NuGet.config-to-account-for-a.patch authored by @crummel, to reconcile a bad casing assumption. However, on Windows and Mac, this patch fails to apply with git apply as those two filenames are considered equivalent. This breaks the build on non-Linux.

We either need a way to separate out source build patches by OS applicability (e.g. eng/source-build-patches/$(TargetOS)/), or need to eliminate the patch.

@ghost ghost added area-upstream-fix Needs a change in a contributing repo untriaged labels Dec 2, 2022
@MichaelSimons MichaelSimons added area-patch-removal Removing patches for contributing repos from source-build and removed area-upstream-fix Needs a change in a contributing repo untriaged labels Dec 8, 2022
@MichaelSimons MichaelSimons moved this to 8.0 Preview 1 in .NET Source Build Dec 8, 2022
@MichaelSimons MichaelSimons added area-upstream-fix Needs a change in a contributing repo and removed area-patch-removal Removing patches for contributing repos from source-build labels Dec 8, 2022
@MichaelSimons
Copy link
Member

[Triage] A fix was accepted in Arcade to address the problem that required the patch. We believe the source-build patch can now be removed.

@directhex
Copy link
Author

Is there a plan to remove the patch from the nuget-client repo? This is still causing build failures on non-Linux:

    Check for patches in /Users/directhex/Projects/dotnet/src/nuget-client/eng/source-build/../../eng/source-build-patches/*.patch
  EXEC : error : NuGet.config: already exists in working directory [/Users/directhex/Projects/dotnet/src/nuget-client/eng/source-build/source-build.proj]
  /Users/directhex/Projects/dotnet/src/nuget-client/eng/source-build/source-build.proj(59,5): error MSB3073: The command "git --work-tree=/Users/directhex/Projects/dotnet/src/nuget-client/eng/source-build/../../ apply --ignore-whitespace --whitespace=nowarn --unsafe-paths "/Users/directhex/Projects/dotnet/src/nuget-client/eng/source-build-patches/0001-Rename-NuGet.Config-to-NuGet.config-to-account-for-a.patch"" exited with code 1.

@MichaelSimons
Copy link
Member

Is there a plan to remove the patch from the nuget-client repo? This is still causing build failures on non-Linux:

Yes, that is what we intend to do with this issue.

@MichaelSimons MichaelSimons moved this from 8.0 Preview 1 to 8.0 Preview 2 in .NET Source Build Feb 13, 2023
@MichaelSimons MichaelSimons moved this from 8.0 Preview 2 to 8.0 Preview 3 in .NET Source Build Mar 2, 2023
@MichaelSimons MichaelSimons moved this from 8.0 Preview 3 to 8.0 Backlog in .NET Source Build Mar 10, 2023
@MichaelSimons MichaelSimons moved this from Needs Review to Backlog in .NET Source Build Jun 8, 2023
@directhex
Copy link
Author

@MichaelSimons what's the status on getting this resolved in nuget-client.git? This patch continues to break building on case-preserving filesystems (i.e. breaks Windows & Mac builds of the VMR)

@MichaelSimons
Copy link
Member

Taking a brief look, I don't think this is necessary anymore. I just ran a full vmr build to confirm. I will check a few more things and then open a PR to remove this.

@MichaelSimons
Copy link
Member

Alright I think I found the fix that eliminated the need for this patch - dotnet/arcade#7549. I will open a PR to remove this.

@directhex
Copy link
Author

@MichaelSimons
Copy link
Member

@MichaelSimons
Copy link
Member

This patch has been removed and flowed into the VMR.

@github-project-automation github-project-automation bot moved this from Backlog to Done in .NET Source Build Sep 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-upstream-fix Needs a change in a contributing repo
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants