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

RFC: What to do about v2-reconfigure #7405

Closed
emilypi opened this issue May 24, 2021 · 13 comments · Fixed by #7402
Closed

RFC: What to do about v2-reconfigure #7405

emilypi opened this issue May 24, 2021 · 13 comments · Fixed by #7402

Comments

@emilypi
Copy link
Member

emilypi commented May 24, 2021

Per a discussion in #7402, in which @ptkato implemented a solution for #5591, @phadej linked a discussion from #7180 in which there are some differences pointed out between v1-(re)configure and v2-(re)configure. Namely, the following: v2-configure is not a useful command, and therefore v2-reconfigure is similarly not useful. v2-configure does two things: generate a build plan, and create a cabal.project.local with project-local settings. The reasons this is not useful are two-fold:

  1. v2-build already generates a build plan autonomously, making the v2-configure build plan step redundant
  2. modification of the cabal.project.local via command-line args passed to v2-configure is under-specified. One can only pass a subset of valid cabal.project.local flags via command line (Oleg points out that you cannot specify remote source repositories, for example). So in that sense, the implementation is under-specified and hides some implementation details.

Here are a few ideas for what we can do here:

  1. Deprecate v2-configure and tell the users why it's not a bad idea, tossing away the v2-reconfigure work from cabal v2-configure #7402. Many people would probably balk at this, since it removes a useful workflow from people's tooling (script-local modifications are faster than hand-editing the local project files) there is a proliferation of content out there that alludes to v1-configure, and we'd have to be very loud and explicit about why that command in particular was deprecated.

  2. Modify v2-configure and re-spec it's purpose so that it only does modifications of the cabal.project.local, and see if we can come up with a decent middle ground for what flags we support on command line.

Your thoughts welcome.

@emilypi emilypi linked a pull request May 24, 2021 that will close this issue
3 tasks
@emilypi
Copy link
Member Author

emilypi commented May 24, 2021

Pinging @fgaz @Mikolaj @gbaz @davean

@Mikolaj
Copy link
Member

Mikolaj commented May 24, 2021

How about the following?

  1. Remove cabal v2-configure, add cabal init --cabal.project.local that does what v2-configure does currently except generating a build plan, with an extra flag --append-only that mimics v2-reconfigure except the build plan.

Of course, deprecate gently, pointing users to the new command over many releases.

Edit: and point users to cabal build --dry-run in case they use configure to quickly check if build is going to be successful.

@emilypi
Copy link
Member Author

emilypi commented May 24, 2021

I'm not sure about cabal init being re-used for this, since I rarely want any kind of prompting to go along with it. However, i like the idea of it being overwrite by default, and an --append-only flag handling the reconfiguration. That I can get on board with!

To modify your #3, a bit, a new command might be in order here. Perhaps cabal local --append-only <FLAGS>

@gbaz
Copy link
Collaborator

gbaz commented May 24, 2021 via email

@Mikolaj
Copy link
Member

Mikolaj commented May 24, 2021

Re new command, fine. I see we have only one similar command user-config:

Display and update the user's global cabal configuration.

Usage: cabal user-config init
   or: cabal user-config diff
   or: cabal user-config update

but it modifies global user configuration and we need a command for local project configuration. So perhaps

project-config       Display and update the local project configuration.

The name cabal local is fine too, mostly due to being short.

@emilypi
Copy link
Member Author

emilypi commented May 24, 2021

oh interesting. I didn't know that command existed! It's a shame that comamnd doesn't have a v2 version that allows you to specify --global, --project-local and --project levels, because I'd be very stoked to have all of those features in this hypothetical new command. (maybe it should?)

@Mikolaj
Copy link
Member

Mikolaj commented May 24, 2021

And can you specify the same things and in the same way in ~/.cabal/config as in cabal.project? I honestly have no clue. If yes (or if we can make it so), sounds like a great idea to extend this command (and possibly rename it, though keeping old names has its merits; the problem of v2-configure is not a bad name, but an unfortunate name clash with v1-configure).

@gbaz
Copy link
Collaborator

gbaz commented May 24, 2021 via email

@Mikolaj
Copy link
Member

Mikolaj commented May 24, 2021

I vote we keep the name configure and the existing workflow intact, just switching to overwrite by default.

Which workflow do you mean? The one that generates the build plan twice and overwrites (granted, with a backup) a possibly manually user-generated config file, by trying to follow the v1 command sequence using v2 commands?

@gbaz
Copy link
Collaborator

gbaz commented May 24, 2021

To be clear I'm fine with it not generating the build plan a second time. And I'm fine with it being reconfigure instead of overwrite or vice versa, which ever people prefer. I think the command has somewhat weird semantics and they can be fixed, but we should do so while keeping the name.

I think in irc we resolved that overloading user-config is not a good idea since they're just fundamentally different things.

@emilypi
Copy link
Member Author

emilypi commented May 24, 2021

Okay, so some of the conversation shifted to IRC. Here's a recap of what we came to:

  • Keep v2-configure, but do the following:

    1. Remove the dry-run functionality. It's unnecessary. This was a historical mistake, and we're fixing the semantics to be more correct in v3.8.
    2. v2-configure should overwrite cabal.project.local files by default, creating a single backup that will be overwritten on subsequent configurations with the previous config. If you want to reconfigure by appending flags, you should call v2-configure --append <FLAGS> to append new flags, and --overwrite to overwrite your local project file in-place.
    3. Configuration settings should be settable in the global config.
  • There is no longer a need for v2-reconfigure, as it will be folded into v2-configure --append

  • It's fine that you can't specify all flags for cabal.project.local.

@emilypi emilypi mentioned this issue May 24, 2021
3 tasks
ptkato added a commit to ptkato/cabal that referenced this issue May 26, 2021
ptkato added a commit to ptkato/cabal that referenced this issue May 26, 2021
ptkato added a commit to ptkato/cabal that referenced this issue May 31, 2021
ptkato added a commit to ptkato/cabal that referenced this issue May 31, 2021
@fgaz
Copy link
Member

fgaz commented May 31, 2021

more bikeshedding, feel free to ignore:

I think --{enable,disable}-overwrite is a bad name, since:

  • it looks like the opposite of --{enable,disable}-append, but it isn't
  • when we enable overwriting we are actually disabling backups, and vice versa

so I suggest to name it --{enable,disable}-backup, with default to enable

@ptkato
Copy link
Collaborator

ptkato commented May 31, 2021

I do agree with that, even I, while implementing the changes, got confused with the names.

ptkato added a commit to ptkato/cabal that referenced this issue Jun 1, 2021
This commit straightens the v2-configure command:

* Removes the pre-build phase
* Adds two flags, --append and --overwrite

Co-authored-by: Emily Pillmore <emilypi@cohomolo.gy>
ptkato added a commit to ptkato/cabal that referenced this issue Jun 1, 2021
This commit straightens the v2-configure command:

* Removes the pre-build phase
* Adds two flags, --append and --overwrite

Co-authored-by: Emily Pillmore <emilypi@cohomolo.gy>
ptkato added a commit to ptkato/cabal that referenced this issue Jun 1, 2021
This commit straightens the v2-configure command:

* Removes the pre-build phase
* Adds two flags, --append and --overwrite

Co-authored-by: Emily Pillmore <emilypi@cohomolo.gy>
emilypi added a commit that referenced this issue Jun 3, 2021
This commit straightens the v2-configure command:

* Removes the pre-build phase
* Adds two flags, --append and --overwrite

Co-authored-by: Emily Pillmore <emilypi@cohomolo.gy>
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 a pull request may close this issue.

5 participants