-
Notifications
You must be signed in to change notification settings - Fork 41
Upgrade Agda base version to 2.8.0-rc3 #416
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
base: master
Are you sure you want to change the base?
Conversation
If any nix experts know how to make the Nix CI pass with a custom commit from Github, please let me know (or push directly to this branch). Otherwise I will just ignore it until we have a proper Agda release. |
39c12a2
to
3ae1a0f
Compare
@@ -2,7 +2,6 @@ | |||
ROOT=containers.agda | |||
AGDA2HS="cabal run agda2hs --" | |||
${AGDA2HS} \ | |||
--local-interfaces \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There might be some weird interaction with Nix here. @liesnikov @HeinrichApfelmus?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removing --local-interfaces
does make the agda2hs
wrapper more compatible with agdaPackages
from nixpkgs
, though I do not know whether this makes it fully compatible.
In any case, the proper fix would be to have agda2hs
provide its own mkDerivation
function, as implemented in #407 .
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The reason why I removed --local-interfaces
is simply because it is no longer supported by the new release of Agda (see https://github.com/agda/agda/blob/ef912c68fd329ad3046d156e3c1a70a7fec19ba1/CHANGELOG.md?plain=1#L76)
I converted all errors to use CustomBackendError instead of Generic(Doc)Error. This has the nice side effect that all error messages are now automatically prepended by |
@HeinrichApfelmus Currently the two new agda2hs libraries are named |
I don't mind either way. I was thinking that in a dream future where every package on Hackage has an equivalent in agda2hs, it would be cumbersome to prefix every package name |
I have added an incantation that pulls in the Agda-2.7.20250601 candidate package from Hackage. Unfortunately, this incantation currently breaks the |
Perhaps it's a problem that the cabal.project pulls the version from Github while your Nix magics get the version from Hackage. Or does the Nix CI ignore cabal.project completely? Anyway, I'm fine with waiting to merge this PR until Agda 2.8.0 is officially released, which will hopefully make the problem disappear. |
The current Nix CI ignores
Most likely not without issues, though — this will require the inclusion of Agda 2.8.0 in nixpkgs, and then an update of the nixpkgs pin in |
dec10ff
to
e077f03
Compare
I managed to fix the incantation, the Nix CI now passes. |
9aceff4
to
b7aa7a4
Compare
b7aa7a4
to
9b07558
Compare
This is a first attempt at upgrading the Agda base version to 2.8.0 (well currently the 3rd release candidate, but I don't expect much more to change). It compiles and runs the tests, however there are some things that should be addressed before merging:
There are a few Haskell build files that show up in lib/containers/haskell/Test/Agda2Hs after runningmake test
. We might need these files but I don't think it's the right place for them (and anyway it's strange that they only show up now and not earlier). @HeinrichApfelmus could you please take a look?There is a regression in three test cases where a function gets inlined that previously didn't. Specifically:test/golden/Issue302.hstest/golden/Superclass.hstest/golden/Test.hsThere's another regression (I think) in test/golden/UnnamedVar.err which just reportsUNEXPECTED_SUCCESS
where it didn't report anything before.The error messages now contain a label for the error, but currently most of them are set toGenericError
orGenericDocError
. We should make use ofCustomBackendError
instead.backendInteractTop
andbackendInteractHole
toNothing
, but perhaps there's something better we could do there.source-repository-package
field should be removed fromcabal.project
).(All of this reminds me again of #153: we really need a better test infrastructure.)