[SofaPython3] Fix ExternalProjectConfig.h

[damienmarchal]

Red alert on the CI ! Fix it now.

---

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).

[jnbrunet]

@damienmarchal can you put origin/master instead?

[damienmarchal]

[ci-build]

[guparan]

You should not put a branch name in GIT_TAG.
Now SOFA master branch depends on SofaPython3. We don't want that.

[jnbrunet]

Yeah but if you don't do it, than we need to manually update the commit number each time SofaPython3 upgrades. They should follow each others.

[guparan]

What is the problem with manually updating the pointer?
In my opinion it is much less a pain than having ALL pull-request going red after a SofaPython3 merge.

[jnbrunet]

What is the problem with manually updating the pointer?

We will forget to do it, I'm sure of it.

In my opinion it is much less a pain than having ALL pull-request going red after a SofaPython3 merge.

How could that happen? No SOFA components depend on it, and SP3 has its own CI on github.

[guparan]

Because we build SofaPython3 on SOFA CI.
It's the starting point of all my work on ExternalProject update workflow that we have been discussing for 3 weeks.
We want to ease the pain of handling ExternalProject pointers while still building those external project on SOFA CI.
See #1961

[jnbrunet]

Yes I know, and I'm not questioning your work nor the 3 weeks of discussions.

But, if you are going to automatically merge the related PR in SP3 to its master branch, than why hard code the commit number? You can automatically update it on the CI, but not actually commit it. Once it passes, no need to change the "origin/master" since it contains your automatically merged SP3 PR.

If you don't do it, then upgrades in SP3 (bug fixes, new features) that do not have a related PR in SOFA won't be taken into account unless we go do a PR in SOFA to update this pointer. I can guarantee you that nobody will think of doing it. Which means that fetching SP3 within SOFA has great chance to be pointing to an old version.

In fact, the reason I asked this to Damien is exactly because of this, SOFA fetched me an outdated version of SP3, which didn't contain the latest bug fix in SP3.

[guparan]

I'm always having a hard time designing this type of workflow...
I agree with you that it's way easier to have a branch than a commit in GIT_TAG.
What will happen though is that SOFA CI could be failing because of changes in SP3/master.

For instance, this will happen:
1. Bob opens a SOFA PR that breaks SP3
-> the SOFA PR fails on SOFA CI
2. Bob opens a SP3 PR to propose a fix to this future change in SOFA that will break SP3
-> the SP3 PR fails on SP3 CI (because it fixes something that is not broken yet)
3. Thanks to my work on ExternalProject, SOFA CI is able to test the SOFA PR with a custom version for SP3 (the version of the SP3 PR)
-> the SOFA PR succeeds on SOFA CI
4. Both PRs are merged (in any order)
5. All the other SOFA PRs are now failing on SOFA CI and must be updated with SOFA/master

EDIT: corrected version below

[jnbrunet]

Yeah I see your issue now, thanks !

I think it could be resolved by changing (improving?) a little bit our current CI strategy. Correct me if I'm wrong, but right now the CI does the following for a PR # XXX into the branch Master:

1. Checkout PR#XXX branch
2. Build SOFA
3. Run tests

I think a better approach should be:

1. Checkout origin/master
2. Merge PR #XXX branch
3. Build SOFA
4. Run tests

This might also prevent some weird issues that we had were the CI passes, we merge, and then the CI start breaking everywhere: The CI never actually tested the merged PR.

I think this would fix your issue 5.

What do you think?

[guparan]

Actually SOFA CI is already building a merge for PRs so I was wrong with my use case before. 😅

Here is a corrected version of what could happen:

1. Bob opens a SOFA PR that breaks SP3
   -> the SOFA PR fails on SOFA CI
2. Bob opens a SP3 PR to propose a fix to this future change in SOFA that will break SP3
   -> the SP3 PR fails on SP3 CI (because it fixes something that is not broken yet)
3. Thanks to my work on ExternalProject, SOFA CI is able to test the SOFA PR with a custom version for SP3 (the version of the SP3 PR)
   -> the SOFA PR succeeds on SOFA CI
4. SP3 PR is merged first (to ensure that SOFA/master always succeeds on SOFA CI)
5. All the other SOFA PRs are now failing on SOFA CI and must wait for Bob's SOFA PR to be merged
6. SOFA PR is merged
7. All the other SOFA PRs are fixed

The problematic point is the duration of step 5.
How to make sure it is short? Is it a good idea to ask our devs to quickly merge 2 PRs from 2 projects? What about when it will be more complicated with 2, 3 or more external projects needing to be fixed?

[jnbrunet]

Hey @guparan ,

I would merge both at the same time (automatically), the trigger being the SOFA merge:

4. SP3 PR is merged first (to ensure that SOFA/master always succeeds on SOFA CI)
   SOFA PR is manually merged
5. SP3 PR is merged automatically (we can setup a workflow for this, or if you have something ready on the CI)
6. All the other SOFA PRs should be fine

Just to emphasis the problem with the alternative (hard-coded commit number instead of a branch):

1. PRs in SP3 can be merged in the master branch before the correction SP3 PR is merged , which means that SOFA will point to an older version of SP3, example:
   a. Bob opens a SOFA PR that breaks SP3
   b. Bob opens a SP3 PR with the correction
   c. Paul comes in an open a SP3 PR
   d. Paul's PR is accepted and merged
   e. Paul opens a SOFA PR to update the commit number
   f. Paul's PR get accepted and merged
   g. Bob's PR get accepted and merged
   f. SOFA commit number is now pointing to Bob's SP3 branch number, which misses Paul's commit
2. How do you deal with squash/rebase merges in SP3? Will your script use the merged commit number? Or the latest commit number of the SP3 PR's branch?
3. How do you deal with release branches? Will we need to manually edit the commit number for each new commit in release branches?