Repository navigation
Conversation
One real test failure: mem-use-tests (TODO)
One existing test failure: cabal-benchmarks
One unrunnable test: integration-test2 (Claude says "inherently unsuited
to a hermetic `buck2 test` run regardless of how it's built")
* Tested `cabal buck2 --enable-tests` against the real cabal repo itself
(15 local packages), working towards `buck2 build //...`. Found and
fixed, in order:
1. `cabal build all --only-dependencies` (which `cabal buck2` needs)
failed with `CannotPruneDependencies` on this repo specifically:
`cabal-testsuite` (build-type Custom) has `setup-depends` on the
local `Cabal-syntax`, and excluding *all* local targets from "only
dependencies" broke that edge - confirmed pre-existing/upstream
(Cabal-7072) by reproducing with plain `cabal build all
--only-dependencies`, no buck2 involved. Fixed generally (not just
for this one case) with a retry loop, `pruneToDependenciesNeeded` in
CmdBuck2.hs: keep exactly the packages a `CannotPruneDependencies`
failure says are still needed, retry until stable.
2. `hackage-security`/`Cabal-syntax` showed "no .conf file" warnings:
hackage-security's `cabal-syntax` flag makes it depend on the local
in-tree Cabal-syntax, forcing it to build "inplace" (registered in
`dist-newstyle/packagedb/`, not the store) with an empty
library-dirs `.conf`. Prebuilt.hs previously only knew "global vs
store"; reworked to a 3-way `PkgDbKind` (Global/Store/Inplace,
classified via the `-inplace` unit-id suffix), with per-package
symlinks into each inplace package's real build directory
(`elabDistDirParams`) so `readPackage` can override the empty
library-dirs. Also stopped walking local packages into the prebuilt
set at all (they already get real `haskell_library()` rules from
Generate.hs).
3. buck2 "Attempted to register target ...:attoparsec twice" -
Prebuilt.hs's `targetName` used only `packageName`, ignoring
`sourceLibName`, so `attoparsec` and `attoparsec-internal` (a named
sub-library) collided. Fixed by exporting and reusing
CabalToBuck.hs's own `libTargetName` (already sub-library-aware).
4. `Cabal-syntax` failed to compile under buck2
(`Distribution.Utils.Structured`: "Illegal visible type
application") - `default-language: GHC2021` (which implies
TypeApplications, among ~40 other extensions) was never translated
to a `-X` flag; only `default-extensions:` was. Fixed by adding a
`-X<Language>` flag (from `defaultLanguage`) in `compilerFlagsArg`.
5. `cabal-testsuite`'s `Test.Cabal.ScriptEnv0` (a Custom-Setup.hs
generated module with no real source file anywhere) made buck2 fail
evaluating that package's *entire* BUCK file, since the old
`resolveModules`/`resolveOne`/`resolveMainIs` guessed a plausible
path and emitted the rule anyway when a module's source couldn't be
found. Restructured all three to return `Maybe`, so a component
with an unresolvable module is cleanly skipped (with a warning)
instead of emitting a rule referencing a nonexistent file - buck2
evaluates every BUCK file for `//...` upfront, so one bad path
anywhere previously hard-failed the whole build's analysis, not
just that one target.
6. `Cabal-syntax` failed again, differently: `ComponentId.hs:65`
ambiguous-type error (`elem`/string-literal, the classic
OverloadedStrings-without-a-type-signature shape) on code that
compiles fine under plain `cabal build`. Root cause:
`buck2/haskell.bzl` (this project's own `haskell_library()`/
`haskell_binary()` wrapper) was unconditionally injecting a
hardcoded `FB_HASKELL_EXTENSIONS` list (including
`-XOverloadedStrings`) into every target's `compiler_flags`, on top
of the correct per-package flags CabalToBuck.hs already computes
from each `.cabal` file - leftover from an unrelated
Facebook/Glean-flavoured buck2/Haskell wrapper (comments referenced
`glean.cabal.in`, `hsthrift`, `gleancli`, ...) that had no business
being in a general cabal-buck2 integration. Removed the
`fb_haskell`/`FB_HASKELL_EXTENSIONS` mechanism entirely -
CabalToBuck.hs is the sole authority for a package's compiler
flags, so there's no reason for the wrapper to inject anything of
its own.
7. Same cleanup surfaced a second, more real bug while removing that
wrapper's other hardcoded default (unconditional `-threaded
-rtsopts` linker_flags on every `haskell_binary`, also
Glean-inherited): buck2's `haskell_binary` only applies
`compiler_flags` to each module's own compile step, not the final
link (a separate `ghc -o` invocation) - unlike Cabal, which applies
a component's whole `ghc-options` to every ghc invocation for it,
compile and link alike. CabalToBuck.hs's `compilerFlagsArg` only
ever wrote `ghc-options` into buck2's `compiler_flags`, so anything
link-relevant in there (`-threaded`, `-rtsopts`,
`-with-rtsopts=...` - all real, used by cabal-install's own
executables) was silently never reaching the linked binary; the
wrapper's hardcoded injection happened to paper over exactly this
gap, for every binary, whether or not it actually wanted it. Fixed
properly in `linkerFlagsArg`: `ghc-options` now also goes into
buck2's `linker_flags` for executables/test-suites (harmless for
compile-only flags that show up there too - GHC's link-mode
invocation just ignores what doesn't apply).
Continued past fix 7, still chasing a real `buck2 build //...` on this
repo (which now got much further - past every package compiling - into
new failures):
8. `-hide-all-packages` was missing from the vendored prelude's own
compile command entirely (`prelude/haskell/compile.bzl`) - without
it, GHC's own *implicit* global package db (always loaded,
independent of whatever `-package-db`s are passed) leaks in
alongside whatever `get_packages_info()` computes, which is
normally harmless but breaks down exactly when a project rebuilds
one of GHC's own boot libraries from source under its own name (as
this one does for `Cabal`/`Cabal-syntax`): GHC then sees *two*
different compiled `Cabal-syntax`s - the real boot one (auto-exposed
via the global db) and this project's own buck2-built one - and
fails with "Ambiguous module name" for anything importing a module
both provide. Fixed with a local prelude fork: added
`-hide-all-packages` to `compile_args` in `compile.bzl`, so only the
deps `get_packages_info()` already explicitly computes (correctly,
per its own comment) are ever visible.
9. Once ambiguity was gone, the *opposite* problem appeared for modules
`Cabal` re-exports from `Cabal-syntax` (`reexported-modules:` in
Cabal.cabal, for backwards compatibility - see haskell#7974):
"Could not load module ... hidden package". buck2's
`haskell_library()` has no concept of a reexported module - unlike a
real `ghc-pkg`-registered `.conf`, which can declare "I also provide
module M, sourced from package P" and have GHC honour it
transparently - so a consumer that only declared `Cabal` as a
dependency (matching its own `.cabal` file, and matching what a
plain `cabal build` needs) but imports one of those modules directly
now correctly fails once hidden packages are actually hidden.
Fixed generally in CabalToBuck.hs: `LocalPackageIndex` now also
carries each local package's reexport origins (resolved via
`moduleReexportOriginalPackage` when explicit, or by searching every
local package's own `exposedModules` for the bare-name form
Cabal.cabal actually uses), and `classifyDeps` closes over them
transitively - so a component depending on `Cabal` automatically
also gets `Cabal-syntax` as an explicit direct dependency, exactly
mirroring what the reexport really means.
10. `cabal-install` itself then failed to compile: `cabal_macros.h`'s
`MIN_VERSION_Cabal-syntax(...)`/`VERSION_x` CPP macros were simply
never generated at all - my generator only ever forwarded
`cpp-options`/`default-extensions`, never Cabal's own auto-generated
per-component macros header, so any module using
`#if MIN_VERSION_x(...)` (extremely common) failed outright
("function-like macro ... is not defined") the moment such a module
was actually reached (Cabal-syntax's own `ComponentId.hs`, then
cabal-install's use of `default-language: GHC2021`'s implied
extensions, masked earlier real macro-related failures until this
point). Fixed by reusing Cabal's *own*
`Distribution.Simple.Build.Macros.generatePackageVersionMacros`
(not reimplementing macro formatting) - CmdBuck2.hs now resolves
every package's solver-picked version once
(`Map PackageName Version`, from `elaboratedPlanOriginal`) and
passes it down; CabalToBuck.hs writes a real `cabal_macros.h` per
component (`<pkg>/buck2-cabal-macros/<component>/cabal_macros.h`,
refreshed on every `cabal buck2` run like everything else
generated) and adds `-optP-include -optP<path>` to
`compiler_flags`, exactly how real Cabal wires this up, just ahead
of time instead of via Setup.hs.
11. `cabal-install` failed again, differently: a real type error
(`Couldn't match type PackageIdentifier ... Actual: PackageId ...`)
between `Cabal-syntax-3.19.0.0` and `root-Cabal-syntax-Cabal-syntax`
- two *different* compiled copies of the identical source. Root
cause: `hackage-security` (an external, ordinarily-prebuilt
dependency) has its own `cabal-syntax` flag making *it* depend on
the local in-tree Cabal-syntax too, forcing hackage-security itself
to build "inplace" (see `pruneToDependenciesNeeded`'s own haddock) -
compiled by plain `cabal build all --only-dependencies` against the
*real* Cabal-syntax, not this project's buck2-rebuilt one. Reusing
that already-compiled hackage-security as a prebuilt blob (the
normal path for every other external dependency) leaves GHC with
two nominally distinct, incompatible copies of Cabal-syntax's types
in the one build the moment cabal-install uses hackage-security's
Security API. Consulted the user on this one (a genuine design
fork, not just a bugfix) - confirmed the general fix over a
hackage-security-specific workaround: any *non-local* package whose
own build is forced inplace by depending on a local one now gets a
real `haskell_library()` rule generated from its own unpacked
source too (CmdBuck2.hs widens its "is this local" predicate from
`elabLocalToProject` to `elabLocalToProject || elabBuildStyle /=
BuildAndInstall`, resolving the real source dir via
`distUnpackedSrcDirectory`/`elabPkgSourceLocation` the same way
`dataDirEnvVarForPackage` already does internally; Prebuilt.hs
excludes the same widened set from its own prebuilt-rule
generation, while still registering its `.conf` in the recached
store db purely so `ghc-pkg`'s own dependency-closure check for
anything *else* that might reference its original unit id still
passes).
12. That surfaced two more, smaller bugs, both fixed as local prelude
forks / general CabalToBuck.hs fixes rather than one-offs:
- buck2's own derived package name/id (`haskell.bzl`, from the
target's buck2 package *path*) failed `ghc-pkg register`
outright ("unexpected Empty component") for
hackage-security's real unpacked directory
(`dist-newstyle/src/hackage-security-0.6.4.0`) - traced
(confirmed empirically against the real `ghc-pkg` binary) to
ghc-pkg's name-token grammar rejecting *any* non-trailing,
purely-numeric hyphen-delimited component (it's parsing for a
trailing name-version split, same as any ordinary package
identifier) - a plain "." literally, or "." naively turned into
another hyphen, both produce exactly that shape from a version
number like "0.6.4.0". Fixed by splicing a letter into the
substitution (`.` -> `"d"`) so no component is ever purely
numeric, confirmed against the real binary before landing it.
- generating a real rule for cabal-testsuite's *library* (skipped -
see item 5) meant OTHER components in the *same* package that
depend on it (its own `test-runtime-deps`/`cabal-tests`
executables) emitted a `deps` reference to a target that no
longer existed - buck2 fails that outright at analysis time
("Unknown target"), for the whole build, not just that package.
Fixed generally: `generatePackageTargets` now computes which of a
package's own libraries will be skipped *before* generating any
component (a quiet, warning-free pre-pass reusing the same
`resolveModules` check), so any other component depending on one
of those skips itself too, with one clear warning naming why -
rather than a confusing downstream buck2 analysis error.
`buck2 build //...` on the real cabal repo (15 packages, `-w ghc-9.6.7`,
`--enable-tests`) succeeds: `BUILD SUCCEEDED`, exit code 0, 84 actions,
0% cache (first build).
* Ran `buck2 test //...` on the real cabal repo: 10 of 16 test targets
pass outright (`Cabal-tests:unit-tests`, `cabal-install-solver:unit-tests`,
`solver-benchmarks:unit-tests`, ...). The 6 failures are three distinct,
diagnosed causes, not one bug - noted here rather than fixed yet (the
first is a real, general gap worth a deliberate design decision before
touching the prelude further; the other two aren't buck2-integration
bugs at all):
- **CWD mismatch (`Cabal-tests:check-tests`, `Cabal-tests:parser-tests`,
`cabal-install:parser-tests`, `cabal-install:unit-tests`)**: these
load fixture files via paths relative to their *own package
directory* (e.g. `tests/fixtures/project-root`,
`tests/ParserTests/regressions/*.cabal`) - exactly matching `cabal
test`'s own convention of running a test suite with its cwd set to
the package root. buck2's `sh_test` (what `haskell_test()` in
buck2/haskell.bzl wraps every test in) has no per-package cwd concept
of its own - confirmed empirically that local test execution here
runs with cwd = the *project* root (one level up from where these
fixtures actually expect), the same convention build actions already
use elsewhere in this setup. Real, general fix needs a small cd-first
wrapper around each test binary (e.g. a `native.genrule()`-produced
shell script using `native.package_name()` for the target directory,
since a macro can't create build actions directly the way a real
rule's `ctx.actions` can) - substantial enough (and with more than
one reasonable shape) that it's worth a design check-in rather than
guessing, so left for the next session rather than rushed.
- **`cabal-install:mem-use-tests`**: "Heap exhausted ... Current
maximum heap size is 16777216 bytes (16 MB)" - this test suite is
deliberately run under a tight `-M16M` RTS heap cap
(`-with-rtsopts=-M16M -K1K` in cabal-install.cabal) to catch memory
regressions; the *buck2-built* binary's actual heap usage for the
same workload differs enough (different optimisation level in `dev`
mode, different linking) to blow that specific budget. Not a
buck2-integration bug - the cap itself is tuned against a `cabal
build`-produced binary's specific memory profile, which was never
guaranteed to match a differently-built one exactly.
- **`cabal-install:integration-tests2`**: downloads the real Hackage
package index over the network
("Downloading the latest package list from hackage.haskell.org") and
exercises real temp-directory git repos - inherently unsuited to a
hermetic `buck2 test` run regardless of how it's built; not something
to chase here.
* Fixed the CWD-mismatch class of failure above generally. User proposed
the design directly: a `cwd` attribute on `haskell_test()`, and when
present, generate the wrapper via `sh_binary()` and run *that* with
`sh_test()`. First cut generated the cd-then-exec wrapper script per
test via `genrule()` (a heredoc with a *quoted* delimiter, so
`$1`/`$bin`/`$@`/`$PWD` survived into the generated script literally
instead of being expanded by the genrule's own shell) - working, but
the user then asked whether the genrule was even needed versus one
static script taking the binary and cwd as plain arguments. It wasn't:
simplified to `buck2/run_in_cwd.sh`, a small checked-in, argument-driven
script (`run_in_cwd.sh <cwd> <binary> [args...]` - still resolves the
binary to an absolute path *before* the `cd`, since whatever
`$(exe :bin)` expands to is only valid from the original cwd), wired up
once via a single shared `sh_binary(name = "run_in_cwd")` in a new
`buck2/BUCK`. `haskell_test()`'s `cwd` handling shrank to just pointing
`sh_test()`'s `test` at `//buck2:run_in_cwd` and prepending
`[cwd, "$(exe :bin)"]` to its `args` - no per-test targets, no
generated-file escaping to get right. CabalToBuck.hs's `testSuite` now
always passes `cwd = <package's own root-relative dir>` - matching real
`cabal test`'s own invariant (it always runs a test-suite with its cwd
set to the package root) unconditionally, not just for the packages
that happened to be failing.
Re-running after the simplification showed `cabal-install:unit-tests`
newly failing on one subtest ("concurrent jobs", a QuickCheck property
test over `JobControl`'s concurrency, "Falsified after 97 tests") -
checked whether the simplification had broken something by re-running
just that target on its own: passed cleanly the second time ("All 632
tests passed"), and the same test had passed in the *previous* (genrule)
run too - confirming it's a pre-existing, timing-sensitive flaky test,
unrelated to either version of the `cwd` mechanism.
Result: `Cabal-tests:check-tests`, `Cabal-tests:parser-tests`,
`cabal-install:parser-tests`, and `cabal-install:unit-tests` now all
pass. `cabal-benchmarks:cabal-benchmarks` (declared as a `test-suite`
but hardcodes project-root-relative paths - `"Cabal/Cabal.cabal"`,
`"cabal-benchmarks/cabal-benchmarks.cabal"` - directly in
`bench/CabalBenchmarks.hs`) started *newly* failing under this fix -
investigated by reproducing with plain `cabal test cabal-benchmarks`
directly (no buck2 involved): it fails the exact same two subtests
there too. So this isn't a regression the fix introduced - the buck2
build was silently passing it by accident before (its cwd, prior to
this fix, happened to be the project root, which happens to be what
this one benchmark's own hardcoded paths want, unlike every other
test-suite in the repo) - it's a real, pre-existing bug in
cabal-benchmarks' own test source, now faithfully reproduced instead of
masked. Left as-is (out of scope - a bug in the cabal repo itself, not
the buck2 integration).
`buck2 test //...` on the real repo: 13 of 16 targets pass. The
remaining 3 (`cabal-benchmarks`, `mem-use-tests`, `integration-tests2`)
are all independently diagnosed as genuine, non-buck2-integration
issues (see above) - `buck2 test //...` now reproduces exactly what a
real `cabal test` on this repo would report, not more and not fewer
failures.
We now have export_file() rules for autogen files so that 1. cabal_macros.h is correctly referenced as a source file from the -optP-include flags, not just invisibly used 2. Custom BUCK rules can easily use the autogen/Paths_pkg.hs file
This is still a bit gross and could probably be cleaned up some more.
Collaborator
|
For a previous discussion on building with different "backend" see #9089 |
Member
Author
|
moving discussion to #12456 |
This file contains hidden or 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
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.
I'm creating this as a draft for now, for discussion. It's not ready for merging yet, but before I spend the time to write docs etc. I'd like to get a sense of whether the Cabal maintainers think this is worth polishing as is or whether we should do something different.
EDIT: I've shipped this as an external command on Hackage for now: cabal-buck2. However, it uses the cabal-install library API heavily, so it's tightly wired to a particular release of cabal-install (currently 3.18). This relates to #12326.
Background:
I've done a round of cleaning up and refactoring the (originally AI-generated) code, so it's in not bad shape. The internals are pretty well factored into distinct pieces and the effects are reasonably localised. It doesn't look to be abusing any Cabal APIs. Someone more familiar with the Cabal internals might be able to find ways to factor things better though.
Main outstanding issues
We need to have a story for shipping/deploying haskell-buck2, which needs to be cloned (or copied) into the root of the Cabal project. It needs to be versioned somehow. Ship a tarball with Cabal or clone it on demand? One branch per Cabal release? Should there be a submodule in the Cabal repo, as in this PR? There's also a versioning issue with Buck2 itself, which moves in sync with its prelude so only certain releases of Buck2 will work with a particular prelude.
No support for the new build hooks yet
Some other missing features:
asm-sources,cmm-sources,js-sources,frameworks,c2hs,foreign-library.Some tidying up of code/comments still needed.
Patches conform to the coding conventions.
[NOT YET] Any changes that could be relevant to users have been recorded in the changelog.
significance: significantin the changelog file.[NOT YET] The documentation has been updated, if necessary.
[NOT YET] Manual QA notes have been included.
Tests have been added. (Ask for help if you don’t know how to write them! Ask for an exemption if tests are too complex for too little coverage!)