Breaking change in importing packages?

[philderbeast]

Describe the bug
I installed cabal-install from source and noticed a breaking change. I took the test from #8686 and created a similar test that shows the problem.

$ pwd
/.../cabal/cabal-testsuite/ProjectTests/Import/PackageGroups
$ tree
.
├── cabal-configs
│   ├── pkg-groups
│   │   └── dep1-dep2.config
│   └── pkgs.config
├── cabal.out
├── cabal.project
├── cabal.test.hs
├── dep1
│   ├── dep1.cabal
│   └── Dep1.hs
├── dep2
│   ├── dep2.cabal
│   └── Dep2.hs
├── Lib.hs
└── main.cabal

Stepping through the indirection of imports:

$ cat cabal.project
import: cabal-configs/pkgs.config

packages:
    ./

$ cat cabal-configs/pkgs.config 
import: cabal-configs/pkg-groups/dep1-dep2.config

$ cat cabal-configs/pkg-groups/dep1-dep2.config 
packages:
    ./dep1/
  , ./dep2/
$ 

In the reproduction, if I import the packages in 1-hop then we're good but in 2-hops we break (when we didn't before).

-- Importing packages in one hop works. Uncomment the line below to see that.
-- import: cabal-configs/pkg-groups/dep1-dep2.config

-- Importing the same packages in two hops works with cabal-install-3.8.1.0 but fails now.
import: cabal-configs/pkgs.config

packages:
    ./

To Reproduce
First off with v3.8.1.0 (this works):

$ ~/.ghcup/bin/cabal clean --project-file=./cabal.project
$ ~/.ghcup/bin/cabal build all --dry-run --project-file=./cabal.project
Resolving dependencies...
Build profile: -w ghc-9.2.5 -O1
In order, the following would be built (use -v for more details):
 - dep1-0.1 (lib) (first run)
 - dep2-0.1 (lib) (first run)
 - main-0.1 (lib) (first run)
$ ~/.ghcup/bin/cabal --version
cabal-install version 3.8.1.0
compiled using version 3.8.1.0 of the Cabal library

Then with v3.9.0.0 (this fails):

$ ~/.cabal/bin/cabal clean
$ ~/.cabal/bin/cabal build all --dry-run --project-file=./cabal.project
cabal-configs/cabal-configs/pkg-groups/dep1-dep2.config:
  withBinaryFile: does not exist (No such file or directory)
$ ~/.cabal/bin/cabal --version
cabal-install version 3.9
compiled using version 3.9.0.0 of the Cabal library 

From the error report, cabal has repeated the first part of the path.

- cabal-configs/pkg-groups/dep1-dep2.config
+ cabal-configs/cabal-configs/pkg-groups/dep1-dep2.config

Expected behavior
I like Dhall's stance on imports. Can we do something like that?

"The import resolution phase replaces all imports with the expression located at that import, transitively resolving imports within the imported expression if necessary.
"SOURCE: Import Resolution Judgment

System information

• Ubuntu 22.04.1 LTS

$ cabal --version
cabal-install version 3.8.1.0
compiled using version 3.8.1.0 of the Cabal library 
$ ghc --version
The Glorious Glasgow Haskell Compilation System, version 9.2.5

[philderbeast]

I've included the reproduction as a pull request.

[philderbeast]

Here's the code change that changes the behaviour:

0db95ca#diff-8280f05bf3effdec103f73157f07a93ca761fa0b6bf1fbe25c0dcf56fc6abfd1L227-R228

-         Nothing -> BS.readFile pci
+         Nothing -> BS.readFile $
+           if isAbsolute pci then pci else takeDirectory source </> pci

[gbaz]

I don't think there's a semantics that resolves the issue resolved by #8686 and also does what is asked here, and I think the semantics there make the most sense.

In the given example, cabal-configs/pkgs.config doesn't make sense on its own -- it refers to its own directory, and only makes sense when invoked from the parent directory. Under the new behavior it will always resolve imports relative to its own directory -- in a sense, the accidental thing that worked before is that if you invoked it specifically from one directory up, it would work. But of course the thing that usually works is invoking things from their own directory. So before, in a sense, relative paths had no fixed semantics, and now we've pinned down their fixed semantics to be the most natural one, which also coincides with how most people expected them to work.

I propose closing this.

[philderbeast]

In the test for #8686 packages are given as paths to cabal files but packages can also be given as paths to folders. Should these continue to be anchored to the root of the project if using a ./etc path or are they now meant to be relative too?

cabal/cabal-testsuite/PackageTests/RelativePathProjectImports/cabal.project

Lines 1 to 6 in 90a2f33

	import: ./dep/cabal.project
	packages:
	main.cabal
	dep/dep.cabal
	dep2/dep2.cabal

[philderbeast]

Could we say more in the docs about where package directory paths are relative to (the file they're in or the project root)?

Package locations can take the following forms:

1. They can specify a Cabal file, or a directory containing a Cabal file
   SOURCE: project packages

[gbaz]

I think the project root relative/absolute issue was addressed in #8454

however i guess the issue is if an included project file should have its packages relative to its own root or to the project root. 🤔

[philderbeast]

however i guess the issue is if an included project file should have its packages relative to its own root or to the project root

Indeed, this seems inconsistent when the imported file is relative one way but the package paths are relative another way.

With #8454 all tests have packages: . in the project itself.

[jberryman]

can someone summarize the migration from 3.8 to 3.10 for us please?

We have chains of cabal.project files which import each other A imports B imports C, we sometimes invoke with --project-file subdir/A sometimes with --project-file subdir/B etc

the way paths in import are meant to work is not documented

[gbaz]

This needs to be thought through and documented carefully.

That said, I think the difference is as follows:

3.8: Project imports are all relative to whatever directory cabal is invoked from
3.10: Project imports in any given file should be relative to that specific file.

The remaining puzzling bit is not about package imports, but the way that project files resolve relative paths of packages they list, which has not changed over the course of the introduction of project imports, but now that we are looking closely at these semantics, we may also want to clean up.

[jberryman]

3.10: Project imports in any given file should be relative to that specific file.

No that's not right. In my example if you do --project-file subdir/A the import of B needs to be file-relative, while import of C needs to be relative to top of project (or maybe from where we invoked cabal, who knows)

I guess moving everything to the top-level is the only option for us; there isn't even a way to do this with 3.10 pinned

[gbaz]

Ah, ok, so what I wrote is the desired semantics for 3.10 and you're saying they still aren't what's implemented, because everything appears to be relative to the initial project file used, rather than each successive project file?

[jberryman]

No, an import-ed project file's import needs to use paths relative to the top-level afaict. Just try an imported chain of files as I describe here #8795 (comment)

[philderbeast]

I only ever use --project-file=cabal.some-thing.project. In these circumstances, going from cabal-3.8 to cabal.3.10, relativity went from being project-relative for everything to import-relative for everything but packages. Packages (when specified by paths to sub-directories) are always relative to the project (when that is at the root of the tree).

[philderbeast]

Project imports are all relative to whatever directory cabal is invoked from

This comment prompted me to try invoking cabal from somewhere other than the project root directory. With cabal-3.8 if I was in some random subdirectory somewhere deep in a project's source tree, invoking cabal build acme-package --project-file=cabal.upgrade.project (when cabal.upgrade.project is in the root) would work the same as if I was in the root. That was a nice "feature", that the same command would work wherever I am in a project tree.

[gbaz]

Ok let me try to explain what I understand is now the case. I created the following structure.

cabal.project containing import: depa/a.project
depa/a.project containing import: ../depb/b.project
depb/b.project containing packages: foo.cabal

In cabal 3.10, this works when calling cabal build from the top level. So, imports are resolved relative to cabal project files.
In cabal 3.8 this does not work when calling cabal build from the top level, but does work if the contents of a.project are instead import: depb/b.project, so imports are resolved relative to the cwd from which cabal is invoked.

So far, this is what I described prior, and I think the new behavior is superior.

Now, in both versions, foo.cabal will always resolve based on the location of the initial cabal.project file (at the root of the import tree), and I believe this is a bug, and propose it resolve relative to the project file containing the packages stanza.

Finally, in both versions, invoking cabal build in any subdirectory does not work -- it appears that cabal constructs the relative path fragment it uses to do an import correctly, but then resolves it against the cwd. Oddly, it does work if the import from cabal.project is another file in the same directory, but not otherwise. I think this is also a bug, and propose that it work the same regardless of subdirectory from which cabal is invoked.

Does this description of behavior and the two bugs to fix sound correct, or are there other changes that people would like to make (or not make)?

(If #8454 were implemented, we might have some workarounds by setting --project-dir explicitly as well, but sadly no such luck)

Edit: thanks for bearing with us on this. Imports were largely motivated by bringing in common stanzas, constraints, etc or picking different flags based on oses or compiler versions. It honestly didn't occur to me that people would use them to structure trees of packages, and consequently the design process really dropped the ball on working out carefully the relative path semantics, especially with regards to the interaction with package stanzas.