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

No way of specifying a remote-repo in a sandbox #1884

Closed
snoyberg opened this issue May 21, 2014 · 29 comments · Fixed by #2226
Closed

No way of specifying a remote-repo in a sandbox #1884

snoyberg opened this issue May 21, 2014 · 29 comments · Fixed by #2226

Comments

@snoyberg
Copy link
Collaborator

I initially asked this as a Stack Overflow questions. Apologies if this is not in fact a bug, but I can't seem to find any way to accomplish what I'm trying to do.

I would like to work on a project in a cabal sandbox. But instead of using the same remote-repo as my non-sandboxed code (i.e., Hackage), I'd like to point to a different remote repo. I tried creating a cabal.config file in the project directory with a remote-repo line, but it seemed to have no effect; running cabal update after that indicated that Hackage was being downloaded, but not my custom repo.

Is this use case supported, and if so, how do I achieve it?

@bergmark
Copy link
Contributor

An issue I ran across might have the same cause: if you generate a cabal.config with cabal freeze you cannot use it with --config-file. Here you might expect that it would combine the user level config and the specified one.

@23Skidoo 23Skidoo self-assigned this May 21, 2014
@AshleyYakeley
Copy link
Member

cabal update looks only at the global .cabal/config.
cabal install looks at both the global .cabal/config and the sandbox cabal.config and uses all the remote-repo lines.

Not sure what the best thing is, but update should at least work the same ways as install.

@ygale
Copy link
Contributor

ygale commented Jun 5, 2014

A few questions:

  1. If one or more remote-repo is specified locally, does that imply that the ones in the global config should not be used in this sandbox? Do we also want to support the use case of adding an additional local remote-repo to the global ones, and if so, how would we distinguish the two semantics?
  2. When a remote-repo is specified locally, where is its package index stored? In my opinion, locally would make the most sense; if two different sandboxes happen to use the same local remote-repo, you don't want an update to affect the way the solver will work in the other.
  3. Depending on the answers to the previous questions, how do we want cabal update to work? Do we want options to allow updating only global repos, only local repos, or both? What should the default be? If indexes remain a global-only concept as they are now, then I would not want the default to be to update a local remote-repo, because that could impact other sandboxes.

@AshleyYakeley
Copy link
Member

  1. Currently most settings (e.g, with-compiler) essentially have type String, and lines in files can only modify the setting by replacing it or not (i.e., const s or id). But the remote-repo setting has type [String]. I propose we keep the notion that a file can only modify the setting by replacing it or not. So, if one or more remote-repo is specified locally, that counts as replacing the setting and the global remote-repo is not used. The only wrinkle is that it is impossible to set remote-repo to the empty list, but I don't think that matters.
  2. The package index location is controlled by remote-repo-cache, which should be independent of remote-repo. It can be set in the sandbox's cabal.config (I hope). As to where it defaults to, my preference is the sandbox.
  3. cabal update should behave the same way as the other commands: it should calculate remote-repo and remote-repo-cache, and update those repos in that location.

@Profpatsch
Copy link

This should be fixed to make sandboxes work as intended. As it is now, they are a somewhat leaky concept.

@luite
Copy link
Member

luite commented Aug 20, 2014

I have implemented the behaviour for remote-repo as described by @AshleyYakeley in the ghcjs branch of the ghcjs fork of Cabal. I'll check if remote-repo-cache is handled too.

Since there appears to be interest in fixing this, I'll split out the patch within a few days and submit it before the rest of the GHCJS patch. Does anyone object to the behaviour where remote-repo in a sandbox competely replaces any existing remote-repo setting?

@ygale
Copy link
Contributor

ygale commented Aug 20, 2014

@luite Thanks for all that!

In my opinion, the only problem with that behavior is that it might be very surprising and perplexing to some people, so it needs to be documented. In fact, from the discussion above, the opposite behavior might be just as surprising to some other people, so it's even more important for whatever we implement to be documented.

The trouble is, I'm not sure where the right place is to put that documentation so that people who need to know about this will have some reasonable chance of seeing it.

@23Skidoo
Copy link
Member

@ygale Once #2041 is done, we can add a comment to the auto-generated cabal.config file.

@dcoutts
Copy link
Contributor

dcoutts commented Aug 23, 2014

Whether things in the local config overrides the global or not is currently rather unclear (to users I mean). What I'd like to see is that we introduce a mechanism to make this fully explicit, and then there's no more confusion. We should have an "include" or similar, and then by default have local config files include the per-user ~/.cabal/config one (or not, depending on the desired behaviour). Similarly, the sandbox config vs the local config, we should not implicitly include both, but have one very explicitly include the other.

If we need a mechanism for selective including then fine.

@23Skidoo
Copy link
Member

@dcoutts

An "include" statement feels a bit coarse-grained. I think that this mechanism should work on per-option basis.

For example:

remote-repo: foo  -- inherit
remote-repo: !foo -- override

