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

Configure logic updates #1180

Merged
merged 5 commits into from
Mar 29, 2019
Merged

Configure logic updates #1180

merged 5 commits into from
Mar 29, 2019

Conversation

rhc54
Copy link
Contributor

@rhc54 rhc54 commented Mar 28, 2019

No description provided.

Thread support was being checked for C++ and Fortran compilers although
we support neither of those.

Signed-off-by: Ralph Castain <rhc@pmix.org>
(cherry picked from commit be0cc71)
Remove stale CXX references

Signed-off-by: Ralph Castain <rhc@pmix.org>
(cherry picked from commit c7f31a4)
@rhc54 rhc54 added the bug label Mar 28, 2019
@rhc54 rhc54 added this to the v3.1.3 milestone Mar 28, 2019
@rhc54 rhc54 self-assigned this Mar 28, 2019
@ggouaillardet
Copy link
Contributor

@rhc54 are you sure flex is needed to build PMIx ?

In the case of Open MPI, flex is checked at autogen.pl time, and the generated *.l files are included in the dist tarball (and so does PMIx, at least in 3.1.2 I just checked)

Long story short, developers do need flex (and they run autogen.pl that checks for it), and other folks (who build from tarballs) might not need it at all.

About C++ compilers, Open MPI still require them because we build the mpic++ wrapper that uses a C++ compiler. There is no such thing in pmix (but there is in prte).
I have no objection not requiring a C++ compiler any more (even if we might spot in advance something that could go wrong if the pmix library is to be used by a C++ linker).

@rhc54
Copy link
Contributor Author

rhc54 commented Mar 28, 2019

@rhc54 are you sure flex is needed to build PMIx ?

I cloned the repo, ran autogen.pl, ran configure, and then typed "make all" - and that's where I hit the error that "flex" wasn't found. Now it is possible that the tarball might not require it. I can perhaps spend a little more time looking at it again. This was on a virgin SLES15 system and was the first time I had ever actually had a system that didn't include flex - I'm willing to bet that nobody tests our build system to check any more.

About C++ compilers, Open MPI still require them because we build the mpic++ wrapper that uses a C++ compiler.

I understand why C++ is required by OMPI, but the problem here is that we had stale m4 code checking for C++ and Fortran pthread support. This failed for me because I didn't have a C++ compiler on the system - which is absurd. This change doesn't prevent someone from using a C++ compiler - it just removes the check for C++ thread support as we don't use it.

@ggouaillardet
Copy link
Contributor

unlike what I wrote earlier, autogen.pl does not check for flex (and to my surprise, the same thing applies to Open MPI).
flex is used at compile time if the *.l files are not present (e.g. you are building from a git tree and not from a tarball).
So I guess the better fix is to test flex in autogen.pl (so you can build PMIx from a tarball on a virgin system that has no flex).

I am fine with the changes related to C++.

@rhc54
Copy link
Contributor Author

rhc54 commented Mar 28, 2019

I can find no mention of "flex" anywhere in autogen.pl - can you explain why you believe autogen.pl is supposed to run flex?

@ggouaillardet
Copy link
Contributor

autogen.pl does not run flex, but since configure might not run it either (e.g. when invoked on a tarball) I believe the best place to check for flex is in autogen.pl. Sorry I did not make that clear earlier.

Signed-off-by: Ralph Castain <rhc@pmix.org>
(cherry picked from commit 5262af9)
@rhc54
Copy link
Contributor Author

rhc54 commented Mar 28, 2019

I really prefer to let autoconf do the work, so I instead moved the check for flex so it is only done in a git directory tree. That should solve the problem, I think.

@ggouaillardet
Copy link
Contributor

@rhc54 fair enough, I issued #1182 in order to support VPATH and git worktrees.
If/when merged, it should be backported into this PR.

ggouaillardet and others added 2 commits March 28, 2019 19:33
test the existence of .git in the top source tree
in order to
 - support VPATH
 - support git worktrees (.git is a file and not
   a directory)

Signed-off-by: Gilles Gouaillardet <gilles@rist.or.jp>
(cherry picked from commit 539599a)
Signed-off-by: Ralph Castain <rhc@pmix.org>
@ggouaillardet ggouaillardet self-requested a review March 29, 2019 02:46
@rhc54 rhc54 merged commit 22a6799 into openpmix:v3.1 Mar 29, 2019
@rhc54 rhc54 deleted the cmr31/config branch March 29, 2019 03:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants