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

Fix $(location) references to data attribute #990

Merged
merged 3 commits into from
Jul 11, 2019
Merged

Conversation

aherrmann
Copy link
Member

The following kind of $(location) expansion failed with an "expression is not a declared prerequisite of this rule" error.

haskell_library(
    ...
    data = ["//some/genrule:target"],
    compiler_flags = ["-DSOME_DEF=$(rootpath //some/genrule:target)"],
)

The reason is that since #959 expand_location was called multiple times, iterating over all relevant attributes. However, expand_location fails immediately if any reference cannot be resolved. This issue didn't come up so far, because certain attributes (e.g. tools) are always included implicitly. However, data is not.

This PR changes the code to instead deduplicate the list of targets and call expand_location only once. This PR also modifies //tests/binary-with-data to serve as a regression test for this issue.

@mboes mboes added the merge-queue merge on green CI label Jul 11, 2019
@aherrmann
Copy link
Member Author

CI was failing on Windows and Linux bindist because //tools/runfiles depended on @stackage. We only need core-packages, so pulling those in via haskell_toolchain_library instead should fix the issue.

All required libraries are part of GHC's core-packages. Therefore, it's
not necessary to pull them in via `@stackage`. Using
`haskell_toolchain_library` instead allows to depend on
`//tools/runfiles` on Linux bindist and Windows as well.
@mergify mergify bot merged commit c1d2c8c into master Jul 11, 2019
@mergify mergify bot deleted the expand_location_2 branch July 11, 2019 15:57
@mergify mergify bot removed the merge-queue merge on green CI label Jul 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants