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

Update usher: don't skip on Mac OSX #38446

Merged
merged 3 commits into from
Jan 3, 2023

Conversation

AngieHinrichs
Copy link
Contributor

Don't skip Mac build for usher; instead, omit ripples-fast if architecture is not x86_64.

Describe your pull request here


Please read the guidelines for Bioconda recipes before opening a pull request (PR).

Please use the following BiocondaBot commands:

Everyone has access to the following BiocondaBot commands, which can be given in a comment:

@BiocondaBot please update Merge the master branch into a PR.
@BiocondaBot please add label Add the please review & merge label.
@BiocondaBot please fetch artifacts Post links to CI-built packages/containers.
You can use this to test packages locally.

For members of the Bioconda project, the following command is also available:

@BiocondaBot please merge Upload built packages/containers and merge a PR.
Someone must approve a PR first!
This reduces CI build time by reusing built artifacts.

Also, the bot watches for comments from non-members that include @bioconda/<team> and will automatically re-post them to notify the addressed <team>.

@AngieHinrichs AngieHinrichs marked this pull request as ready for review December 16, 2022 22:07
@AngieHinrichs
Copy link
Contributor Author

@BiocondaBot please add label

@bgruening
Copy link
Member

It's a bit confusing for users I guess. That the same package with same versions has different binaries. It might be confusing for users and hard to debug. Can we maybe add a pseudo binary that prints a meaningful error message?

@AngieHinrichs
Copy link
Contributor Author

It's a bit confusing for users I guess. That the same package with same versions has different binaries. It might be confusing for users and hard to debug. Can we maybe add a pseudo binary that prints a meaningful error message?

Yes, it's not a very satisfying solution and is just a short-term measure to get usher v0.6.1 to build on Mac so that Mac users can have the amazing new usher-sampled program, which will make pangolin run a lot faster. So far there has been some interest in ripples and ripples-fast, but nobody has complained yet about ripples-fast not being available on Mac. 🙂 But pangolin has a lot of users and is a very high priority for us.

Hopefully soon we'll find out how to include the right headers in the bioconda Mac build environment so we can use Intel SIMD instructions like __builtin_ia32_pminsw128 that is breaking ripples-fast, and then we can re-enable ripples-fast instead of this ugly workaround.

Thanks @bgruening!

@AngieHinrichs
Copy link
Contributor Author

@bgruening I updated build.sh to make a placeholder script version of ripples-fast when skipping it in the build, that prints out this message:

Sorry, ripples-fast is temporarily unavailable from bioconda for Mac.
Please chime in on https://github.com/yatisht/usher/issues/310 if this inconveniences you.

@AngieHinrichs
Copy link
Contributor Author

Thanks @rpetit3! Is anything else needed for a merge?

@AngieHinrichs
Copy link
Contributor Author

Happy New Year Bioconda team! Can this be merged, so that pangolin can use the new faster usher-sampled on Mac as well as linux? Thanks!

@rpetit3
Copy link
Member

rpetit3 commented Jan 3, 2023

@BiocondaBot please merge

@BiocondaBot
Copy link
Collaborator

I will attempt to upload artifacts and merge this PR. This may take some time, please have patience.

@BiocondaBot
Copy link
Collaborator

I received an error uploading the build artifacts or merging the PR!

@rpetit3
Copy link
Member

rpetit3 commented Jan 3, 2023

@BiocondaBot please update

@rpetit3
Copy link
Member

rpetit3 commented Jan 3, 2023

@BiocondaBot please merge

@BiocondaBot
Copy link
Collaborator

I will attempt to upload artifacts and merge this PR. This may take some time, please have patience.

@BiocondaBot BiocondaBot merged commit 23c416a into bioconda:master Jan 3, 2023
cokelaer pushed a commit to cokelaer/bioconda-recipes that referenced this pull request Apr 28, 2023
Merge PR bioconda#38446, commits were: 
 * Merge remote-tracking branch 'upstream/master' into usher.v0.6.mac
 * When not building ripples-fast on Mac, add a stub ripples-fast shell script that prints out a message pointing to yatisht/usher#310.
 * Don't skip Mac build for usher; instead, omit ripples-fast when building on Mac.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
please review & merge set to ask for merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants