Detect duplicate non-cyclical project imports#9933
Detect duplicate non-cyclical project imports#9933philderbeast wants to merge 3 commits intohaskell:masterfrom
Conversation
c618bef to
45e6f73
Compare
|
@TeofilC I'm inviting your review as cabal could perhaps be enhanced to output a list of files imported from a project in the future for use by other tools, input-output-hk/haskell.nix#2144 (comment). |
45e6f73 to
62a67ca
Compare
|
@alt-romes: could you maybe have a look? |
62a67ca to
1d189d5
Compare
|
Looks like you may need to rebase on |
c5ab2b7 to
b708112
Compare
|
Thanks @Kleidukos for pointing out the stray changelog entry. Rebasing didn't clear it so I deleted it and then squashed related commits. |
50ebe1d to
1d1b3e7
Compare
|
@GrayJay do you have time to review this? |
c423f02 to
06065fb
Compare
908ce01 to
6d1a639
Compare
6d1a639 to
0b668c0
Compare
|
This change was split from #9578. When I was reviewing #9578, I disagreed with the duplicate project imports error because it wasn't clear to me that it should be an error (I don't think that duplicate non-cyclic imports cause build problems, and it is possible that existing projects are using them intentionally.), and it didn't seem worth the complexity of an IORef. This new PR is mostly outside of the solver, so I'll defer to others who are more familiar with project files. |
0b668c0 to
cfd9ca8
Compare
cfd9ca8 to
3461f3b
Compare
- A test for detecting when the same config is imported via many different paths - Error on duplicate imports - Do the filtering in duplicateImportMsg - Use duplicateImportMsg for cycles too - Add haddocks to IORef parameter - Add changelog entry - Use ordNub instead of nub - Use NubList - Share implement of duplicate and cyclical messages - Update expectation for non-cyclical duplicate import
3461f3b to
505a04a
Compare
|
@philderbeast Great PR, I like that we are tackling this behaviour. I will join @GrayJay in that introducing a breaking change in |
@Kleidukos, I've read the wiki and it is unclear to me what the following means:
Are we talking the |
|
@philderbeast Sorry for the late answer! Indeed this can be confusing. We mean major versions, so X.Y (epoch.major). At a rate of 2 major releases per year, this is something like 2 years maximum. |
|
Anything I can do to help review/revamp this and so save this fine PR? |
|
A very kind ping! |
|
Closing in favour of #10933 which is much the same but issues a warning instead of an error. |
A follow on from #9578. Uses an
IORefto better report on duplicates (including cycles) and to detect duplicates that are not cycles. This was originally included in #9578 but punted at review, #9578 (comment).