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

Make mix tests optional #21

Merged
merged 6 commits into from
Sep 27, 2017
Merged

Conversation

davidozog
Copy link
Contributor

This addresses #20 by adding --enable-mpi / --disable-mpi flag to configure.

David Ozog added 3 commits September 14, 2017 13:33
This commit adds an enable/disable option for the 'mix' tests.  The
default is to enable.  Relies on the ENABLE_MIX macro to include/exclude
appropriate source files.

Signed-off-by: David Ozog <[email protected]>
@swx-jenkins2
Copy link
Collaborator

Can one of the admins verify this patch?

@mike-dubman
Copy link
Contributor

@davidozog - could you please squash commits.
we are connecting jenkins CI to this project, hope it will be operational next week to smoke test & review it.
@MrBr-github

@jdinan
Copy link
Contributor

jdinan commented Sep 14, 2017

@miked-mellanox You should be able to squash when you merge the commit by clicking the down arrow on the right side of the merge button and selecting "Squash and merge".

@davidozog
Copy link
Contributor Author

@miked-mellanox Would you prefer that I "push --force" a squashed commit to this PR, open a new PR, or let you do the green-button "squash and merge"? I'm fine with whatever you prefer.

@ireed
Copy link
Contributor

ireed commented Sep 15, 2017

I was unaware that you were working on the MPI separation, and i was preparing a pull-request until i checked here.
I have a proposed version that auto-configures based on the existence/detection of -lmpi instead of depending on a hard-coded define. This method follows the style of the original architect(s).

ireed@aeadfd4

@jdinan
Copy link
Contributor

jdinan commented Sep 15, 2017

I think the explicit --disable-mpi option will work better. It's common to have MPI installed in the development environment; however, the OpenSHMEM build may still not support mixed mode operation.

MPI also does not have a standard library name, so checking for libmpi is not sufficient to detect MPI. Something like this would be more robust:

AC_COMPILE_IFELSE([#include <mpi.h>
int main(void) { MPI_Init(NULL, NULL); MPI_Finalize(); return 0; }], ...)

@swx-jenkins2
Copy link
Collaborator

Can one of the admins verify this patch?

@mike-dubman
Copy link
Contributor

ok to test

@mike-dubman
Copy link
Contributor

bot:retest

@mike-dubman
Copy link
Contributor

@davidozog - merge conflict

@mike-dubman mike-dubman merged commit 2e78434 into openshmem-org:master Sep 27, 2017
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