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

Fallback when caching fails #1093

Merged
merged 3 commits into from Jan 21, 2016

Conversation

Projects
None yet
2 participants
@matthid
Copy link
Collaborator

commented Jan 20, 2016

See discussion in #1088.

matthid added some commits Jan 20, 2016

@matthid matthid changed the title WIP - Fallback when caching fails Fallback when caching fails Jan 20, 2016

@forki

This comment has been minimized.

Copy link
Member

commented Jan 21, 2016

so when are we rerunning the script exactly?

@matthid

This comment has been minimized.

Copy link
Collaborator Author

commented Jan 21, 2016

When an exception occured in the loading code (from the caching logic). Because if a error happens in user code it is handled here and the function returns false. If this doesn't answer it can you clearify the question?

@forki

This comment has been minimized.

Copy link
Member

commented Jan 21, 2016

Yes!

So normal build error in user code still fails in first try?

Excellent!
On Jan 21, 2016 9:10 AM, "Matthias Dittrich" notifications@github.com
wrote:

When an exception occured in the loading code (from the caching logic).
Because if a error happens in user code it is handled here
https://github.com/matthid/FAKE/blob/17463458004722d18530769fc826324532b386f1/src/app/FakeLib/FSIHelper.fs#L318
and the function returns false. If this doesn't answer it can you
clearify the question?


Reply to this email directly or view it on GitHub
#1093 (comment).

@matthid

This comment has been minimized.

Copy link
Collaborator Author

commented Jan 21, 2016

Yes that's how it's supposed to be, maybe that would be a good test though :)

@matthid

This comment has been minimized.

Copy link
Collaborator Author

commented Jan 21, 2016

Well it's not actually that simple, there are a wide range of scenarios where this is not correct, for example: Loading the assembly already executes user code (for example assembly level attribute constructors...). If they throw we will recompile as well... On the other hand if the @main method doesn't exist we assume user code failed. Those are real edge cases though.

@forki

This comment has been minimized.

Copy link
Member

commented Jan 21, 2016

so what do we do?

@matthid

This comment has been minimized.

Copy link
Collaborator Author

commented Jan 21, 2016

Nevertheless I think this is an improvement for now. It quite unlikely that people do such crasy things in their build scripts or that the cache can be loaded, has the type we are looking for, but it is missing the member we search for (maybe with an FCS update, but even then we should see it here first in Bootstrap/Unit tests)

@matthid

This comment has been minimized.

Copy link
Collaborator Author

commented Jan 21, 2016

I can add a unit test for the obvious case of user code failing later either in this or a separate PR. If I find some time I might add some edge cases just for the fun of it ;)

@forki

This comment has been minimized.

Copy link
Member

commented Jan 21, 2016

sounds great

forki added a commit that referenced this pull request Jan 21, 2016

@forki forki merged commit f2c4997 into fsharp:master Jan 21, 2016

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@matthid matthid deleted the matthid:fallbackOnCacheErrors branch Jun 16, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.