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

build: Remove unnecessary configure tests #1290

Merged
merged 1 commit into from
Mar 16, 2022

Conversation

bwbarrett
Copy link
Contributor

Many of the configure tests that PRRTE runs are either not used
(ie, checking for a case sensitive file system or 128 bit integers)
or we use the tested item without conditionals throughout the code
(ie, we use stdbool.h in multiple places and none of them have
conditional checks). The second really isn't an issue; of the
places I found that behaivor, we're likely leaving behind some
ancient systems, but that hasn't proven to be a problem at this
point.

This patch removes all the tests where we didn't use the results.
There is likely more shortening of the configure script that
we could do, but this was the low hanging fruit.

Signed-off-by: Brian Barrett bbarrett@amazon.com

Many of the configure tests that PRRTE runs are either not used
(ie, checking for a case sensitive file system or 128 bit integers)
or we use the tested item without conditionals throughout the code
(ie, we use stdbool.h in multiple places and none of them have
conditional checks).  The second really isn't an issue; of the
places I found that behaivor, we're likely leaving behind some
ancient systems, but that hasn't proven to be a problem at this
point.

This patch removes all the tests where we didn't use the results.
There is likely more shortening of the configure script that
we could do, but this was the low hanging fruit.

Signed-off-by: Brian Barrett <bbarrett@amazon.com>
@bwbarrett bwbarrett requested a review from rhc54 March 16, 2022 17:23
@bwbarrett
Copy link
Contributor Author

@rhc54 I'm totally fine if you don't want to take this. It was an experiment based on your comments about configure runtime in containers during last night's IU basketball game. The code should work, it does make configure faster, but it's also code changes, so it might just muck things up.

@rhc54
Copy link
Contributor

rhc54 commented Mar 16, 2022

I suspect a lot of these went away when we shifted utilities and other shared infrastructure over to PMIx. I see no problem with doing it - if we miss some system, they'll let us know 😄

@rhc54
Copy link
Contributor

rhc54 commented Mar 16, 2022

Sorry - I've tried redelivering the signed-off-by payload multiple times with no success. No idea what the problem is - probably best to just push something into it?

@bwbarrett
Copy link
Contributor Author

GitHub Actions are experiencing problems according to https://www.githubstatus.com/. I think the right answer is to wait until GitHub is healthy again.

@rhc54 rhc54 merged commit 9eb9667 into openpmix:master Mar 16, 2022
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.

2 participants