[WIP] only mangle CC/etc if on mac and not told not to #175
Conversation
|
(Rebased onto #176 to avoid the flake8 error.) |
setup.py
Outdated
| if sys.platform.startswith('darwin'): | ||
| if sys.platform.startswith('darwin') and not os.environ.get('POT_LEAVE_CC', ''): | ||
| os.environ["CC"] = "g++" | ||
| os.environ["CXX"] = "g++" |
There was a problem hiding this comment.
why do we need to do this in the first place? POT does not compile with clang or icc?
There was a problem hiding this comment.
|
this is for me not the right fix.
I typically do this when using clang on my mac:
CFLAGS='-stdlib=libc++' pip install -e .
and not need for a hack in setup.py but maybe there is a simpler way.
… |
|
I don't know why this would ever be generally needed; my worry is that the current behavior is over-reacting to one user's broken build system. But @ncourty can clarify maybe. |
rflamary
changed the title
|
Hello all. Yes, I do not think changing the CC flag is needed anymore. I remember it was an old solution to OSX installation problems before clang becomes the main c++ compiler on OSX. Clearly it is a reminiscence from the past, I guess we can remove it now ? |
|
I propose to remove it and see if CIs break. If not we can remove
|
|
good for me |
|
OK now this PR is just removing the flags and it still works in CI. Since @djsutherland basically had to remove it for it to build for conda I guess we should merge it if everyone is OK with it. |
|
ok with this ! |
|
For the record, I don't love the |
|
Can you maybe make a new release that includes this change? It's been more than a year. |
|
Or how about a |
As mentioned in conda-forge/pot-feedstock#9 (comment) and following up on #130 – always setting
CC=g++is the wrong thing to do, and also always doing theCFLAGSmangling on Mac, regardless of what compilers you're doing/etc, is also the wrong thing to do. This only setsCCon Mac, and allows you to bail out of doing any of this (as e.g. conda-forge will want) by settingPOT_LEAVE_CC.CC @ncourty