p4-plugin icon indicating copy to clipboard operation
p4-plugin copied to clipboard

[JENKINS-71364] Check if parent job is MatrixConfiguration instead of MatrixProject

Open lleroy opened this issue 2 years ago • 5 comments

Update PerforceScm.java - Check if parent job is MatrixConfiguration instead of MatrixProject when doing a matrix build: checkouts should be the same for each run of the matrix build see also https://issues.jenkins.io/projects/JENKINS/issues/JENKINS-71364

Testing done

### Submitter checklist
- [x] Make sure you are opening from a **topic/feature/bugfix branch** (right side) and not your main branch!
- [x] Ensure that the pull request title represents the desired changelog entry
- [x] Please describe what you did
- [x] Link to relevant issues in GitHub or Jira
- [x] Link to relevant pull requests, esp. upstream and downstream changes
- [x] Ensure you have provided tests - that demonstrates feature works or fixes the issue

lleroy avatar Jul 12 '23 09:07 lleroy

@p4paul : could you have a look to have this (rather trivial) fix merged? I need it on my production server and now have to use a custom-build version of the plugin, but would like to use an official (future) release

lleroy avatar Oct 16 '23 11:10 lleroy

thanks @lleroy for creating this PR. We appreciate your effort to improve the p4-plugin.

When running matrix builds perforce gives an option to run builds simultaneously and sequentially. If builds are executed simultaneously, all builds will sync with the same changelist. But if builds are executed sequentially and a new change is committed while the first build is running, the second build will sync to the new change.

To toggle between this we have added the "Run each Configuration Sequentially" checkbox. image

Could you please check if this can be used to solve your issue? I need to check if the behaviors to sync to the latest for the second build was intended.

skumar7322 avatar Feb 22 '24 08:02 skumar7322

No, we do not want to "Run each Configuration Sequentially". And Yes, it's definitely indented to sync on the same changelist.

"If builds are executed simultaneously, all builds will sync with the same changelist." That is what is expected, and that was the case before (or with the "old" perforce plugin), but I think due to some code change elsewhere, this functionality was broken.

The reason we need this, is that we use a shared workspace between the different axis that run on the same build slave (due to the workspace taking >5GB, and containing >200K files) We do this via the "Advanced Project Options", "Use custom workspace" and "Use custom child workspace"+"Child Directory"="."

The builds take a long time, and there are many variants to build. Sometimes a commit was done that changes an interface (in a C++ header) and so the code before or after that commit compiles, but due to the shared workspace, files compiled before and after the sync in a different axis were no longer compatible. I found this out by adding extra logging in the P4 plugin, and saw that the baseclass was sometimes MatrixConfiguration instead of MatrixProject. I have not read through the complete code where it comes from, but it fixes my problem. (It was hard to diagnose, because it only happens from time to time, but frequent enough to bite our developments)

Thanks for considering the fix - I hope you integrate it as it's a very simple, one-line fix. -- lode

On 2/22/24, skumar7322 @.***> wrote:

thanks @lleroy for creating this PR. We appreciate your effort to improve the p4-plugin.

When running matrix builds perforce gives an option to run builds simultaneously and sequentially. If builds are executed simultaneously, all builds will sync with the same changelist. But if builds are executed sequentially and a new change is committed while the first build is running, the second build will sync to the new change.

To toggle between this we have added the "Run each Configuration Sequentially" checkbox. image

Could you please check if this can be used to solve your issue? I need to check if the behaviors to sync to the latest for the second build was intended.

-- Reply to this email directly or view it on GitHub: https://github.com/jenkinsci/p4-plugin/pull/189#issuecomment-1958928187 You are receiving this because you were mentioned.

Message ID: @.***>

lleroy avatar Feb 22 '24 08:02 lleroy

agree the fix is simple... But if we merge it, then the builds running sequentially or simultaneously will always sync to the changelist that the parent has identified for the first build. There will be no way to toggle between sync to the latest change or change identified when the first build started (need to check how valid this scenario is).

I feel we should look into the last changes when this bug was introduced. It would be helpful if you could provide the last version of the p4-plugin in which this was working.

skumar7322 avatar Feb 22 '24 09:02 skumar7322

We're not using the "Perforce: Matrix Options", but the "Groovy Script Matrix Executor Strategy".

Maybe to be (more) sure to be backward compatible, you could check if the "MatrixOptions" is in use:

((job instanceof MatrixConfiguration) && (! getMatrixExecutionStrategy(job) instanceof MatrixOptions))

[edit: actually that will not make a difference since getMatrixExecutionStrategy() will return null when it's instanceof(MatrixConfiguration) because that's not instanceof MatrixProject]

Unfortunately, I cannot say when the problem started showing - it was quite some time before I submitted the fix (Jul 12, 2023). It took some time to realize there was an intermittent problem, and it took some time before I got around to investigate the problem at the code level. [edit: I think the change is in the jenkins/hudson framework rather than in the p4 plugin]

lleroy avatar Feb 22 '24 09:02 lleroy