Skip to content

Loading…

The sandbox config should inherit from ~/.cabal/config instead of including a copy #1123

Closed
tibbe opened this Issue · 16 comments

2 participants

@tibbe
Haskell member

Commit fada98e had the sandbox config contain a copy of the user config file (i.e. ~/.cabal/config). This might be frustrating for users who e.g. try to enable profiling by setting enable-library-profiling: True in in the user config, only to have the change not be reflected in their sandbox, which config contains enable-library-profiling: False, as copied form the user config at some earlier time.

I suggest we instead have the sandbox config inherit from the user config, like so:

combinedConfig = userConfig `mappend` sandboxConfig

We have to be careful though: sandboxing requires that we change certain fields (e.g. the install prefix and the package DB location). mappend for list-fields concatenates the fields and we must make sure that this isn't a problem for the field that the sandbox config needs to set explicitly.

@23Skidoo 23Skidoo was assigned
@23Skidoo
Haskell member

One issue here is that we already allow explicitly inheriting from other config files, so adding an additional implicit inheritance mechanism might be confusing.

@23Skidoo
Haskell member

Another issue to think about: what if the user wants to set some option to the default value in the sandbox, but to a custom value in .cabal/config? Without this change, it means just not setting this option in the package environment file. With this change, it becomes impossible.

@tibbe
Haskell member

(If you haven't seen it already, Duncan's original design for the flags/config system is here: http://www.haskell.org/pipermail/cabal-devel/2007-December/001509.html)

Lets pretend that the ability to explicitly inherit from other config files doesn't exist for a second. In that case we want to mappend the configs like so:

defaultConfig `mappend`
    userConfig `mappend`
    sandboxConfig `mappend`
    packageEnvironment `mappend`
    commandLineFlags

where

  • defaultConfig = config created internally in cabal (mempty?)
  • userConfig = ~/.cabal/config
  • sandboxConfig = $PWD/cabal.sandbox.config
  • packageEnvironment = $PWD/cabal.config

(If I remember the file names correctly.)

Now, should the presence of the inheritance mechanism change this? I don't think so. It would make the user responsible for setting the correct inherit-from flag in the package environment config depending on if a sandbox was used or not (as in the former case it should inherit from $PWD/cabal.sandbox.config and in the latter it should inherit from ~/.cabal/config).

Lets leave the inheritance mechanism as way for users to refactor big configs into many files.

@23Skidoo
Haskell member

Lets leave the inheritance mechanism as way for users to refactor big configs into many files.

I think that the main use case for it is the cabal-meta functionality (setting constraints on package versions).

I'm not arguing to remove explicit inheritance. I just think that having two slightly different inheritance mechanisms might be confusing (where inherit: is just mappend and implicit inheritance is mappend that also overrides some fields). Additionally, implicit inheritance breaks the sandbox metaphor a little bit (a sandbox is an isolated environment, but not really - modifying ~/.cabal/config affects all sandboxes).

I'm starting to think that having an explicit sandbox refresh-config command for updating cabal.sandbox.config with current values from ~/.cabal.config would be more intuitive.

@tibbe
Haskell member

I think that the main use case for it is the cabal-meta functionality (setting constraints on package versions).

I'm not sure if I follow. Probably because I'm only vaguely familiar with cabal-met. If I want to set constraints on package versions, I should put those in the package environment file, which I store at the root of the current project. Why do I need the inheritance?

implicit inheritance is mappend that also overrides some fields

What do you mean by overriding some fields? Aren't we just mappend-ing the sandbox config on top of the user config (which is already mappend-ed with the default config)?

Additionally, implicit inheritance breaks the sandbox metaphor a little bit (a sandbox is an isolated environment, but not really - modifying ~/.cabal/config affects all sandboxes).

I don't think it needs to be isolated in that sense. The idea (in my mind anyway) of sandboxes is that they approximate hermetic builds (as described in my blog post I'm sure you've read). The ~/.cabal/config file should just be short-hand for flags you could give on the command line anyway.

I'm starting to think that having an explicit sandbox refresh-config command for updating cabal.sandbox.config with current values from ~/.cabal.config would be more intuitive.

I don't think so. I think we should be (implicitly) inheriting from that file. Otherwise we risk that things get out of sync and users get frustrated.

Perhaps this is helpful: think of a future state where we, instead of sandboxes, uses an immutable package database (ala Nix) to store all packages and thus achieve hermetic builds. Does the settings, concepts, etc. still make sense in such a scenario?

I'm trying to avoid having the user even know of the presence of cabal.sandbox.config. That file should really just contain a few lines that change the package DB location and the prefix. Users shouldn't edit it. They should use the package environment file instead. We should be able to remove cabal.sandbox.config and replace it with a Nix data store without users noticing (or at least the vast majority of them). Does that make sense?

@23Skidoo
Haskell member

If I want to set constraints on package versions, I should put those in the package environment file, which I store at the root of the current project. Why do I need the inheritance?

So that you can use the package db in ~/.cabal instead of the sandboxed one.

Aren't we just mappend-ing the sandbox config on top of the user config (which is already mappend-ed with the default config)?

Some list-valued fields (e.g. remote-repo, package-db) need to be overridden (for implicit inheritance).

We should be able to remove cabal.sandbox.config and replace it with a Nix data store without users noticing (or at least the vast majority of them). Does that make sense?

Yes, it makes more sense when you look at it this way. I still see some problems with this solution:

  1. No way to explicitly set options in ./cabal.config to default values.

  2. Most often you modify ~/.cabal/config to enable something like profiling. Now, imagine that you have foo and bar installed in the sandbox. Then you set profiling: True in ~/.cabal/config and try to install baz which depends on foo and bar. The build fails since foo and bar do not have profiling versions installed. Since updating a value in ~/.cabal/config still means manually updating all sandboxes, we might just as well make refreshing ./.cabal.sandbox.config explicit.

The second problem will be presumably easier to solve with the Nix-like package store since install will know whether there are profiling/shared/... versions of the libraries installed.

@tibbe
Haskell member

So that you can use the package db in ~/.cabal instead of the sandboxed one.

I think that once you use the sandbox you have given up on using the user package DB. Once we have a Nix like package store which package DB to use will be mostly a moot point anyway. The fact that sandboxes use a separate package DB is just a mechanism to achieve hermetic builds, not something that should be a user visible thing.

Some list-valued fields (e.g. remote-repo, package-db) need to be overridden (for implicit inheritance).

Ah yes, so we need to special case them (just like you did).

No way to explicitly set options in ./cabal.config to default values.

Couldn't you just not specify a values in the config and thus get the default? I'm starting to feel that the default value issue is getting too much attention given its relative importance. We could just add a --parallel flag to solve the issue at hand and perhaps modify the cabal help page to output the default values we use so users can set those values explicitly in case they want the default value (which means that they would have to update their config if we change our defaults). Example:

$ cabal help install
 -j --jobs[=NUM]                    Run NUM jobs simultaneously. (default: 8)

Most often you modify ~/.cabal/config to enable something like profiling...

This will be solved once we track the "ways" libraries are installed (e.g. profiling, dynamic). Lets wait for the right fix here.

@23Skidoo
Haskell member

I think that once you use the sandbox you have given up on using the user package DB.

Some users want this and inherit was part of the original package environment design.

I'm starting to feel that the default value issue is getting too much attention given its relative importance.

It's not only about -j, it's about being able to set, say, split-objs: True in ~/.cabal/config and split-objs: $DEFAULT in ./cabal.config.

Couldn't you just not specify a values in the config and thus get the default?

No, you get the value from ~/.cabal/config (because of how mappend works).

@23Skidoo
Haskell member

It's not only about -j, it's about being able to set, say, split-objs: True in ~/.cabal/config and split-objs: $DEFAULT in ./cabal.config.

I should also say that cabal-dev supports this in its cabal-config.in file.

@tibbe
Haskell member

Some users want this and inherit was part of the original package environment design.

I'm not saying "get rid of inherit", just "don't expect to say that you want a sandboxed package DB and expect to be able to override it with a non-sandboxed one". In fact I don't see how that would ever make sense. Since the package DB (and some install prefixes) is all the sandbox give you, overriding them and still wanting to use sandboxing doesn't make much sense to me. But perhaps I'm missing something.

It's not only about -j, it's about being able to set, say, split-objs: True in ~/.cabal/config and split-objs: $DEFAULT in ./cabal.config.

But there's a trade-off. To be able to write $DEFAULT instead of False in this example, users are getting a more complicated configuration scheme. The current configuration scheme is based on flags. All the configuration in e.g. ~/.cabal/config works as-if you had added all those flags to your command line. This is why $DEFAULT is giving us trouble. There's really no command line equivalent. I think this is a nice property to have.

No, you get the value from ~/.cabal/config (because of how mappend works).

I see. You want users to be able to override ~/.cabal/config but still get a default in the package environment.

I should also say that cabal-dev supports this in its cabal-config.in file.

We'll support it if it makes sense. :)

@23Skidoo
Haskell member

I'm not saying "get rid of inherit", just "don't expect to say that you want a sandboxed package DB and expect to be able to override it with a non-sandboxed one". In fact I don't see how that would ever make sense.

As I understand it, the initial idea was to distinguish between isolated package environments (sandboxes) and non-isolated (a set of constraints).

But there's a trade-off. To be able to write $DEFAULT instead of False in this example, users are getting a more complicated configuration scheme.

Not to mention the more complicated implementation. Perhaps the cost/benefit ratio is too high and the users can live with having to use False instead of $DEFAULT.

This will be solved once we track the "ways" libraries are installed (e.g. profiling, dynamic). Lets wait for the right fix here.

Since the persistent package store will obsolete sandboxes, this behaviour will persist for the entire lifespan of the feature.

@tibbe
Haskell member

As I understand it, the initial idea was to distinguish between isolated package environments (sandboxes) and non-isolated (a set of constraints).

Sorry, I've not communicated clearly enough.

Package environments are mainly a convenient way for users to lock down package versions and cabal flags for a project they're currently working on and share that config with other developers (via source control). I don't expect users to use package environments to specify package DBs, even though that is possible (which is nice, because general mechanisms are nice). In other words, I intend package environments to be used much like Ruby's lockfiles.

Package environments also happens to be a mechanism we can use to implement sandboxes. We could have taken a different approach, such as creating a totally different way to mark a working directory as using a sandbox e.g. we could have had an empty .uses-sandbox file that would have told cabal that it should look for the package DB in a different place (hard-coded in the cabal source).

Sandboxes are not the goal, hermetic builds are. Sandboxes (and their use of package environments) is just a way to get there.

Not to mention the more complicated implementation. Perhaps the cost/benefit ratio is too high and the users can live with having to use False instead of $DEFAULT.

At least for now I think we're better of just fixing the current issue (parallel builds by default) and move on.

Since the persistent package store will obsolete sandboxes, this behaviour will persist for the entire lifespan of the feature.

We plan to track ways separate from having a persistent package store. We mainly need to change the package DB format. Once we do that sandboxes will install profiling libraries as needed automatically.

@23Skidoo
Haskell member

Package environments are mainly a convenient way for users to lock down package versions and cabal flags for a project they're currently working on and share that config with other developers (via source control).

I think we can actually get rid of inherit if we follow this rule when loading package environment files:

If the directory contains cabal.sandbox.config (and optionally a cabal.config) - this is a sandbox, use an isolated package db, overriding the userConfig settings.

If the directory contains only a cabal.config - just mapppend it with the config file settings.

Sandboxes are not the goal, hermetic builds are.

No disagreement here.

At least for now I think we're better of just fixing the current issue (parallel builds by default) and move on.

Agreed.

We plan to track ways separate from having a persistent package store. We mainly need to change the package DB format. Once we do that sandboxes will install profiling libraries as needed automatically.

Great.

@tibbe
Haskell member

this is a sandbox, use an isolated package db, overriding the userConfig settings.

Assuming you mean just override the necessary settings (i.e. package DB and install prefixes), this sounds good to me.

I'm kinda ambivalent when it comes to the inherit feature. I never argued for it so it don't feel strongly about its behavior (while I do feel strongly about the behavior of sandboxes). I don't want to outright remove it without talking to the person who wanted it in the first place (was it @dcoutts)?

@23Skidoo
Haskell member

Assuming you mean just override the necessary settings (i.e. package DB and install prefixes), this sounds good to me.

Yes, that's what I meant.

@23Skidoo 23Skidoo added a commit to 23Skidoo/cabal that referenced this issue
@23Skidoo 23Skidoo Makes sandboxes inherit from '~/.cabal/config'.
Previously that file was copied at sandbox creation time. Fixes #1123.
1a0f293
@23Skidoo
Haskell member

Implemented in #1217. I decided not to remove the inherit feature for now (could be useful for constructing nested sandboxes as discussed in #1196).

@23Skidoo 23Skidoo added a commit that closed this issue
@23Skidoo 23Skidoo Makes sandboxes inherit from '~/.cabal/config'.
Previously that file was copied at sandbox creation time. Fixes #1123.
1794af3
@23Skidoo 23Skidoo closed this in 1794af3
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.