Skip to content

Redesign 'cabal path' command to account for projects (backport #9583) #9893

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

Merged
merged 1 commit into from
Apr 16, 2024

Conversation

mergify[bot]
Copy link
Contributor

@mergify mergify bot commented Apr 16, 2024

Previously, cabal path was only able to query from the global configuration file, e.g., ~/.cabal/config or the XDG equivalent. This was deemed not useful enough to warrant a new cabal command, which is a user facing change.

We take the foundations and turn them into cabal v2-path which takes project configuration, such as cabal.project into account. Note, the command is still named cabal path, but for the sake of disambiguation, we refer to this new iteration of the command as cabal v2-path.

In addition, we add support for multiple output formats, such as key-value pairs and json. Specifying the output format is currently mandatory. This allows us to postpone the decision of the default output format and encourages users to explicitly specify the output format in the future, too.

The json output format is versioned by the cabal-install version, which is part of the json object.
Thus, all result objects contain at least the key "cabal-install-version".

The key-value pair output prints a line for each queried key and its respective value:

key1: value2
key2: value2

If only a single key is queried, we print only the value, for example:

value1

We expand the cabal v2-path to also produce information of the compiler that is going to be used in a cabal build or cabal repl invocation. To do that, we rebuild the install plan and query for the configured compiler program.
This is helpful for downstream tools, such as HLS, to figure out the GHC version required to compile a project with.

QA Notes

The name of the command is cabal path. It should give the user the option to query certain paths cabal will be using.

The following things might be of interest to validate

  • Check, that the output of cabal path --help informs the user of its purpose and stability promises.
  • Any combination of flags to cabal path should succeed. Order should not matter and the output does not honour the order of flags.
    • Exception, what is the expected behaviour when the given ghc executable cannot be found? Currently, no output is produced and cabal exits with a non-zero exit status.
  • As the command is intentionally requiring the flag output-format, did the error message when you forget to supply this flag help you figure out how to solve the issue?
  • Does the command work in both XDG and legacy cabal dir layouts?
  • Are cabal.project files honoured?
  • Are the paths you can query of interest to you?
  • Can you ignore the project context via -z?
  • Can you supply custom --cabal-config locations?
  • The command cabal path must never produce any caching artefacts, i.e. no folder in dist-newstyle is ever modified or created. Neither is dist-newstyle.
  • Are all output-formats adequately explained or self-explanatory?

Template Α: This PR modifies cabal behaviour

Include the following checklist in your PR:

  • Patches conform to the coding conventions.
  • Any changes that could be relevant to users have been recorded in the changelog.
  • The documentation has been updated, if necessary.
  • Manual QA notes have been included.
  • Tests have been added. (Ask for help if you don’t know how to write them! Ask for an exemption if tests are too complex for too little coverage!)

This is an automatic backport of pull request #9583 done by [Mergify](https://mergify.com).

Copy link
Contributor Author

mergify bot commented Apr 16, 2024

Cherry-pick of 4a8a7c5 has failed:

On branch mergify/bp/3.12/pr-9583
Your branch is up to date with 'origin/3.12'.

You are currently cherry-picking commit 4a8a7c5d2.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Changes to be committed:
	modified:   cabal-install/cabal-install.cabal
	new file:   cabal-install/src/Distribution/Client/CmdPath.hs
	modified:   cabal-install/src/Distribution/Client/Config.hs
	modified:   cabal-install/src/Distribution/Client/Errors.hs
	modified:   cabal-install/src/Distribution/Client/Main.hs
	modified:   cabal-install/src/Distribution/Client/ScriptUtils.hs
	modified:   cabal-testsuite/PackageTests/Path/All/cabal.out
	modified:   cabal-testsuite/PackageTests/Path/All/cabal.test.hs
	new file:   cabal-testsuite/PackageTests/Path/Compiler/cabal.out
	new file:   cabal-testsuite/PackageTests/Path/Compiler/cabal.test.hs
	new file:   cabal-testsuite/PackageTests/Path/Config/cabal.out
	new file:   cabal-testsuite/PackageTests/Path/Config/cabal.test.hs
	new file:   cabal-testsuite/PackageTests/Path/Config/config.cabal
	new file:   cabal-testsuite/PackageTests/Path/Config/fake-cabal.config
	new file:   cabal-testsuite/PackageTests/Path/Config/fake.cabal.project
	deleted:    cabal-testsuite/PackageTests/Path/Single/cabal.out
	deleted:    cabal-testsuite/PackageTests/Path/Single/cabal.test.hs
	modified:   cabal-testsuite/src/Test/Cabal/Monad.hs
	modified:   cabal-testsuite/src/Test/Cabal/OutputNormalizer.hs
	modified:   cabal-testsuite/src/Test/Cabal/Prelude.hs
	new file:   changelog.d/pr-9583
	modified:   doc/cabal-commands.rst

Unmerged paths:
  (use "git add <file>..." to mark resolution)
	both modified:   cabal-install/src/Distribution/Client/Setup.hs

To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally

@Mikolaj
Copy link
Member

Mikolaj commented Apr 16, 2024

@fendor: any idea which PRs we are missing to make it easier to rebase? I assume the original cabal path PR is already on branch 3.12 (which is why I backported this PR), but something must be missing. If it's one of the big refactorings we are not going to include, then tough luck, somebody needs to rebase this by hand. :/

@fendor
Copy link
Collaborator

fendor commented Apr 16, 2024

I don't think there is any big PR needed for this, I suspect there is a minor conflict that someone added a line close to where I deleted the path commands.

@fendor
Copy link
Collaborator

fendor commented Apr 16, 2024

Is it ok if I amend the PR? Or will mergify complain?

@Mikolaj
Copy link
Member

Mikolaj commented Apr 16, 2024

Oh, no, it would be great if you'd amend it and mergify would be very happy too. :D

@fendor fendor force-pushed the mergify/bp/3.12/pr-9583 branch from e796d66 to b5ef324 Compare April 16, 2024 10:26
Copy link
Member

@Mikolaj Mikolaj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fendor: thanks a lot, the conflicts seem to be gone. Let me approve and set the label right now so that no further CI is triggered.

@Mikolaj Mikolaj added merge me Tell Mergify Bot to merge and removed conflicts labels Apr 16, 2024
@Mikolaj
Copy link
Member

Mikolaj commented Apr 16, 2024

Doh, the moment I said that, tests failed. May this be related to the conflict (the test is to do with Setup, I think)?

@fendor
Copy link
Collaborator

fendor commented Apr 16, 2024

Hm, I don't really know what is going wrong here. I don't think that test case should be affected in any way. Perhaps one of the changes in the test output normaliser? 🤔 Perhaps we reused, by accident, a build directory because I amended the commit instead of pushing a new one?

@Mikolaj
Copy link
Member

Mikolaj commented Apr 16, 2024

Huh, must have been a fluke and an anonymous benefactor guessed that and restarted the CI and it seems to be going strong.

@fendor
Copy link
Collaborator

fendor commented Apr 16, 2024

Huh, must have been a fluke and an anonymous benefactor guessed that and restarted the CI and it seems to be going strong.

Oh yeah that was me, years of experience with HLS's CI taught me: When CI fails, restart until it is green 👼

however, CI might still be failing, the job that failed last time hasn't succeeded yet.

* Redesign 'cabal path' command to account for projects

Previously, `cabal path` was only able to query from the global
configuration file, e.g., `~/.cabal/config` or the XDG equivalent.
Adding support for cabal project is a huge boost to usability.

We take the foundations and turn them into `cabal v2-path` which takes
project configuration, such as `cabal.project` into account.
Note, the command is still named `cabal path`, but for the sake of
disambiguation, we refer to this new iteration of the command as `cabal
v2-path`.

In addition, we add support for multiple output formats, such as
key-value pairs and json.

The key-value pair output prints a line for each queried key and its
respective value:

    key1: value2
    key2: value2

If only a single key is queried, we print only the
value, for example:

    value1

The json output format is versioned by the cabal-install version which
is part of the json object.
Thus, all result objects contain at least the key "cabal-install-version".

We expand the `cabal v2-path` to also produce information of the
compiler that is going to be used in a `cabal build` or `cabal repl` invocation.
To do that, we rebuild the install plan and query for the configured
compiler program.
This is helpful for downstream tools, such as HLS, to figure out the GHC
version required to compile a project with.

We also add an exhaustive test suite for 'cabal path' cmd

We test that each query honours cabal.project files, cli parameters, and
is composable with the other query flags.

We extend the test output normalisers for ghc compiler location and
cabal-install version, as the 'cabal path' command outputs the exact ghc
and ghc-pkg location. In addition, the json output format is versioned
on the cabal-install version.

Currently, we query the cabal-install version on each test normalisation
run. This might be unnecessary expensive, and could be avoided by
introducing a 'cabalProgram' that specifies how the program version can
be found. This way, we can cache the version query.

Add '--cache-home' flag thats shows the cabal's cache root

Rename '--cache-dir' to the more correct '--remote-repo-dir'.

* Update 'cabal path' documentation

* Add changelog.d entry

---------

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
(cherry picked from commit 4a8a7c5)
@Mikolaj Mikolaj force-pushed the mergify/bp/3.12/pr-9583 branch from b5ef324 to 354224c Compare April 16, 2024 13:56
@mergify mergify bot merged commit b6c1ed6 into 3.12 Apr 16, 2024
42 checks passed
@mergify mergify bot deleted the mergify/bp/3.12/pr-9583 branch April 16, 2024 14:05
@Mikolaj
Copy link
Member

Mikolaj commented Apr 16, 2024

Audacity paid off. :D

@fendor, thank you again.

@ulysses4ever ulysses4ever mentioned this pull request May 13, 2024
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport merge me Tell Mergify Bot to merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants