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

Can't compile mrtrix3 with conda compilers on macOS #1519

Closed
duncanmmacleod opened this issue Dec 14, 2018 · 10 comments

Comments

Projects
None yet
3 participants
@duncanmmacleod
Copy link

commented Dec 14, 2018

I am trying to build mrtrix3 with conda build (an attempt to build a conda packge for mrtrix3). My build is failing at the ./configure step due to (I think) compiler flags being incorrectly passed directly to the linker (ld). From the configure.log`:

REPORT: Checking for C++11 compliance:

COMPILE /tmp/tmprl18qe94.cpp:
---

#include <cstddef>
struct Base {
    Base (int);
};
struct Derived : Base {
    using Base::Base;
};

int main() {
  Derived D (int); // check for contructor inheritance
  return 0;
}

---
EXEC <<
CMD: x86_64-apple-darwin13.4.0-clang++ -c -std=c++11 -DMRTRIX_BUILD_TYPE="release version" -DMRTRIX_MACOSX -fPIC -mmacosx-version-min=10.9 -march=64 -march=core2 -mtune=haswell -mssse3 -ftree-vectorize -fPIC -fPIE -fstack-protector-strong -O2 -pipe -I/Users/duncan/opt/miniconda3/conda-bld/mrtrix3_1544798506638/_h_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placeho/include -fdebug-prefix-map=${SRC_DIR}=/usr/local/src/conda/${PKG_NAME}-${PKG_VERSION} -fdebug-prefix-map=${PREFIX}=/usr/local/src/conda-prefix /tmp/tmprl18qe94.cpp -o /tmp/tmprl18qe94.o
EXIT: 0
>>

EXEC <<
CMD: /Users/duncan/opt/miniconda3/conda-bld/mrtrix3_1544798506638/_build_env/bin/x86_64-apple-darwin13.4.0-ld /tmp/tmprl18qe94.o -mmacosx-version-min=10.9 -Wl,-pie -Wl,-headerpad_max_install_names -Wl,-dead_strip_dylibs -Wl,-rpath,/Users/duncan/opt/miniconda3/conda-bld/mrtrix3_1544798506638/_h_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placeho/lib -L/Users/duncan/opt/miniconda3/conda-bld/mrtrix3_1544798506638/_h_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placeho/lib -o a.out
EXIT: 1
STDERR:
ld: unknown option: -mmacosx-version-min=10.9

I think the fundamental problem is that a list of indirect linker flags is being built as ld_flags, but is then being passed directly to the ld command-line tool, rather than as arguments to the compiler clang++.

If the flags are to be passed directly, I think the LDFLAGS_LD variable should be used instead of LDFLAGS (according to conda activate settings).

I attach the conda recipe I have created, hopefully that will help people reproduce this error. Roughly this should do it:

$ # install miniconda
$ conda install -c conda-forge conda-build
$ tar -xf mrtrix-conda.tar.gz
$ conda build mrtrix3 -c conda-forge

It's also likely that I just don't understand compiling c/c++ as well as I should and I'm making a trivial error somewhere, so apologies if that is true.

@jdtournier

This comment has been minimized.

Copy link
Member

commented Dec 17, 2018

I'm not sure how this conda thing works, but by default, the configure script assumes the same command for both compiler and linker. The fact that it seems to be invoking a different executable is unexpected. Do you have the LD environment variable set? If so, try unsetting it before invoking ./configure.

Even then, there's a chance that might not be sufficient to avoid the issue. The option it's complaining about is specific to macOS. It's possible that conda's version of clang++ wasn't compiled to support that option...

@duncanmmacleod

This comment has been minimized.

Copy link
Author

commented Dec 17, 2018

@jdtournier, unsetting LD works, but suggests to me that there really is a bug in the ./configure script. Should the handling of LD in ./configure be removed?

@jdtournier

This comment has been minimized.

Copy link
Member

commented Dec 17, 2018

Not sure. The expectation is that the LD environment variable would only be set if actually required - i.e. if the default doesn't work. Your setup is unusual in that the conda environment uses a non-default compiler and linker, and presumably sets these up itself (I assume you didn't set them up yourself?).

But more broadly, you might be right in that we don't expect to invoke the linker directly, but the compiler with the expectation that it would then invoke the linker with the right flags. I expect the linker proper doesn't accept the same flags as the compiler (otherwise what's the point of the -Wl directives?). So maybe we should use a different variable here, other than LD - maybe LINK_CMD or something. Not sure...

@duncanmmacleod

This comment has been minimized.

Copy link
Author

commented Dec 17, 2018

Not sure. The expectation is that the LD environment variable would only be set if actually required - i.e. if the default doesn't work. Your setup is unusual in that the conda environment uses a non-default compiler and linker, and presumably sets these up itself (I assume you didn't set them up yourself?).

Yes, conda includes an activate script that sets a number of compiler environment variables, including LD, LDFLAGS, etc.

But more broadly, you might be right in that we don't expect to invoke the linker directly, but the compiler with the expectation that it would then invoke the linker with the right flags. I expect the linker proper doesn't accept the same flags as the compiler (otherwise what's the point of the -Wl directives?). So maybe we should use a different variable here, other than LD - maybe LINK_CMD or something. Not sure...

I'm almost entirely unfamiliar with how mrtrix3 works, or how it is built, I've just started dabbling in MRI applications as a side project, so I don't want to comment on that.

Since my issue was resolved, should I close this ticket? Or should this be left open until the LD option issue is resolved?

@jdtournier

This comment has been minimized.

Copy link
Member

commented Dec 17, 2018

Thanks, let's leave it open, I'd like to hear back from the others whether they have an opinion on this...

@maxpietsch

This comment has been minimized.

Copy link
Member

commented Dec 17, 2018

Can I ask what the use-case of an mrtrix3 conda package is? The anaconda compiler tools depend on the macOS 10.9 SDK so I am not sure what the advantage of using anaconda is.

We currently explicitly strip anaconda from the PATH so I suspect the build would not work at all if you used anaconda instead of miniconda?

@duncanmmacleod

This comment has been minimized.

Copy link
Author

commented Dec 17, 2018

@maxpietsch, a Conda package is designed to make installing mrtrix3 (and its dependencies) a little easier, rather than every user building from source each time.

Can I ask why you strip anaconda from PATH? The conda package is designed to make distributing and installing mrtrix3 a little easier. I was able to build mrtrix3 just fine even with this restriction, mainly because conda builds use environment variables to set compiler environment variables, and similar.

You are right that the builds require macOS 10.9 SDK, but once built, the packages are forward compatible, so will run on any macOS version to date. One also can get linux and Windows builds with the same package configuration.

In general, if a Conda package is something the mrtrix3 team does not want, I will stop my work now; I believe it would be a good thing in the long run.

@maxpietsch

This comment has been minimized.

Copy link
Member

commented Dec 17, 2018

In general, if a Conda package is something the mrtrix3 team does not want, I will stop my work now; I believe it would be a good thing in the long run.

Not at all, I am not familiar with conda packages and was just curious about what the difference is to the homebrew package. If conda packages allow distributing pre-compiled software (especially if this works with qt) then this might be an alternative way to distribute release candidates.

Can I ask why you strip anaconda from PATH?

We had multiple users report issues for which anaconda turned out to be interfering with the installation. For examples see
http://community.mrtrix.org/search?q=anaconda

@jdtournier

This comment has been minimized.

Copy link
Member

commented Dec 18, 2018

Can I ask why you strip anaconda from PATH?

We had multiple users report issues for which anaconda turned out to be interfering with the installation. For examples see
http://community.mrtrix.org/search?q=anaconda

I might add that we only strip it from the PATH during the configure and build stages. It's not generally an issue at runtime, once built (as far as I remember).

In general, if a Conda package is something the mrtrix3 team does not want, I will stop my work now; I believe it would be a good thing in the long run.

I will echo @maxpietsch's response here: this is something we would be keen to support, especially if it can done with little additional effort, as is the case here. The problem we're encountering on macOS is that there are many 3rd party package installers out there, and they often conflict in unexpected ways, particularly during the configure and build stages. But if these can be used to distribute binary versions, it actually should make life a lot easier for us in the long run.

The issue we have is primarily one of familiarity: I don't have a Mac, some people use macports, some people use homebrew, and others install from source manually. So if you're willing to work out an anaconda package, we're all for it. 👍

@duncanmmacleod

This comment has been minimized.

Copy link
Author

commented Dec 18, 2018

@jdtournier, @maxpietsch, I understand your concerns. I think I'm 95% finished with a working Conda package, I just need to figure out some problems with the python layer that sits on top. Once I have something finished, I will post it as a pull request on the conda-forge/staged-recipes repository, which is where new conda-forge packages go for an initial review.

It's not really ideal for me to be the only maintainer of the package, especially since I don't actually work in brain imaging (I'm an astrophysicist involved in some inter-departmental work). Can you name one or two other names (maybe yourselves) who are happy to be named as co-maintainers. The workload is very small, and is only really anything when new releases come out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.