Skip to content
This repository has been archived by the owner on Jan 11, 2024. It is now read-only.

In init-tools.sh, lenient casing for restored packages #1114

Merged
merged 1 commit into from
Oct 26, 2016

Conversation

dagood
Copy link
Member

@dagood dagood commented Oct 10, 2016

Uses find to allow compatibility with CLI versions that have the NuGet change that lowercases package ids in the packages directory.

This should fix the buildtools side of #1111.

Tested on OSX and openSUSE 13.2 with dotnet/coreclr@master...dagood:init-tools-insensitive.

@janvorli @chcosta @mellinoe @ericstj
/cc @weshaggard @gkhanna79

Uses find to allow compatibility with CLI versions that have the NuGet change to lowercase package ids in the packages directory.
@chcosta
Copy link
Member

chcosta commented Oct 10, 2016

I don't understand how the package name got lower-cased.

@dagood
Copy link
Member Author

dagood commented Oct 10, 2016

It's a breaking NuGet change that we're exposed to through dotnet restore: NuGet/Home#2522

@dagood
Copy link
Member Author

dagood commented Oct 26, 2016

Ping--this is needed to restore buildtools with a CLI version past the NuGet breaking change.

@chcosta
Copy link
Member

chcosta commented Oct 26, 2016

I'm not super familiar with 'find'. Does this change the behavior? ie, is this going to hide not finding those packages if they don't exist?

@dagood
Copy link
Member Author

dagood commented Oct 26, 2016

Good point, if something happens that lets the restore pass but the packages aren't there, this won't report an error. Also, if multiple copies of the same package are in the packages dir with different directory casing it would also copy each one's contents. Those are the only differences I'm aware of.

Are we currently relying on cp -R on the literal paths to return an error? The only place I've seen that error help so far is when dealing with the NuGet capitalization breaking change, which this PR makes non-breaking as far as init-tools is concerned.

@chcosta
Copy link
Member

chcosta commented Oct 26, 2016

I'd expect us to not see an issue with the multiple packages because it would be differently cased packages, but with the same version; and that seems unlikely. Also, I guess it's not too concerning if the directories aren't present because i'd generally expect that to manifest as an issue during the previous "package restore" step. I guess there are cases where the package restore step doesn't catch things though.

LGTM

@dagood
Copy link
Member Author

dagood commented Oct 26, 2016

Agreed, unlikely, but with some potential cases. Filed #1162 to track maybe fixing that, although I wonder if we'll be able to replace that copy with something better at some point.

Merging, thanks.

@dagood dagood merged commit 2d3d950 into dotnet:master Oct 26, 2016
@dagood dagood deleted the case-insensitive-init branch October 26, 2016 19:42
@dagood
Copy link
Member Author

dagood commented Nov 7, 2016

This is breaking some xplat builds when tools are initialized:

chcosta/dotnetcore:fedora23_prereqs corefx build

/root/corefx/packages/Microsoft.DotNet.BuildTools/1.0.27-prerelease-01002-04/lib/init-tools.sh: line 101: find: command not found

chcosta/dotnetcore:alpine_prereqs corefx build

2016-11-07T14:03:47.4123920Z find: unrecognized: -iwholename

It looks like find isn't as stable across distros as I'd hoped, or even available--we need an alternative to find the name of a folder in a case insensitive way. I'm not familiar enough with the ecosystem to know how to check this without just trying various things.

In the meantime, @ericstj plans to make the init-tools copy and msbuild import conditional. (#1200)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants