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 the add feelpp package PR for spack: add boost defaults, readline, python@:3.11 #5

Open
wants to merge 1 commit into
base: 4-add-package-for-feelpp
Choose a base branch
from

Conversation

bernhardkaindl
Copy link

@bernhardkaindl bernhardkaindl commented Oct 12, 2024

@prudhomm @vincentchabannes, this "PR for the PR" contains the fixes that I've found so far:

PS, further FIXMEs before the build can be done on a freshly installed host in spack:

PPS: Your last Merge branch 'develop' into 4-add-package-for-feelpp merged a two bad commit from the develop branch into your PR branch https://github.com/feelpp/spack/tree/4-add-package-for-feelpp:

These two bad commits have been reverted in develop afterwards, but your PR branch has not been updated or rebased to a newer develop that reverts those bad commits yet.

The result is that when trying to build feelpp, the spack compiler wrapper script (which are bad in this commit) are very very very very slow: A single compiler call has about one second of additional CPU time which makes all compiler probing take ages. This caused many builds in CI to take longer than 6 hours and was a cause of major failures and backlogs. Therefore, you need to urgently rebase it to a new develop branch before resuming test builds for testing your PR:

git remote add upstream https://github.com/feelpp/feelpp.git
git rebase upstream/develop

When updating your branch, DO NOT USE "Update with merge commit" but "Update with rebase".

Using rebase keeps (or brings your commits) on top of the commit tree, which is what you want to review your commits.

You may also possibly consider squashing your commits for the new recipe into one to consolidate the commits (only now, not always) as many commits accumulated in the PR branch.

@prudhomm
Copy link
Member

@bernhardkaindl thank you so much for your input, I am looking into your contribution

@bernhardkaindl
Copy link
Author

bernhardkaindl commented Oct 23, 2024

@prudhomm: You do not have to use Boost.with_default_variants: I now found that it is just a method to or a transition from an old generic boost. You can replace it with the parts of boost that are needed. But it's also not a problem to use it.

You can apply the changes I suggest on your own, or you can merge this PR into your PR branch - just approve it here if that's ok.

And, then, please run a git rebase develop to update your branch and the push it to the PR using git push -f

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