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.

Sign up for GitHub

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

Jump to bottom

New build exception handling #3416

Merged
merged 3 commits into from
May 14, 2016
Merged

New build exception handling #3416

merged 3 commits into from
May 14, 2016

Conversation

dcoutts
Copy link
Contributor

@dcoutts dcoutts commented May 9, 2016

Some improvements to the exception handling during new-build, plus tests.

Yet TODO: better reporting, handling download exceptions. Covering more cases in tests. Still, it should be a strict improvement.

@dcoutts
Add better exception handling in new-build
51da3c7 
When executing the install plan, the various failures are supposed to be
caught and put into the residual install plan. This adds the main cases
for that, including the configure and build cases. Download is still not
covered.

This is a step towards better reporting of what failed when there are
failures in deps etc.
@23Skidoo
Copy link
Member

23Skidoo commented May 9, 2016

Travis failure is due to stale extra-source-files.

23Skidoo
23Skidoo reviewed May 9, 2016
View reviewed changes
cabal-install/tests/IntegrationTests2.hs
@@ -0,0 +1,258 @@
module Main where

import Distribution.Client.DistDirLayout
Copy link
Member

Choose a reason for hiding this comment

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

This will slow down build times on Travis. Would be nice if we could use an internal library in cabal-install.cabal.

Copy link
Contributor Author

@dcoutts dcoutts May 9, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

It doesn't depend on all the cabal-install modules, but yes fair point. Once we feel we can rely on the named internal libs feature in Cabal then we could use it here.

Or we could add this to the unit-tests test exe and find some way to skip the slow integration tests.

@23Skidoo
Copy link
Member

23Skidoo commented May 9, 2016

Looks OK.

@dcoutts
Copy link
Contributor Author

dcoutts commented May 9, 2016

Travis failure is due to stale extra-source-files.

Ahh yes, thanks. Will fix.

This was referenced May 11, 2016
Closed
Open
@dcoutts
Copy link
Contributor Author

dcoutts commented May 13, 2016 •
edited
Loading

@23Skidoo so I updated the script so it catches cabal.project files (of which there are a few now in the existing tests). So assuming travis is happy, I think this is ok to go in, unless you're unhappy with having a separate test exe slowing down travis builds.

@23Skidoo
Copy link
Member

@dcoutts Sure, let's merge.

@23Skidoo
Copy link
Member

Fails due to -Werror now:

tests/IntegrationTests2.hs:17:1: Warning:
    The import of ‘Data.Monoid’ is redundant
      except perhaps to import instances from ‘Data.Monoid’
    To import instances alone, use: import Data.Monoid()

@dcoutts
Update the extra-source-files script and list
30d6d9f 
Include cabal.project files too.
@dcoutts
Add new integration tests, initially covering build exceptions
40846f0 
These integration tests, unlike the existing ones, don't call cabal as
an external processes. Instead they use the cabal code directly. This
makes it possible to conveniently test catching exceptions.

Add a couple tests for exceptions in finding projects. There should be a
lot more for the various phases of planning.

Also add a couple tests for exceptions in the configure and build
phases. These test the previous patch that improves the exception
handling so that failures are added into the residual plan rather than
just propagating immediately.
@dcoutts dcoutts force-pushed the new-build-exception-handling branch from ceec812 to 40846f0 Compare May 14, 2016 16:56
@dcoutts dcoutts merged commit ff6e495 into haskell:master May 14, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
blocked: action-required cabal-install: nix-local-build type: enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

new-build: Outputs to console after "exiting"?
2 participants
@dcoutts @23Skidoo