Skip to content

Commit

Permalink
Replace usage of stdenvNoCC.
Browse files Browse the repository at this point in the history
- For Linux, the real fix is `hardeningDisable` of "fortify";
- For macOS, the real fix is setting the C and CXX compilers in `desktop/build.sh` as well as disabling CMake install step;
- Unfortunately for iOS we still need stdenvNoCC (or find a way to use GNU's ld instead of Apple's)

Signed-off-by: Pedro Pombeiro <pombeirp@users.noreply.github.com>
  • Loading branch information
Pedro Pombeiro committed Mar 27, 2019
1 parent e8f762e commit 6eb806d
Show file tree
Hide file tree
Showing 5 changed files with 21 additions and 5 deletions.
5 changes: 3 additions & 2 deletions default.nix
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,8 @@ with pkgs;
"ios" = true;
"" = true;
}.${target-os} or false;
_stdenv = stdenvNoCC; # TODO: Try to use stdenv for Darwin
# TODO: Try to use stdenv for iOS. The problem is with building iOS as the build is trying to pass parameters to Apple's ld that are meant for GNU's ld (e.g. -dynamiclib)

This comment has been minimized.

Copy link
@hlolli

hlolli Apr 2, 2019

Contributor

Have you checked this out? https://nixos.wiki/wiki/Using_Clang_instead_of_GCC For clang compilers you can use stdenv = pkgs.clangStdenv;

This comment has been minimized.

Copy link
@pedropombeiro

pedropombeiro Apr 2, 2019

Contributor

Will try pushing a commit to CI and see how that works out.

This comment has been minimized.

Copy link
@pedropombeiro

pedropombeiro Apr 2, 2019

Contributor

Same error:

16:09:55  ▸ Linking RSKImageCropper
16:09:55  
16:09:55  ❌  ld: unknown option: -dynamiclib
16:09:55  
16:09:55  
16:09:55  ▸ Linking RSKImageCropper
16:09:55  
16:09:55  ❌  ld: unknown option: -dynamiclib

This comment has been minimized.

Copy link
@hlolli

hlolli via email Apr 2, 2019

Contributor

This comment has been minimized.

Copy link
@hlolli

hlolli Apr 2, 2019

Contributor

How can I reproduce this error?

This comment has been minimized.

Copy link
@pedropombeiro

pedropombeiro Apr 2, 2019

Contributor

I'm not very familiar with iOS builds. CI uses Fastlane to build the release, and the Makefile tells you to use XCode (which is slightly broken atm), so I don't have a way to reproduce without trying to set up the Fastlane environment locally. I'm planning on finding some time with an iOS developer in the team in the next few days so we can work out the local iOS build kinks.

This comment has been minimized.

Copy link
@hlolli

hlolli Apr 2, 2019

Contributor

One thing I don't understand, is the argument target-os, because XCode is only platform dependant (darwin) but shouldn't be an issue for target. We should be able to create a nix-shell that targets iOS and Android trough both Linux and Darwin platforms? Just wonder if this target-os arg is useful at all, it isn't provided when the nix-shell is called in the Makefile.

This comment has been minimized.

Copy link
@pedropombeiro

pedropombeiro Apr 2, 2019

Contributor

It's more of a sanity check that we are only including exactly what is required to build for each platform. Right now it is used in Jenkins. If Jenkins is building iOS, it won't try to install Android SDK and all that, for instance. For make shell we install everything. I'm sure there are better ways to organize the code, but I haven't yet spent time refactoring that.

This comment has been minimized.

Copy link
@hlolli

hlolli Apr 2, 2019

Contributor

That's how I understood it, just all the dependencies in one environment. And to make the shell open faster, you may want to look into some way of creating a cache, like https://cachix.org/

In the end I'm just looking to get status-react running so I can solve some clojurescript stuff with @yenda. For that reason, maybe you can put gcc back in there :D (and look again at my PR). If I'm bit annoying, I can always just make my own nix-expression to start status-react env. Always very willing to help!

_stdenv = if target-os == "ios" || target-os == "" then stdenvNoCC else stdenv;
statusDesktop = callPackage ./scripts/lib/setup/nix/desktop { inherit target-os; stdenv = _stdenv; };
statusMobile = callPackage ./scripts/lib/setup/nix/mobile { inherit target-os; stdenv = _stdenv; };
nodeInputs = import ./scripts/lib/setup/nix/global-node-packages/output {
Expand Down Expand Up @@ -53,7 +54,6 @@ with pkgs;
wget
] ++ nodePkgs
++ lib.optional isDarwin cocoapods
++ lib.optional isLinux gcc7

This comment has been minimized.

Copy link
@hlolli

hlolli Apr 2, 2019

Contributor

By removing the c-compiler I get

[nix-shell:~/forks/status-react]$ make run-desktop 
scripts/prepare-for-platform.sh desktop
Finished!
yarn install v1.13.0
[1/4] Resolving packages...
success Already up-to-date.
$ patch-package
patch-package: Applying patches...
metro@0.48.5 ✔
metro-config@0.48.5 ✔
Done in 0.76s.
Finished!
npx react-native run-desktop
Found desktop external modules: node_modules/react-native-languages/desktop,node_modules/react-native-config/desktop,node_modules/react-native-fs/desktop,node_modules/react-native-http-bridge/desktop,node_modules/react-native-webview-bridge/desktop,node_modules/react-native-keychain/desktop,node_modules/react-native-securerandom/desktop,modules/react-native-status/desktop,modules/react-native-desktop-linking/desktop,modules/react-native-desktop-menu/desktop,modules/react-native-desktop-config/desktop,modules/react-native-desktop-shortcuts/desktop,modules/react-native-desktop-notification/desktop,node_modules/google-breakpad
Found desktop JS bundle path: undefined
Found desktop fonts: ../../../../../resources/fonts/Inter-BlackItalic.otf,../../../../../resources/fonts/Inter-Black.otf,../../../../../resources/fonts/Inter-BoldItalic.otf,../../../../../resources/fonts/Inter-Bold.otf,../../../../../resources/fonts/Inter-ExtraBoldItalic.otf,../../../../../resources/fonts/Inter-ExtraBold.otf,../../../../../resources/fonts/Inter-ExtraLight-BETA.otf,../../../../../resources/fonts/Inter-ExtraLightItalic-BETA.otf,../../../../../resources/fonts/Inter-Italic.otf,../../../../../resources/fonts/Inter-Light-BETA.otf,../../../../../resources/fonts/Inter-LightItalic-BETA.otf,../../../../../resources/fonts/Inter-MediumItalic.otf,../../../../../resources/fonts/Inter-Medium.otf,../../../../../resources/fonts/Inter-Regular.otf,../../../../../resources/fonts/Inter-SemiBoldItalic.otf,../../../../../resources/fonts/Inter-SemiBold.otf,../../../../../resources/fonts/Inter-Thin-BETA.otf,../../../../../resources/fonts/Inter-ThinItalic-BETA.otf
Building the app...

Command failed: ./build.sh -e "node_modules/react-native-languages/desktop;node_modules/react-native-config/desktop;node_modules/react-native-fs/desktop;node_modules/react-native-http-bridge/desktop;node_modules/react-native-webview-bridge/desktop;node_modules/react-native-keychain/desktop;node_modules/react-native-securerandom/desktop;modules/react-native-status/desktop;modules/react-native-desktop-linking/desktop;modules/react-native-desktop-menu/desktop;modules/react-native-desktop-config/desktop;modules/react-native-desktop-shortcuts/desktop;modules/react-native-desktop-notification/desktop;node_modules/google-breakpad" -f "../../../../../resources/fonts/Inter-BlackItalic.otf;../../../../../resources/fonts/Inter-Black.otf;../../../../../resources/fonts/Inter-BoldItalic.otf;../../../../../resources/fonts/Inter-Bold.otf;../../../../../resources/fonts/Inter-ExtraBoldItalic.otf;../../../../../resources/fonts/Inter-ExtraBold.otf;../../../../../resources/fonts/Inter-ExtraLight-BETA.otf;../../../../../resources/fonts/Inter-ExtraLightItalic-BETA.otf;../../../../../resources/fonts/Inter-Italic.otf;../../../../../resources/fonts/Inter-Light-BETA.otf;../../../../../resources/fonts/Inter-LightItalic-BETA.otf;../../../../../resources/fonts/Inter-MediumItalic.otf;../../../../../resources/fonts/Inter-Medium.otf;../../../../../resources/fonts/Inter-Regular.otf;../../../../../resources/fonts/Inter-SemiBoldItalic.otf;../../../../../resources/fonts/Inter-SemiBold.otf;../../../../../resources/fonts/Inter-Thin-BETA.otf;../../../../../resources/fonts/Inter-ThinItalic-BETA.otf"
CMake Error at CMakeLists.txt:27 (project):
  The CMAKE_C_COMPILER:

    gcc

  is not a full path and was not found in the PATH.

  Tell CMake where to find the compiler by setting either the environment
  variable "CC" or the CMake cache entry CMAKE_C_COMPILER to the full path to
  the compiler, or to the compiler name if it is in the PATH.


CMake Error at CMakeLists.txt:27 (project):
  The CMAKE_CXX_COMPILER:

    g++

  is not a full path and was not found in the PATH.

  Tell CMake where to find the compiler by setting either the environment
  variable "CXX" or the CMake cache entry CMAKE_CXX_COMPILER to the full path
  to the compiler, or to the compiler name if it is in the PATH.




Error: Command failed: ./build.sh -e "node_modules/react-native-languages/desktop;node_modules/react-native-config/desktop;node_modules/react-native-fs/desktop;node_modules/react-native-http-bridge/desktop;node_modules/react-native-webview-bridge/desktop;node_modules/react-native-keychain/desktop;node_modules/react-native-securerandom/desktop;modules/react-native-status/desktop;modules/react-native-desktop-linking/desktop;modules/react-native-desktop-menu/desktop;modules/react-native-desktop-config/desktop;modules/react-native-desktop-shortcuts/desktop;modules/react-native-desktop-notification/desktop;node_modules/google-breakpad" -f "../../../../../resources/fonts/Inter-BlackItalic.otf;../../../../../resources/fonts/Inter-Black.otf;../../../../../resources/fonts/Inter-BoldItalic.otf;../../../../../resources/fonts/Inter-Bold.otf;../../../../../resources/fonts/Inter-ExtraBoldItalic.otf;../../../../../resources/fonts/Inter-ExtraBold.otf;../../../../../resources/fonts/Inter-ExtraLight-BETA.otf;../../../../../resources/fonts/Inter-ExtraLightItalic-BETA.otf;../../../../../resources/fonts/Inter-Italic.otf;../../../../../resources/fonts/Inter-Light-BETA.otf;../../../../../resources/fonts/Inter-LightItalic-BETA.otf;../../../../../resources/fonts/Inter-MediumItalic.otf;../../../../../resources/fonts/Inter-Medium.otf;../../../../../resources/fonts/Inter-Regular.otf;../../../../../resources/fonts/Inter-SemiBoldItalic.otf;../../../../../resources/fonts/Inter-SemiBold.otf;../../../../../resources/fonts/Inter-Thin-BETA.otf;../../../../../resources/fonts/Inter-ThinItalic-BETA.otf"
CMake Error at CMakeLists.txt:27 (project):
  The CMAKE_C_COMPILER:

    gcc

  is not a full path and was not found in the PATH.

  Tell CMake where to find the compiler by setting either the environment
  variable "CC" or the CMake cache entry CMAKE_C_COMPILER to the full path to
  the compiler, or to the compiler name if it is in the PATH.


CMake Error at CMakeLists.txt:27 (project):
  The CMAKE_CXX_COMPILER:

    g++

  is not a full path and was not found in the PATH.

  Tell CMake where to find the compiler by setting either the environment
  variable "CXX" or the CMake cache entry CMAKE_CXX_COMPILER to the full path
  to the compiler, or to the compiler name if it is in the PATH.



    at ChildProcess.exithandler (child_process.js:281:12)
    at emitTwo (events.js:126:13)
    at ChildProcess.emit (events.js:214:7)
    at maybeClose (internal/child_process.js:915:16)
    at Process.ChildProcess._handle.onexit (internal/child_process.js:209:5)

