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

Lower case group folder name #2150

Merged
merged 1 commit into from Feb 23, 2017

Conversation

Projects
None yet
3 participants
@JonCanning
Contributor

JonCanning commented Feb 23, 2017

generate-include-scripts fails on Linux systems when the group name is upper case since the folder is created lower case

This causes the dir.Exists check here:
https://github.com/fsprojects/Paket/blob/master/src/Paket.Core/NuGetV2.fs#L609
to fail

Lower case group folder name
generate-include-scripts fails on Linux systems when the group name is upper case since the folder is created lower case

This causes the dir.Exists check here:
https://github.com/fsprojects/Paket/blob/master/src/Paket.Core/NuGetV2.fs#L609
to fail
@smoothdeveloper

This comment has been minimized.

Show comment
Hide comment
@smoothdeveloper

smoothdeveloper Feb 23, 2017

Contributor

@JonCanning thanks for the report, could you give a paket.dependencies that you are looking at and a summary of folder names where they mismatch (wether in file structure or in the generated script clauses)?

Thanks!

Contributor

smoothdeveloper commented Feb 23, 2017

@JonCanning thanks for the report, could you give a paket.dependencies that you are looking at and a summary of folder names where they mismatch (wether in file structure or in the generated script clauses)?

Thanks!

@smoothdeveloper

This comment has been minimized.

Show comment
Hide comment
@smoothdeveloper

smoothdeveloper Feb 23, 2017

Contributor

Mhh this is a PR already 😄 yes lowercasing the group name seems correct, we probably need same fix in core3 branch:

let groupFolder = if groupName = Constants.MainDependencyGroup then "" else "/" + groupName.ToString()

If you can consider adding a test that would be nice.

Contributor

smoothdeveloper commented Feb 23, 2017

Mhh this is a PR already 😄 yes lowercasing the group name seems correct, we probably need same fix in core3 branch:

let groupFolder = if groupName = Constants.MainDependencyGroup then "" else "/" + groupName.ToString()

If you can consider adding a test that would be nice.

@@ -384,7 +384,7 @@ type Dependencies(dependenciesFileName: string) =
| None -> failwithf "Package %O is not installed in group %O." packageName groupName
| Some resolvedPackage ->
let packageName = resolvedPackage.Name
let groupFolder = if groupName = Constants.MainDependencyGroup then "" else "/" + groupName.ToString()
let groupFolder = if groupName = Constants.MainDependencyGroup then "" else "/" + groupName.ToString().ToLower()

This comment has been minimized.

@forki

forki Feb 23, 2017

Member

the groupd should have a compare name - I will use that one.
ANd thanks for fixing!

@forki

forki Feb 23, 2017

Member

the groupd should have a compare name - I will use that one.
ANd thanks for fixing!

@forki forki merged commit 2132e51 into fsprojects:master Feb 23, 2017

0 of 2 checks passed

continuous-integration/appveyor/pr AppVeyor build failed
Details
continuous-integration/travis-ci/pr The Travis CI build failed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment