Skip to content
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

(GH-1231) Use DirectoryInfo to evaluate CacheLocation #1232

Closed
wants to merge 11 commits into from

Conversation

mwrock
Copy link
Member

@mwrock mwrock commented Apr 3, 2017

The former fix (GH-1210) fails when the CacheLocation ends with a slash. This uses a DirectoryInfo instance to examine the last directory in the path in a more robust manner

@mwrock
Copy link
Member Author

mwrock commented Apr 3, 2017

I just tested the appveyor drop with boxstarter and this looks to be working.

@mwrock
Copy link
Member Author

mwrock commented Apr 3, 2017

fixes #1231

Copy link
Member

@ferventcoder ferventcoder left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Waiting for requested changes.

// the API.
if(!config.CacheLocation.EndsWith("chocolatey")) {
// the API.
if(new DirectoryInfo(config.CacheLocation).Name.ToLower() != "chocolatey") {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use fileSystem.get_directory_info() and use either .to_lower() or Name.is_equal_to(). The latter is preferred.

@mwrock mwrock force-pushed the fix_temp_for_reals branch 2 times, most recently from 0d8f34c to 9835125 Compare April 3, 2017 16:09
@mwrock
Copy link
Member Author

mwrock commented Apr 3, 2017

hmm this is dying with 'string' does not contain a definition for 'is_equal_to' but that seems to be a valid extension method used throughout this file. My c# is very rusty. Will look more later tonight.

@ferventcoder
Copy link
Member

hmm this is dying with 'string' does not contain a definition for 'is_equal_to' but that seems to be a valid extension method used throughout this file. My c# is very rusty. Will look more later tonight.

My guess?

new DirectoryInfo(config.CacheLocation).Name.ToLower() != "chocolatey" was changed to fileSystem.get_directory_info(config.CacheLocation).Name.is_equal_to() != "chocolatey" instead of !fileSystem.get_directory_info(config.CacheLocation).Name.is_equal_to("chocolatey"). The first change would definitely produce the error, it's needs a value passed to the method.

@mwrock
Copy link
Member Author

mwrock commented Apr 3, 2017

well !fileSystem.get_directory_info(config.CacheLocation).Name.is_equal_to("chocolatey") is exacty what's in this pr.

@mwrock
Copy link
Member Author

mwrock commented Apr 3, 2017

actually its get_directory_info_for not get_directory_info but I think thats right.

@ferventcoder
Copy link
Member

So it is. So it is.

@mwrock
Copy link
Member Author

mwrock commented Apr 3, 2017

ok. I just ran MSBuild.exe C:\dev\choco\src\chocolatey\chocolatey.csproj /t:Build and it compiles just fine. So I'm not entirely sure why that fails in appveyor. The appveyor log is huge and theer might be something in there I am missing.

BTW: the build.bat breaks on nuget.exe in the choco repo. If I swap in the latest nuget.exe the nuget installs work but then the build fails in ilmerge.exe. I think the nuget problems are due to API incompatibilities with older nuget clients (just an educated guess).

Anyways I found that vanilla msbuild ended up being the easiest way for me to locally sanity check the build.

@mwrock
Copy link
Member Author

mwrock commented Apr 3, 2017

Looking again at the appveyor log I actually think this is a test setup issue and not related directly to the config builder itself. Perhaps somthing needs to be imported or added to a container.

@ferventcoder
Copy link
Member

Don't get too hung up on the appveyor issues. Also, as a committer, you may think about using stable and not master.

ferventcoder and others added 8 commits April 3, 2017 18:16
PowerShell is not very good at telling the difference between ANSI
and UTF8 w/out BOM, so PowerShell scripts are not able to be verified
by what is built into PowerShell authenticode verification if they use
UTF8 no BOM with unicode characters. This follows up on chocolatey#1209 where it
was discovered that the authenticode verification would not work with
LF line endings. That was resolved and later a unicode character
(copyright) was introduced, which broke the functionality again.

Convert all scripts to UTF8 w/BOM.
Ensure NOTICE lists Chocolatey Software as the new copyright holder.
The Chocolatey Package was getting the license copied, but the library
was not. Ensure that it receives the licensing files as well. Add the
notice and credits as outputs to that.
Ensure NOTICE looks like what choco bakes in
Choco already unpacks the License file. It is the same text as the
NOTICE file, so ensure that is is used and not overwritten.
Ensure credits is set properly with licenses.
The former fix (chocolateyGH-1210) fails when the CacheLocation ends with a slash. This uses a DirectoryInfo instance to examine the last directory in the path in a more robust manner
@mwrock mwrock force-pushed the fix_temp_for_reals branch from 9835125 to fb396c3 Compare April 4, 2017 06:30
@mwrock
Copy link
Member Author

mwrock commented Apr 4, 2017

closing in favor of #1233 targeting stable

@mwrock mwrock closed this Apr 4, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants