Skip to content

[CMake] ADD option to fetch pull-request commits in ExternalProject #1961

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.

Sign up for GitHub

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

Jump to bottom
Merged
jnbrunet merged 6 commits into from
May 21, 2021
Merged

[CMake] ADD option to fetch pull-request commits in ExternalProject #1961

jnbrunet merged 6 commits into from
May 21, 2021

Conversation

@guparan
Copy link
Contributor

@guparan guparan commented Mar 29, 2021 •
edited
Loading

When your SOFA PR fails on CI somewhere in an external project, it may be because your change is breaking for that external project.
In that case, the best (and preferred) solution is that you push a fix in the external project so that when your PR is merged in SOFA, it does not fail on CI.
Here is the typical workflow for that use case.

  1. You open a PR in SOFA (let's call it "SOFA PR").
  2. SOFA CI fails to build/test your SOFA PR because some external projects are enabled and you didn't enable them in your local tests.
  3. You realize that the failure is due to your changes.
  4. You fix the issue in the external project and open a PR (let's call it "external PR").
  5. You reference your external PR in the description of your SOFA PR with
    [ci-depends-on https://github.com/<org>/<repo>/pull/<id>]
  6. SOFA CI can now build/test your PR. It will automatically replace the external project with the "result of the merge of your external PR".
  7. As soon as SOFA CI is green and external PR is merged, your SOFA PR is ready to merge.

This way, you do not need to touch any ExternalProject.cmake file in your SOFA PR 👍

The only change in this PR consists in adding GIT_CONFIG "remote.origin.fetch=+refs/pull/*:refs/remotes/origin/pr/*" to all ExternalProject.cmake files.
This change permits to have access to any commit in any pull-request (including merge commits). See this nice cheatsheet about how to fetch PRs.
All other changes have been done in CI scripts and are ready to use.

By submitting this pull request, I acknowledge that
I have read, understand, and agree SOFA Developer Certificate of Origin (DCO).

Reviewers will merge this pull-request only if

  • it builds with SUCCESS for all platforms on the CI.
  • it does not generate new warnings.
  • it does not generate new unit test failures.
  • it does not generate new scene test failures.
  • it does not break API compatibility.
  • it is more than 1 week old (or has fast-merge label).

@guparan guparan added the pr: status wip Development in the pull-request is still in progress label Mar 29, 2021
@hugtalbot hugtalbot added the pr: experimental Demonstrate an experimental feature label Mar 29, 2021
@sofa-framework sofa-framework deleted a comment from sofabot Mar 30, 2021
@sofa-framework sofa-framework deleted a comment from sofabot Mar 30, 2021
@sofa-framework sofa-framework deleted a comment from sofabot Mar 30, 2021
@sofa-framework sofa-framework deleted a comment from sofabot Mar 30, 2021
@guparan guparan added pr: status to review To notify reviewers to review this pull-request pr: status wip Development in the pull-request is still in progress and removed pr: status wip Development in the pull-request is still in progress pr: status to review To notify reviewers to review this pull-request labels Mar 30, 2021
@sofa-framework sofa-framework deleted a comment from sofabot Apr 15, 2021
@guparan guparan added pr: status to review To notify reviewers to review this pull-request and removed pr: experimental Demonstrate an experimental feature pr: status wip Development in the pull-request is still in progress labels Apr 15, 2021
@guparan
Copy link
Contributor Author

guparan commented Apr 15, 2021

[ci-build]

@guparan
Merge remote-tracking branch 'sofa-framework/master' into improve_ext…
9fa8999 
…ernalproject_options

# Conflicts:
#	applications/plugins/SofaPython3/ExternalProjectConfig.cmake.in
@sofabot
Copy link
Collaborator

sofabot commented Apr 15, 2021

[ci-depends-on] detected during build #54.

All dependencies are merged/closed and all ExternalProject pointers are up-to-date. Congrats! 👍

@guparan guparan mentioned this pull request Apr 15, 2021
Merged
@hugtalbot hugtalbot added pr: status wip Development in the pull-request is still in progress and removed pr: status to review To notify reviewers to review this pull-request labels Apr 21, 2021
@jnbrunet
Copy link
Contributor

jnbrunet commented Apr 21, 2021

See discussion here

@sofabot
Copy link
Collaborator

sofabot commented May 6, 2021

[ci-depends-on] detected during build #59.

To unlock the merge button, you must

@guparan guparan added pr: enhancement About a possible enhancement pr: status to review To notify reviewers to review this pull-request and removed pr: status wip Development in the pull-request is still in progress labels May 18, 2021
@sofa-framework sofa-framework deleted a comment from sofabot May 18, 2021
@sofa-framework sofa-framework deleted a comment from sofabot May 18, 2021
@sofa-framework sofa-framework deleted a comment from sofabot May 18, 2021
@sofa-framework sofa-framework deleted a comment from sofabot May 18, 2021
@sofa-framework sofa-framework deleted a comment from sofabot May 18, 2021
@sofa-framework sofa-framework deleted a comment from sofabot May 18, 2021
@sofa-framework sofa-framework deleted a comment from sofabot May 18, 2021
@sofa-framework sofa-framework deleted a comment from sofabot May 18, 2021
@sofa-framework sofa-framework deleted a comment from sofabot May 18, 2021
@sofa-framework sofa-framework deleted a comment from sofabot May 18, 2021
@sofa-framework sofa-framework deleted a comment from sofabot May 18, 2021
@hugtalbot hugtalbot added pr: status ready Approved a pull-request, ready to be squashed and removed pr: status to review To notify reviewers to review this pull-request labels May 19, 2021
@guparan
Copy link
Contributor Author

guparan commented May 20, 2021

[ci-build]

1 similar comment
@guparan
Copy link
Contributor Author

guparan commented May 20, 2021

[ci-build]

@sofabot
Copy link
Collaborator

sofabot commented May 20, 2021

[ci-depends-on] detected during build #62.

All dependencies are merged/closed and all ExternalProject pointers are up-to-date. Congrats! 👍

@sofa-framework sofa-framework deleted a comment from sofabot May 20, 2021
@jnbrunet
Copy link
Contributor

jnbrunet commented May 20, 2021

What's up with the CI? Build failed on centos and ubuntu?

@guparan
Copy link
Contributor Author

guparan commented May 21, 2021

What's up with the CI? Build failed on centos and ubuntu?

A weird error that happens sometimes... I relaunched and it's OK now.

@jnbrunet jnbrunet merged commit 5fe4f72 into sofa-framework:master May 21, 2021
@guparan guparan added this to the v21.06 milestone Jun 28, 2021
@guparan guparan changed the title [All] ADD option to fetch pull-request commits in ExternalProject [CMake] ADD option to fetch pull-request commits in ExternalProject Jun 28, 2021
@guparan guparan deleted the improve_externalproject_options branch October 29, 2021 08:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

pr: enhancement About a possible enhancement pr: status ready Approved a pull-request, ready to be squashed

Projects

None yet

Milestone

v21.06

Development

Successfully merging this pull request may close these issues.

4 participants

@guparan @sofabot @jnbrunet @hugtalbot