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

Add support for cabal.project.local #632

Merged
merged 4 commits into from
Jun 1, 2020
Merged

Conversation

hamishmack
Copy link
Collaborator

Running cabal v2-configure overwrites the cabal.project.local file
so this change adds the contents onto the end of the cabal.project
file before running cabal v2-configure.

You can override this behavior passing cabalProjectLocal = ""

Running `cabal v2-configure` overwrites the `cabal.project.local` file
so this change adds the contents onto the end of the `cabal.project`
file before running `cabal v2-configure`.

You can override this behavior passing `cabalProjectLocal = ""`
@hamishmack hamishmack requested a review from michaelpj May 28, 2020 10:15
@michaelpj
Copy link
Collaborator

I have slightly mixed feelings about this. Conceptually it makes sense to me that haskell.nix should pick this up, since the guiding principle is to do whatever cabal does. But usually I have a cabal.project.local for my "dev" process, whereas I'm using the Nix stuff for more "prod" process (and e.g. I still want to get cache hits for my local version).

As I write that, though, I guess this might be solved by using an appropriate source cleaner? I.e. if you use cleanGit and cabal.project.local is gitignored then it won't be picked up?

@hamishmack
Copy link
Collaborator Author

As I write that, though, I guess this might be solved by using an appropriate source cleaner? I.e. if you use cleanGit and cabal.project.local is gitignored then it won't be picked up?

It bypasses the cleaner and goes straight to the file location. We could make it respect the filter in the cleaner, but then we would have to find a way to let it through cleanGit for that to be useful.

Perhaps we could rename it cabalProjectExtra and just default it to "" (never use __readFile for it). Then caller is free to pass in __readFile "cabal.project.local" if they wish?

@michaelpj
Copy link
Collaborator

haskell/cabal#6016 (comment) alleges that the old cabal.project.local is moved to a backup location: could we cat it back on to the end of cabal.project.local after we do configure?

@angerman
Copy link
Collaborator

Hmm... IDK. local is local. I see that it would be useful for "local" development/nix build/nix shell, but maybe we don't want cleanGit in that instance anyway?

@hamishmack
Copy link
Collaborator Author

haskell/cabal#6016 (comment) alleges that the old cabal.project.local is moved to a backup location: could we cat it back on to the end of cabal.project.local after we do configure?

The problem is that it ignores the current cabal.project.local file when constructing the plan.json. I think appending to cabal.project will work ok (we throw it away anyway once we have the plan.json file).

I think the main thing we have to decide is the default behaviour of haskell-nix.cabalProject. I have no strong feelings either way, but here are what I think are the pros of each option.

Include cabal.project.local by default

  • nix-shell will have right dependencies needed for cabal build.
  • cabal build x:y == nix-build -A pkg.components.x.y.
  • Might fix stuff that does not currently work.

Excluded cabal.project.local by default

  • nix-build locally will match CI builds.
  • Won't break anything that currently works.

@michaelpj
Copy link
Collaborator

This is a problem we have because we actually have the ability to share builds with CI.

Looking at it again, I'm convinced by the benefits. IMO nix build == cabal build is a very nice property that we should try and keep as absolutely true as we can. And if people complain that it doesn't share with CI, we can just point out that they've asked for something else.

So 👍 to the functionality here.

On an implementation point of view, it seems like the cabal config files get treated in two different ways:

  • cabal.project gets pulled in specially
  • cabal.project.freeze just gets pulled in via the source and used by the builder

Does cabal.project.local need to behave like the former, or could we make it work more like the latter? Or maybe the freeze file should also be pulled in specially: if you gitignored your freeze file then I think currently it might not be true that nix build == cabal build.

@angerman
Copy link
Collaborator

Do not that we still want to have cabal.project.ci (and possibly have that extension configurable per job or something). This should superceed any configureArgs; and would need to be copied into cabal.project.local prior to running cabal configure.

This usecase needs to mesh with this as well.

Will still be used if it is in the .gitignore
@hamishmack
Copy link
Collaborator Author

I have updated this PR so both cabal.project.local and cabal.project.freeze are read directly and will still be used if filtered out be cleanGit. For cabal.project.ci would it make sense just to require:

project = cabalProject {
  cabalProjectLocal = __readFile "cabal.project.ci";
  configureArgs = "";
  ...
};

@hamishmack hamishmack requested a review from angerman May 29, 2020 23:50
@hamishmack hamishmack merged commit e5a0522 into master Jun 1, 2020
@iohk-bors iohk-bors bot deleted the hkm/cabal-project-local branch June 1, 2020 11:12
booniepepper pushed a commit to booniepepper/haskell.nix that referenced this pull request Feb 4, 2022
Running `cabal v2-configure` overwrites the `cabal.project.local` file
so this change adds the contents onto the end of the `cabal.project`
file before running `cabal v2-configure`.

You can override this behavior passing `cabalProjectLocal = ""`

Treats cabal.project.feeze the same way as `cabal.project` and
`cabal.project.local` in that it will still use it if it is filtered by cleanGit.

This change also adds `cabalProjectFileName` arg so that
we can override the default `cabal.project` name.
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.

3 participants