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

What should we report when there are duplicate imports? #10509

Closed
philderbeast opened this issue Nov 3, 2024 · 4 comments · Fixed by #10546
Closed

What should we report when there are duplicate imports? #10509

philderbeast opened this issue Nov 3, 2024 · 4 comments · Fixed by #10546

Comments

@philderbeast
Copy link
Collaborator

What should we report when there are duplicate imports?

Is it acceptable that there are duplicates reported for configuration, such as the 5 times for - yops/yops-9.config with the project added in #10508?

$ cabal run cabal-install:exe:cabal -- build --dry-run \
  --project-file=cabal-testsuite/PackageTests/ConditionalAndImport/yops-0.project
Warning: this is a debug build of cabal-install with assertions enabled.
When using configuration from:
  - yops-0.project
  - yops-2.config
  - yops-4.config
  - yops-4.config
  - yops-6.config
  - yops-6.config
  - yops-6.config
  - yops-8.config
  - yops-8.config
  - yops-8.config
  - yops-8.config
  - yops/yops-1.config
  - yops/yops-3.config
  - yops/yops-3.config
  - yops/yops-5.config
  - yops/yops-5.config
  - yops/yops-5.config
  - yops/yops-7.config
  - yops/yops-7.config
  - yops/yops-7.config
  - yops/yops-7.config
  - yops/yops-9.config
  - yops/yops-9.config
  - yops/yops-9.config
  - yops/yops-9.config
  - yops/yops-9.config
The following errors occurred:
  - The package directory '.' does not contain any .cabal file.

#9933 would reject duplicate imports.

@geekosaur
Copy link
Collaborator

geekosaur commented Nov 3, 2024

As a C programmer, I'm somewhat inclined to take the C #include stance: multiple imports allowed, but imports must be idempotent. But that just kicks the can up the road, because at present I don't think there's a way to make a project file idempotent. It also could be argued that this is a reaction to how cpp works, and something more/differently principled is more appropriate.

@philderbeast
Copy link
Collaborator Author

We don't want anything like include guards or #pragma once, do we?

@geekosaur
Copy link
Collaborator

no, I think the more Haskelly way is ensuring that importing multiple times can't change the outcome regardless of its contents, and otherwise not worrying about it. If I import two project files each of which import the same other project file, it should behave just like that third file had only been imported once. (I think this is already true?)

The other side of this, which is why I go toward the C route, is that every include/import is self-contained: I don't need to require the developer to "just know" that something else must be imported first for it to work. This requires idempotence as a consequence, since two "self-contained" imports may pull in the same "dependency".

@philderbeast
Copy link
Collaborator Author

If we were to separate the reporting of duplicates (as warnings) from the "When using configuration from" message then the output could be trimmed to:

$ cabal run cabal-install:exe:cabal -- build --dry-run \
  --project-file=cabal-testsuite/PackageTests/ConditionalAndImport/yops-0.project
Warning: this is a debug build of cabal-install with assertions enabled.
When using configuration from:
  - yops-0.project
  - yops-2.config
  - yops-4.config
- - yops-4.config
  - yops-6.config
- - yops-6.config
- - yops-6.config
  - yops-8.config
- - yops-8.config
- - yops-8.config
- - yops-8.config
  - yops/yops-1.config
  - yops/yops-3.config
- - yops/yops-3.config
  - yops/yops-5.config
- - yops/yops-5.config
- - yops/yops-5.config
  - yops/yops-7.config
- - yops/yops-7.config
- - yops/yops-7.config
- - yops/yops-7.config
  - yops/yops-9.config
- - yops/yops-9.config
- - yops/yops-9.config
- - yops/yops-9.config
- - yops/yops-9.config
The following errors occurred:
  - The package directory '.' does not contain any .cabal file.

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

Successfully merging a pull request may close this issue.

2 participants