-
-
Notifications
You must be signed in to change notification settings - Fork 15k
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
[WIP] Fix taffybar #40186
[WIP] Fix taffybar #40186
Conversation
This was already used in Stack builder.
Fixed in 809c70d0594bc6cc898841ec3b2684c61822c9ba
It's needed to propagate GI dependencies.
@@ -214,6 +214,10 @@ stdenv.mkDerivation ({ | |||
|
|||
LANG = "en_US.UTF-8"; # GHC needs the locale configured during the Haddock phase. | |||
|
|||
# XXX: workaround for https://ghc.haskell.org/trac/ghc/ticket/11042. | |||
LD_LIBRARY_PATH = stdenv.lib.makeLibraryPath (stdenv.lib.filter (x: x != null) systemBuildInputs); | |||
# ^^^ Internally uses `getOutput "lib"` (equiv. to getLib) |
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.
Is this comment is pointing out the definition of the function or is it something special?
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.
Pointing out the definition -- I copied that from stack-builder.nix
.
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.
I'm sketched out by adding this. is ghci really needed? The rest looks good.
@Ericson2314 I've noticed that hooks are executed several times for propagated build inputs in some cases. For example:
From a quick look at our source code this shouldn't happen because we keep track of traversed packages. |
Yeah, this library loading is used by doctests. We have the same code in stack-builder.nix, I only copied it.
This was supposed to be in answer to @Ericson2314's comment on ghci.
|
any progress on the gcc error? |
@IvanMalison I don't see a good solution now; this could either be fixed in stdenv (removing duplication), in cc-wrapper (adding support for argument files) or maybe somehow in Cabal. The first way is, if I understood @Ericson2314, problematic; the second is doable but still constitutes a mass rebuild. I'm not sure what could we try to do in Cabal. |
Ah doctests. Good use. I'd maybe mention that (as an example) explicitly. Yeah I want to teach *-wrapper to use response files, so that's a good approach. Cabal may in fact use them, but the wrappers need to unpack to use their black magic. |
For the duplication, add |
@@ -214,6 +214,10 @@ stdenv.mkDerivation ({ | |||
|
|||
LANG = "en_US.UTF-8"; # GHC needs the locale configured during the Haddock phase. | |||
|
|||
# XXX: workaround for https://ghc.haskell.org/trac/ghc/ticket/11042. | |||
LD_LIBRARY_PATH = stdenv.lib.makeLibraryPath (stdenv.lib.filter (x: x != null) systemBuildInputs); |
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.
I think it's a bad idea to set LD_LIBRARY_PATHS
in the generic builder. The haskell-gi
build is supposed to fail, honestly, because upstream calls their doctest suite with incorrect flags. I don't want us to add kludges to our code so that their broken code can succeed. The proper solution is to fix the haskell-gi
build. I've committed 3e05301c94d72979da3c5f304196c7dd01f76a15 to accomplish that until a new release comes out.
Ah, so it can be done with proper flags - I see now! Why is this used in Stack builder though?
--
Nikolay.
|
I have no clue what that stack builder is doing. :-/ |
@@ -144,6 +144,9 @@ self: super: builtins.intersectAttrs super { | |||
gtk = disableHardening (addPkgconfigDepend (addBuildTool super.gtk self.gtk2hs-buildtools) pkgs.gtk2) ["fortify"]; | |||
gtksourceview2 = addPkgconfigDepend super.gtksourceview2 pkgs.gtk2; | |||
gtk-traymanager = addPkgconfigDepend super.gtk-traymanager pkgs.gtk3; | |||
gi-gdkx11 = addPkgconfigDepend super.gi-gdkx11 pkgs.gtk3; |
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.
I believe that this override is no longer necessary due to NixOS/cabal2nix@2923434. gi-gdkx11
compiles fine in haskell-updates
without this override.
@@ -144,6 +144,9 @@ self: super: builtins.intersectAttrs super { | |||
gtk = disableHardening (addPkgconfigDepend (addBuildTool super.gtk self.gtk2hs-buildtools) pkgs.gtk2) ["fortify"]; | |||
gtksourceview2 = addPkgconfigDepend super.gtksourceview2 pkgs.gtk2; | |||
gtk-traymanager = addPkgconfigDepend super.gtk-traymanager pkgs.gtk3; | |||
gi-gdkx11 = addPkgconfigDepend super.gi-gdkx11 pkgs.gtk3; | |||
gi-dbusmenu = super.gi-dbusmenu.override { dbusmenu-glib = pkgs.libdbusmenu-glib; }; | |||
gi-dbusmenugtk3 = super.gi-dbusmenugtk3.override { dbusmenu-gtk3 = pkgs.libdbusmenu-gtk3; }; |
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.
These two overrides should be unnecessary after NixOS/cabal2nix@d8bc043.
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.
I think NixOS/cabal2nix#353 is also needed, unless upstream is an even better place.
@peti Nice! That leaves us only with a libdbusmenu GTK propagation patch and some way to fix taffybar. I'm testing response files patch for cc-wrapper now, if that doesn't help I'll try |
@abbradar see 469fd89 and the new conditions based on |
It seems response files don't fix this because gcc itself calls binaries with too long arguments:
So... what do we do from here? EDIT: reading gcc's source code -- there's no visible way to force it to pass arguments to its child processes via a file too. |
@abbradar fwiw, that error message can be a little misleading, if you didn't know: Everything in the environment is counted as well as the command line arguments. If the content of the variables was just stuck in response files but the variables themselves never cleared, they still count towards the limit. |
Yeah, I understand - the problem is that this limit is being hit inside GCC itself where we have no control over usage of response files (which get overall size under the limit). I thought of clearing environment variables to get over this limit but that feels too hacky and may lead to subtle errors if we clear NIX_*.
…On May 11, 2018 2:48:56 PM GMT+03:00, Sarah Brofeldt ***@***.***> wrote:
@abbradar fwiw, that error message can be a little misleading, if you
didn't know: Everything in the environment is counted _as well_ as the
command line arguments. If the content of variables were just stuck in
response files but the variables themselves never cleared, they still
count towards the limit.
--
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub:
#40186 (comment)
--
Nikolay.
|
Yes, it does seem hacky. It's a fundamental builder problem if we take up this much space in the environment, though... |
#40539 means one can do |
Post-#40529 do |
@abbradar Are we still stuck? |
@Ericson2314 I'm running in to this issue with stack builds of taffybar as well (I have the nix integration enabled). How would I set strictDeps = true; for this case? |
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.
I think either NixOS/cabal2nix#353 or haskell-gi/haskell-gi#172 are better places for these changes.
@@ -144,6 +144,9 @@ self: super: builtins.intersectAttrs super { | |||
gtk = disableHardening (addPkgconfigDepend (addBuildTool super.gtk self.gtk2hs-buildtools) pkgs.gtk2) ["fortify"]; | |||
gtksourceview2 = addPkgconfigDepend super.gtksourceview2 pkgs.gtk2; | |||
gtk-traymanager = addPkgconfigDepend super.gtk-traymanager pkgs.gtk3; | |||
gi-gdkx11 = addPkgconfigDepend super.gi-gdkx11 pkgs.gtk3; | |||
gi-dbusmenu = super.gi-dbusmenu.override { dbusmenu-glib = pkgs.libdbusmenu-glib; }; | |||
gi-dbusmenugtk3 = super.gi-dbusmenugtk3.override { dbusmenu-gtk3 = pkgs.libdbusmenu-gtk3; }; |
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.
I think NixOS/cabal2nix#353 is also needed, unless upstream is an even better place.
@abbradar I believe that you should be able to fully compile taffybar-2.1.2 with this pull request because I have removed all hsc files. The gcc call with too many arguments was occuring in this call, (cabal actually seems to properly handle deduplicating these flags AFAICT). Can you verify? |
Is this still an issue? I've been using taffybar from 18.03 for a while and just discovered that taffybar works in nixos-unstable channel without this patch. |
Yeah, I think it's fine now @glglwty |
Let's close it; it's stale now. |
Motivation for this change
Fix taffybar.
Things done
build-use-sandbox
innix.conf
on non-NixOS)nix-shell -p nox --run "nox-review wip"
./result/bin/
)This makes all the dependencies for taffybar build but taffybar itself fails with:
Not sure how to fix this -- there may be duplicated arguments in
configureFlags
for Cabal, should we filter them somehow? @peti, any ideas?For testing one can skip 76f629e and 7c698dd -- those are mass rebuild changes.