Skip to content

Fix computation of transitive dependencies - #1669

Merged
mergify[bot] merged 9 commits into
masterfrom
fd/narrowing-fixes
Dec 22, 2021
Merged

Fix computation of transitive dependencies#1669
mergify[bot] merged 9 commits into
masterfrom
fd/narrowing-fixes

Conversation

@facundominguez

Copy link
Copy Markdown
Member

The most important fix here is exposing interface files coming from transitive cross-library dependencies. Otherwise, the strengthened tests would fail.

Comment thread haskell/experimental/private/module.bzl Outdated

def _collect_narrowed_deps_module_files(ctx_label, per_module_transitive_files, dep):
direct_cross_library_deps = dep[HaskellModuleInfo].direct_cross_library_deps
transitive_cross_library_dep_labels = dep[HaskellModuleInfo].transitive_cross_library_dep_labels.to_list()

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This flattening could be problematic because the depset could be large. I'm still thinking how to avoid it.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I reverted this and created a better solution in af97995.

@aherrmann aherrmann left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thank you!

One small comment.

Comment thread haskell/private/haskell_impl.bzl Outdated
Co-authored-by: Andreas Herrmann <andreas.herrmann@tweag.io>
@facundominguez facundominguez added the merge-queue merge on green CI label Dec 22, 2021
@mergify
mergify Bot merged commit 16ac181 into master Dec 22, 2021
@mergify
mergify Bot deleted the fd/narrowing-fixes branch December 22, 2021 14:56
@mergify mergify Bot removed the merge-queue merge on green CI label Dec 22, 2021
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