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

Duplicate module name when importing transitive dependencies #2929

Closed
GeorgSchneider opened this issue Sep 17, 2019 · 1 comment · Fixed by #2978
Closed

Duplicate module name when importing transitive dependencies #2929

GeorgSchneider opened this issue Sep 17, 2019 · 1 comment · Fixed by #2978
Assignees
Labels
language/now Language team priority NOW language Language team work

Comments

@GeorgSchneider
Copy link
Contributor

When I daml build the project in auth.zip I get:

File:     Library.daml
Range:    1:0-100001:0
Source:   compiler
Severity: DsError
Message: 
  Data.NameMap.fromList: duplicate name ModuleName {unModuleName = ["Fact","Claim"]}
  CallStack (from HasCallStack):
  error, called at libs-haskell/da-hs-base/src/Data/NameMap.hs:190:13 in
  libs-haskellZSda-hs-baseZSda-hs-base:Data.NameMap

Fact.Claim is a dependency of Rule.Application, and it is also imported explicitly in Library.daml. I'd expect the compiler to recognise that these are the same module. Note that in the IDE everything works fine, it's just daml build that complains.

@GeorgSchneider GeorgSchneider added the language Language team work label Sep 17, 2019
@cocreature cocreature self-assigned this Sep 17, 2019
@cocreature cocreature added the language/now Language team priority NOW label Sep 17, 2019
@cocreature
Copy link
Contributor

The issue appears to be that we end up with ./Fact/Claim.hs and Fact/Claim.hs which we treat as distinct files. Not quite sure where the culprit is.

cocreature added a commit to haskell/ghcide that referenced this issue Sep 23, 2019
This only matters for the DAML codebase (so I’ll add a test on that
side) where we use relative paths:

Previously, we would produce the include dir "." for moduleImportPath
"./A.daml"
and "" for moduleImportPath "./A/B.daml". This resulted in us ending
up with ./A.daml and A.daml in the Shake graph which resulted in
issues like digital-asset/daml#2929.

We should move this logic completely over to the DAML repo at some
point but I’ll leave that for a separate PR.
cocreature added a commit that referenced this issue Sep 23, 2019
See haskell/ghcide#114 for the actual
fix.
This PR just bumps ghcide and adds a regression test. I’ll change the
revision before merging, I just want to test CI for now.

fixes #2929
cocreature added a commit that referenced this issue Sep 23, 2019
See haskell/ghcide#114 for the actual
fix.
This PR just bumps ghcide and adds a regression test. I’ll change the
revision before merging, I just want to test CI for now.

fixes #2929
cocreature added a commit to haskell/ghcide that referenced this issue Sep 23, 2019
This only matters for the DAML codebase (so I’ll add a test on that
side) where we use relative paths:

Previously, we would produce the include dir "." for moduleImportPath
"./A.daml"
and "" for moduleImportPath "./A/B.daml". This resulted in us ending
up with ./A.daml and A.daml in the Shake graph which resulted in
issues like digital-asset/daml#2929.

We should move this logic completely over to the DAML repo at some
point but I’ll leave that for a separate PR.
cocreature added a commit that referenced this issue Sep 23, 2019
See haskell/ghcide#114 for the actual
fix.
This PR just bumps ghcide and adds a regression test. I’ll change the
revision before merging, I just want to test CI for now.

fixes #2929
@mergify mergify bot closed this as completed in #2978 Sep 23, 2019
mergify bot pushed a commit that referenced this issue Sep 23, 2019
* Use a consistant include dir for cwd

See haskell/ghcide#114 for the actual
fix.
This PR just bumps ghcide and adds a regression test. I’ll change the
revision before merging, I just want to test CI for now.

fixes #2929

* Switch to proper ghcide revision

* writeIfacesAndHie no longer exists

* Add changelog entry

* Maybe I should try to compile code before committing but I don’t want to

* Fix ghcide exe
pepeiborra pushed a commit to pepeiborra/ide that referenced this issue Dec 29, 2020
This only matters for the DAML codebase (so I’ll add a test on that
side) where we use relative paths:

Previously, we would produce the include dir "." for moduleImportPath
"./A.daml"
and "" for moduleImportPath "./A/B.daml". This resulted in us ending
up with ./A.daml and A.daml in the Shake graph which resulted in
issues like digital-asset/daml#2929.

We should move this logic completely over to the DAML repo at some
point but I’ll leave that for a separate PR.
pepeiborra pushed a commit to pepeiborra/ide that referenced this issue Dec 29, 2020
This only matters for the DAML codebase (so I’ll add a test on that
side) where we use relative paths:

Previously, we would produce the include dir "." for moduleImportPath
"./A.daml"
and "" for moduleImportPath "./A/B.daml". This resulted in us ending
up with ./A.daml and A.daml in the Shake graph which resulted in
issues like digital-asset/daml#2929.

We should move this logic completely over to the DAML repo at some
point but I’ll leave that for a separate PR.
pepeiborra pushed a commit to pepeiborra/ide that referenced this issue Dec 29, 2020
This only matters for the DAML codebase (so I’ll add a test on that
side) where we use relative paths:

Previously, we would produce the include dir "." for moduleImportPath
"./A.daml"
and "" for moduleImportPath "./A/B.daml". This resulted in us ending
up with ./A.daml and A.daml in the Shake graph which resulted in
issues like digital-asset/daml#2929.

We should move this logic completely over to the DAML repo at some
point but I’ll leave that for a separate PR.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
language/now Language team priority NOW language Language team work
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants