-
Notifications
You must be signed in to change notification settings - Fork 23
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
Fix install SofaPython3 CI action. #82
Fix install SofaPython3 CI action. #82
Conversation
I guess the broken link to SofaPython3 master nightly release is due to that current CI of SofaPython3 is down, awaiting sofa-framework/SofaPython3#364. |
001732a
to
65209c8
Compare
65209c8
to
e097062
Compare
.github/workflows/ci.yml
Outdated
if [[ "${{ matrix.sofa_branch }}" == "master" ]]; then | ||
url="${url}/release-master-nightly/SofaPython3_master-nightly_python-${{ matrix.python_version }}_for-SOFA-master_${{ runner.os }}.zip" | ||
else | ||
url="${url}/release-${{ matrix.sofa_branch }}/SofaPython3_${{ matrix.sofa_branch }}_python-${{ matrix.python_version }}_for-SOFA-${{ matrix.sofa_branch }}_${{ runner.os }}.zip" | ||
fi |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if [[ "${{ matrix.sofa_branch }}" == "master" ]]; then | |
url="${url}/release-master-nightly/SofaPython3_master-nightly_python-${{ matrix.python_version }}_for-SOFA-master_${{ runner.os }}.zip" | |
else | |
url="${url}/release-${{ matrix.sofa_branch }}/SofaPython3_${{ matrix.sofa_branch }}_python-${{ matrix.python_version }}_for-SOFA-${{ matrix.sofa_branch }}_${{ runner.os }}.zip" | |
fi | |
url="${url}/release-${{ matrix.sofa_branch }}/SofaPython3_${{ matrix.sofa_branch }}_python-${{ matrix.python_version }}_for-SOFA-${{ matrix.sofa_branch }}_${{ runner.os }}.zip" |
Why searching for master-nightly
and not master
? I suspect that master-nightly
is not even built nightly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a good question. Honestly I don't know, I kept the master-nightly
as it was already there.
I don't see any drawback switching to non-nightly build, and it would make the code simpler.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The term nightly comes from
https://github.com/sofa-framework/SofaPython3/blob/c5c1603cbfaa1ef5f11a5402186e83635edfba3b/.github/workflows/ci.yml#L6, which is triggered only manually. Unless there is a reason I don't see, I would not base the other plugins on this package.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alright. I've made the changes.
* Fix install SofaPython3 CI action. * Use non-nightly SofaPython3 builds for CI.
This was initially done for #81.
This PR intend to fix broken CI when building a release branch, defined in the
sofa_branch
field in theci.yml
:Cosserat/.github/workflows/ci.yml
Line 18 in ca56f59
For a given plugin release that needs to install SofaPython3 plugin, the expected behavior should be to install the corresponding release of the plugin (instead of
master-nightly
).So far, the path was broken as it was a mix of current
sofa_branch
value and hard-codedmaster
ormaster-nightly
.This solution should be fine as long we limit the scope of branches that can be specified in the
sofa_branch
field ofci.yml
tomaster
and release branches, as for non-master branches it will try to install a release of SofaPython3 defined by the branch name, and it will only exists if it a release. This is open for discussion.This should be integrated into master of all plugins for which the CI rely on SofaPython3 installation for the future releases.