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

Jump to bottom

dante: add darwin support #68418

Merged
merged 1 commit into from Sep 23, 2019
Merged

dante: add darwin support #68418

merged 1 commit into from Sep 23, 2019

Conversation

arnarg
Copy link
Contributor

@arnarg arnarg commented Sep 10, 2019 •
edited

Motivation for this change

This is a dependency of aerc that I want to install.
I looked at how homebrew does it: https://github.com/Homebrew/homebrew-core/blob/master/Formula/dante.rb

It seems that dependency tracking is a requirement on Mac OS.
When compiled with --with-libc=libc.so.6 socksify fails with: socksify: error: dante client not built with preloading support.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nix-review --run "nix-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.
Notify maintainers

cc @

@ofborg ofborg bot added 6.topic: darwin Running or building packages on Darwin 8.has: package (new) labels Sep 10, 2019
@ofborg ofborg bot requested a review from Shados September 10, 2019 13:18
Shados
Shados reviewed Sep 10, 2019
View reviewed changes
Copy link
Member

@Shados Shados left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Other than these comments, LGTM, although I don't have a darwin box to test with.

pkgs/servers/dante/default.nix Outdated Show resolved Hide resolved
pkgs/servers/dante/default.nix Outdated
configureFlags = if stdenv.isDarwin then ["--with-libc=libc.dylib"]
else ["--with-libc=libc.so.6"];

