-
Notifications
You must be signed in to change notification settings - Fork 691
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
Expand and unify --keep-temp-files
#10292
base: master
Are you sure you want to change the base?
Expand and unify --keep-temp-files
#10292
Conversation
69f0104
to
1e05052
Compare
One concern I have is that I don't fully understand how the I'm not objecting to this PR (which I am happy to see), but I would like to make sure that you have taken into consideration the logical separation between the Cabal library and |
Yes, I wasn't entirely sure where to put this flag. There seem to be a lot of flags that are shared between multiple commands (like I picked |
e0ea1a0
to
4156c93
Compare
I think the patch looks good, apart from I think it won't work if you are using a custom setup built with an older version of Cabal. See my comment. |
We need an old-ghcs test job with custom setup, as we already have backward compatibility breakage that leaked through (#10379). |
4156c93
to
8255fc1
Compare
This lets the `--keep-temp-files` options to `cabal haddock` and `cabal repl` work with an older Cabal version. Pointed out by @mpickering: haskell#10292 (comment)
b209b2f
to
d797bc4
Compare
This lets the `--keep-temp-files` options to `cabal haddock` and `cabal repl` work with an older Cabal version. Pointed out by @mpickering: haskell#10292 (comment)
d797bc4
to
8cefa2c
Compare
This lets the `--keep-temp-files` options to `cabal haddock` and `cabal repl` work with an older Cabal version. Pointed out by @mpickering: haskell#10292 (comment)
8cefa2c
to
e9016e8
Compare
In haskell#10292, we will move the `--keep-temp-files` setting into `CommonSetupFlags` and out of `ReplFlags`/`HaddockFlags`. This means that the flag-filtering behavior (which adapts flags from new versions of `cabal-install` to old version of `Cabal`) will need to know which command is being run to provide the correct `CommonSetupFlags`. Therefore, this change adds several new `filterFooFlags` functions to provide this behavior, and removes the `commonFlags` used for all subcommands in `Distribution.Client.ProjectBuilding.UnpackedPackage`.
e9016e8
to
f7369cf
Compare
This PR has gotten too large to effectively review, so I've split out many of the changes into separate, smaller PRs:
Once those PRs are all merged, this change will only have ~100 lines to review. |
facc238
to
b60ee3f
Compare
0fb8ef7
to
93a5b0b
Compare
`addFields` should be in `ParseUtils` with the rest of the field helpers.
Currently, `cabal repl` has a `--keep-temp-files` option, and `cabal.project` has a `keep-temp-files` option but it only effects Haddock builds. This patch adds `--keep-temp-files` to `CommonSetupFlags`, making it available to all commands. The expanded `--keep-temp-files` flag is used for the `cabal repl` command and Haddock builds (retaining compatibility with the previous behavior) but is also used to determine when to keep response files.
93a5b0b
to
7d29dc2
Compare
Could you have another look @sheaf and @mpickering? |
This is blocking #9367.
Currently,
cabal repl
has a--keep-temp-files
option, andcabal.project
has akeep-temp-files
option but it only affects Haddock builds.This patch adds
--keep-temp-files
toCommonSetupFlags
, making it available to all commands. The expanded--keep-temp-files
flag is used for thecabal repl
command and Haddock builds (retaining compatibility with the previous behavior). The flag will also be used for response files; see #9367.Template Α: This PR modifies behaviour or interface
Include the following checklist in your PR:
Depends-on: #10395
Depends-on: #10392
Depends-on: #10391
Depends-on: #10390
Depends-on: #10389
Depends-on: #10388
Depends-on: #10393