Fix handling of packages with clashing names #403
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mboes
force-pushed
the
package-name-clash
branch
from
4ec0a25 to
b01583f
Compare
|
Incidentally, this bug could have been avoided had we factored things out a bit more. Currently, |
This was referenced Aug 17, 2018
guibou
approved these changes
Aug 18, 2018
| @@ -124,6 +124,13 @@ def link_binary( | |||
|
|
|||
| args.add(["-o", compile_output.path, dummy_static_lib.path]) | |||
|
|
|||
| # In compile.bzl, we pass this just before all -package-id | |||
There was a problem hiding this comment.
This comment should be copy pasted in front of all similar lines. This will motivate us to factorise and document more.
There was a problem hiding this comment.
Added a reference. Duplication in comments is worse than in code, because comments are not checked.
Package names should not need to be unique. After all, that's the very reason for the package-name / package-id distinction. It turns out that the root cause for the original problem was very subtle, but very simple to fix. The problem was that while we were passing `-hide-all-packages` to GHC during compilation, we were not doing so during linking. This flag makes sense, and indeed not passing it causes bizarre `module not found` errors on import. It turns out that not passing it at link time also causes the bizarre link errors we were seeing (which only show up when two packages share the same name). I can't explain why `-hide-all-packages`, or lack thereof, has this effect. But I do know that using the same flags during linking as during compilation is the right thing to do. With this one line change to the source code, we can continue to have arbitrary non-unique package names, relying on package id's generated by rules_haskell internally to be unique. Closes #395. Closes #396.
mboes
force-pushed
the
package-name-clash
branch
from
b01583f to
44eab16
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In #396, @guibou proposes to resolve #395 by making not just package
id's unique but also package names. That's a really big hammer, indeed
one that would cause collateral damage. Hazel would need to be
modified, because CPP macros generated by GHC would no longer be using
the names that upstream code pulled from Hackage expects. Also, the
names in Haddock would look worse.
Package names should not need to be unique. After all, that's the very
reason for the package-name / package-id distinction. It turns out
that the root cause for the original problem was very subtle, but very
simple to fix. The problem was that while we were passing
-hide-all-packagesto GHC during compilation, we were not doing soduring linking. This flag makes sense, and indeed not passing it
causes bizarre
module not founderrors on import. It turns out thatnot passing it at link time also causes the bizarre link errors we
were seeing (which only show up when two packages share the same
name).
I can't explain why
-hide-all-packages, or lack thereof, has thiseffect. But I do know that using the same flags during linking as
during compilation is the right thing to do. With this one line change
to the source code, we can continue to have arbitrary non-unique
package names, relying on package id's generated by rules_haskell
internally to be unique.
cc @guibou @Profpatsch @judah @lunaris
Closes #395.
Closes #396.