@luite
Copy link
Member

luite commented Aug 23, 2014

I like the idea of include. Inserting the contents of a config file would have the same effect as an include directive at the same location, which makes things nice and clear.

Selective imports sound a bit tricky though. Having a way to reset a field to a default value might be better:

remote-repo: hackage.haskell.org:http://hackage.haskell.org/packages/archive
remote-repo: <unset>
remote-repo: hdiff.luite.com:http://hdiff.luite.com/packages/archive

where a <> unset = mempty

your syntax could be usable as a shortcut for unset followed by a new value.

@gilligan
Copy link

Any progress on this ?

@luite
Copy link
Member

luite commented Sep 11, 2014

I think most people were at ICFP last week, which could've been the reason for the low activity. I'm back now, and unless anyone has objections or steps up I'll try to implement the approach from my last message next weekend.

@luite
Copy link
Member

luite commented Sep 16, 2014

I have an almost complete but very untested implementation of the override idea, where the types of "appendable" fields have been changed to something that supports the notion of unset. Unfortunately it looks like it requires changes to various data types like ConfigFlags and I'm not sure if it's worth it at this time.

Duncan's explicit imports idea is orthogonal and I think that even without overriding (or filtering) on a per-field basis it's already quite usable. Just have both your .cabal/config and your sandbox config include a common config file if you want to share settings. I'm going to implement this first now and leave the unset field code for now (perhaps it can still be added later when we're making other breaking changes).

@gilligan
Copy link

gilligan commented Oct 1, 2014

I'd like to switch to stackage but don't want to give up on sandboxes - is there any hack/patch/workaround that would me allow that until it's properly implemented ?

@ygale
Copy link
Contributor

ygale commented Oct 1, 2014

Here is one suggestion:

Have a different global cabal config files your different projects, and write a front end script for cabal that runs cabal --config-file=foo with foo the appropriate cabal config file for the current sandbox.

Something else I did in the past: have a script that initializes work with a specific project by swapping the global cabal config to the right one for that project. You have to remember to run that script though. Anyway, that did work for me.

@mrak
Copy link

mrak commented Nov 13, 2014

Due to the lack of a voting mechanism on issues:

1st-class support for cabal sandboxes + remote-repo 🙏

@snoyberg
Copy link
Collaborator Author

What I'm really concerned about is that:

  1. There are clearly a number of people who find this problem very troubling
  2. There's already a proposed patch that solves the problem
  3. There's no clear way to move forward from where we are right now

Can someone who has the authority to make a move on this issue comment on how we can precede? This doesn't seem to be stalled due to any lack of energy towards fixing it, but simply due to lack of a clear process.

@23Skidoo
Copy link
Member

@snoyberg

There's already a proposed patch that solves the problem

Where? I don't see a pull request.

This doesn't seem to be stalled due to any lack of energy towards fixing it, but simply due to lack of a clear process.

The process is simple: send us pull requests, and we'll either merge them (usually) or reject them (sometimes) after a code review.

@snoyberg
Copy link
Collaborator Author

@luite implemented it, see: #1884 (comment)

@23Skidoo
Copy link
Member

Well, it's up to @luite then to send a pull request when he thinks that the patch is ready to be merged.

@snoyberg
Copy link
Collaborator Author

@23Skidoo There's a discussion (which you participated in) following @luite's comment above which demonstrates that it is premature to make a pull request, as neither you nor @dcoutts seem satisfied with what @luite has implemented so far. So we seem to be in a situation where:

  • A number of users want this feature to be fixed.
  • There's at least one implementation of it in existence.
  • For design reasons, that implementation is not acceptable.
  • And now we have no clear process forward to get a patch in place that meets (seemingly) unspecified design criterion.

Put another way: if I knew the cabal codebase backwards and forwards and was willing to invest the next 20 hours straight in getting this implemented, I'd still have no idea where to start. So I'm asking: how do we get this process unstuck?

@23Skidoo
Copy link
Member

@snoyberg

Put another way: if I knew the cabal codebase backwards and forwards and was willing to invest the next 20 hours straight in getting this implemented, I'd still have no idea where to start.

Some possible solutions:

  1. Make Flag [a] work like Flag a (i.e. Flag [a,b] <> Flag [c] == Flag [c]). The easiest to implement, but needs testing. Some parts of the code may rely on old behaviour.
    Edit: I looked at the code, and apparently globalRemoteRepos is not a Flag [a], but just a list (and there's no Monoid a => Monoid (Flag a) instance).
  2. Make overriding the default behaviour. Wrap all list fields in the config in a Flag or a newtype. Won't work for all list options, though: e.g. you want --ghc-option=a --ghc-option=b to continue to work. Perhaps only change the SavedConfig Monoid instance?
  3. Add a syntax for explicitly overriding options (suggested by me above). So remote-repo: something in your ./cabal.config would mean ~/.cabal/config::remote-repo <> ./cabal.config::remote-repo and remote-repo: !something would mean ./cabal.config::remote-repo.
  4. Add an include mechanism (suggested by @dcoutts above). So we'd have a ~/.cabal/config.common file included by both ~/.cabal/config and ./cabal.config. Then we could set remote-repo to different values in ~/.cabal/config and ./cabal.config. By default, ./cabal.config would include ~/.cabal/config (for backwards compat), so if you wanted to use a different remote-repo, you'd change that line to include ~/.cabal/config.common.

@snoyberg
Copy link
Collaborator Author

@dcoutts Can you comment on the possible solutions that @23Skidoo mentions? Are any of them acceptable?

@snoyberg
Copy link
Collaborator Author

I'm looking into this now, and just noting an extra problem: cabal update currently does not load the sandbox config at all. That needs to be changed in addition to which of @23Skidoo's modifications go through.

snoyberg added a commit to snoyberg/conduit that referenced this issue Nov 14, 2014
This follows @23Skidoo's option (3) at:
haskell/cabal#1884 (comment)

If a config file has a repo name that begins with `!`, it's treated as
*overriding*, meaning that any previous repos are ignored. This allows a
sandbox to either add additional repos (by omitting the `!`), or ignore
the main config's repos and use a separate repo (which is currently the
use case I'm trying to design for).
snoyberg added a commit to snoyberg/cabal that referenced this issue Nov 14, 2014
snoyberg added a commit to snoyberg/cabal that referenced this issue Nov 14, 2014
snoyberg added a commit to snoyberg/cabal that referenced this issue Nov 14, 2014
This follows @23Skidoo's option (3) at:
haskell#1884 (comment)

If a config file has a repo name that begins with `!`, it's treated as
*overriding*, meaning that any previous repos are ignored. This allows a
sandbox to either add additional repos (by omitting the `!`), or ignore
the main config's repos and use a separate repo (which is currently the
use case I'm trying to design for).
snoyberg added a commit to snoyberg/cabal that referenced this issue Nov 14, 2014
snoyberg added a commit to snoyberg/cabal that referenced this issue Nov 14, 2014
This follows @23Skidoo's option (3) at:
haskell#1884 (comment)

If a config file has a repo name that begins with `!`, it's treated as
*overriding*, meaning that any previous repos are ignored. This allows a
sandbox to either add additional repos (by omitting the `!`), or ignore
the main config's repos and use a separate repo (which is currently the
use case I'm trying to design for).
@snoyberg
Copy link
Collaborator Author

I've implemented both the cabal update fix and @23Skidoo's approach (3) above, namely, override fields. This is now available as pull request: #2221. Please review.

@luite
Copy link
Member

luite commented Nov 14, 2014

The patch that I mentioned required more changes in the data types than I was comfortable with, since it supported an unset value for every flag. @snoyberg's patch is a lot smaller than that, since it makes remote-repo a special case, and override part of the RemoteRepo data type. I have to admit that I'm not too keen on this, but it could be a pragmatic fix to address an immediate need if we can extend this override syntax later to all fields and implement it in a more principled way.

I personally still like the idea of explicitly including things best, even when keeping it simple (no partial imports), since it's easy to explain and understand and immediately works for all fields. Unfortunately I didn't complete the implementation since I ran out of time, and many other things here have been taking more time than I hoped (like making GHCJS work with GHC HEAD again) recently.

I'll dig up what i got for my include implementation this weekend, I'm don't remember how close to ready it is or what it still takes to finish it, but I guess letting it sit on my hard drive only delays a solution.

Somewhat off-topic: Since GHC 7.10 is nearing, I feel that there must be more problems similar to this one that we really want fixed before the branch for 7.10 is cut, but are in need of some attention to get unstuck. I can spend some time on this, Sunday-Wednesday for example, be online on IRC (google talk or other is ok too) and try to help others navigating the source and discuss solutions. Should we do something like this, agree on a bunch of higher priority tickets and do a distributed "mini-hackathon"?

23Skidoo pushed a commit to 23Skidoo/cabal that referenced this issue Nov 15, 2014
23Skidoo added a commit to 23Skidoo/cabal that referenced this issue Nov 15, 2014
Fixes haskell#1884. See comments in the code for more details.
23Skidoo added a commit to 23Skidoo/cabal that referenced this issue Nov 15, 2014
23Skidoo added a commit to 23Skidoo/cabal that referenced this issue Nov 16, 2014
@snoyberg
Copy link
Collaborator Author

Note that I have backported @23Skidoo's patch to cabal 1.20 and am now using it on my local system with Stackage, and it appears to work just fine.

snoyberg pushed a commit to snoyberg/cabal that referenced this issue Nov 17, 2014
snoyberg pushed a commit to snoyberg/cabal that referenced this issue Nov 17, 2014
@luite
Copy link
Member

luite commented Nov 17, 2014

Great, thanks for doing the work!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

10 participants