conda-forge / ctng-compilers-feedstock

A conda-smithy repository for ctng-compilers.
BSD 3-Clause "New" or "Revised" License
12 stars 28 forks source link

Let's configure GCC to work straight from the box #90

Closed timsnyder closed 2 years ago

timsnyder commented 2 years ago

Comment:

When we were working with the Continuum folks during the development of conda-build 3, my colleagues and I suggested rather strongly that -I $PREFIX/include and the 4 required linking arguments -Wl,-rpath,$PREFIX/lib -Wl,-rpath-link,$PREFIX/lib -Wl,-disable-new-dtags -L $PREFIX/lib be built into the configuration of GCC so that when someone does a conda install gcc and then tries to build something with gcc it operates like one would expect, treating the conda environment like the 'local' include and lib path and setting things appropriately to use RPATH everywhere.

I don't recall why Ray and Michael stuck with setting *FLAGS in the activation scripts. To their credit, we have been able to get very far with that. However, to continue polishing the end-user experience, I feel that we should still continue to use the activation scripts for building packages but that their use should be optional for users who just want to compile their own code using conda to install the prerequisites of a C/C++ development environment.

Since you guys moved away from ctng, it is probably easier to do this now. Also, it looks like LLVM/clang has the ability to define configuration files and accomplish the equivalent behavior as the spec-file chicanery required to get the RPATHS by default in GCC. Again, I think it's time to make this work.

@conda-forge/ctng-compilers folks, I'm interested to hear your feedback. I have recipe changes building locally that I'm going to Q/A a bit before opening a PR.

timsnyder commented 2 years ago

@isuruf , I see that you enabled binary relocation in https://github.com/conda-forge/ctng-compilers-feedstock/pull/74 but that only fixes RPATH and things like setting --with-local-prefix and --with-spec can end up embedding the PREFIX in binaries in non-RPATH locations that need to be patched on *nix. Similarly in binutils, the ld SEARCH-DIR entries (see $LD --verbose | grep SEARCH_DIR) could benefit from binary_files_with_prefix: true or explicitly being listed in binary_has_prefix_files.

If patching binary files is problematic, then we could get this configuration done by patching the specfile and/or wrapping LD.

I do see that someone has patched the specfile to include -rpath $PREFIX/lib in https://github.com/conda-forge/ctng-compilers-feedstock/blob/9d0bb475945ebe080a219885fefccc03b7f34a71/recipe/install-gcc.sh#L141-L144 but it should probably be guarded inside at least !static and also be accompanied by the -disable_new_dtags i.e. something like '%{!static:%x{-rpath='${PREFIX}'/lib} %x{-rpath-link='${PREFIX}'/lib} %x{-disable-new-dtags}}'. The !static makes sure that we don't add those options when trying to build a static lib and %x builds the options up into what should go to the linker. Since we're patching link_command directly, the %x{} parts aren't really needed.

jakirkham commented 2 years ago

Have you tried the compilers package?

timsnyder commented 2 years ago

Have you tried the compilers package?

Yes. And compilers is a nice way to abstractly be able to install the compilers as an end user but it still relies on having the activation scripts set environment variables that work in many cases but I still feel like it would be beneficial for $PREFIX/lib(-L, -rpath, and -rpath_link), $PREFIX/include and -disable_new_dtags to to be baked into the tools because there are use-cases where it is highly annoying that they aren't.

beckermr commented 2 years ago

I think the original design here was correct. Building the compilers themselves is quite finicky. Some of them needed to be built by hand in the past, for example. At a practical level separating out the flags from the builds is very useful.

We could add patches to embed values from a special flags env var automatically (say CONDA_TOOLCHAIN_FLAGS or whatever) and then provide the correct values in an activation script. This would leave the system flexible but still automated for users who do not use the env vars set currently.

timsnyder commented 2 years ago

I definitely agree that having the FLAGS set via activation scripts is the right way to go for everything that conda-forge is doing and it is definitely the preferred pattern for build systems IMHO. However, there are many things I have run into since conda-build 3.0 was released that "just work" when a non-relocatable GCC toolchain is found but don't when the conda toolchain is. It always boils down to a few small issues:

it would be really useful for only the "functionally required" FLAGS to be baked into the tools as a backstop to cover the only remaining gap between the usability of system-packaged, non-relocatable toolchains and the better ones provided by conda-forge. The annoying thing is that GCC makes it really challenging to just always pass a set of flags. You have to use it's arcane and somewhat confusing specfiles so that the flags you pass are only applied in certain situations. For example, if you always pass what we currently have in LDFLAGS to the gcc-driver, then it is always going to think it needs to link something and that will definitely break when you really just want to compile something into an object file (or use the driver to do the 999 other things it can do).

I'm very familiar with how finicky building the toolchains is, especially GCC. I used to have to build it by hand for my team and after moving to using conda for everything else, I convinced my boss at a previous company that we needed to pay Continuum to accelerate conda-build 3.0 and get the toolchains into conda-build.

I don't want to change anything that conda-forge is doing currently with it's recipes and how packages are built. However, I would like to make it possible for large, equally annoying and finicky software stacks to build using the compiler toolchain without having to rework their build systems to introspect whether they are being built using a 'conda' toolchain and thus allow FLAGS to be injected from the environment and possibly filter down the FLAGS into ones that their codebase is alright with accepting.

At a minimum, I want to revisit this topic for myself and document why binary_prefix patching breaks GCC. Possibly see if there is a 'fix' for that by determining if there is a way that binary_prefix patching can be improved in conda either in its application or in automating tests to determine when it can not be applied without breaking the binary in question.

beckermr commented 2 years ago

My solution of a special env var that has the small set of flags you want to pass in it and patching the compiler to read this env var if it is present solves your issue without permanent changes to the compiler or any build system. The compiler itself would read the env var as needed.

timsnyder commented 2 years ago

@beckermr I think it would depend on exactly how and where it was implemented. Do you know where you would make your patch? I'd like to try it.

beckermr commented 2 years ago

I do not know the tool chain well enough to make the patch.

Fwiw we already patch clang to point to a special conda OS X sdk through an env var if it is set.

timsnyder commented 2 years ago

Fair enough. I'll look to see if I can find a spot to make the patch. From what I know about the guts of gcc it would be challenging to take a single env var of command-line options and apply it in a single spot to get the results we want. It could be done with a few lines in the specfile but that doesn't look like a simple list of cmdline options. I'd suggest that we possibly have CONDA_LIBRARY_PATH and CONDA_INCLUDE_PATH that have the following behavior:

Or, we could use the existing ${CONDA_PREFIX}/include and ${CONDA_PREFIX}/lib

beckermr commented 2 years ago

Or, we could use the existing ${CONDA_PREFIX}/include and ${CONDA_PREFIX}/lib

-1 on this implementation. It conflates two different things. We want this option to be totally separate from anything else so that folks can turn it off independently.

isuruf commented 2 years ago

-1 on env vars like that or patching GCC (except adding specs files). conda only env vars and patches are hard to debug and ugly.

First of all, we already have -L$BUILD_PREFIX/lib and -Wl,-rpath,$BUILD_PREFIX/lib added by default. This is not intended for using all conda libraries, but mostly for libgcc_s.so, libstdc++.so, libgfortran.so.

Adding -isystem $BUILD_PREFIX/include for example to cpp: in a specs file would make it have higher priority than the flag we add using *FLAGS which is -isystem $PREFIX/include, but this is not what we want. Also, -isystem $BUILD_PREFIX/include has the annoying habit that the same option added with -I $BUILD_PREFIX/include is ignored and therefore the user has no way to override that if needed. (It's almost the same situation with adding -isystem $BUILD_PREFIX/include in *FLAGS, but the user can override the env variable).

If you can come up with a solution that,

  1. doesn't break conda-build envs (i.e. $BUILD_PREFIX != $PREFIX)
  2. allows the user to override when necessary
  3. No patching GCC except *specs files then I'm all ears.
timsnyder commented 2 years ago

Ok. I'll see what I can come up with. At a minimum, I'll at least have a better understanding of why things are the way they are. Thanks everyone for taking time to read and respond.