Uploaded image for project: 'Jenkins'
  1. Jenkins
  2. JENKINS-8835

SVN_URL and SVN_REVISION (Including SVN_URL_X and SVN_REVISION_X) are never set when using svnurl@revision

    • Icon: Patch Patch
    • Resolution: Duplicate
    • Icon: Major Major
    • subversion-plugin
    • None
    • Windows 2003 server

      It seems that if you use a url such as http://myserver/svn/myrepo@HEAD or http://myserver/svn/myrepo@1234 then the SVN_REVISION and SVN_URL environment variables are never set.

      I can confirm this is the issue as removing @HEAD or @REV results in them being populated again (so in this case only using http://myserver/svn/myrepo).

          [JENKINS-8835] SVN_URL and SVN_REVISION (Including SVN_URL_X and SVN_REVISION_X) are never set when using svnurl@revision

          Paul M added a comment -

          It appears the problem is this:

          Line 449 SubversionSCM.java
          if(svnLocations.length==1) {
          Long rev = revisions.get(svnLocations[0].remote);
          if(rev!=null) {

          svnLocations[0].remote contains http://myserver/svn/repo@HEAD, and revisions[0] key contains http://myserver/svn/repo hence the value of the map "rev" is never found and is always null.

          The root of the @HEAD being set in svnLocations[0].remote seems to happen in:

          Line 2090 in SubversionScm.java here:

          public ModuleLocation(String remote, String local) {
          this.remote = Util.removeTrailingSlash(Util.fixNull(remote).trim());

          String remote = http://myserver/svn/repo@HEAD, I believe it should search for the last @ char and remove everything after it?

          Paul M added a comment - It appears the problem is this: Line 449 SubversionSCM.java if(svnLocations.length==1) { Long rev = revisions.get(svnLocations [0] .remote); if(rev!=null) { svnLocations [0] .remote contains http://myserver/svn/repo@HEAD , and revisions [0] key contains http://myserver/svn/repo hence the value of the map "rev" is never found and is always null. The root of the @HEAD being set in svnLocations [0] .remote seems to happen in: Line 2090 in SubversionScm.java here: public ModuleLocation(String remote, String local) { this.remote = Util.removeTrailingSlash(Util.fixNull(remote).trim()); String remote = http://myserver/svn/repo@HEAD , I believe it should search for the last @ char and remove everything after it?

          Paul M added a comment -

          I've made a patch that appears to resolve the issue, I've changed the constructor previously mentioned on line 2090 to this:

          @DataBoundConstructor
          public ModuleLocation(String remote, String local) {
          String tmpRemote = Util.removeTrailingSlash(Util.fixNull(remote).trim());

          // Look for an @ char in the case of http://svnserver/repos@HEAD or
          // http://svnserver@rev and remove @rev/@HEAD as only the first part of the URL is used in
          // the map for pulling out the revisions.
          int atPos = tmpRemote.lastIndexOf('@');
          if(atPos!=-1)

          { tmpRemote = tmpRemote.substring(0, atPos); }

          this.remote = tmpRemote;
          this.local = fixEmptyAndTrim(local);
          }

          Paul M added a comment - I've made a patch that appears to resolve the issue, I've changed the constructor previously mentioned on line 2090 to this: @DataBoundConstructor public ModuleLocation(String remote, String local) { String tmpRemote = Util.removeTrailingSlash(Util.fixNull(remote).trim()); // Look for an @ char in the case of http://svnserver/repos@HEAD or // http://svnserver@rev and remove @rev/@HEAD as only the first part of the URL is used in // the map for pulling out the revisions. int atPos = tmpRemote.lastIndexOf('@'); if(atPos!=-1) { tmpRemote = tmpRemote.substring(0, atPos); } this.remote = tmpRemote; this.local = fixEmptyAndTrim(local); }

          Paul M added a comment -

          Adding a real patch from svn.

          Paul M added a comment - Adding a real patch from svn.

          Paul M added a comment -

          After more testing it seems this patch breaks the @REV functionality as the edited URL is the one used to do the checkout (I.e the url http://myserver/svn/repos is always used).

          Not sure what the best way to fix this is? Perhaps not using a map for the revisions, or adding a new remotePlain field to the ModuleLocation class?

          Paul M added a comment - After more testing it seems this patch breaks the @REV functionality as the edited URL is the one used to do the checkout (I.e the url http://myserver/svn/repos is always used). Not sure what the best way to fix this is? Perhaps not using a map for the revisions, or adding a new remotePlain field to the ModuleLocation class?

          Paul M added a comment -

          Updated patch that works in all cases (no revision, revision and keyword such as HEAD).

          Attempted to keep code changes down by just calling another function inside map.get(), e.g map.get(getPlainUrl()).

          Paul M added a comment - Updated patch that works in all cases (no revision, revision and keyword such as HEAD). Attempted to keep code changes down by just calling another function inside map.get(), e.g map.get(getPlainUrl()).

          Sylvain C. added a comment -

          Hi, could you fix it in the next release ? thanks

          Sylvain C. added a comment - Hi, could you fix it in the next release ? thanks

          kutzi added a comment -

          I can have a look at it, if time permits.
          I didn't see any unit test in the patch. Paul could you add one please?

          kutzi added a comment - I can have a look at it, if time permits. I didn't see any unit test in the patch. Paul could you add one please?

          Sylvain C. added a comment -

          Hi, sorry to try again but this fix would be very important for me and my company, could you add some test to close the bug.
          Thanks

          Sylvain C. added a comment - Hi, sorry to try again but this fix would be very important for me and my company, could you add some test to close the bug. Thanks

          kutzi added a comment -

          Fixed with JENKINS-10942

          kutzi added a comment - Fixed with JENKINS-10942

            kutzi kutzi
            paulm Paul M
            Votes:
            1 Vote for this issue
            Watchers:
            4 Start watching this issue

              Created:
              Updated:
              Resolved: