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
Reunification with NixOS/nixpkgs #312
Conversation
0a92b10
to
81d4327
Compare
Excellent! :) Super glad to see movement on this front. I think it would be a good idea to merge this only once all the changes needed to nixpkgs are upstreamed. See #313 |
I am just going to post this here in case someone know what this comes from:
All of the version look correct - but not Cabal still give that error. |
@matthewbauer #312 (comment) |
I don't think it's related to doctest - tests for distributive should be disabled already in reflex-platform. I really am not sure what's going on though. I've opened an issue in Nixpkgs (where it also shows up) here: NixOS/nixpkgs#41806. |
f7a3c94
to
463db77
Compare
So we have now opened two issues in Nixpkgs related to this PR: |
I also think the iOS logic needs to be reworked but will leave that out for now. Latest Nixpkgs should not have to look for the /Applications/Xcode.app directory because it now uses requireFile. |
af09ea4
to
03980a3
Compare
default.nix
Outdated
}; | ||
}; | ||
}; | ||
let config = { allowUnfree = true; }; |
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.
We should use allowUnfreePredicate
so that we only allow the unfree derivations that we actually need.
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.
Since this was originally set, I'm not going to try to modify it here for consistency. But definitely would be worth looking into using allowUnfreePredicate instead of allowUnfree.
So most things are now working! Currently I've gotten reflex-todomvc built on:
We are still waiting for iOS and there may be some changes necessary here.
iOS fails on GHC with:
|
4e96fc6
to
d847258
Compare
Note that iOS simulator is broken in both sides of the reunification, so it shouldn't block this |
9611c2b
to
2561c83
Compare
Ok everything seems to be working now! iOS is building but I have not had the chance to test the app & need to set up the team id & certificate stuff. The new rebased reunification Nixpkgs is at: https://github.com/obsidiansystems/nixpkgs/tree/reunification3 We now have very few differences with Nixpkgs master:
Staging has merged into master so NixOS/nixpkgs@645f03b is now available! Also ghcWithPackages is still broken & the reflexEnv stuff will not work. See NixOS/nixpkgs#42032. @Ericson2314 thinks he will have a fix for this soon. There is some issue with how the package-db is generated in Nixpkgs. The jsaddle version in this tree points to this currently open PR: ghcjs/jsaddle#67. I hope to get it merged soon. I really need to get the Hydra job running again but there is some evaluation error. Does anyone know how to fix it? This is the evaluation error: https://hydra.reflex-frp.org/jobset/reflex-platform/reflex-platform-pr-312#tabs-errors |
8ad6514
to
b649401
Compare
Do this to make sure we have all reflex profiler fixes
Thankfully, most of these changes are cherry-picks from commits that made it into 8.4. The little at the end that isn't is a) stuff that should be upstreamed for all versions, and b) a quick hack just disabling some linker routines and iserv.
That way they can be installed at the same time
See comment. We couldn't start due to a missing symbol.
Enable the Reflex profiler in all profiled builds
for ios mobile fixes
Finally! |
Awesome!
…On Tue, Oct 9, 2018, 19:39 John Ericson ***@***.***> wrote:
Finally!
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#312 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABGlYDHKpmUBW6awkihJ_alEeDOurDqEks5ujTOegaJpZM4Ug_vt>
.
|
Hey, this commit d9eed03 - This seems to have masked that ghcjs upstream tests have been failing for a long time (years?) because nobody seems to actually run them. In particular, we see this one failing:
That seems scary, because rountripping strings between Haskell and Javascript is somewhat important. Any idea what this could be? |
@luigy Is it possible that this has anything to do with your recent string-related changes in https://github.com/ghcjs/ghcjs-base/commits/master ? |
@nh2 I think in general there is a bunch of ghcjs stuff here that needs to get upstreamed into nixpkgs. We might have indeed disabled too many tests, I suspect WIP versions of the "text-jsstring" patch had something to do with it. |
Work in progress reunification branch.
Uses Nixpkgs commit: NixOS/nixpkgs@f3fc04b
Current Status
Currently GHC 8.4 works great. You can build desktop apps for macOS & Linux works as well as Linux->Android and macOS->iOS. We have been able to build a few different apps built on obelisk this way.
On the other hand, GHCJS 8.4 is still not ready yet. There have been some changes internally in GHC that GHCJS has not kept up with, leading to some critical issues. We can build a GHCJS app such as reflex-todomvc, but only when the text-jsstring is disabled. When text-jsstring is enabled, two issues occur:
h$pstr
of the output. Inlining strings is crucial for performance in Reflex-based applications. This appears to be a bug in GHCJS itself where JSString, Text, and String literals still end up in h$pstr. (inlining is now fixed, but second part mentioned still remains. Tracked here ghcjs-8.2-8.4: JSString inlined literals should not be placed in the string initializer ghcjs/ghcjs#694)The text-jsstring is important for us to get working & inlined correctly to ensure the generated JavaScript is as performant as possible.