Fixing loss of unshelved changes - Update ClientHelper.java [JENKINS-25724]

[mohanraj-r]

Moving revert -k operation in unshelveFiles() to before the unshelving process. Without this, when revert -k is done after the unshelving process, the unshelved changes are lost. This results in the review build option to be unusable and also renders the swarm jenkins integration unusable.

I am not a Java developer and don't have the resources to test / verify this change. I tried mvn package, but ran into build errors related from some other dependency. If this change is valid in theory and if you can help me build the package, I can test it. Our team is moving to use the new p4 plugin and this is one the critical features for us and one that will also help convince adoption of this plugin to other teams in our organization.

I guess this issue has already been reported : JENKINS-25724

Can you please review this change @p4paul

[jenkinsadmin]

Thank you for a pull request! Please check this document for how the Jenkins project handles pull requests

[p4paul]

Hi Mohan,

Thank you for looking into the issue and the suggested change.

The 'revert -k' option should not remove the unshelved content from the workspace. The '-k' option just removes the associated metadata from Perforce's have table.

It turns out on closer inspection that I was actually setting '-n' by accident:

rOpts.setNoUpdate(true);

The code should be...

rOpts.setNoClientRefresh(true);

Even so '-n' (preview) will not remove the shelved content.

I'm wondering if it is your intended usage that is causing the issue? A subsequent build WILL remove the unshelved content, unless it was building the same shelf.

If the content is missing then you may have found a P4JAVA bug.

Kind regards,
Paul

On 27 Mar 2015, at 02:58, Mohan Raj Rajamanickam notifications@github.com wrote:

Moving 'revert -k' operation in unshelveFiles() to before the unshelving process. Without this, when 'revert -k' is done after the unshelving process, the unshelved changes are lost. This results in the review build option (https://github.com/jenkinsci/p4-plugin#review) to be unusable and also renders the swarm jenkins integration (http://www.perforce.com/blog/140910/continuous-integration-new-perforce-server-plugin-jenkins) unusable.

I am not a Java developer and don't have the resources to test / verify this change. I tried mvn package, but ran into build errors related from some other dependency. If this change can is valid in theory and if you can help me build the package, I can test it. Our team is moving to use the new p4 plugin and this is one the critical features for us and one that will also help convince adoption of this plugin to other teams.

I guess this issue has already been reported : https://issues.jenkins-ci.org/browse/JENKINS-25724

Can you please review this change @p4paul

You can view, comment on, or merge this pull request online at:

#10

Commit Summary

• Update ClientHelper.java
File Changes

• M src/main/java/org/jenkinsci/plugins/p4/client/ClientHelper.java (13)
Patch Links:

• https://github.com/jenkinsci/p4-plugin/pull/10.patch
• https://github.com/jenkinsci/p4-plugin/pull/10.diff
—
Reply to this email directly or view it on GitHub.

---

This email and any files transmitted with it are confidential and intended
solely for the use of the individual or entity to whom they are addressed. If
you have received this email in error please notify the system manager. Please
note that any views or opinions presented in this email are solely those of the
author and do not necessarily represent those of Perforce Software. Finally,
the recipient should check this email and any attachments for the presence of
viruses. Perforce Software accepts no liability for any damage caused by any
virus transmitted by this email.

Perforce Software UK Ltd is registered in England and Wales as company no.
3816019 at the following address: West Forest Gate, Wellington Road, Wokingham,

RG40 2AT, UK

[mohanraj-r]

Thanks @p4paul

I got the mvn package to work (had to point it to the newer JDK) and test out some changes.

I am testing these changes with the p4 plugin option set to 'Forced clean and sync'. Created a shelve which added a file that causes the tests to fail and hence the review build should fail.

1. In unshelveFiles() I replaced with rOpts.setNoClientRefresh(true) and tried. Still unshelve wasn't working.
   • Build passes. watch-ing and ls-ing during build doesn't show the new file in shelve.
     • Could see that the workspace gets cleaned (as expected due to the 'Forced clean and sync' option) and workspace gets synced to the latest checked in CL - but the shelved files don't show up.
2. Commented out all the revert related code in unshelveFiles(). Still same result as above - Build passes and shelved file doesn't show up.
3. At this point I am wondering if the iclient.unshelveChangelist(review, null, 0, true, false) command is working?
   • There are some reports of the API not working: report1, report2
   • I am wondering if the clientChangelistId of 0 in iclient.unshelveChangelist(..) could be causing the issue.
     • when I tried a local command got an error saying 0 is invalid

$ p4 unshelve -f -s 9999999 -c 0
Invalid changelist number '0'.

     - maybe the p4java library is treating `0` as a special value and doing something special?

 -  If `0` is the issue, then the workaround maybe to create a new CL and give it to -c and then delete it at the end ?

Paul, If the issue is in the p4java library can you please help get this escalated to their notice and fix it (assuming you are working for perforce).

[p4paul]

I am a bit puzzled as shelving/review works for me and many other Jenkins users, there is also test coverage for unshelving stage.

I'm guessing there is a configuration issue with your builds. How are you triggering a review build? Unshelving can be invoked either through Swarm or another web app calling the review/build/ URL, or by using the 'Build Review' button.

To verify your Job configuration please click on the 'Build Review' and fill put the following fields.

review: 12345 (shelf to review)
change: (leave empty for head or specify a submitted change)
status: shelved

(ignore the other fields)

Click build and check you workspace for the unshelved content. Please let me know the result and send me the Jenkins console output for the build.

To answer your questions; p4java uses a '0' to indicate the default changelist, this will not work on the command line.

Kind regards,
Paul

On 27 Mar 2015, at 20:42, Mohan Raj Rajamanickam notifications@github.com wrote:

Thanks @p4paul

I got the mvn package to work (had to point it to the newer JDK) and test out some changes.

I am testing these changes with the p4 plugin option set to 'Forced clean and sync'. Created a shelve which added a file that causes the tests to fail and hence the review build should fail.

• In unshelveFiles() I replaced with rOpts.setNoClientRefresh(true) and tried. Still unshelve wasn't working.

  • Build passes. watch-ing and ls-ing during build doesn't show the new file in shelve.
      • Could see that the workspace gets cleaned (as expected due to the 'Forced clean and sync' option) and workspace gets synced to the latest checked in CL - but the shelved files don't show up.

• Commented out all the revert related code in unshelveFiles(). Still same result as above - Build passes and shelved file doesn't show up.

• At this point I am wondering if the iclient.unshelveChangelist(review, null, 0, true, false) command is working?

  • There are some reports of the API not working: report1, report2
  • I am wondering if the clientChangelistId of 0 in iclient.unshelveChangelist(..) could be causing the issue.
      • when I tried a local command got an error saying 0 is invalid

$ p4 unshelve -f -s 9999999 -c 0
Invalid changelist number '0'.

 - maybe the p4java library is treating `0` as a special value and doing something special?

• If 0 is the issue, then the workaround maybe to create a new CL and give it to -c and then delete it at the end ?

Paul, If the issue is in the p4java library can you please help get this escalated to their notice and fix it (assuming you are working for perforce).

—
Reply to this email directly or view it on GitHub.

---

This email and any files transmitted with it are confidential and intended
solely for the use of the individual or entity to whom they are addressed. If
you have received this email in error please notify the system manager. Please
note that any views or opinions presented in this email are solely those of the
author and do not necessarily represent those of Perforce Software. Finally,
the recipient should check this email and any attachments for the presence of
viruses. Perforce Software accepts no liability for any damage caused by any
virus transmitted by this email.

Perforce Software UK Ltd is registered in England and Wales as company no.
3816019 at the following address: West Forest Gate, Wellington Road, Wokingham,

RG40 2AT, UK

[mohanraj-r]

Thanks @p4paul
I have triggered the builds by using both the 'Build Review' button and the review build url (.../review/build?review=9999999&status=shelved).

I triggered a build using the 'Build Review' button as you have specified and following is the output.
The unshelving doesn't work - because

• the build passes when it should fail because the shelve contains a file with errors
• the ls command on the workspace doesn't show the file in shelve.

Connected to server: p4.xxx.xxx.com:1999
Connected to client: xx.xx.xx.critic.tmp
SCM Task: cleanup workspace: xx.xx.xx.critic.tmp
SCM Task: reverting all pending and shelved revisions.
... [list] = revert
... size[list] = 1
... rm [list] | ABANDONED
... duration: (52ms)
SCM Task: syncing files at change: 0
... sync /home/hudson/jenkins_home/workspace/xx.xx.xx.critic.tmp/...@0
... force update true
... bypass have true
... duration: 0m 1s
... rm -rf /home/hudson/jenkins_home/workspace/xx.xx.xx.critic.tmp
SCM Task: syncing files at change: 9988888
... sync /home/hudson/jenkins_home/workspace/xx.xx.xx.critic.tmp/...@9988888
... force update true
... bypass have true
... duration: 0m 5s
SCM Task: unshelve review: 9999999
... unshelve -f -s 9999999
... revert -k /home/hudson/jenkins_home/workspace/xx.xx.xx.critic.tmp/...
... duration: (129ms)
Calculating built changes... 
Saved to file... 
[xx.xx.xx.critic.tmp] $ /bin/sh -xe /tmp/hudson4860246412020756479.sh
+ ls /home/hudson/jenkins_home/workspace/xx.xx.xx.critic.tmp/...
<ls output>

...

As a workaround for now can you give me a exec*Cmd e.g execInputStringStreamCmd that I can use to try the unshelve command directly (as discussed in some of the error reports)?

I tried p4.execStreamCmd("unshelve", new String[]{"-s", "-f", ""+review}); instead of p4.unshelveFiles(review); in p4/tasks/CheckoutTask.java but got a compile error that the object doesn't contain the method. Not sure how to get the server object.

[p4paul]

Hi Mohan,

This is most likely a configuration issue with your Jenkins or Perforce deployment.

As I stated in my earlier email; the plugin's unshelve feature is widely used both here in Perforce and across many of our customers.

Please test you can unshelve the change on the Jenkins master/slave in the working area as the Jenkins user.

cd <workspace_area_used_by_jenkins>
p4 -u <jenkins_user> -p -c <workspace_used_by_jenkins> unshelve -s <shelved_change_number>

Check list:

• Does the Perforce Jenkins user defined in the credential have permission to read/open the shelved files.
• Does the Jenkins Workspace view mapping include the area where the files are shelved.
• Are there files actually in the shelf or are they just in the pending change.
• Attempt the same with a new Perforce workspace, encase the working/have records are in a strange state.

If you are unable to find the issue, please raise a ticket with our support team (support@perforce.com) and provide the following details. Feel free to CC me on the support case.

(1) p4 -Ztag info

(2) p4 client -o <jenkins_workspace>

(3) p4 -u <jenkins_user> protects

(4) p4 describe -s <shelved_change>

(5) config.xml file for the Jenkins job

I do not recommend making changes to the source code until you have resolved the configuration issue. In addition I do not recommend the use of execMap/Stream functions! For reference your parameters are out of order:

...{"-s" + review, "-f"}

(you need to append the change number to the '-s' flag and not as a separate parameter)

Kind regards,
Paul

On 30 Mar 2015, at 19:05, Mohan Raj Rajamanickam notifications@github.com wrote:

Thanks @p4paul
I have triggered the builds by using both the 'Build Review' button and the review build url (.../review/build?review=9999999&status=shelved).

I triggered a build using the 'Build Review' button as you have specified and following is the output.
The unshelving doesn't work - because

• the build passes when it should fail because the shelve contains a file with errors
• the ls command on the workspace doesn't show the file in shelve.
Connected to server: p4.xxx.xxx.com:1999
Connected to client: xx.xx.xx.critic.tmp
SCM Task: cleanup workspace: xx.xx.xx.critic.tmp
SCM Task: reverting all pending and shelved revisions.
... [list] = revert
... size[list] = 1
... rm [list] | ABANDONED
... duration: (52ms)
SCM Task: syncing files at change: 0
... sync /home/hudson/jenkins_home/workspace/xx.xx.xx.critic.tmp/...@0
... force update true
... bypass have true
... duration: 0m 1s
... rm -rf /home/hudson/jenkins_home/workspace/xx.xx.xx.critic.tmp
SCM Task: syncing files at change: 9988888
... sync /home/hudson/jenkins_home/workspace/xx.xx.xx.critic.tmp/...@9988888
... force update true
... bypass have true
... duration: 0m 5s
SCM Task: unshelve review: 9999999
... unshelve -f -s 9999999
... revert -k /home/hudson/jenkins_home/workspace/xx.xx.xx.critic.tmp/...
... duration: (129ms)
Calculating built changes...
Saved to file...
[xx.xx.xx.critic.tmp] $ /bin/sh -xe /tmp/hudson4860246412020756479.sh

• ls /home/hudson/jenkins_home/workspace/xx.xx.xx.critic.tmp/...
  

...

As a workaround for now can you give me a exec*Cmd e.g execInputStringStreamCmd that I can use to try the unshelve command directly (as discussed in some of the error reports)?

I tried p4.execStreamCmd("unshelve", new String[]{"-s", "-f", ""+review}); instead of p4.unshelveFiles(review); in p4/tasks/CheckoutTask.java but got a compile error that the object doesn't contain the method. Not sure how to get the server object.

—
Reply to this email directly or view it on GitHub.

---

This email and any files transmitted with it are confidential and intended
solely for the use of the individual or entity to whom they are addressed. If
you have received this email in error please notify the system manager. Please
note that any views or opinions presented in this email are solely those of the
author and do not necessarily represent those of Perforce Software. Finally,
the recipient should check this email and any attachments for the presence of
viruses. Perforce Software accepts no liability for any damage caused by any
virus transmitted by this email.

Perforce Software UK Ltd is registered in England and Wales as company no.
3816019 at the following address: West Forest Gate, Wellington Road, Wokingham,

RG40 2AT, UK

[mohanraj-r]

Thanks @p4paul

Turns out it was a permission issue with the jenkins p4 user we were using.
Configuring the build to use another regular p4 user fixes the issue.

Sorry for the confusion.
It might be helpful if it is possible to detect the error ("No permission for operation on file(s).") with the p4java lib during unshelving and print it out in the plugin.

Closing this ..

[p4paul]

Hi Mohan,

I'm glad you were able to resolve the issue. It turned out to be a useful exercise as there was a minor bug with the '-n' flag instead of '-k'.

I agree there is some work to be done on error logging; errors seem to sometimes end up in the Jenkins log and not the Job's console output.

Kind regards,
Paul

On 31 Mar 2015, at 21:03, Mohan Raj Rajamanickam notifications@github.com wrote:

Thanks @p4paul

Turns out it was a permission issue with the jenkins p4 user we were using.
Configuring the build to use another regular p4 user fixes the issue.

Sorry for the confusion.
It might be helpful if it is possible to detect the error ("No permission for operation on file(s).") with the p4java lib during unshelving and print it out in the plugin.

Closing this ..

—
Reply to this email directly or view it on GitHub.

---

This email and any files transmitted with it are confidential and intended
solely for the use of the individual or entity to whom they are addressed. If
you have received this email in error please notify the system manager. Please
note that any views or opinions presented in this email are solely those of the
author and do not necessarily represent those of Perforce Software. Finally,
the recipient should check this email and any attachments for the presence of
viruses. Perforce Software accepts no liability for any damage caused by any
virus transmitted by this email.

Perforce Software UK Ltd is registered in England and Wales as company no.
3816019 at the following address: West Forest Gate, Wellington Road, Wokingham,

RG40 2AT, UK