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

ttyrec: fix build on Darwin #27500

Merged
merged 2 commits into from
Jul 19, 2017
Merged

ttyrec: fix build on Darwin #27500

merged 2 commits into from
Jul 19, 2017

Conversation

therealpxc
Copy link
Contributor

Motivation for this change

ttyrec doesn't build on Darwin

Things done
  • Tested using sandboxing
    (nix.useSandbox on NixOS,
    or option build-use-sandbox in nix.conf
    on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • Linux
  • 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/)
  • Fits CONTRIBUTING.md.

Sorry, something went wrong.

@Mic92 Mic92 merged commit 010163d into NixOS:master Jul 19, 2017
@@ -11,7 +11,7 @@ stdenv.mkDerivation rec {

patches = [ ./clang-fixes.patch ];

makeFlags = [ "CFLAGS=-DSVR4" ]
makeFlags = stdenv.lib.optional buildPlatform.isLinux "CFLAGS=-DSVR4"
Copy link
Member

Choose a reason for hiding this comment

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

Is this really a buildPlatform thing? cc @Ericson2314 who knows best 😄

Copy link
Member

Choose a reason for hiding this comment

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

I think it should be hostPlatform though.

Copy link
Member

@Ericson2314 Ericson2314 Jul 20, 2017

Choose a reason for hiding this comment

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

When in doubt, hostPlatform. We can't really test to prove until I get more cross stuff working, but yeah let's change 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.

Sorry I mixed this up, guys. I'll reread the docs before the next time I try to use it.

Copy link
Member

@Ericson2314 Ericson2314 Jul 20, 2017

Choose a reason for hiding this comment

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

@therealpxc I am mainly pleased people are figuring out about *Platform at all :). This is new to most, so no worries.

[Also, one can now do stdenv.*Platform.is*, if you don't want to add the additional parameter.]

bugworm pushed a commit to bugworm/nixpkgs that referenced this pull request Jul 23, 2017
* ttyrec: fix build on Darwin

* ttyrec: remove pointless empty list
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants