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.

Sign up for GitHub

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

Fix Synergy building on macOS #43523

Merged
merged 1 commit into from
Jul 21, 2018
Merged

Fix Synergy building on macOS #43523

merged 1 commit into from
Jul 21, 2018

Conversation

Enzime
Copy link
Member

@Enzime Enzime commented Jul 14, 2018

Update gtest and gmock to fix clang compiler issues, as well as patch CMakeLists.txt in multiple places to fix other issues on macOS.

Things done
  • Tested without sandboxing on macOS
  • 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 nox --run "nox-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)
  • Fits CONTRIBUTING.md.

@Enzime
Copy link
Member Author

Enzime commented Jul 14, 2018

@GrahamcOfBorg build synergy

adisbladis
adisbladis requested changes Jul 15, 2018
View reviewed changes
pkgs/applications/misc/synergy/default.nix
@@ -14,17 +12,30 @@ stdenv.mkDerivation rec {
sha256 = "0ksgr9hkf09h54572p7k7b9zkfhcdb2g2d5x7ixxn028y8i3jyp3";
};

patches = [ ./openssl-1.1.patch ];
patches = [./openssl-1.1.patch ./update_gtest_gmock.patch
Copy link
Member

Choose a reason for hiding this comment

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

Please use fetchpatch to avoid checking in patches to nixpkgs.

Copy link
Member Author

Choose a reason for hiding this comment

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

If this is a patch I made for Nixpkgs, where should I host it?

Copy link
Member

Choose a reason for hiding this comment

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

My bad. I didn't notice these patches were made by you.
I assumed they were from upstream.

pkgs/applications/misc/synergy/default.nix Outdated

patch_gcc6 = fetchpatch {
url = https://raw.githubusercontent.com/gentoo/gentoo/20e2bff3697ebf5f291e9907b34aae3074a36b53/dev-cpp/gmock/files/gmock-1.7.0-gcc6.patch;
sha256 = "0j3f381x1lf8qci9pfv6mliggl8qs2w05v5lw3rs3gn7aibg174d";
};

gmock_zip = fetchurl {
Copy link
Member

Choose a reason for hiding this comment

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

I don't like this. How are these files supposed to be maintained and updated?
If this is just a temporary measure and expected to be removed in the next release please add a comment stating so.

Copy link
Member Author

Choose a reason for hiding this comment

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

Added a comment noting that it was only related to this version of Synergy.

pkgs/applications/misc/synergy/default.nix Outdated
sed -i -e '/HAVE_X11_EXTENSIONS_XRANDR_H/c \
set(HAVE_X11_EXTENSIONS_XRANDR_H true)
' CMakeLists.txt
'';

cmakeFlags = [ "-DOSX_TARGET_MAJOR=10" "-DOSX_TARGET_MINOR=7" ];
Copy link
Member

Choose a reason for hiding this comment

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

These cmake flags should only be applied on darwin.

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed.

@adisbladis
Copy link
Member

cc @aszlig

@adisbladis
Copy link
Member

@Enzime To use @GrahamcOfBorg you need to be in the whitelist: https://github.com/NixOS/ofborg#trusted-users-vs-known-users

@GrahamcOfBorg GrahamcOfBorg added the 6.topic: darwin Running or building packages on Darwin label Jul 15, 2018
LnL7
LnL7 reviewed Jul 15, 2018
View reviewed changes
pkgs/applications/misc/synergy/default.nix
sed -i -e '/HAVE_X11_EXTENSIONS_XRANDR_H/c \
set(HAVE_X11_EXTENSIONS_XRANDR_H true)
' CMakeLists.txt
'';

cmakeFlags = lib.optionals stdenv.isDarwin [ "-DOSX_TARGET_MAJOR=10" "-DOSX_TARGET_MINOR=7" ];
Copy link
Member

Choose a reason for hiding this comment

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

What's this for? We already set MACOSX_DEPLOYMENT_TARGET in the stdenv.

Copy link
Member Author

Choose a reason for hiding this comment

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

Synergy's CMakeLists.txt expects these variables, and doesn't infer from MACOSX_DEPLOYMENT_TARGET.

pkgs/applications/misc/synergy/default.nix Outdated
cmake xlibsWrapper libX11 libXi libXtst libXrandr xinput curl openssl
];
cmake curl openssl
] ++ lib.optionals stdenv.isDarwin (with darwin.apple_sdk.frameworks;
Copy link
Member

Choose a reason for hiding this comment

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

nit: do this in with callPackage overrides in all-packages.nix

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed.

@Enzime
Fix Synergy building on macOS
fe69ee8 
Update gtest and gmock to fix clang compiler issues, as well as patch
CMakeLists.txt in multiple places to fix other issues on macOS.
@grahamc
Copy link
Member

grahamc commented Jul 17, 2018

@GrahamcOfBorg build synergy

@GrahamcOfBorg
Copy link

Success on x86_64-linux (full log)

Attempted: synergy

Partial log (click to expand)

installing
post-installation fixup
shrinking RPATHs of ELF executables and libraries in /nix/store/q85h06ac7iba7smyp4mx7jw76nr8fp4m-synergy-1.8.8
shrinking /nix/store/q85h06ac7iba7smyp4mx7jw76nr8fp4m-synergy-1.8.8/bin/synergyc
shrinking /nix/store/q85h06ac7iba7smyp4mx7jw76nr8fp4m-synergy-1.8.8/bin/synergys
shrinking /nix/store/q85h06ac7iba7smyp4mx7jw76nr8fp4m-synergy-1.8.8/bin/synergyd
strip is /nix/store/90vmpr41dzsx350k5argycaf693hnl1l-binutils-2.30/bin/strip
stripping (with command strip and flags -S) in /nix/store/q85h06ac7iba7smyp4mx7jw76nr8fp4m-synergy-1.8.8/bin 
patching script interpreter paths in /nix/store/q85h06ac7iba7smyp4mx7jw76nr8fp4m-synergy-1.8.8
checking for references to /tmp/nix-build-synergy-1.8.8.drv-0 in /nix/store/q85h06ac7iba7smyp4mx7jw76nr8fp4m-synergy-1.8.8...

@GrahamcOfBorg
Copy link

Success on x86_64-darwin (full log)

Attempted: synergy

Partial log (click to expand)

[----------] 2 tests from ServerArgsParsingTests (0 ms total)

[----------] Global test environment tear-down
[==========] 139 tests from 12 test cases ran. (14 ms total)
[  PASSED  ] 139 tests.
installing
post-installation fixup
strip is /nix/store/8axizw5mf6dx7wip65nbkyrmlkhmjhc5-cctools-binutils-darwin/bin/strip
stripping (with command strip and flags -S) in /nix/store/11iybbpfjzm39wlir1l6fpb2b7djid73-synergy-1.8.8/bin
patching script interpreter paths in /nix/store/11iybbpfjzm39wlir1l6fpb2b7djid73-synergy-1.8.8

@GrahamcOfBorg
Copy link

Success on aarch64-linux (full log)

Attempted: synergy

Partial log (click to expand)

installing
post-installation fixup
shrinking RPATHs of ELF executables and libraries in /nix/store/myk6kndhr2ska9s7x25pkhlqbh0w6br0-synergy-1.8.8
shrinking /nix/store/myk6kndhr2ska9s7x25pkhlqbh0w6br0-synergy-1.8.8/bin/synergyd
shrinking /nix/store/myk6kndhr2ska9s7x25pkhlqbh0w6br0-synergy-1.8.8/bin/synergys
shrinking /nix/store/myk6kndhr2ska9s7x25pkhlqbh0w6br0-synergy-1.8.8/bin/synergyc
strip is /nix/store/7iyn7gn33i7xxjgmwf25k20246y6nd9d-binutils-2.30/bin/strip
stripping (with command strip and flags -S) in /nix/store/myk6kndhr2ska9s7x25pkhlqbh0w6br0-synergy-1.8.8/bin
patching script interpreter paths in /nix/store/myk6kndhr2ska9s7x25pkhlqbh0w6br0-synergy-1.8.8
checking for references to /build in /nix/store/myk6kndhr2ska9s7x25pkhlqbh0w6br0-synergy-1.8.8...

@LnL7 LnL7 merged commit d4943ea into NixOS:master Jul 21, 2018
@Enzime Enzime deleted the fix/synergy-on-darwin branch July 24, 2018 11:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@adisbladis adisbladis adisbladis requested changes

@LnL7 LnL7 LnL7 approved these changes

Assignees
No one assigned
Labels
6.topic: darwin Running or building packages on Darwin 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@Enzime @adisbladis @grahamc @GrahamcOfBorg @LnL7