Skip to content
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

Show import tree provenance (backport #9578) #9877

Merged
merged 1 commit into from
Apr 9, 2024
Merged

Conversation

mergify[bot]
Copy link
Contributor

@mergify mergify bot commented Apr 9, 2024

Fixes #9562. This adds the path in the project import tree with the constraint conflict just below the solver's rejecting message (sample taken from the change log entry).

$ cabal build all --dry-run
...
[__1] next goal: hashable
[__1] rejecting: hashable-1.4.3.0
      (<ROOT>/1-web-import-constraints.project requires ==1.4.2.0)
[__1] rejecting: hashable-1.4.2.0
      (https://www.stackage.org/nightly-2023-12-07/cabal.config requires ==1.4.3.0)
        imported by: <ROOT>/1-web-import-constraints.project

Important

The way imports worked changed between cabal-install-3.8 and cabal-install-3.10. With the earlier version imports were anchored to the project root (relative to a fixed location) but this was changed to make them relative to their importer. I had to pay close attention to the check for seen imports, something I added tests for. These tests merged already with #9665 and #9680. I've also added verbose logging at the info level so we can see the import we're processing, the seen imports and the import we're fetching.

Because relative imports may contain .. and the only way to resolve this is to use canonicalizePath I had to take an extra dependency on directory in cabal-install-solver. I also needed network-uri to check for URIs and avoid messing with them. I assume that once we import a URI config then imports stop there. Put another way, something imported via https://... doesn't itself import another config. Those extra dependencies we should be able to remove when the suggested change of @andreabedini lands, #9642.

If testing manually, I suggest trying cabal build --dry-run with a simple cabal.project, a project with imports and a bare package with no project. To see the change, a solver rejection will need to be triggered by introducing a version constraint.

Note

I'd tried to use a tree-like layout but settled on a list instead after review.

I used +-- ASCII chars to mark tree nodes (and not the same ones as tree does). Can we use the nicer unicode chars?

$ tree -P 'cabal.project*' --prune -L 1 --charset ascii
.
|-- cabal.project
|-- cabal.project.buildinfo
|-- cabal.project.coverage
|-- cabal.project.doctest
|-- cabal.project.latest-ghc
|-- cabal.project.libonly
|-- cabal.project.meta
|-- cabal.project.release
|-- cabal.project.validate
|-- cabal.project.validate.libonly
`-- cabal.project.weeder

$ tree -P 'cabal.project*' --prune -L 1 --charset utf8
.
├── cabal.project
├── cabal.project.buildinfo
├── cabal.project.coverage
├── cabal.project.doctest
├── cabal.project.latest-ghc
├── cabal.project.libonly
├── cabal.project.meta
├── cabal.project.release
├── cabal.project.validate
├── cabal.project.validate.libonly
└── cabal.project.weeder
```<hr>This is an automatic backport of pull request #9578 done by [Mergify](https://mergify.com).

@mergify mergify bot added the conflicts label Apr 9, 2024
Copy link
Contributor Author

mergify bot commented Apr 9, 2024

Cherry-pick of f8cd563 has failed:

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

You are currently cherry-picking commit f8cd56341.
  (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-solver/cabal-install-solver.cabal
	modified:   cabal-install-solver/src/Distribution/Solver/Modular/Message.hs
	modified:   cabal-install-solver/src/Distribution/Solver/Types/ConstraintSource.hs
	new file:   cabal-install-solver/src/Distribution/Solver/Types/ProjectConfigPath.hs
	modified:   cabal-install/src/Distribution/Client/ProjectConfig.hs
	modified:   cabal-install/src/Distribution/Client/ProjectConfig/Types.hs
	modified:   cabal-install/src/Distribution/Client/ProjectPlanning.hs
	modified:   cabal-install/src/Distribution/Client/ScriptUtils.hs
	modified:   cabal-install/tests/UnitTests/Distribution/Client/ProjectConfig.hs
	modified:   cabal-install/tests/UnitTests/Distribution/Client/TreeDiffInstances.hs
	modified:   cabal-testsuite/PackageTests/ConditionalAndImport/cabal.out
	modified:   cabal-testsuite/PackageTests/ConditionalAndImport/cabal.test.hs
	new file:   cabal-testsuite/PackageTests/ConditionalAndImport/oops-0.project
	new file:   cabal-testsuite/PackageTests/ConditionalAndImport/oops-2.config
	new file:   cabal-testsuite/PackageTests/ConditionalAndImport/oops-4.config
	new file:   cabal-testsuite/PackageTests/ConditionalAndImport/oops-6.config
	new file:   cabal-testsuite/PackageTests/ConditionalAndImport/oops-8.config
	new file:   cabal-testsuite/PackageTests/ConditionalAndImport/oops/oops-1.config
	new file:   cabal-testsuite/PackageTests/ConditionalAndImport/oops/oops-3.config
	new file:   cabal-testsuite/PackageTests/ConditionalAndImport/oops/oops-5.config
	new file:   cabal-testsuite/PackageTests/ConditionalAndImport/oops/oops-7.config
	new file:   cabal-testsuite/PackageTests/ConditionalAndImport/oops/oops-9.config
	new file:   cabal-testsuite/PackageTests/ConditionalAndImport/oops/oops.cabal
	new file:   cabal-testsuite/PackageTests/ConditionalAndImport/repo/hashable-1.4.2.0/hashable.cabal
	new file:   cabal-testsuite/PackageTests/ConditionalAndImport/repo/hashable-1.4.3.0/hashable.cabal
	modified:   cabal-testsuite/PackageTests/ConditionalAndImport/same-filename/noncyclical-same-filename-b.config
	modified:   cabal-testsuite/PackageTests/VersionPriority/0-local.out
	modified:   cabal-testsuite/PackageTests/VersionPriority/1-local.out
	modified:   cabal-testsuite/PackageTests/VersionPriority/1-web.out
	modified:   cabal-testsuite/PackageTests/VersionPriority/2-local.out
	modified:   cabal-testsuite/PackageTests/VersionPriority/2-web.out
	modified:   cabal-testsuite/PackageTests/VersionPriority/3-web.out
	modified:   cabal-testsuite/PackageTests/VersionPriority/repo/hashable-1.4.2.0/hashable.cabal
	modified:   cabal-testsuite/PackageTests/VersionPriority/repo/hashable-1.4.3.0/hashable.cabal
	new file:   changelog.d/issue-9578

Unmerged paths:
  (use "git add <file>..." to mark resolution)
	both modified:   cabal-install/src/Distribution/Client/ProjectConfig/Legacy.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

@mergify mergify bot mentioned this pull request Apr 9, 2024
5 tasks
@mergify mergify bot added the backport label Apr 9, 2024
@ffaf1 ffaf1 added squash+merge me Tell Mergify Bot to squash-merge and removed conflicts labels Apr 9, 2024
@geekosaur geekosaur force-pushed the mergify/bp/3.12/pr-9578 branch from 58d0a56 to 1ac56af Compare April 9, 2024 19:06
@geekosaur
Copy link
Collaborator

geekosaur commented Apr 9, 2024

Backports don't accept "squash+merge me" (https://github.com/haskell/cabal/blob/master/.github/mergify.yml#L59-L72). I was hoping the "fixup!" prefixes would get the additional commits squashed (https://git-scm.com/docs/git-commit#Documentation/git-commit.txt---fixupamendrewordltcommitgt) but I suppose that doesn't work here.

@geekosaur geekosaur force-pushed the mergify/bp/3.12/pr-9578 branch from 1ac56af to dece5e8 Compare April 9, 2024 21:54
With this change to the solver message rendering, I also fix some bugs around project imports, adding tests for those cases. Reviewers asked that the Y-shaped import checks (using IORef) be made on a separate pull request. Removing those lead to cascading deletions.

- Regenerate expected .out files
- Show tree provenance of import constraint
- Add trimmed down PackageTests/VersionPriority
- Add changelog entry
- Use NonEmpty
- Fix check for cyclical import
- Use primes for next iteration
- Remove unused LANGUAGE pragmas
- Rename to projectConfigPathRoot
- Docs for ProjectConfigPath and showProjectConfigPath
- Renaming
- Add cyclical import tests with 1 and 2 hops in cycle
- Use full path for cyclical error message
- Expected output has project with full project path
- Add fullPath local function
- Project directory as FilePath, not Maybe FilePath
- Use (_, projectFileName) binding splitFileName
- Need full path to project parsing legacy
- Inline seenImports conversion
- Add cyclical checks with same file names and hops
- Add noncyclical tests that hop over folders
- Add a project testing skipping in and out of a folder
- Update expectations of cyclical tests
- Use canonicalizePath for collapsing .. when possible
- Capture trace for later
- Add module for ProjectConfigPath
- Move functions for ProjectConfigPath to its module
- Fetch URI is not prefixed with ./https://etc
- Document normaliseConfigPath
- Add doctests for normaliseConfigPath
- Add doctest of canonicalizeConfigPath
- Show an example of canonical paths
- Use importer and importee in canonicalizeConfigPaths
- Add logging
@geekosaur geekosaur force-pushed the mergify/bp/3.12/pr-9578 branch from dece5e8 to af9d5df Compare April 9, 2024 21:55
@geekosaur geekosaur added merge me Tell Mergify Bot to merge and removed squash+merge me Tell Mergify Bot to squash-merge labels Apr 9, 2024
@mergify mergify bot merged commit bd0d321 into 3.12 Apr 9, 2024
54 checks passed
@mergify mergify bot deleted the mergify/bp/3.12/pr-9578 branch April 9, 2024 23:56
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.

3 participants