dropbox / dbx_build_tools

Dropbox's Bazel rules and tools
Other
208 stars 36 forks source link

Remove use of deprecated `compiler_executable` and `ar_executable`. #21

Closed jsharpe closed 3 years ago

jsharpe commented 3 years ago

These values assume that the compiler and ar exist in the path of the toolchain definition and so for some toolchains incorrectly guess the location of ar / gcc.

Update the configuration to use the cc_common.get_tool_for_action API which returns the tool configured by the toolchain.

jsharpe commented 3 years ago

Thanks @benjaminp. Did you see the second commit I made on this branch? I needed that for building numpy with the intel compiler as one of the build stages seems to search the path for icc and so I needed to add the c compiler's location to the path passed into vpip.

benjaminp commented 3 years ago

Did you see the second commit I made on this branch? I needed that for building numpy with the intel compiler as one of the build stages seems to search the path for icc and so I needed to add the c compiler's location to the path passed into vpip.

No, sorry. That must have happened after I started the import.

jsharpe commented 3 years ago

No problem; open for suggestions as to whether this is the correct thing to do or if there is another solution? It does feel a bit hacky

benjaminp commented 3 years ago

Do you know why icc is different from gcc in this regard?

jsharpe commented 3 years ago

You have to pass a specific config flag to numpy's build system which uses a specific code path that searches for the Intel C++ and Fortran compilers on the path during part of the build despite having passed the compiler explicitly.

I don't think this is the correct solution, so happy to just carry it locally - I should probably vendor numpy and use the local pip rule to do this properly as I can patch up the logic so that it doesn't need the path set then.