Extended project files (conditionals and imports)

[gbaz]

Adds conditionals and imports to cabal.project files. The basic idea is we introduce a ProjectSkeleton that reuses the conditional architecture from cabal files (CondTree) to hold a tree of project data, resolving imports as we go. Then the project skeletons need to be instantiated with concrete information for OS, compiler, platform before use.

Additional checks (not yet added) should prevent setting of compiler info within conditionals (since we can have conditionals depend on this info). Additionally, we should enforce that cabal.project.local files cannot have conditionals or imports, as we don't have to want to write machine generation to preserve that structure for now (necessary for cabal reconfigure).

---

Please include the following checklist in your PR:

• Any changes that could be relevant to users have been recorded in the changelog.
• The documentation has been updated, if necessary.

Please also shortly describe how you tested your change. Bonus points for added tests!

[phadej]

we should enforce that cabal.project.local files cannot have conditionals or imports

I'd rather make reconfigure fail in that case. (I never use cabal configure or cabal reconfigure, but I do write my .local files manually for various in development needs).

[gbaz]

This last pr finishes up the core logic -- now it needs some testing and debugging.

[Mikolaj]

@Mergifyio rebase

[mergify]

rebase

❌ Base branch update has failed

Git reported the following error:

Rebasing (1/11)
Rebasing (2/11)
Auto-merging cabal-install/src/Distribution/Client/ProjectConfig.hs
CONFLICT (content): Merge conflict in cabal-install/src/Distribution/Client/ProjectConfig.hs
Auto-merging cabal-install/src/Distribution/Client/ProjectConfig/Legacy.hs
Auto-merging cabal-install/src/Distribution/Client/ProjectPlanning.hs
error: could not apply 2ccd991a... first compiling pass
hint: Resolve all conflicts manually, mark them as resolved with
hint: "git add/rm <conflicted_files>", then run "git rebase --continue".
hint: You can instead skip this commit: run "git rebase --skip".
hint: To abort and get back to the state before "git rebase", run "git rebase --abort".
Could not apply 2ccd991a... first compiling pass

err-code: BDECE

[jneira]

Maybe i am a little bit paranoic and probably the code already take it in account but maybe testing an indirect cycle would be good: cabal-root.project -> cabal-child.project -> cabal-root.project

[jneira]

Also i miss a simple positive test over imports and conditionals, to exercise and demosntrate the pr main goal.

[hasufell]

#7783 (comment)

[jneira]

Also i miss a simple positive test over imports and conditionals, to exercise and demosntrate the pr main goal.

oops, the same test is testing both things, in addition to cycles, sorry, i was assuming they were to going placed in two separate tests (not needed)

[jneira]

#7783 (comment)

afaics the test only checks a cabal.project cant import itself but no a cycle with more than one node (maybe it is not possible by design??)

[jneira]

just checked it locally and the cycle check works perfectly across several project files, but i can imagine a bug breaking that invariant while not allowing auto imports

[jneira]

and a test checking remote imports would not be bad as well, it could be in other pr as well

[gbaz]

i'm fine with a remote import test -- currently we have no tests relying on network connectivity so i didn't want to add one. but i think the cycle check test is fine as is -- indeed by design we check in a way that's robust to any cycle, so checking for 3-cycles vs 2-cycles isn't telling us anything special.

[jneira]

i'm fine with a remote import test -- currently we have no tests relying on network connectivity so i didn't want to add one.

i've just checked

packages: .
import: file://D:/ws/haskell/cabal/cabal.project

works. which is really nice. It would test part of the code path (not sure if a interesting enough part) and will not go out the file system, no?

[jneira]

This pr is amazing and it will make cabal.project dramatically more flexible.
One caveat i can think of it is you will not be able to parse cabal.project's with a simple grep.
Doing that is fragile indeed but you have a static set of possible sources of project config and this change will make it so fragile than it cant be used anymore. And there are script out there doing such greps. Adding "ifs" to a config "language" is not for free, neither from an user pov.

I agree is totally necessary and increase complexity and break such fragile scripts is justified.

But it think it would be good to merge cabal status asap after this, before the release if possible, to allow users other ways to query the final configuration of the project and gives them another, better, alternative.

[hasufell]

One caveat i can think of it is you will not be able to parse cabal.project's with a simple grep.

I don't know of any such script. That's what https://hackage.haskell.org/package/cabal-install-parsers-0.4.5/docs/Cabal-Project.html is for and it shall be extended to resolve the project configuration.

[jneira]

Just tested a possible corner case: conditions over the compiler, being the compiler one of the values you can change in the cabal.project itself

• Setting the compiler in a imported cabal.project inside a if throws an error: 👍

Cannot set compiler in a conditional clause of a cabal project file

• conditions over compiler version honours the compiler set in the same cabal.project indpendently of its position 👍

Such corner cases could be new tests but they could be added after merging this

[jneira]

In windows a remote import like:

packages: .

import: https://raw.githubusercontent.com/haskell/cabal/master/cabal.project

is failing for me with

D:\ws\haskell\issues\importifs\dist-newstyle\src\htt2033.project: renameFile:renamePath:MoveFileEx "\\\\?\\D:\\ws\\haskell\\issues\\importifs\\dist-newstyle\\src\\htt2033.project" Just "\\\\?\\D:\\ws\\haskell\\issues\\importifs\\dist-newstyle\\src\\https:XXraw.githubusercontent.comXhaskellXcabalXmasterXcabal.project":invalid argument (El nombre de archivo, el nombre de directorio o la sintaxis de la etiqueta del volumen no son correctos.)

cabal-install compiled with ghc-8.6.5 and executed also with ghc-8.6.5
it was predicted by @hasufell here so i guess it is matter to apply the suggestion?

[jneira]

One caveat i can think of it is you will not be able to parse cabal.project's with a simple grep.

I don't know of any such script. That's what https://hackage.haskell.org/package/cabal-install-parsers-0.4.5/docs/Cabal-Project.html is for and it shall be extended to resolve the project configuration.

"I've seen things you people wouldn't believe. Attack ships on fire...", etc etc :-P
But it was an example, see how ifs in .cabal file made the duplicate module checks harder and error prone recently

[jneira]

Additional checks (not yet added) should prevent setting of compiler info within conditionals (since we can have conditionals depend on this info). Additionally, we should enforce that cabal.project.local files cannot have conditionals or imports, as we don't have to want to write machine generation to preserve that structure for now (necessary for cabal reconfigure).

so the first check is done but just tested the second one is not, conditions and imports are allowed in cabal.project.local

[jneira]

And the last one, i promise, there is a minor caveat related with syntax error reporting. If you have a cabal.project:

packages: .
import: cabal.1.project

and a cabal.1.project

package: foo

the warning does not include the offending file but it does for the line number:

Warning: D:\ws\haskell\issues\importifs\cabal.project: Unrecognized field
'package' on line 4

but it does it better for other errors like a non existing package (maybe it should only mention the offending file and no all of them but still):

When using configuration(s) from D:\ws\haskell\issues\importifs\cabal.project, cabal.1.project, the following errors occurred:
The package location 'foo' does not exist.

[gbaz]

"so the first check is done but just tested the second one is not, conditions and imports are allowed in cabal.project.local"

I changed my mind on that. Instead we just error if attempting to update-reconfigure when there's conditionals. So they're allowed, but we just block the failing action: https://github.com/haskell/cabal/pull/7783/files#diff-50fda1f9b998a33220632ddccf47717d9c21b9992268697b40fdaf149bd2c700R139

[gbaz]

improved the error message a bit to list all files, and fixed the filename on windows thing (i think) and added one more test. I think this is ready for merge now and more tests can be added as desired?

[jneira]

improved the error message a bit to list all files, and fixed the filename on windows thing (i think) and added one more test. I think this is ready for merge now and more tests can be added as desired?

thanks for take in account review comments
agree about add more tests later, will open an issue about if you dont mind

[fgaz]

Maybe we should have a stackage example too?

[bapcyk]

So what is the right way to set with-compiler: conditionally?

[Kleidukos]

I personally would generate a local project file with this information.