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

Add a Nix flake #111

Merged
merged 4 commits into from
Mar 21, 2023
Merged

Conversation

sellout
Copy link
Contributor

@sellout sellout commented Feb 20, 2023

No description provided.

@sellout
Copy link
Contributor Author

sellout commented Feb 20, 2023

I recommend adding https://garnix.io/ to the repo, which will do Nix flake builds/checks without any additional configuration.

It can’t yet replace GH Actions, since the flake doesn’t yet run tests, but it’ll make sure the flake stays in working order, and is a cache for anyone using the flake.

Copy link
Contributor

@wavewave wavewave left a comment

Choose a reason for hiding this comment

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

This is great! I just put minor comments. thanks!

flake.nix Outdated Show resolved Hide resolved
flake.nix Outdated Show resolved Hide resolved
flake.nix Outdated Show resolved Hide resolved
Copy link
Contributor

@mikesperber mikesperber left a comment

Choose a reason for hiding this comment

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

My Nix-expert coworker @kenranunderscore reviewed, and it looks good!

Many thanks!

Maybe should squash before merging?

flake.nix Outdated
"concat-hardware"
# TODO: Tests pass with `cabal new-test`, but not with `cabal test`, and
# I’m guessing the latter is what `callCabal2nix` uses.
"concat-plugin"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I would really like to remove this line – i.e., get the gold-tests working via Nix.

I’m ok with merging before that happens, but this prevents us from replacing GH Actions with Garnix.

Copy link
Contributor

Choose a reason for hiding this comment

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

@sellout What exactly fails with the gold tests?

Copy link
Contributor

Choose a reason for hiding this comment

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

One thing I can think of is that misc-examples and misc-trace aren't proper test suites.

Just turned them into executables in a0f2aff

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, great – the same should be done for graphics-trace in concat-graphics.

Then we should also expose the executables in the flake 😄

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Your executable change fixes some of what I was running into, but there are still a few issues. Here’s what happens on GHC 9.0.2 (the default):

$ nix develop github:con-kitty/concat/updated-flake#ghc902_concat-plugin
⋮
❄️$ cabal test gold-tests
⋮
[3 of 4] Compiling BasicTests       ( test/BasicTests.hs, dist/build/gold-tests/gold-tests-tmp/BasicTests.o )
[4 of 4] Compiling Main             ( test/GoldTests.hs, dist/build/gold-tests/gold-tests-tmp/Main.o )
Linking dist/build/gold-tests/gold-tests ...
/nix/store/…-clang-wrapper-11.1.0/bin/ld: line 256: 51086 Segmentation fault: 11  /nix/store/…-cctools-binutils-darwin-973.0.1/bin/ld ${extraBefore+"${extraBefore[@]}"} ${params+"${params[@]}"} ${extraAfter+"${extraAfter[@]}"}
clang-11: error: linker command failed with exit code 139 (use -v to see invocation)
`cc' failed in phase `Linker'. (Exit code: 139)
❄️$ cabal v2-test gold-tests
⋮
5 out of 83 tests failed (0.14s)

Test suite gold-tests: FAIL
Test suite logged to:
/…/build/aarch64-osx/ghc-9.0.2/concat-plugin-0.3.0.0/t/gold-tests/test/concat-plugin-0.3.0.0-gold-tests.log
0 of 1 test suites (0 of 1 test cases) passed.
Error: cabal: Tests failed for test:gold-tests from concat-plugin-0.3.0.0.

❄️$ 

cabal test falls during linking, but cabal v2-test builds (then fails some test cases). Once the linking step succeeds, cabal test gold-tests will thereafter have the same result as cabal v2-test gold-tests.

When replicating this issue, I noticed a “bug”. Everything gets built with the correct GHC, but if you enter a dev shell (as above) the GHC that’s exposed in the shell is always the default (currently GHC 9.0.2). So,

$ nix develop github:con-kitty/concat/updated-flake#ghc8107_concat-plugin
⋮
❄️$ cabal test gold-tests

will build and run tests with GHC 9.0.2, not 8.10.7. I tried a few things locally to fix that, but didn’t get anything that works.

However nix build github:con-kitty/concat/updated-flake#ghc8107_concat-plugin builds just fine, no linking error, unlike 9.0.2 – and the tests pass on 8.10.7.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, I pushed a commit that changed the other things that should be executable and then enables all checks (which will still result in some builds failing).

However, I don’t think any of the failing builds are depended on by Categorifier 😁

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So, to summarize the current state:

  • concat-hardware panics during tests (on all GHCs)
  • concat-plugin fails tests only on GHC 9.0.2
  • all of the other failures are probably fine, but are blocked by the above failures.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just enabled darwin builds, too, so we’ll see how that goes.

Copy link
Contributor

Choose a reason for hiding this comment

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

The "5 out of 83 tests failed" are probably false negatives: You can see that the github ci on master also fails similarly, as the output is equivalent but similar. I'm guessing a slight skew in base versions, leading to different inlining results. If that's the case here (of which I'm pretty sure), we can just re-generate the gold output and be done with it.

@sellout sellout force-pushed the updated-flake branch 3 times, most recently from 545e93c to b94caec Compare March 1, 2023 17:34
wavewave and others added 3 commits March 11, 2023 18:16
They really are – they don’t run tests, and are hard to use without human interaction.
Mostly because Haddock thinks commented-out guards are docs.
- add a Nix formatter;
- add a default package & devShell;
- restructure the overlays;
- move the inputs to the end;
- don’t repeat the GHC version list;
- move lib into its own file (and add more functions);
- add nixConfig;
- provide a “Haskell overlay”;
- disable GHC 9.4 and HEAD builds, as they fail;
- enable all packages now that they work with newer GHC (except for concat-hardware);
- enable `checkPhase` for all Cabal packages;
- replace string passing with a package set;
- extract "ghc902" to `defaultCompiler`;
- add a Garnix config to enable darwin builds; and
- expose executables from Cabal packages.
@sellout
Copy link
Contributor Author

sellout commented Mar 21, 2023

Ok, this is mergable now, IMO. The failures that are happening are known. concat-plugin on 9.0.2 sounds fixable after this, and then we can figure out concat-hardware. Everything else is good.

@mikesperber mikesperber merged commit 451b112 into compiling-to-categories:master Mar 21, 2023
@mikesperber
Copy link
Contributor

Thanks - will fix 9.0.2 build subsequently.

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.

None yet

3 participants