-
Notifications
You must be signed in to change notification settings - Fork 228
Conversation
@@ -57,7 +58,18 @@ public IEnumerable<PackageInfo> FindPackagesById(string packageId) | |||
continue; | |||
} | |||
|
|||
packages.Add(new PackageInfo(_repositoryRoot, id, version, versionDir)); | |||
// Get the accurate package id to ensure case-sensitivity | |||
var manifestFileName = Path.GetFileName( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are we scanning files here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As my comments suggest, we want to get accurate package name (i.e. package name in correct casing) here. This accurate name will be used to do casing check before installation of the package.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure how I feel about this since we spent a whole bunch of time making this avoid scanning in the first place. Why can't this check be done later? Or why can't restore just fix it up before the runtime sees it?
@davidfowl , |
|
||
public PackageRepository(string path) | ||
public PackageRepository(string path, bool isInRuntimeContext) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rename this variable to checkPackageIdCase
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in the latest commit.
- PackageRepository has different behaviors in different contexts
648a1a5
to
c5de200
Compare
@@ -137,7 +138,7 @@ public PackageInfo BuildModel(string id, XElement element) | |||
|
|||
return new PackageInfo | |||
{ | |||
Id = id, | |||
Id = element.Element(_xnameTitle).Value, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This broke the EF build. There is a case difference between the id and title of the redis-64 package. The NuGet URL is expecting the Id to be passed, not the title.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the id afaik
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This could also be exacerbated by an issue on the NuGet server where it rewrites the id to uppercase because that's what it's been historically.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, I think NuGet was changing underneath us. Things seem to be ok if we use 'Redis-64' now.
parent #581
Shows user-friendly warning:
Unable to locate System.console >= 4.0.0.0. Do you mean System.Console?