Skip to content

Fix monitoring/caching of imported project files#11567

Open
hasufell wants to merge 5 commits intohaskell:masterfrom
hasufell:issue-10255
Open

Fix monitoring/caching of imported project files#11567
hasufell wants to merge 5 commits intohaskell:masterfrom
hasufell:issue-10255

Conversation

@hasufell
Copy link
Member

@hasufell hasufell commented Mar 3, 2026

See #10255

However, it appears there's still a bug in the patch, where the following assert gets triggered after I change the imported cabal.project.common file:

cabal/cabal-install/src/Distribution/Client/ProjectPlanning.hs

Lines 268 to 272 in d67898a

-- the stanzas explicitly disabled should not be available
. assert
( optStanzaSetNull $
optStanzaKeysFilteredByValue (maybe False not) elabStanzasRequested `optStanzaSetIntersection` elabStanzasAvailable
)

@hasufell hasufell marked this pull request as draft March 3, 2026 10:37
@hasufell hasufell linked an issue Mar 3, 2026 that may be closed by this pull request
Changing a file imported by cabal.project does not trigger recompilation #10255
Open
@hasufell
Copy link
Member Author

hasufell commented Mar 3, 2026

Actually... this assert happens with upstream cabal as well.

Looks like I ran over another bug.

@philderbeast
Copy link
Collaborator

philderbeast commented Mar 3, 2026

I will return to look at this in detail. Can you alter your test or add another project to the tests so that its imports are relative to each other and not all in the same folder. This issue has some discussion on relative imports, introduced in 3.10, #8795 (comment).

@hasufell hasufell mentioned this pull request Mar 3, 2026
Open
@hasufell
Copy link
Member Author

hasufell commented Mar 3, 2026

#11568

@hasufell hasufell linked an issue Mar 4, 2026 that may be closed by this pull request
Assertion failure for "stanzas explicitly disabled should not be available" #11568
Open
@hasufell hasufell force-pushed the issue-10255 branch 2 times, most recently from 2443bac to e0b7900 Compare March 4, 2026 14:00
@hasufell hasufell marked this pull request as ready for review March 4, 2026 14:00
hasufell
hasufell commented Mar 4, 2026
View reviewed changes
@philderbeast
Copy link
Collaborator

philderbeast commented Mar 4, 2026 •
edited
Loading

A couple of patches, more indirection with the imports and one -Wunused-imports warning.

Add-more-indirection-for-project-imports.patch
Wunused-imports.patch

I made a typo you may like to correct:

-   assertOutputDoesNotContain "Test suite not yet implement" result'
+   assertOutputDoesNotContain "Test suite not yet implemented" result'

@hasufell hasufell force-pushed the issue-10255 branch 2 times, most recently from c3ea457 to 504ccd5 Compare March 4, 2026 15:56
angerman
angerman reviewed Mar 5, 2026
View reviewed changes
cabal-testsuite/PackageTests/ProjectImport/FileMonitoring/test/Main.hs
module Main (main) where

main :: IO ()
main = puStrLn "Test suite not yet implemented."
Copy link
Collaborator

@angerman angerman Mar 5, 2026

Choose a reason for hiding this comment

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

Is this intentional to make the test fail to compile?

Copy link
Member Author

@hasufell hasufell Mar 5, 2026

Choose a reason for hiding this comment

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

Yes.

@hasufell hasufell force-pushed the issue-10255 branch 2 times, most recently from 980e55c to 9d5dcb3 Compare March 6, 2026 11:44
@hasufell
Copy link
Member Author

hasufell commented Mar 6, 2026 •
edited
Loading

I redesigned ProjectConfigPath, because I did not like how we stuff URIs into FilePaths: c6ae276

@hasufell hasufell force-pushed the issue-10255 branch 4 times, most recently from 00a4e5f to ac6d6c4 Compare March 6, 2026 12:21
hasufell added 3 commits March 9, 2026 14:17
@hasufell
Fix assertion failure wrt haskell#11568
5021c05 
'lookupLocalPackageConfig' would ignore 'projectConfigAllPackages'
(`package *`) and thus diverge from 'lookupPerPkgOption'.

This would then cause further divergence between 'elabStanzasRequested'
and 'elabStanzasAvailable'.
@hasufell
Add test for haskell#10255
5d3b2ff
@hasufell
Monitor imported cabal.project files as well
c8f2c84 
Fixes haskell#10255
@hasufell hasufell force-pushed the issue-10255 branch 3 times, most recently from 9d96a0b to aa45123 Compare March 9, 2026 07:41
@hasufell
Be stricter about URI imports
67685ca 
This behavior is documented, but not enforced. We
redesign the 'ProjectConfigPath' type to better
express the properties we expect.
@hasufell hasufell requested a review from gbaz March 10, 2026 07:35
Bodigrim
Bodigrim reviewed Mar 13, 2026
View reviewed changes
Cabal-syntax/Cabal-syntax.cabal
, directory >= 1.2 && < 1.4
, filepath >= 1.3.0.1 && < 1.6
, mtl >= 2.1 && < 2.4
, network-uri >= 2.6.0.2 && < 2.7
Copy link
Collaborator

@Bodigrim Bodigrim Mar 13, 2026

Choose a reason for hiding this comment

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

Unfortunately we cannot do it: Cabal-syntax is a boot package and cannot depend on a non-boot network-uri.

Bodigrim
Bodigrim reviewed Mar 13, 2026
View reviewed changes
Copy link
Collaborator

@Bodigrim Bodigrim left a comment

Choose a reason for hiding this comment

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

Looks good except the last commit; perhaps needs a changelog?..

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@angerman angerman angerman left review comments

@Bodigrim Bodigrim Bodigrim left review comments

@philderbeast philderbeast Awaiting requested review from philderbeast

@andreabedini andreabedini Awaiting requested review from andreabedini

@gbaz gbaz Awaiting requested review from gbaz

At least 2 approving reviews are required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Assertion failure for "stanzas explicitly disabled should not be available" Changing a file imported by cabal.project does not trigger recompilation

4 participants

@hasufell @philderbeast @angerman @Bodigrim