make: *** [Makefile:146: _run-desktop] Error 1

This comment has been minimized.

Copy link
@pedropombeiro

pedropombeiro Apr 8, 2019

Contributor

In the feature/shell-nix branch I'm bringing back gcc7, fwiw.

++ lib.optional targetDesktop statusDesktop.buildInputs
++ lib.optional targetMobile statusMobile.buildInputs;
shellHook =
Expand All @@ -68,4 +68,5 @@ with pkgs;
fi
set +e
'';
hardeningDisable = statusDesktop.hardeningDisable;
}
10 changes: 9 additions & 1 deletion desktop/build.sh
Original file line number Diff line number Diff line change
Expand Up @@ -34,4 +34,12 @@ echo "build.sh desktop fonts: "$desktopFonts
rm -rf CMakeFiles CMakeCache.txt cmake_install.cmake Makefile

# Build project
cmake -DCMAKE_BUILD_TYPE=Debug -DEXTERNAL_MODULES_DIR="$ExternalModulesPaths" -DJS_BUNDLE_PATH="$JsBundlePath" -DDESKTOP_FONTS="$desktopFonts" . && make
cmake -DCMAKE_BUILD_TYPE=Debug \
-DEXTERNAL_MODULES_DIR="$ExternalModulesPaths" \
-DJS_BUNDLE_PATH="$JsBundlePath" \
-DDESKTOP_FONTS="$desktopFonts" \
-DCMAKE_C_COMPILER='gcc'\
-DCMAKE_CXX_COMPILER='g++' \
-DCMAKE_INSTALL_COMPONENT='' \
. && \
make
3 changes: 3 additions & 0 deletions scripts/build-desktop.sh
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,9 @@ if is_windows_target; then
CMAKE_EXTRA_FLAGS="$CMAKE_EXTRA_FLAGS -DCMAKE_C_COMPILER='x86_64-w64-mingw32.shared-gcc'"
CMAKE_EXTRA_FLAGS="$CMAKE_EXTRA_FLAGS -DCMAKE_CXX_COMPILER='x86_64-w64-mingw32.shared-g++'"
CMAKE_EXTRA_FLAGS="$CMAKE_EXTRA_FLAGS -DCMAKE_RC_COMPILER='x86_64-w64-mingw32.shared-windres'"
elif is_macos; then
CMAKE_EXTRA_FLAGS="$CMAKE_EXTRA_FLAGS -DCMAKE_C_COMPILER='gcc'"
CMAKE_EXTRA_FLAGS="$CMAKE_EXTRA_FLAGS -DCMAKE_CXX_COMPILER='g++'"
fi

STATUSREACTPATH="$(cd "$SCRIPTPATH" && cd '..' && pwd)"
Expand Down
5 changes: 3 additions & 2 deletions scripts/lib/setup/nix/desktop/default.nix
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,8 @@ in
'' else ''
export QT_PATH="${qt5.full}"
export PATH="${stdenv.lib.makeBinPath [ qt5.full ]}:$PATH"
'') + (lib.optionalString isDarwin ''
export MACOSX_DEPLOYMENT_TARGET=10.9
'');

# Fixes Cgo related build failures (see https://github.com/NixOS/nixpkgs/issues/25959 )
hardeningDisable = linuxPlatform.hardeningDisable;
}
3 changes: 3 additions & 0 deletions scripts/lib/setup/nix/desktop/linux/default.nix
Original file line number Diff line number Diff line change
Expand Up @@ -11,4 +11,7 @@ let
in
{
buildInputs = [ appimagekit linuxdeployqt patchelf baseImage ];

# Fixes Cgo related build failures (see https://github.com/NixOS/nixpkgs/issues/25959 )
hardeningDisable = [ "fortify" ];
}

0 comments on commit 6eb806d

Please sign in to comment.