Skip to content

Add binaries from haskell packages to PATH#12

Merged
avdv merged 7 commits intomercuryfrom
cb/haskell-bin-tools
Jun 14, 2024
Merged

Add binaries from haskell packages to PATH#12
avdv merged 7 commits intomercuryfrom
cb/haskell-bin-tools

Conversation

@avdv
Copy link
Copy Markdown

@avdv avdv commented Jun 11, 2024

  • Remove duplicate variable
  • Remove superfluous -package-db flags passed to ghc
  • Keep track of the path for each haskell package db
  • Add --nix-pkg argument to ghc_wrapper.py and generate_target_metadata.py
  • Pass --nix-pkg flag for each direct toolchain library dependency
  • Allow to declare dependencies for haskell sources manually

@avdv avdv requested a review from aherrmann June 11, 2024 15:50
@avdv avdv force-pushed the cb/haskell-bin-tools branch 3 times, most recently from abe75b1 to 6ebf50d Compare June 13, 2024 11:31
avdv added 3 commits June 13, 2024 13:32
The same flags are already part of `packages_info.packagedb_args`.
- add package `path` projection to `HaskellPackageDbTSet`
@avdv avdv force-pushed the cb/haskell-bin-tools branch from 6ebf50d to e587d49 Compare June 13, 2024 11:53
Copy link
Copy Markdown

@aherrmann aherrmann left a comment

Choose a reason for hiding this comment

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

Thank you, looks good! Just a few comments.

Comment thread decls/haskell_common.bzl Outdated
Comment on lines +28 to +29
Allows to declare dependencies for sources manually, in case the automatic
dependency mechanism is not able to detect them.
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The way it's phrased it sounds like it would override the automatic dependency detection. But, IIUC it extends the set of automatically discovered dependencies, correct?

Copy link
Copy Markdown
Author

@avdv avdv Jun 13, 2024

Choose a reason for hiding this comment

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

How about:

Allows to declare dependencies for sources manually, additionally to the dependencies automatically detected.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

That sounds good, thanks!

Comment thread haskell/compile.bzl Outdated
ghc_args.add(ctx.attrs.compiler_flags)

md_args = cmd_args(md_gen)
md_args.add(packages_info.nix_packages)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Is this really Nix specific, or would the same approach be needed for other forms of GHC toolchains? I'm having the goal to upstream to facebook/buck2-prelude in mind. If there's a more general pattern, then it would be good to call that out. (Not necessarily in this PR though, but good to keep in mind as a follow-up)

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Well, this would work for anything that refers to a directory (symlink) which has a bin folder. Maybe we can change this to bin_path instead. I'll see if that works.

Comment thread haskell/compile.bzl Outdated
if aux_deps:
compile_args_for_file.hidden(aux_deps)

for (path, src) in srcs_to_pairs(ctx.attrs.srcs):
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Would these aux_deps suffice to replace this srcs_to_pairs loop?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Yes, that's what I thought too, but I was a bit reluctant to remove this just yet. Maybe we should warn about other sources other than haskell being listed here and advice on using srcs_deps instead?

@avdv avdv force-pushed the cb/haskell-bin-tools branch from e587d49 to e9ef00f Compare June 13, 2024 15:16
@avdv avdv merged commit b968190 into mercury Jun 14, 2024
@avdv avdv deleted the cb/haskell-bin-tools branch June 14, 2024 06:55
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