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

Don't crash in DownloadHashFile #2376

Merged
merged 1 commit into from May 28, 2017

Conversation

Projects
None yet
2 participants
@vbfox
Contributor

vbfox commented May 28, 2017

When the cache folder didn't exists DownloadHashFile was crashing instead of working as expected.

See #2368 (comment) comment by @matthid for context (Sorry).

Don't crash in DownloadHashFile
When the cache folder didn't exists DownloadHashFile was crashing instead of working as expected.
@matthid

This comment has been minimized.

Show comment
Hide comment
@matthid

matthid May 28, 2017

Member

No worries :). I'm actually still happy that I don't need to context switch back and forth.

Interstingly though I still see it https://ci.appveyor.com/project/SteffenForkmann/fake/build/1.0.3225#L873.
But I don't think its because of your changes. I think what happens is that msbuild starts all the restore operations at the same time so all the bootstrapper processes check the cache and then start downloading.

I'll try to verify this by invoking the bootstrapper manually in the build process...

Member

matthid commented May 28, 2017

No worries :). I'm actually still happy that I don't need to context switch back and forth.

Interstingly though I still see it https://ci.appveyor.com/project/SteffenForkmann/fake/build/1.0.3225#L873.
But I don't think its because of your changes. I think what happens is that msbuild starts all the restore operations at the same time so all the bootstrapper processes check the cache and then start downloading.

I'll try to verify this by invoking the bootstrapper manually in the build process...

@matthid

This comment has been minimized.

Show comment
Hide comment
@matthid

matthid May 28, 2017

Member

That also possibly explains why it couldn't fall back to an existing paket.exe...
Sorry, if I'd have come up with this earlier the fix wouldn't have been urgent at all..
I've logged this here. I guess we need to decide if there is even a good fix for that at all...

Member

matthid commented May 28, 2017

That also possibly explains why it couldn't fall back to an existing paket.exe...
Sorry, if I'd have come up with this earlier the fix wouldn't have been urgent at all..
I've logged this here. I guess we need to decide if there is even a good fix for that at all...

@matthid matthid merged commit c946efc into fsprojects:master May 28, 2017

2 checks passed

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