dontAddDisableDepTrack = stdenv.lib.optional stdenv.isDarwin true;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have you tested without this set? I see the comment in the homebrew formula you linked, but I'm not certain this is actually relevant within Nix's build environment (and note, there's nearly no usage of dontAddDisableDepTrack in the entirety of nixpkgs).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When I disable dependency tracking the build fails.

Output of configure with disable on:

flags:  '--disable-static' '--disable-dependency-tracking' '--prefix=/nix/store/dbpj8hcyh7smryikwkvch8br9gdw2gfy-dante-1.4.2' '--with-libc=libc.dylib' 'CC=clang'
CC: clang (clang)
CFLAGS: -arch i386 -arch x86_64 -g -O2
CPP: clang -E
CPPFLAGS: -DDEBUG=0 -D_FORTIFY_SOURCE=2 -DFD_SETSIZE=65536
LDFLAGS: -arch i386 -arch x86_64
LD_LIBRARY_PATH:
LIBS:
DLIBDEPS:
SOCKDDEPS:
compiler flags:
warning flags:
FEATURES: nopreload sess2 mon-data mon-disconnect
compat: daemon getifaddrs hstrerror inet_pton issetugid memmove pselect seteuid setproctitle sockatmark strlcpy strvis vsyslog
socket options (socket level): SO_BROADCAST SO_DEBUG SO_DONTROUTE SO_KEEPALIVE SO_LINGER SO_OOBINLINE SO_RCVBUF SO_RCVLOWAT SO_RCVTIMEO SO_SNDBUF SO_SNDLOWAT SO_SNDTIMEO SO_TIMESTAMP SO_USELOOPBACK
socket options (ipv4 level): IP_PORTRANGE IP_TOS IP_TTL
socket options (ipv6 level): IPV6_UNICAST_HOPS IPV6_2292DSTOPTS IPV6_2292HOPLIMIT IPV6_2292HOPOPTS IPV6_2292PKTINFO IPV6_2292PKTOPTIONS IPV6_2292RTHDR IPV6_CHECKSUM IPV6_IPSEC_POLICY IPV6_JOIN_GROUP IPV6_LEAVE_GROUP IPV6_PORTRANGE IPV6_TCLASS IPV6_UNICAST_HOPS IPV6_V6ONLY
socket options (tcp level): TCP_KEEPCNT TCP_KEEPINTVL TCP_MAXSEG TCP_NODELAY TCP_NOOPT TCP_NOPUSH
socket options (udp level):
socket option arguments: IP_PORTRANGE(IP_PORTRANGE_DEFAULT IP_PORTRANGE_HIGH IP_PORTRANGE_LOW) IP_TOS.DSCP(AF11 AF12 AF13 AF21 AF22 AF23 AF31 AF32 AF33 AF41 AF42 AF43 CS0 CS1 CS2 CS3 CS4 CS5 CS6 CS7 EF) IP_TOS.PREC(NETCONTROL INTERNETCONTROL CRITIC_ECP FLASHOVERRIDE FLASH IMMEDIATE PRIORITY ROUTINE) IP_TOS.TOS(LOWDELAY THROUGHPUT RELIABILITY)
checking that generated files are newer than configure... done
configure: creating ./config.status
[...]

                     Configure status:

Client:            Enabled
Server:            Enabled
Preloading:        Client preloading might not be reliable on this platform
select:            Unexpected select behaviour on unconnected sockets,
                   operations on nonblocking sockets might fail
                   on this platform when using socksify
Libwrap:           Disabled, tcpd.h missing
BSD Auth:          Disabled, usable bsd_auth.h not found
PAM:               Disabled, security/pam_appl.h missing
GSSAPI:            Not found/disabled
KRB5:              Not found/disabled
SASL:              Not found/disabled
UPNP:              Not found/disabled
Compatability:     daemon getifaddrs hstrerror inet_pton issetugid memmove pselect seteuid setproctitle sockatmark strlcpy strvis vsyslog

                     Modules:

redirect:          Not found
bandwidth:         Not found
ldap:              Not found

Output of configure with dependency tracking enabled:

flags:  '--disable-static' '--prefix=/nix/store/h2gzqlciqfq779apy4m2sqznhl7l1irn-dante-1.4.2' '--with-libc=libc.dylib' 'CC=clang'
CC: clang (clang)
CFLAGS: -g -O2 -pipe
CPP: clang -E
CPPFLAGS: -DDEBUG=0 -D_FORTIFY_SOURCE=2 -DFD_SETSIZE=65536 -I/nix/store/q1f1pmchkkx8yh9g0wz7gm68n2g0qq6h-libkrb5-1.17-dev/include -I/nix/store/q1f1pmchkkx8yh9g0wz7gm68n2g0qq6h-libkrb5-1.17-dev/include -I/usr/include -I/usr/include
LDFLAGS: -L/nix/store/hy8x74slxcmzihwj5sx9qngwl2scs34z-libkrb5-1.17/lib -dynamic -Wl,-search_paths_first -L/nix/store/hy8x74slxcmzihwj5sx9qngwl2scs34z-libkrb5-1.17/lib -dynamic -Wl,-search_paths_first -L/usr/lib -L/usr/lib
LD_LIBRARY_PATH:
LIBS: -lpam  -lresolv   -lgssapi_krb5 -lkrb5 -lk5crypto -lcom_err  -lresolv
DLIBDEPS: -L/nix/store/hy8x74slxcmzihwj5sx9qngwl2scs34z-libkrb5-1.17/lib -dynamic -Wl,-search_paths_first -lgssapi_krb5 -lkrb5 -lk5crypto -lcom_err -lminiupnpc  -lresolv   -lgssapi_krb5 -lkrb5 -lk5crypto -lcom_err  -lresolv
SOCKDDEPS: -L/nix/store/hy8x74slxcmzihwj5sx9qngwl2scs34z-libkrb5-1.17/lib -dynamic -Wl,-search_paths_first -lgssapi_krb5 -lkrb5 -lk5crypto -lcom_err -lminiupnpc  -lresolv   -lgssapi_krb5 -lkrb5 -lk5crypto -lcom_err  -lresolv   -lm
compiler flags: -pipe
warning flags:
FEATURES: gssapi upnp pam sess2 mon-data mon-disconnect preload
compat: setproctitle
socket options (socket level): SO_BROADCAST SO_DEBUG SO_DONTROUTE SO_KEEPALIVE SO_LINGER SO_OOBINLINE SO_RCVBUF SO_RCVLOWAT SO_RCVTIMEO SO_SNDBUF SO_SNDLOWAT SO_SNDTIMEO SO_TIMESTAMP SO_USELOOPBACK
socket options (ipv4 level): IP_PORTRANGE IP_TOS IP_TTL
socket options (ipv6 level): IPV6_UNICAST_HOPS IPV6_2292DSTOPTS IPV6_2292HOPLIMIT IPV6_2292HOPOPTS IPV6_2292PKTINFO IPV6_2292PKTOPTIONS IPV6_2292RTHDR IPV6_CHECKSUM IPV6_IPSEC_POLICY IPV6_JOIN_GROUP IPV6_LEAVE_GROUP IPV6_PORTRANGE IPV6_TCLASS IPV6_UNICAST_HOPS IPV6_V6ONLY
socket options (tcp level): TCP_KEEPCNT TCP_KEEPINTVL TCP_MAXSEG TCP_NODELAY TCP_NOOPT TCP_NOPUSH
socket options (udp level):
socket option arguments: IP_PORTRANGE(IP_PORTRANGE_DEFAULT IP_PORTRANGE_HIGH IP_PORTRANGE_LOW) IP_TOS.DSCP(AF11 AF12 AF13 AF21 AF22 AF23 AF31 AF32 AF33 AF41 AF42 AF43 CS0 CS1 CS2 CS3 CS4 CS5 CS6 CS7 EF) IP_TOS.PREC(NETCONTROL INTERNETCONTROL CRITIC_ECP FLASHOVERRIDE FLASH IMMEDIATE PRIORITY ROUTINE) IP_TOS.TOS(LOWDELAY THROUGHPUT RELIABILITY)
checking that generated files are newer than configure... done
configure: creating ./config.status
[...]

                     Configure status:

Client:            Enabled
Server:            Enabled
Preloading:        Enabled
Libwrap:           Disabled, tcpd.h missing
BSD Auth:          Disabled, usable bsd_auth.h not found
PAM:               Enabled
GSSAPI:            Enabled
KRB5:              Enabled
SASL:              Enabled
UPNP:              Enabled
Compatability:     setproctitle

                     Modules:

redirect:          Not found
bandwidth:         Not found
ldap:              Not found

This seems to take in some paths from system-wide /usr which is not optimal in nix, right?

The full output with dependency tracking disabled: https://hastebin.com/basuhoyayu.coffeescript

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, /usr paths are worrying. I know some measure of impurity is required on darwin, but I don't think it's to that degree. I'm really not sure why dependency tracking is causing this degree of changes.

Can you give it a try without dontAddDisableDepTrack, and with the below changes? This should regenerate the autoconf scripts.

stdenv.mkDerivation rec {
  ...
  postPatch = ''
    sed -i -e '/^PATH/d' include/redefgen.sh
  '';
  nativeBuildInputs = [
    autoreconfHook
  ];
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As far as I can tell, I get the same errors: https://gist.github.com/arnarg/76e54e6d5a503f86a3808f4451b2e55c

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would just add dontAddDisableDepTrack=true unconditionally. Using lib.optional gives you a list and we don't want that here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But do we want that on platforms that don't need it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Shados @matthewbauer What should I do regarding this?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not needed on non-Darwin platforms, so unconditionally setting it to true would be undesirable. Using a list here (via lib.optional) is harmless, but unnecessary: dontAddDisableDepTrack = stdenv.isDarwin; should be fine. I still think something strange is going on with that, but I don't have any realistic way of looking into it, as I don't have access to any Darwin systems.

@ofborg ofborg bot requested a review from Shados September 10, 2019 15:02
@lheckemann lheckemann added this to the 20.03 milestone Sep 12, 2019
@Mic92
Copy link
Member

Mic92 commented Sep 23, 2019

@GrahamcOfBorg build dante

@Mic92
Copy link
Member

Mic92 commented Sep 23, 2019

@GrahamcOfBorg build dante

@Mic92 Mic92 merged commit 9dc1f08 into NixOS:master Sep 23, 2019
@arnarg arnarg deleted the fix-dante-darwin branch September 24, 2019 10:25
This was referenced Nov 18, 2019
Closed
Merged
r0man pushed a commit to asahi-guix/guix that referenced this pull request Dec 23, 2023
@McSinyx @civodul
gnu: dante: Enable preloading support.
4a1b383 
Fixes <https://issues.guix.gnu.org/63250>.

Dante uses dlopen which doesn't work with libc.so but libc.so.6.
This fixes socksify, which is patched to skip invoking uname needlessly.

* gnu/packages/networking.scm (dante): Enable preloading support.
* gnu/packages/patches/dante-non-darwin.patch: New file.
* gnu/local.mk (dist_patch_DATA): Add it.

References: NixOS/nixpkgs#68418 (comment)
Change-Id: I03a5b6b6e11cf6a0453a4968ff00c6dd627249a8
Signed-off-by: Ludovic Courtès <ludo@gnu.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@matthewbauer matthewbauer matthewbauer left review comments

@Mic92 Mic92 Mic92 left review comments

@LnL7 LnL7 LnL7 left review comments

@Shados Shados Awaiting requested review from Shados

Assignees
No one assigned
Labels
6.topic: darwin Running or building packages on Darwin 8.has: package (new) 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10 12. first-time contribution
Projects
None yet
Milestone
20.03
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@arnarg @Mic92 @matthewbauer @Shados @LnL7 @lheckemann @Janik-Haag