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

Fix builds on Solaris/AIX using native compilers. #1259

Closed
wants to merge 3 commits into from

Conversation

apaprocki
Copy link
Contributor

These commits fix the build on Solaris using Oracle Studio and the build on AIX using IBM Visual Age compilers. I built these changes on both Solaris/AIX using the native compilers as well as using GCC 6 just to verify that there were no regressions. Each commit log entry has an explanation.

The current `configure.ac` has a TODO to detect `-pthread` vs
`-pthreads` and the forced use of `-pthreads` in `Makefile.am`
breaks the build on Solaris when using Oracle Studio instead of
GCC.

The open-source `ax_pthread.m4` macro exists to detect the
appropriate `libpthread` flags, libraries, and compiler (in the
case of AIX when not using GCC) to use.
The auto-detection of GCC specific options does not work when
building with Oracle Studio, as that compiler interprets the flags
differently without returning a bad return code for `autoconf` to
detect.
The `HAVE_SYS_AHAFS_EVPRODS_H` define is properly auto-detected by
`autoconf` and must not be forced on for systems that do not have
`ahafs` installed.  There is no way to auto-detect this in `gyp`,
so the flag remains in `uv.gyp`.
@gibfahn
Copy link

gibfahn commented Mar 17, 2017

cc/ @joransiu @jBarz @jbajwa for the AIX code.

Copy link
Contributor

@gireeshpunathil gireeshpunathil left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@apaprocki - the enforcement of this flag has a reason, and here is the history:
#1156 (comment)

In short, the community agreed that we enforce installation of AHAFS on AIX systems, as the fs.watch APIs critically depend on it. If the user is not interested in AHAFS related usages, he can always remove the flag manually and built. On the other hand, the pre-built binary will just work fine.

@gireeshpunathil
Copy link
Contributor

gireeshpunathil commented Mar 18, 2017

@apaprocki - curious to know - in general, what is your motivation behind building with non-gcc compilers? any performance goals? or integration of libuv with some legacy systems? or some new use case?

@apaprocki
Copy link
Contributor Author

@gireeshpunathil I implemented AHAFS detection in #372 and it worked perfectly fine until #1156 broke the code. I agree you need to force it on in the gyp build because it has no auto-detection functionality, but auto-detection works perfectly fine in autoconf.

We use the native compilers on Solaris and AIX because we link libuv into binaries that do not (and can not) build with GCC.

@gireeshpunathil
Copy link
Contributor

@apaprocki - can you elaborate on until #1156 broke the code part? Are you hinting at the current code failure on compilation complaining sys/ahafs_evProds.h is missing? (for both autoconf and gyp)

Are you proposing that the AHAFS feature should be opted out through autoconf via auto-detection, if the package is missing, and retain the behavior when built via gyp?

@gireeshpunathil
Copy link
Contributor

On the use case part, thanks for the explanation. I thought that the GCC-built images can link and co-exist with natively built libraries. That isn't the case?

@sam-github
Copy link
Contributor

This is adding GPL code into a MIT licensed project, I don't think that is workable.

@apaprocki
Copy link
Contributor Author

apaprocki commented Mar 20, 2017

@sam-github Ah, I mis-read the special exemption text. It only applies to the output of autoconf and not to the macro itself. I'll have to withdraw that one and come up with an alternate route that doesn't break Solaris+Studio.

@apaprocki
Copy link
Contributor Author

@gireeshpunathil After #1156, -DHAVE_SYS_AHAFS_EVPRODS_H is always passed on the command line instead of only being passed if the system has the header installed. gyp has no way to auto-detect features, so if the gyp build wants to force AHAFS on and the community wants that, then it is fine. The autoconf build, on the other hand, was auto-detecting the header just fine and does not need help by putting -DHAVE_SYS_AHAFS_EVPRODS_H in Makefile.am.

@gireeshpunathil
Copy link
Contributor

@apaprocki : yes, agree - but then this will have two implications:

  1. Building through the two means will exhibit two different behaviors
  2. The autoconf build will now allow building a libuv with handicapped watch feature.

I am not arguing the current one is better over the proposal etc., just placing things into perspectives and letting the reviewers know the context better.

@cjihrig
Copy link
Contributor

cjihrig commented Mar 3, 2018

It seems like this PR might have run its course. I'm going to close. Please let me know if that is the wrong thing to do.

@cjihrig cjihrig closed this Mar 3, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants