Skip to content

make cabal less chatty w.r.t. project files in use #10940

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
Jun 20, 2025

Conversation

ulysses4ever
Copy link
Collaborator

@ulysses4ever ulysses4ever commented Apr 26, 2025

fix #10885

After #10507, cabal prints out what project files are in use on every run.
This looked too noisy for some users (#10885). In this patch, we implement a more nuanced
strategy: print out this info only when cabal is run below the root project directory.
As before, you can get this information uncoditionally if run in the verbose mode.

Bonus: we now also print the porject root directory along with the file names.
Before, we only printed, say, cabal.project, but it wasn't clear where this file is
coming from (can be enywhere up the directory tree).
The change tries to avoid confusion when cabal picks up stray project files in
ancestor directories, see discussion in #7930.

The diff in tests is big but it should mostly just undo the changes from #10507.

Manual QA

  • Create a simple package, e.g. cabal init -nm --lib.
  • Create a project file, e.g. echo 'packages: .' > cabal.project
  • Run cabal build all --dry -- expect NO message about project files.
  • cd src and run cabal again just as in the previous step -- expect a message about cabal.project.

Template Α: This PR modifies behaviour or interface

Include the following checklist in your PR:

@ulysses4ever ulysses4ever force-pushed the cabal-project-message-less-chatty-t10885 branch 2 times, most recently from d2194da to 45dcb0d Compare April 26, 2025 23:49
@ulysses4ever ulysses4ever force-pushed the cabal-project-message-less-chatty-t10885 branch 6 times, most recently from 4501f79 to cc63eaf Compare May 13, 2025 23:09
@ulysses4ever ulysses4ever force-pushed the cabal-project-message-less-chatty-t10885 branch 3 times, most recently from 528cf85 to fae5b09 Compare May 19, 2025 15:59
@Mikolaj
Copy link
Member

Mikolaj commented May 27, 2025

Are you still seeing the problems in the CI? As in

I'm seeing weird (seemingly) unrelated test failures in CI (just several) on my draft PR and can't explain them.
They all seem to start with output:


+ /usr/bin/git ls-files --cached --modified


so that's a new line in the output, right? No idea where it comes from...

Here's an example run https://github.com/haskell/cabal/actions/runs/15117706253/job/42492471019

Would appreciate any insights...

@ulysses4ever
Copy link
Collaborator Author

@Mikolaj yes, that's why the pipelines are currently failing I believe

@Mikolaj
Copy link
Member

Mikolaj commented May 28, 2025

I don't think this + indicates an error, because it's not in the context of "Actual output differs from expected:" and there's no patch syntax (---, +++, @@ -1,7 +1,5 @@, etc.). See this real error from the same file for comparison:

Actual output differs from expected:

stderr:
--- /tmp/cabal-testsuite-154925/cabal.yes.dist/cabal.yes.out.normalized	2025-05-19 16:23:19.039827000 +0000
+++ /tmp/cabal-testsuite-154925/cabal.yes.dist/cabal.yes.comp.out.normalized	2025-05-19 16:23:19.039827000 +0000
@@ -1,7 +1,5 @@
 # cabal clean
 # cabal v2-repl
-Configuration is affected by the following files:
-- cabal.project
 Resolving dependencies...
 Build profile: -w ghc-<GHCVER> -O1
 In order, the following will be built:

*** unexpected failure for PackageTests/MultiRepl/CustomSetupKeepTempFiles/cabal.test.hs

@ulysses4ever
Copy link
Collaborator Author

Oh my, I had a feeling I lost my way in the output... I wonder if in the future we could try to avoid generating +'s like the one that confused me.

The actual failure is expected and will be fixed. The reason it wasn't fixed before is that when I ran the testsuite locally --accept'ing the necessary changes, this test failed with an exception in ProjectaOrchestration.hs. This is a discrepancy between my local setup and CI that probably deserves a separate ticket but let's not get bogged down by this.

Thanks for unlocking me! Now I can probably finish fixing tests at least.

@ulysses4ever ulysses4ever force-pushed the cabal-project-message-less-chatty-t10885 branch 2 times, most recently from 2f4b051 to df7fc70 Compare May 29, 2025 15:04
@ulysses4ever ulysses4ever force-pushed the cabal-project-message-less-chatty-t10885 branch from df7fc70 to 5c90821 Compare May 29, 2025 17:52
@ulysses4ever
Copy link
Collaborator Author

ulysses4ever commented May 29, 2025

2025-05-29T15:32:41.9907347Z PackageTests/JS/JSPPOptions/other-arch.test.hs                                                                    FAIL (2.94s)

I don't see this test in my tree — is it a ghost?

@geekosaur
Copy link
Collaborator

@ulysses4ever
Copy link
Collaborator Author

@geekosaur

Seems to be from https://github.com/haskell/cabal/pull/10722/files#diff-5b2c8500c79636c2f408a166261b24ddbdf12e9aaa31c75b0fb50f8a954188f4 ?

Exactly: that one isn't merged yet, so there shouldn't be tests from there on my branch...


I'm having trouble understanding this:

Actual output differs from expected:

stderr:
--- /private/var/folders/w5/_8wgjw3j5cg6mgrth3s2kg9m0000gn/T/cabal-testsuite-73838/cabal.dist/cabal.out.normalized	2025-05-29 18:38:06
+++ /private/var/folders/w5/_8wgjw3j5cg6mgrth3s2kg9m0000gn/T/cabal-testsuite-73838/cabal.dist/cabal.comp.out.normalized	2025-05-29 18:38:06
@@ -11,7 +11,7 @@
 Running 1 test suites...
 Test suite test: RUNNING...
 Test suite test: PASS
-Test suite logged to: cabal.dist/work/./dist/build/<ARCH>/ghc-<GHCVER>/plain-0.1.0.0/test/plain-0.1.0.0-test.log
-Package coverage report written to cabal.dist/work/./dist/build/<ARCH>/ghc-<GHCVER>/plain-0.1.0.0/hpc/vanilla/html/hpc_index.html
+Test suite logged to: <ROOT>/cabal.dist/work/./dist/build/<ARCH>/ghc-<GHCVER>/plain-0.1.0.0/test/plain-0.1.0.0-test.log
+Package coverage report written to <ROOT>/cabal.dist/work/./dist/build/<ARCH>/ghc-<GHCVER>/plain-0.1.0.0/hpc/vanilla/html/hpc_index.html
 1 of 1 test suites (1 of 1 test cases) passed.
-Package coverage report written to cabal.dist/work/./dist/build/<ARCH>/ghc-<GHCVER>/plain-0.1.0.0/hpc/vanilla/html/hpc_index.html
+Package coverage report written to <ROOT>/cabal.dist/work/./dist/build/<ARCH>/ghc-<GHCVER>/plain-0.1.0.0/hpc/vanilla/html/hpc_index.html

*** unexpected failure for PackageTests/CustomTestCoverage/cabal.test.hs

@Bodigrim thanks for your comments. I'll incorporate them when/if I finish my fight with the test suite. Currently, it's not clear who will come out as a victor, I or it...

@geekosaur
Copy link
Collaborator

I've never really understood all the normalization stuff, tbh. And I think I poked at it once and found it a bit hacky.

@ulysses4ever
Copy link
Collaborator Author

Okay, test suite issues aside, there's an actual technical question: if we want to print project files out only when cabal is run from a directory below the project root, we need to be able to determine, somehow, that we are not in the project root. How?

My first idea was to look at the path to project files, 'cause I thought their paths would be stored relative to CWD but that's of course not the case --- their paths are relative to the project root (it makes sense: otherwise we'd have to reconfigure everytime we change directory we run cabal from).

So, I'm currently looking for ideas on how to tell that cabal is run not from the project root.

@ulysses4ever
Copy link
Collaborator Author

all right, looks like distProjectRootDirectory /= cwd should work.

@ulysses4ever ulysses4ever force-pushed the cabal-project-message-less-chatty-t10885 branch 5 times, most recently from bdcf9aa to ab84f4f Compare June 12, 2025 16:16
@ulysses4ever
Copy link
Collaborator Author

this branch is rebased now but the PR won't merge because @philderbeast "requested changes"

@Mikolaj
Copy link
Member

Mikolaj commented Jun 17, 2025

@philderbeast: could we have your ban lifted? :)

Actually, assuming you are fine with it and in the interest of time, let me lift it myself, but please correct me, if I got the situation wrong.

@Mikolaj Mikolaj requested a review from philderbeast June 17, 2025 14:17
@Mikolaj Mikolaj dismissed philderbeast’s stale review June 17, 2025 14:18

I understand this no longer applies after the other PR was merged. Please correct me if not.

@Mikolaj
Copy link
Member

Mikolaj commented Jun 17, 2025

@mergify rebase

Copy link
Contributor

mergify bot commented Jun 17, 2025

rebase

✅ Branch has been successfully rebased

@Mikolaj Mikolaj force-pushed the cabal-project-message-less-chatty-t10885 branch from 0977e83 to 739ab41 Compare June 17, 2025 14:40
@philderbeast
Copy link
Collaborator

If @ulysses4ever agrees to rebase on top of #10688, then let's indeed speed up its merge.

#10684 has the fix for the duplicate "Configuration is affected by" notices so ideally that would go in first so we'd see its fix in the test output changes.

@philderbeast
Copy link
Collaborator

Sorry for the delay @ulysses4ever.

@ulysses4ever
Copy link
Collaborator Author

no problem @philderbeast!

@ulysses4ever ulysses4ever force-pushed the cabal-project-message-less-chatty-t10885 branch from 81ff394 to 3231c72 Compare June 17, 2025 21:57
@geekosaur
Copy link
Collaborator

I think I've mentioned this before (and I know this was a rebase, not new commits) but fixup! and friends will soon be useless: Mergify is disabling autosquash and their announcement implied that they consider it a bad idea. (They didn't explain why, and I don't recall git itself warning about their use. I also don't see a blog entry about it, or a web link in the newsletter they sent me.)

👋 Bye autosquash, you won’t be missed
Autosquash is now disabled by default and will be deprecated soon.
You probably weren't using it. If you were… let's be honest, you probably shouldn't be.

@ulysses4ever
Copy link
Collaborator Author

@geekosaur thanks, quite true. This is more of a convention for me rather than a tool.

@ulysses4ever
Copy link
Collaborator Author

ulysses4ever commented Jun 18, 2025

@geekosaur there are hidden perks that "fixup!" brings, e.g. Magit comments out these messages when doing squash locally and forming the resulting commit message

@ulysses4ever ulysses4ever force-pushed the cabal-project-message-less-chatty-t10885 branch from 3231c72 to 00b179e Compare June 18, 2025 13:09
@ulysses4ever ulysses4ever force-pushed the cabal-project-message-less-chatty-t10885 branch from 00b179e to bd0760e Compare June 18, 2025 14:24
@ulysses4ever
Copy link
Collaborator Author

@mergify refresh

Copy link
Contributor

mergify bot commented Jun 18, 2025

refresh

✅ Pull request refreshed

@mergify mergify bot added the merge delay passed Applied (usually by Mergify) when PR approved and received no updates for 2 days label Jun 20, 2025
@mergify mergify bot merged commit df88415 into master Jun 20, 2025
55 checks passed
@mergify mergify bot deleted the cabal-project-message-less-chatty-t10885 branch June 20, 2025 17:37
@github-project-automation github-project-automation bot moved this from Waiting to To Test in Manual QA board Jun 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
attention: needs-manual-qa PR is destined for manual QA merge delay passed Applied (usually by Mergify) when PR approved and received no updates for 2 days re: project-file Concerning cabal.project files ready and waiting Mergify is waiting out the cooldown period squash+merge me Tell Mergify Bot to squash-merge
Projects
Status: To Test
Development

Successfully merging this pull request may close these issues.

cabal got annoyingly chatty
7 participants