-
Notifications
You must be signed in to change notification settings - Fork 245
Add Backpack support. #1806
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 Backpack support. #1806
Conversation
builder/comp-builder.nix
Outdated
if lib.strings.hasInfix "inplace" instLine then | ||
"$(read packageId < <(read sedline < <(echo ${instLine} | sed -n -e 's/.*=\\(.*\\)-inplace-\\(.*\\):.*/\\1-\\\\\\\\([^\"]*\\\\\\\\)-\\2/p') ; sed -n -e \"s/.*\$\{sedline\}/\\1/p\" ${configFiles}/configure-flags | head -1) ; echo ${instLine} | sed -n -e \"s/\\(.*\\)-inplace-\\(.*\\)/\\1-\$\{packageId\}-\\2/p\")" | ||
else instLine; |
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.
This needs a comment, explaining what we try to rewrite, from what to what and how.
builder/comp-builder.nix
Outdated
@@ -162,8 +167,12 @@ let | |||
if configureAllComponents | |||
then ["--enable-tests" "--enable-benchmarks"] | |||
else ["${haskellLib.componentTarget componentId}"] | |||
) ++ [ "$(cat ${configFiles}/configure-flags)" | |||
] ++ commonConfigureFlags); | |||
) ++ [ "$(sed 's/--dependency=\\(.*\\)+\\(.*\\)/--dependency=\\1/' ${configFiles}/configure-flags)" |
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.
Needs a comment, that explains what (and why) we are running sed on the configure-flags. E.g. why are they wrong by construction, and what do we need them to look like, and why?
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.
This is great!
@andreabedini can I get your input on this?
@ak3n If I understand correctly, the logic is the following
Is this correct? edit: I am not sure what |
Side note: Isn't Backpack also unmaintained in GHC? (Though I'd love to use it) |
Did you test this in the dev shell with exactDeps = true? |
I'm afraid I too think it mostly is, but I'm in the same boat as you. I want to use it ;) |
@angerman thanks for the review! Fixed some moments. Will add comments a bit later after refacting the code. Added the passing test for now. @andreabedini yep, that's it. The problem at the moment is that we do not have instantiated libraries — cabal instantiates them itself while building with |
In #1775 we started moving away from reading |
@L-as nope, testing right now, thanks! @angerman, @andreabedini, I have updated the PR, refactored the code, added tests and comments. Please take a look. It should be more clear now. UPD: @L-as how exactly to test this? Is starting a |
Yep, but do note it doesn't work with source-repository-package well. To make it work with source-repository-package, those stanzas must only be visible to haskell.nix, so you must put them into the cabalProjectLocal argument to cabalProject'. |
@L-as yeah, that works for me. For some reason It tries to patch |
Now that #1775 is merged I can integrate this. Sorry for the wait @ak3n! |
@andreabedini thanks! but actually I paused this a bit — I have started porting backpack tests from cabal and discovered new situations which are not processed properly yet, marked as draft for now. |
@andreabedini there is something really wrong with my CI checks...
is it a known issue? |
@ak3n yes, @hamishmack should have fixed that in #1846. |
Add test with 2 sigs instantiated together. Add comments. Refactor cabal2nix.
Ok, this time I'm blocked by something strange (well, all previous blockers looked so). I have added the
It says that there is a missing package containers anyway (generated nix plan). Need some time again to reset the focus I suppose. |
I'm going to close this because for some reason the update bot keeps pushing commits to it and it's probably pointlessly burning CI resources. |
This PR adds support for Backpack libraries. Should fix #244.
TODO: