Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Best effort stack init when some of the source packages have conflicts #1621

Closed
harendra-kumar opened this issue Jan 8, 2016 · 14 comments
Closed

Comments

@harendra-kumar
Copy link
Collaborator

When there are a number of packages under the directory tree and even one of them is not compatible with the snapshot compiler then stack init fails. Sometimes there is stray or old unmaintained package under the tree or maybe even a test package which is designed to fail.

Instead what we can do is to filter out the failing packages and init the stack.yaml based on the rest. This will allow more repositories/packages to be automatically initialized by stack init. We can keep the failing packages as commented out in stack.yaml with an appropriate reason comment automatically inserted.

For example the diagrams-ci tree fails with this:

* Rejected lts-4.0
    ghc-7.10.3 cannot be used for these packages:
        - active-semantics
        - dhtest
    base version 4.8.2.0 found
        - active-semantics requires ==4.6.*
        - dhtest requires ==4.6.*
        - dhtest flags: inside = False

These packages can be moved out of the way automatically.

@mgsloan
Copy link
Contributor

mgsloan commented Jan 8, 2016

Makes sense to me!

For #1616 , maybe it makes sense to have a mode that does this for any package that's problematic (and not just the ones that aren't compatible witht eh compiler). Perhaps a flag called something like --omit-packages

@harendra-kumar
Copy link
Collaborator Author

I was thinking of using --force itself instead of introducing a new flag as we are already using that to force write incomplete config. Both compiler conflicts and regular dependency conflicts should come under the same flag. We shouldn't do this automatically. The user should know what's going on.

The overloading of --force looks a bit weird though. We suggest to use that flag for overwriting the file when it exists. But the user may not realize that he is also saying yes to an incomplete config as a side effect. Also losing his old and perhaps working config. We should perhaps make backup every time we overwrite.

Maybe we can postpone the 'use --force to overwrite' message to the end after it has been determined whether the config will be successfully created or not i.e. fail just before we are actually going to write it - instead of right at the beginning as the first message. That way the message will show up after the user knows whether he will get an incomplete or complete config. Any other ideas?

@mgsloan
Copy link
Contributor

mgsloan commented Jan 8, 2016

Hmm, yeah, now that the meaning of --force would change to a different strategy, I'm thinking two separate flags would be better.

One question is what happens when none of the packages can be built. In that circumstance, it may still be valuable to output a stack.yaml file, even if it won't build.

@harendra-kumar
Copy link
Collaborator Author

The cases that we have can be summarized as:

  • overwrite an existing file (irrespective of whether the new config is good or broken)
  • generate a config
    • default when config is self contained in a snapshot
    • optional when config uses external deps i.e. not contained in a snapshot
    • optional when the config is known to be broken

What will be option names for these cases? Which one keeps --force or none? Do we overload the last two?

Another option could be to simply generate the config always with a bold warning when it is not self contained or broken. More importantly, generate a config param to indicate that the config is not self contained or broken. We then provide an elaborate warning every time a stack command using the config is run. The warning can be suppressed by explicitly editing the config param to say I accept it the way it is and don't bother me anymore.

When none of the packages can be built, all of them will be commented out in stack.yaml with everything else in place. The file can then be fixed by the user.

@mgsloan
Copy link
Contributor

mgsloan commented Jan 8, 2016

I'm thinking that --force should only mean "overwrite existing file", as it did before.

I like the idea of putting a message in the .yaml file that gets printed when you use it. With that addition, I think it makes sense to default to writing a config file, even if it's broken. Some might not like this behavior, so it'd be good to wrap it up in a flag. How about having the flags be --allow-broken-config (default) / --no-allow-broken-config?

I can also see this being useful when / if we have multiple configurations in the stack file (#1568). For example, if the user really should pick a configuration, then the default config can just consist of a message that tells them about that.

How about something like user-message: "Warning: Configuration known to be broken when generated by init. You can suppress this message by removing it from your stack.yaml"?

@harendra-kumar
Copy link
Collaborator Author

Maybe we should file a bug with the title 'stack supports too many options' :-) I wouldn't add the flags unless someone asks for it and makes a good case for it.

How about having case specific messages for the two cases I listed above. In the first case we can add something to the effect - Some dependencies were not found in the snapshot. External dependencies were added for those. and for the second case - packages x,y,z were excluded from the config because of incompatibility with the resolver compiler.

Also, how about user-warning for the first case and user-error for the second case? Error will mean that we will refuse to load the config and display the error. The error will force the user to edit the file before he can use it. That will be almost equivalent to having a flag.

@mgsloan
Copy link
Contributor

mgsloan commented Jan 9, 2016

Sure, sounds great! True, maybe it'd be good not to add flags anticipating some unknown usecase :)

@harendra-kumar
Copy link
Collaborator Author

I was going to remove --force and I found an inconsistency. Once we remove that and rely on the user message for ignored-packages then we should perhaps remove --solver too and rely on the user-message for extra-deps as well. If we ignore incompatible src packages without a forced switch then why not add extra-deps without a forced switch?

This will make both the cases consistent and also make stack init just work out of the box without much thinking on the part of the user - doing whatever best is possible and telling the user what it did via the user message.

@mgsloan
Copy link
Contributor

mgsloan commented Jan 15, 2016

Hmm, I think --solver is still useful. I don't quite follow the reasoning for removing it.

Is the issue that it's difficult to implement the "leave out some packages" logic with --solver? In that case, maybe --solver should refuse to generate a stack.yaml if it can't figure out the extra-deps.

Another thing that's tricky about leaving out some packages is how this relates to snapshot selection. I'm not 100% sure exactly the metric that's being used here, so this might not be an issue. Is it possible that a snapshot can get picked because it works better for some packages which end up getting left out?

I'm starting to think that automatically leaving out packages might be more effort than it's worth. How about this alternative:

In the list of ways to resolve, provide the CLI invocation that only specifies the packages that have a workable build plan. It would be cool to provide the full CLI including flags the user passed, but this is a bit tricky. It'd be fine to just provide the list of folders, such that it can be copy+pasted into the user's invocation of new / init.

@harendra-kumar
Copy link
Collaborator Author

Is the issue that it's difficult to implement the "leave out some packages" logic with --solver?

No, its purely a matter of consistent behavior and not implementation complexity. To elaborate the reasoning, here are two conflicting points:

  • When we cannot meet all the dependencies via a snapshot we stop and advise the user to use --solver if he wants to add extra dependencies. For a user --solver means nothing but using packages outside the snapshot.
  • On the other hand when we leave out packages which do not compile we will be going ahead without any user intervention (based on our discussion above) like we do in case of extra deps. In this case we are saying we will rely on the user warning message in stack.yaml to warn the user that packages have been left out.

What I was trying to say was that in both cases we can just go ahead and use the same warning message in stack.yaml. That will make the behavior for both the cases same and reduce command line flags.

I'm starting to think that automatically leaving out packages might be more effort than it's worth

I just implemented this today. Its pretty elegant, not complex. I have a few more things to add and then I will send a PR. Meanwhile I can push it to my fork if you want to take a look.

Is it possible that a snapshot can get picked because it works better for some packages which end up getting left out?

We pick a snapshot which can compile more packages.

@mgsloan
Copy link
Contributor

mgsloan commented Jan 16, 2016

I see! Yes, it makes sense to have the warning mention --solver. I'd interpreted the earlier comment as removing "--solver" entirely.

I just implemented this today. Its pretty elegant, not complex. I have a few more things to add and then I will send a PR. Meanwhile I can push it to my fork if you want to take a look.

Cool, sounds good! In that case, lets go ahead with the feature. If you want feedback sooner, feel free to push a fork. Otherwise, I can take a look when it's ready.

@harendra-kumar
Copy link
Collaborator Author

I'd interpreted the earlier comment as removing "--solver" entirely.

That's what I meant earlier and now :-) I mean the CLI switch and not the functionality of course. Because otherwise we would be ignoring packages by default, without requiring any explicit switch and on the other hand we require --solver for adding extra deps. Why that difference in behavior although both these things are similar? Is adding extra-deps more 'harmful' than ignoring packages therefore requiring one more level of protection via explicit --solver switch? I don't think so.

@mgsloan
Copy link
Contributor

mgsloan commented Jan 16, 2016

--solver does, however, require that you have a localy cabal-install executable, which isn't always the case.

You make a good point about the consistency! Maybe have an --omit-packages flag, instead of defaulting? There should definitely be a way to disable the behavior (--no-omit-packages), so I suppose this is a question of defaults.

@harendra-kumar
Copy link
Collaborator Author

Maybe have an --omit-packages flag

that was my backup plan :-) I would prefer the simpler no flags and stack.yaml warning. Once the flags are introduced its becomes harder to remove them.

harendra-kumar added a commit to harendra-kumar/stack that referenced this issue Jan 19, 2016
Packages which are not compatible with the resolver are added to the
config but commented out.

See commercialhaskell#1621
harendra-kumar added a commit to harendra-kumar/stack that referenced this issue Jan 19, 2016
When there are some issues with the config file add warning messages for the
user to be displayed everytime the config file is read. The messages can be
suppressed by a user action i.e. remove it from the config if the user accepts
the config.

See commercialhaskell#1621
harendra-kumar added a commit to harendra-kumar/stack that referenced this issue Jan 19, 2016
Remove the overloading of --force option and use it exclusively for overwriting
an existing config. Use a separate --omit-packages option for excluding
conflicting or incompatible packages from generated config.

See commercialhaskell#1621
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

No branches or pull requests

2 participants