-
-
Notifications
You must be signed in to change notification settings - Fork 42
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
Rebuild for openmpi 5, add wget dep, fix ftp.unidata refs, increase ctest timeout #192
Conversation
Hi! This is the friendly automated conda-forge-linting service. I just wanted to let you know that I linted all conda-recipes in your PR ( I do have some suggestions for making it better though... For recipe:
|
It seems like this got missed in: |
Hi! This is the friendly automated conda-forge-linting service. I just wanted to let you know that I linted all conda-recipes in your PR ( |
@conda-forge-admin, please rerender |
Needed by conda-forge/netcdf4-feedstock#163 and presumably others |
@conda-forge/libnetcdf, anyone able to investigate the failing tests? They seem unrelated to OpenMPI 5 and rather to be due to missing files or something:
|
Appears to be a simple |
Eh, no perms to push @xylar |
@akrherz, it doesn't look like you're a maintainer of this feedstock. I can add you as a collaborator on my fork. |
Oh, I'm really sick this week and it's clear that my head is just too foggy. |
Thanks @akrherz! |
Now the error are updates Unidata did
They are dangerously close to a new libnetcdf release, so maybe wait a bit more? |
Okay, but that'll put the whole OpenMPI migration on hold, right? |
Thanks @akrherz! That seems to be doing the trick! |
@xylar , I don't see an openmpi migration applied here? I see this PR is green, but I think this needs updated. Doing so now. |
@conda-forge-admin please rerender |
serenity now, transient CDN error
@conda-forge-admin please restart ci |
@akrherz, good catch! I added the migrator file locally but forgot to |
Okay, I see. We need to migrate |
I've seen the openssh thing be necessary on other feedstocks. I assumed it was always required, so was surprised it wasn't here, too. |
Fair. Am going to hope to avoid it completely, I have the next commit teed up, awaiting a green mpiopenmpi job first :) |
No dice :-( |
If this doesn't work, will try your |
This doesn't make sense to me as the testing is done within the build phase, so if this somehow works.... |
No, right, I was just pointing out that it was probably needed for a different purpose in those other repos, not that moving to |
At least putting it in |
Holy cow, I need to review this more and back out some debugging before somebody thinks about merging... @xylar feel free to do that, or I will get to it in a couple of hours |
my debugging triggered an unrelated failure
now I think openssh needs to be in |
If this gets green, I then what to check if we can remove openssh dep and just use OMPI_MCA_plm_ssh_agent=false. It continues to be very confusing with issues and conflating |
osx_64 failure may be transient timeout
|
…nda-forge-pinning 2024.06.06.00.39.40
no longer needed for ompi 5
@akrherz, I think it's better to stick with this PR than to move to #193. It just makes things more confusing as far as I'm concerned and I think the history here is useful. If we merge that one, folks won't necessarily know to look here for the discussion. With 2 PRs open, it's not clear where folks should contribute. I updated the name of this one to basically match yours. I have brought most of those commits over here (skipping the one that conflicted with @minrk's) and applied @minrk's commit after them. |
Assuming all the tests pass, I'll merge this later this afternoon (European time). |
Checklist
conda-smithy
(Use the phrase@conda-forge-admin, please rerender
in a comment in this PR for automated rerendering)closes #193