-
Bug
-
Resolution: Fixed
-
Major
-
None
-
P4 2015.2 (Win 2008 R@), Jenkins 2.14 (Win 2008 R2), P4-Plugin 1.4.3
I’ve found that the latest versions of files that are mapped in a workspace and also in an “Exclude changes from Depot path” polling filter, are not being synced at build time.
Here’s the scenario. I’m currently set Jobs to use Manual workspaces set in Jenkins, to do a Force Sync, and have the “Populate have list” flag set.
The Workspace includes an example path:
PathA/…
PathB/…
Also configured with the Polling Filter - “Exclude changes from Depot path” is set to a path under PathB (actually in my test it was the same path).
What happens:
- A check-in under Path A is recognized from polling and triggers a build which has a quiet period associated with it.
- During the quiet period, one or more check-ins occur under Path B. These files naturally have a higher changelist number than what triggered the build.
- When the quiet period expires, the build starts and syncs Path A’s files to latest available changelist for that Path, whatever it may be – All is good!
- For the files in Path B, however, the build syncs to the changelist number from Path A that triggered the build, NOT the latest available changelist. That causes the files under Path B, to be sync’ed to non-latest state.
- Again, if check-ins occur in the quiet period under Path A – the latest versions of those files ARE being synced.
For me, I have files under Path B that must be at latest when the build kicks off. I don’t, however, want to trigger builds with these changes, because unrelated changes occur here all day long. This behavior of sync’ing old files in that path is actually breaking our builds and forcing us to stay at version 1.3.8 which prevents us from using the latest version of the plugin (1.3.9 is where this issues was first noticed).
Btw – Polling does recognize the changelists in Path B (See below - in my test 4938865 and 4938866 are under Path B), but they are not being synced to the workspace. 4938864 is the change in Path A that triggered the build – that is being synced AND the files in Path B are being synced to the 4938864 level also. The two Path B changes are also not listed/described on the Changes page for the job.
I can see this maybe being by design, but the Exclude Filter specifically says it’s for polling – not syncing files, plus syncing the latest files from all paths at build time and using an Exclude in Polling path worked up through 1.3.8.
Started on Jul 22, 2016 9:47:01 AM
Polling SCM changes on master
P4: Polling on: master with:Test-Perforce-Plugin
... p4 client -o Test-Perforce-Plugin
+
... p4 info
+
P4 Task: establishing connection.
... server: atl-p4-test:1818
... node: atl-build-test
P4: Polling with range: 4938863,now
... p4 changes -m100 //Test-Perforce-Plugin/...@4938863,now
+
... p4 change -o 4938866
+
... p4 describe -s 4938866
+
... p4 change -o 4938865
+
... p4 describe -s 4938865
+
... p4 change -o 4938864
+
... p4 describe -s 4938864
+
... found change: 4938864
Done. Took 0.18 sec
Changes found
[JENKINS-36883] Jobs with “Exclude Changes” Polling Filter Not Syncing Latest at Build Time
Jason Davis
added a comment - Thanks a ton for working on this! It sounds like perhaps the change was to enable 'now' work as a label after a quiet period. Is that the only change in the works, or will exclude filtered files now also be synced to the latest change without the label? I could update all my jobs to use 'now', but then I'd have to use that by default when using the plugin (to accommodate the excluded files + quiet period problem) and it makes me wonder what I should do if I ever need to use the plugin with a different label other than 'now'.
Jason Davis
added a comment - Thanks a ton for working on this! It sounds like perhaps the change was to enable 'now' work as a label after a quiet period. Is that the only change in the works, or will exclude filtered files now also be synced to the latest change without the label? I could update all my jobs to use 'now', but then I'd have to use that by default when using the plugin (to accommodate the excluded files + quiet period problem) and it makes me wonder what I should do if I ever need to use the plugin with a different label other than 'now'. 
Paul Allen
added a comment - Yes the change is to allow 'now' to always sync to the latest, at the point of sync and not when the build was scheduled. If you specify any other label or change then that is what will get synced.
The Polling Filter for excluded files is only to decide to when to build and not how to sync.
Paul Allen
added a comment - Yes the change is to allow 'now' to always sync to the latest, at the point of sync and not when the build was scheduled. If you specify any other label or change then that is what will get synced.
The Polling Filter for excluded files is only to decide to when to build and not how to sync. Jason Davis
added a comment - OK - however, I'm not certain this is a good fix for the problem. Changes after a trigger should be allowed during a quiet period without requiring the now label - not doing so thwarts the purpose of having a quiet period in Jenkins. Additionally, files are in fact being synced to different changeset levels at build start time. For ex. here's a contrived workspace spec in Jenkins:
//SCM/Tools/Test/... //${JOB_NAME}/SCM/Tools/Test/...
"//SCM/buildinfo/..." "//${JOB_NAME}/buildinfo/..."
then I "Exclude Changes from Depot Path" with this:
//SCM/buildinfo
Next:
- A build is triggered by a change under //SCM/Tools/Test, let's say with changeset 100.
- Quiet period starts. During the quiet period, changes 101 through 105 are checked in, with 103 being the last change in the /buildinfo/... path and 105 being the last change under the Tools/Test/... path
- The quiet period ends.
- The build launches - the latest available changeset for the workspace is 105. What is occurring is that the build is sync'ing the files changed during the quiet period under Tools/Test/... to changeset 105 and the files changed under buildinfo/... to changeset 100. I've confirmed this with a simple Jenkins job using the above spec and a quiet period. This is inconsistent - the files in buildinfo/... should be synced to the latest changeset for the whole workspace (105) at build time without the use of the now label, or everything should be synced to changeset 100, however, I'd argue syncing to changeset 100 would runs against the purpose of the quiet period AND is not what occurred through v1.3.8 of the plugin: all files in the workspace spec were synced to the latest at build time.
My hope is that everything in the workspace spec would be synced to the latest at build time (unless you use Polling per Change), because that would be consistent no matter how the build was launched.
Jason Davis
added a comment - OK - however, I'm not certain this is a good fix for the problem. Changes after a trigger should be allowed during a quiet period without requiring the now label - not doing so thwarts the purpose of having a quiet period in Jenkins. Additionally, files are in fact being synced to different changeset levels at build start time. For ex. here's a contrived workspace spec in Jenkins:
//SCM/Tools/Test/... //${JOB_NAME}/SCM/Tools/Test/...
"//SCM/buildinfo/..." "//${JOB_NAME}/buildinfo/..."
then I "Exclude Changes from Depot Path" with this:
//SCM/buildinfo
Next:
A build is triggered by a change under //SCM/Tools/Test, let's say with changeset 100.
Quiet period starts. During the quiet period, changes 101 through 105 are checked in, with 103 being the last change in the /buildinfo/... path and 105 being the last change under the Tools/Test/... path
The quiet period ends.
The build launches - the latest available changeset for the workspace is 105. What is occurring is that the build is sync'ing the files changed during the quiet period under Tools/Test/... to changeset 105 and the files changed under buildinfo/... to changeset 100. I've confirmed this with a simple Jenkins job using the above spec and a quiet period. This is inconsistent - the files in buildinfo/... should be synced to the latest changeset for the whole workspace (105) at build time without the use of the now label, or everything should be synced to changeset 100, however, I'd argue syncing to changeset 100 would runs against the purpose of the quiet period AND is not what occurred through v1.3.8 of the plugin: all files in the workspace spec were synced to the latest at build time.
My hope is that everything in the workspace spec would be synced to the latest at build time (unless you use Polling per Change), because that would be consistent no matter how the build was launched. Jason Davis
added a comment - Please - before closing - reconsider that not syncing all files in a workspace spec to latest by default goes philosophically against the quiet period, and that currently (without the now label), the build is syncing non-filtered files to the latest changelist, while filtered files in the workspace spec are syncing to a different, earlier changelist, which is not the same behavior (tried to explain better in my last comment).
Jason Davis
added a comment - Please - before closing - reconsider that not syncing all files in a workspace spec to latest by default goes philosophically against the quiet period, and that currently (without the now label), the build is syncing non-filtered files to the latest changelist, while filtered files in the workspace spec are syncing to a different, earlier changelist, which is not the same behavior (tried to explain better in my last comment). Paul Allen
added a comment - Think I am misunderstanding something, or your environment is different to mine. All files should get sync'ed to @now (the latest change). If you look in the log output you should see p4 sync //<workspace>/...@now.
"Exclude Changes from Depot Path" is only a polling filter to prevent build events from occurring on changes in the specified path.
Paul Allen
added a comment - Think I am misunderstanding something, or your environment is different to mine. All files should get sync'ed to @now (the latest change). If you look in the log output you should see p4 sync //<workspace>/...@now.
"Exclude Changes from Depot Path" is only a polling filter to prevent build events from occurring on changes in the specified path. Jason Davis
added a comment - Hi Paul - I agree with you that the polling filter should only prevent builds from occurring on the changes in the filter path! 
That feature is working wonderfully, but there’s something else going on that’s causing files in the workspace spec to be sync’ed to _different _changelist levels after the Jenkins Quiet Period - and that started happening with v1.3.9 of the plugin. Up through 1.3.8, ALL files in a workspace spec did sync to the latest CL after a Quiet Period at build start time (without @now) – but that’s not happening any longer.
Please see the above example. I explain there that, right now and without the use of @now in the workspace spec or P4 label setting, non-filtered files are currently being sync'ed to latest (CL 105), while filtered files are being sync'ed to trigger level (CL 100 instead of CL 103). I’ve confirmed this by testing with the above spec in a simple Jenkins job. Just make some checkins to files both in and out of a filtered path during a Quiet Period to see that they’re not synced to the latest CL at build start time.
You suggested above in the Sept. 8 comment, that the build should sync to the CL at trigger time - not the CL at start time, so that other changes don't sneak in to the changes report. Also that @now should be used as a workaround. I’m further suggesting that @now shouldn’t be required to make the filtered files sync to latest at build start, because Jenkins has a configurable Quiet Period setting, which by design is to enable Devs to submit other changes after a trigger as an "oops" measure. If @now becomes required in the P4 Plugin label field or workspace spec to sync to latest after a Quiet Period, then that prevents use of other P4 labels I might like to use, essentially states that more configuration is required to use the Jenkins Quiet Period setting, would make the P4 plugin work by differently than all other SCM plugins I’m familiar with since I started using Jenkins around 8-9 years ago (TFS, SVN, Community Perforce), and the P4 plugin itself did not have this requirement until v1.3.9.
In the end, I'm first hoping that ALL the files in the workspace spec are synced to the same and latest CL at build start time like it once did (and that’s the same CL that the changes also report uses) – and second, that the build should not require @now to deal with those changes checked in during the Jenkins Quiet Period because I believe that alters what one would expect from SCM plugins when using a Quiet Period (as well as other reasons mentioned in the above comments).
Jason Davis
added a comment - Hi Paul - I agree with you that the polling filter should only prevent builds from occurring on the changes in the filter path! That feature is working wonderfully, but there’s something else going on that’s causing files in the workspace spec to be sync’ed to _different _changelist levels after the Jenkins Quiet Period - and that started happening with v1.3.9 of the plugin. Up through 1.3.8, ALL files in a workspace spec did sync to the latest CL after a Quiet Period at build start time (without @now) – but that’s not happening any longer.
Please see the above example. I explain there that, right now and without the use of @now in the workspace spec or P4 label setting, non-filtered files are currently being sync'ed to latest (CL 105), while filtered files are being sync'ed to trigger level (CL 100 instead of CL 103). I’ve confirmed this by testing with the above spec in a simple Jenkins job. Just make some checkins to files both in and out of a filtered path during a Quiet Period to see that they’re not synced to the latest CL at build start time.
You suggested above in the Sept. 8 comment, that the build should sync to the CL at trigger time - not the CL at start time, so that other changes don't sneak in to the changes report. Also that @now should be used as a workaround. I’m further suggesting that @now shouldn’t be required to make the filtered files sync to latest at build start, because Jenkins has a configurable Quiet Period setting, which by design is to enable Devs to submit other changes after a trigger as an "oops" measure. If @now becomes required in the P4 Plugin label field or workspace spec to sync to latest after a Quiet Period, then that prevents use of other P4 labels I might like to use, essentially states that more configuration is required to use the Jenkins Quiet Period setting, would make the P4 plugin work by differently than all other SCM plugins I’m familiar with since I started using Jenkins around 8-9 years ago (TFS, SVN, Community Perforce), and the P4 plugin itself did not have this requirement until v1.3.9.
In the end, I'm first hoping that ALL the files in the workspace spec are synced to the same and latest CL at build start time like it once did (and that’s the same CL that the changes also report uses) – and second, that the build should not require @now to deal with those changes checked in during the Jenkins Quiet Period because I believe that alters what one would expect from SCM plugins when using a Quiet Period (as well as other reasons mentioned in the above comments). 
Karl Wirth
added a comment - Have reproduced this (ignoring the 'now' label for this example). Setup a polling period of 2 minutes but a Quiet setting on the job of 3 minutes. The job will run with the latest polled changelist of the 2 polls. For example:
- Poll 1 determines changelist 256 is the latest - triggers job#36 with a quiet period of 3 mins.
- 3 more changes made after quiet period starts and before next poll period. CL=260 on included branch, 261 and 262 on excluded from poll branch.
- Poll 2 determines that changelist 260 is the latest.
- Job#36 runs with changelist 260 as the latest changelist.
So the `bug` is that the running job picks up the latest poll CL not the poll CL that was available when the job was started. Note that this is not the latest CL in the workspace because there were later CLs on the branch that was excluded from the polling filter. Attaching JENKINS-36883-repro.txt
full log output.
The desired behavior of putting 'now' in the 'Pin build at Perforce label' field would get up to 262 which is the desired behavior.
Karl Wirth
added a comment - Have reproduced this (ignoring the 'now' label for this example). Setup a polling period of 2 minutes but a Quiet setting on the job of 3 minutes. The job will run with the latest polled changelist of the 2 polls. For example:
Poll 1 determines changelist 256 is the latest - triggers job#36 with a quiet period of 3 mins.
3 more changes made after quiet period starts and before next poll period. CL=260 on included branch, 261 and 262 on excluded from poll branch.
Poll 2 determines that changelist 260 is the latest.
Job#36 runs with changelist 260 as the latest changelist.
So the `bug` is that the running job picks up the latest poll CL not the poll CL that was available when the job was started. Note that this is not the latest CL in the workspace because there were later CLs on the branch that was excluded from the polling filter. Attaching JENKINS-36883-repro.txt full log output.
The desired behavior of putting 'now' in the 'Pin build at Perforce label' field would get up to 262 which is the desired behavior. Code changed in jenkins
User: Paul Allen
Path:
src/main/java/org/jenkinsci/plugins/p4/PerforceScm.java
src/main/java/org/jenkinsci/plugins/p4/changes/P4Revision.java
src/main/java/org/jenkinsci/plugins/p4/client/ClientHelper.java
src/main/java/org/jenkinsci/plugins/p4/tasks/AbstractTask.java
src/main/java/org/jenkinsci/plugins/p4/tasks/CheckoutTask.java
src/main/java/org/jenkinsci/plugins/p4/tasks/PollTask.java
src/test/java/org/jenkinsci/plugins/p4/client/ConnectionTest.java
http://jenkins-ci.org/commit/p4-plugin/f4c24f7964bd1de847ace7b43546be81e51d2ccc
Log:
Polling Fix for use with quiet period.
Switched all uses of change/label to P4Revision object and implemented
Comparable. The changes to build are now calculated at build time
(after the quiet period) not during the polling phase.
Ready for release