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

notify-osd no longer builds #15074

Closed
purefn opened this issue Apr 29, 2016 · 10 comments
Closed

notify-osd no longer builds #15074

purefn opened this issue Apr 29, 2016 · 10 comments
Labels
0.kind: bug Something is broken 6.topic: closure size The final size of a derivation, including its dependencies

Comments

@purefn
Copy link
Contributor

purefn commented Apr 29, 2016

Issue description

Compilation of notify-osd fails because it can't find dbus-binding-tool.

Making all in src
make[2]: Entering directory '/tmp/nix-build-notify-osd-0.9.34.drv-0/notify-osd-0.9.34/src'
/nix/store/d20f169ryps7ds2qak0r5n1f4hhxr80h-bash-4.3-p42/bin/bash ../libtool --mode=execute /nix/store/hxsbjbjn7g1j1cf60n228yi9wnzrl4yk-dbus-glib-0.104/bin/dbus-binding-tool --prefix=stack --mode=glib-server --output=stack-glue.h notify-osd.xml
../libtool: line 10051: /nix/store/hxsbjbjn7g1j1cf60n228yi9wnzrl4yk-dbus-glib-0.104/bin/dbus-binding-tool: No such file or directory
Makefile:1109: recipe for target 'stack-glue.h' failed
make[2]: *** [stack-glue.h] Error 127
make[2]: Leaving directory '/tmp/nix-build-notify-osd-0.9.34.drv-0/notify-osd-0.9.34/src'
Makefile:330: recipe for target 'all-recursive' failed
make[1]: *** [all-recursive] Error 1
make[1]: Leaving directory '/tmp/nix-build-notify-osd-0.9.34.drv-0/notify-osd-0.9.34'
Makefile:261: recipe for target 'all' failed
make: *** [all] Error 2
builder for ‘/nix/store/i03p6rzqg8vjvkdkncawvzf8p6bllv8m-notify-osd-0.9.34.drv’ failed with exit code 2

Steps to reproduce

nix-env -i notify-osd

Technical details

  • System: (NixOS: nixos-version, Ubuntu/Fedora: lsb_release -a, ...)
    16.09pre82105.7465bcd (Flounder)
  • Nix version: (run nix-env --version)
    nix-env (Nix) 1.11.2
  • Nixpkgs version: (run nix-instantiate --eval '<nixpkgs>' -A lib.nixpkgsVersion)
    "16.09pre82105.7465bcd"
@dezgeg
Copy link
Contributor

dezgeg commented Apr 29, 2016

This is because notify-osd locates the binary via pkgconfig, which has:

prefix=/nix/store/hxsbjbjn7g1j1cf60n228yi9wnzrl4yk-dbus-glib-0.104
exec_prefix=${prefix}

I've seen this several times now; I wonder if we should globally pass --exec-prefix as well when passing --bindir.

cc @vcunat

@vcunat
Copy link
Member

vcunat commented Apr 29, 2016

I don't think exec_prefix should have anything to do with executables, and both of them are closest in $out. IIRC it's for systems where during installation you put files into some directory (prefix) but when actually using the files, they're located in a different one (exec_prefix).

@vcunat vcunat added the 0.kind: bug Something is broken label Apr 29, 2016
@dezgeg
Copy link
Contributor

dezgeg commented Apr 29, 2016

So to be clear, the dbus_glib package has:

outputs = [ "dev" "out" "docdev" ];
outputBin = "dev";

... so the pkgconfig file would need to refer to dev for this to work.

The GNU autoconf manual gives this explanation:

Similar to ‘--prefix’, except that it sets the location of installed files which are architecture-dependent. The compiled ‘emacs’ binary is such a file. If this option is not given, the default ‘exec-prefix’ value inserted into generated files is set to the same value as the ‘prefix’.

which is not very helpful (to me) in describing what it should actually mean; i.e. are the apps that do things like

DBUS_GLIB_BIN="`$PKG_CONFIG --variable=exec_prefix dbus-glib-1`/bin"

mis-using the variable (that is what notify-osd uses).

@dezgeg dezgeg added the 6.topic: closure size The final size of a derivation, including its dependencies label Apr 29, 2016
@vcunat
Copy link
Member

vcunat commented Apr 29, 2016

I suppose we should patch this single package, unless there are (many) others using exec_prefix in this meaning.

@dezgeg
Copy link
Contributor

dezgeg commented Apr 29, 2016

Besides this, I've noticed 584d884 and c4ab7e2 in the repo so far.

@groxxda
Copy link
Contributor

groxxda commented Apr 29, 2016

I think the problem is that autotools uses many more variables (by default) than pkgconfig exposes in most packages (bindir, sbindir, libdir, includedir)

When I came across this issue, I was thinking about submitting upstream patches to fix dbus-glib-1.pc to include the other used variables (mainly bindir)
Then fix notify-osd (and others) to query bindir instead of using $exec_prefix/bin which is a breaking change so it will be hard to convince upstream or at least make their configure script accept something like DBUS_BINDING_TOOL which we could automatically pass in when dbus-glib is a build input.

Setting exec_prefix to bindir will not fix all problems, because EPREFIX is used as a default for lib and libexec aswell.
So another package might assume libdir is EPREFIX/lib and consequently break.

Another thought I had was to base our splits on the structure that's exposed by it's pkgconfig file (maybe with the exception of documentation..?)
So here we would only set prefix and exec_prefix for configure and not the more detailed variables (bindir, etc.)

@purefn
Copy link
Contributor Author

purefn commented May 4, 2016

@dezgeg, @groxxda, I understand there is possibly a bigger issue at play here and it should be solved in a proper way, but I'm wondering if you could tell me the one off changes required to notify-osd specifically working here. I'm not up to speed on all the changes from the closure-size merge and can't seem to wrap my head around what is necessary.

dezgeg added a commit that referenced this issue May 5, 2016
Without this notify-osd fails to find dbus-binding-tool, since the
pkgconfig file would contain e.g.:

````
prefix=/nix/store/hxsbjbjn7g1j1cf60n228yi9wnzrl4yk-dbus-glib-0.104
exec_prefix=${prefix}
````

... and notify-osd is using `exec_prefix` to locate the binaries.
Set it to $dev to match the location of installed binaries (we have
`outputBin = "dev";`).

Issue #15074.
@dezgeg
Copy link
Contributor

dezgeg commented May 5, 2016

Heh, indeed too much talk and not enough action ;) I now pushed 367b2aa since it's not (yet) clear that the other alternative(s) would be any better/more.

@vcunat
Copy link
Member

vcunat commented May 7, 2016

I confirm it builds on master.

@vcunat vcunat closed this as completed May 7, 2016
@purefn
Copy link
Contributor Author

purefn commented May 9, 2016

👍 thanks @dezgeg!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
0.kind: bug Something is broken 6.topic: closure size The final size of a derivation, including its dependencies
Projects
None yet
Development

No branches or pull requests

4 participants