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

Tests for #2731 (Ignore dependencies that are not Buildable) #3039

Merged
merged 7 commits into from
Jan 15, 2016

Conversation

grayjay
Copy link
Collaborator

@grayjay grayjay commented Jan 13, 2016

I added a package test for the Cabal component and several unit tests for the solver component. This PR also resolves a conflict between #2731 and #2732. I had to make a few changes to the solver DSL:

  • Allow a flag to be used multiple times in a package. Reusing a flag previously caused this error:

    Exception: internal error: could not construct a valid install plan.
    The proposed (invalid) plan contained the following problems:
    Package B-1.0.0 has an invalid configuration, in particular:
      missing an assignment for the flag: my-flag
    
  • Represent the Buildable field. Now both sides of a conditional can be either not buildable, or buildable with a list of dependencies.

  • Allow a compiler to support zero languages or extensions. I changed the signature of DSL.exResolve to distinguish between not checking languages/extensions and not allowing any languages/extensions.

I thought of two other improvements, but I left them for separate issues or PRs. I didn't see an easy way to check flag choices directly in the DSL. I instead had to check the dependencies that the flags introduced. I also noticed that the default flag value is false in the DSL and true in cabal-install/Cabal. Some of these tests rely on the default, so it should probably be consistent.

@BardurArantsson
Copy link
Collaborator

LGTM. Have you run tests, etc. for each individual commit?

@23Skidoo
Copy link
Member

/cc @kosmikus

I think this can be merged now.

@grayjay
Copy link
Collaborator Author

grayjay commented Jan 14, 2016

@BardurArantsson I tested each commit with GHC 7.10.2, but I can push the commits one at a time to build them with Travis. I'd like to make sure that the two commits from #2731 build.

@BardurArantsson
Copy link
Collaborator

Fair enough, just checking :). I wonder if there's something that could be done to get Travis to always build each commit in a series...

kosmikus and others added 7 commits January 14, 2016 11:13
When doing the index conversion prior to dependency solving, we now
consider the "Buildable" flag for package components.  In particular, if
the "Buildable" flag of a component is "True" only under certain
conditions, then all build dependencies of that component will be placed
under the same conditions.
When configuring a package, the condition trees in the package
descriptions are evaluated according to the known configuration
and flag assignment.

During this process, it becomes also known whether a component
has its "Buildable" flag set to True or False. We now disregard
all dependencies of non-buildable components.
…r DSL.

'DSL.exResolve' now takes a 'Maybe [Extension]' for supported extensions and
a 'Maybe [Language]' for supported languages.  'Nothing' means that
extensions/languages are not checked by the solver, and 'Just []' means that
no extensions/languages are allowed.
@ezyang
Copy link
Contributor

ezyang commented Jan 14, 2016

https://docs.travis-ci.com/user/triggering-builds/ might be of help.

@BardurArantsson
Copy link
Collaborator

I was hoping for something which could be set up once and for all for the whole repository.

(Oh, how I long for a proper review + CI system...)

@ezyang
Copy link
Contributor

ezyang commented Jan 14, 2016

Well, we do have a Phabricator instance :) (Though I don't think builds are setup.)

@BardurArantsson
Copy link
Collaborator

No direct experience with Phab, but doesn't it require some sort of weird custom command line tool instead of just using standard git commands? (When reviewing and pushing patches to review, obviously.)

Anyway, this is probably enough off-topic spam for this thread...

BardurArantsson added a commit that referenced this pull request Jan 15, 2016
Tests for #2731 (Ignore dependencies that are not Buildable)
@BardurArantsson BardurArantsson merged commit cce3ae2 into haskell:master Jan 15, 2016
@BardurArantsson
Copy link
Collaborator

Looks like they all succeeded. Merged

@grayjay
Copy link
Collaborator Author

grayjay commented Jan 15, 2016

Thanks!

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.

5 participants