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

Are linkopts propagated from copts and/or deps ? #7288

Closed
riless opened this issue Feb 6, 2017 · 8 comments
Closed

Are linkopts propagated from copts and/or deps ? #7288

riless opened this issue Feb 6, 2017 · 8 comments
Labels
stat:awaiting tensorflower Status - Awaiting response from tensorflower type:build/install Build and install issues

Comments

@riless
Copy link

riless commented Feb 6, 2017

The linker complains about not finding -lpthread, while I didn't add this flag to linkopts.

I've checked the executed command, and in fact there is extra flags on it -lz -lpthread ....

Where did they came from ? Is there a workaround for this ?

More details

BUILD file

cc_binary(
    name = "libfoo.so",
    srcs = glob([
         "jni/**/*.cc",
         "jni/**/*.h",
    ]),
    copts = [ "-fexceptions", "-DEIGEN_AVOID_STL_ARRAY",
              "-mfpu=neon", "-std=c++11",
              "-DMIN_LOG_LEVEL=0", "-DTF_LEAN_BINARY",
              "-O2", ],
    linkopts = [
        "-llog",
        "-lm",
    ],
    linkshared = 1,
    deps = [
        "@org_tensorflow//tensorflow/core:android_tensorflow_lib",
        "@boringssl//:crypto",
    ],
)

Command

bazel build -c opt //:libfoo.so --crosstool_top=//external:android/crosstool --cpu=armeabi-v7a --host_crosstool_top=@bazel_tools//tools/cpp:toolchain --verbose_failures --sandbox_debug --strategy=CppLink=standalone

Full error

...
  external/bazel_tools/tools/cpp/link_dynamic_library.sh no ignored ignored ignored external/androidndk/ndk/toolchains/arm-linux-androideabi-4.9/prebuilt/darwin-x86_64/bin/arm-linux-androideabi-gcc -shared -o bazel-out/arm-linux-androideabi-4.9-v7a-gnu-libstdcpp-opt/bin/libfoo.so -Wl,-whole-archive bazel-out/arm-linux-androideabi-4.9-v7a-gnu-libstdcpp-opt/bin/external/org_tensorflow/tensorflow/core/libandroid_tensorflow_lib.lo -Wl,-no-whole-archive -Wl,-whole-archive bazel-out/arm-linux-androideabi-4.9-v7a-gnu-libstdcpp-opt/bin/external/org_tensorflow/tensorflow/core/kernels/libandroid_tensorflow_kernels.lo -Wl,-no-whole-archive -Wl,-whole-archive bazel-out/arm-linux-androideabi-4.9-v7a-gnu-libstdcpp-opt/bin/external/org_tensorflow/tensorflow/core/libandroid_tensorflow_lib_lite.lo -Wl,-no-whole-archive -Wl,-whole-archive bazel-out/arm-linux-androideabi-4.9-v7a-gnu-libstdcpp-opt/bin/external/org_tensorflow/tensorflow/core/libprotos_all_cc.a -Wl,-no-whole-archive -Wl,-whole-archive bazel-out/arm-linux-androideabi-4.9-v7a-gnu-libstdcpp-opt/bin/external/protobuf/libprotobuf.a -Wl,-no-whole-archive -Wl,-whole-archive bazel-out/arm-linux-androideabi-4.9-v7a-gnu-libstdcpp-opt/bin/external/protobuf/libprotobuf_lite.a -Wl,-no-whole-archive -Wl,-whole-archive bazel-out/arm-linux-androideabi-4.9-v7a-gnu-libstdcpp-opt/bin/external/boringssl/libcrypto.a -Wl,-no-whole-archive external/androidndk/ndk/sources/cxx-stl/gnu-libstdc++/4.9/libs/armeabi-v7a/libgnustl_static.a external/androidndk/ndk/sources/cxx-stl/gnu-libstdc++/4.9/libs/armeabi-v7a/libsupc++.a -llog -lm -lz -lpthread -static-libgcc -no-canonical-prefixes '-march=armv7-a' -Wl,--fix-cortex-a8 '--sysroot=external/androidndk/ndk/platforms/android-21/arch-arm'): com.google.devtools.build.lib.shell.BadExitStatusException: Process exited with status 1.
external/androidndk/ndk/toolchains/arm-linux-androideabi-4.9/prebuilt/darwin-x86_64/bin/../lib/gcc/arm-linux-androideabi/4.9.x/../../../../arm-linux-androideabi/bin/ld: error: cannot find -lpthread
collect2: error: ld returned 1 exit status
Target //:libfoo.so failed to build

@riless riless changed the title Are linkopts propagated from copts or deps ? Are linkopts propagated from copts or/and deps ? Feb 6, 2017
@riless riless changed the title Are linkopts propagated from copts or/and deps ? Are linkopts propagated from copts and/or deps ? Feb 6, 2017
@riless
Copy link
Author

riless commented Feb 6, 2017

$ cd tensorflow
$ git log -1
commit ac352bc807410c8a863d0b95202cceeb62ed0f10
Author: gunan <gunan@google.com>
Date:   Sun Feb 5 22:19:11 2017 -0800

    Simplify cuDNN library handling in configure script. (#7269)
    
    cuda_configure.bzl now handles the more complicated operations.

@girving
Copy link
Contributor

girving commented Feb 6, 2017

@jart Have you seen this before?

@girving girving added stat:awaiting tensorflower Status - Awaiting response from tensorflower type:build/install Build and install issues labels Feb 6, 2017
@jart
Copy link
Contributor

jart commented Feb 6, 2017

Since copts and linkopts are viral and propagate to dependencies, -lpthread is most likely being inherited from @boringssl//:crypto. See: https://github.com/google/boringssl/blob/bbcaa15b0647816b9a1a9b9e0d209cd6712f0105/BUILD#L91

If linking against BoringSSL for an Android build is genuinely a good thing to do, then this could be a bug. Hey @petewarden what's the correct way to get SSL functionality for Android code? Should our friend be using a different library?

Unrelated suggestions:

name = "libfoo.so",

Just name it "foo" because when Bazel builds this, it will prefix lib and append .so by default.

copts = [ "-fexceptions", "-DEIGEN_AVOID_STL_ARRAY",
"-mfpu=neon", "-std=c++11",
"-DMIN_LOG_LEVEL=0", "-DTF_LEAN_BINARY",
"-O2", ],

Many of these flags might already be inherited from the @org_tensorflow//tensorflow/core:android_tensorflow_lib dependency.

The -O2 (or maybe -O3) flag is set by default when you say bazel build -c opt.

Bazel should already pass flags like -std=c++11 by default.

Another thing to note is that bazel build -s //blah is good for troubleshooting.

@riless
Copy link
Author

riless commented Feb 6, 2017

@jart Thank you for your answer.

I think a similar question is discussed here. but I don't understand proposed solution, can you help me ?

Just name it "foo" because when Bazel builds this, it will prefix lib and append .so by default.

BUILD:40:18: in linkshared attribute of cc_binary rule //:foo: 'linkshared' used in non-shared library.

Many of these flags might already be inherited from the @org_tensorflow//tensorflow/core:android_tensorflow_lib dependency.

You are right, only -std=c++11 is required here.

@andrewharp
Copy link
Contributor

@riless @jart
-lpthread is not necessary or possible on Android, so it sounds like the solution would be to add another condition for the select statement as in the linked commit protocolbuffers/protobuf#1386:

    linkopts = select({
        ":mac_x86_64": [],
        ":android": [],
        "//conditions:default": ["-lpthread"],
    }),

which would necessitate adding a config setting like so:

config_setting(
    name = "android",
    values = {
        "crosstool_top": "//external:android/crosstool",
    },
    visibility = ["//visibility:public"],
)

The only other workaround I know of that doesn't require editing the other repository is to create a dummy libpthread.so target, but that's pretty hacky.

@riless
Copy link
Author

riless commented Feb 7, 2017

I'd like to go with the dummy libpthread.so target solution.

I don't know how to do this. I've tried to add a target like this.

cc_library(
    name = "pthread",
    srcs = ["third_party/empty.cc"],
    visibility = ["//visibility:public"],
    linkstatic = 1
)

And added it to deps of libfoo.so

cc_binary(
    name = "libfoo.so",
   ...
   deps = [
        ":pthread",
        ...
  ]
)

but I still have the cannot find -lpthread error.

@andrewharp
Copy link
Contributor

andrewharp commented Feb 7, 2017

@riless See cd53f3c#diff-06b90d704990e9b4f5adadb18c837b64

This has since been removed, as obviously for a long-term solution it was preferable to fix the protobuf source instead. If we need boringssl on Android builds we should do the same for that as well.

@riless
Copy link
Author

riless commented Feb 8, 2017

@andrewharp it works now, thank you.

I'll still go with this solution as I want boringssl to be downloaded on build.

@riless riless closed this as completed Feb 8, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stat:awaiting tensorflower Status - Awaiting response from tensorflower type:build/install Build and install issues
Projects
None yet
Development

No branches or pull requests

4 participants