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

Subversion plugin doesn't adhere getAffectedPaths() contract

    XMLWordPrintable

    Details

    • Similar Issues:

      Description

      While trying to get the maven incremental build to work, I noticed the subversion changesets reported always start at the root of the subversion server.

      In fact, maven (And the contract) expect the changesets to be relative to the checkout url.

      e.g.
      http://subversion/path/to/trunk/
      When changes are done in "modulename" inside trunk, the changeset reports:
      /path/to/trunk/modulename/changedfile.java
      While maven (And the contract) expect:
      modulename/changedfile.java

      This completely breaks the incremental case, and possibly some other (Yet undetected) cases.

        Attachments

          Issue Links

            Activity

            Show
            cpf_ Mathias Teugels added a comment - See https://github.com/jenkinsci/subversion-plugin/pull/23
            Hide
            kutzi kutzi added a comment -

            In fact, I've successfully used Maven incremental builds with svn-plugin in the past. So you might have a different problem.
            Also I see code in the svn-plugin which seems to handle this already (preparePath method).
            Have you looked at this and can you explain why it does not work in your case?

            Show
            kutzi kutzi added a comment - In fact, I've successfully used Maven incremental builds with svn-plugin in the past. So you might have a different problem. Also I see code in the svn-plugin which seems to handle this already (preparePath method). Have you looked at this and can you explain why it does not work in your case?
            Hide
            kutzi kutzi added a comment -

            Unfortunately, the contract of getAffectedPath() and getAffectedFiles() is not very clear (at least to me). https://github.com/jenkinsci/jenkins/blob/master/core/src/main/java/hudson/scm/ChangeLogSet.java

            In fact, it implies that paths should be relative to workspace root, but doesn't say so explicitely.
            Also getAffectedFiles() seems to be differ substantially in the svn implementation, because - AFAI can see - it doesn't use the preparePath method.
            This seems to be a big mess. However, unfortunately I cannot see, how to change this without severely breaking backwards compatibility.

            Show
            kutzi kutzi added a comment - Unfortunately, the contract of getAffectedPath() and getAffectedFiles() is not very clear (at least to me). https://github.com/jenkinsci/jenkins/blob/master/core/src/main/java/hudson/scm/ChangeLogSet.java In fact, it implies that paths should be relative to workspace root, but doesn't say so explicitely. Also getAffectedFiles() seems to be differ substantially in the svn implementation, because - AFAI can see - it doesn't use the preparePath method. This seems to be a big mess. However, unfortunately I cannot see, how to change this without severely breaking backwards compatibility.
            Hide
            cpf_ Mathias Teugels added a comment -

            At the very least, it's supposed to be normalized to not start with a '/'
            Also, and this might not be seen as the best way of proceeding, but we might want to check out the implementations of other SCM plugins.

            Another source for information might be the usages of the function, where it is used that I know for sure is the maven plugin. This plugin assumes that the normalization at the very least has been taken care of (removing the leading '/'), and also assumes it is a relative path to the workspace. Perhaps this last assumption is wrong and might need fixing there, but in the end, one change we are sure about is the leading '/'!

            Show
            cpf_ Mathias Teugels added a comment - At the very least, it's supposed to be normalized to not start with a '/' Also, and this might not be seen as the best way of proceeding, but we might want to check out the implementations of other SCM plugins. Another source for information might be the usages of the function, where it is used that I know for sure is the maven plugin. This plugin assumes that the normalization at the very least has been taken care of (removing the leading '/'), and also assumes it is a relative path to the workspace. Perhaps this last assumption is wrong and might need fixing there, but in the end, one change we are sure about is the leading '/'!
            Hide
            danielbeck Daniel Beck added a comment -

            Possible solution for this issue: https://github.com/jenkinsci/subversion-plugin/pull/80

            I'd like to get some feedback whether this actually works, so please check it out.

            Show
            danielbeck Daniel Beck added a comment - Possible solution for this issue: https://github.com/jenkinsci/subversion-plugin/pull/80 I'd like to get some feedback whether this actually works, so please check it out.
            Hide
            scm_issue_link SCM/JIRA link daemon added a comment -

            Code changed in jenkins
            User: Stephen Connolly
            Path:
            src/main/java/hudson/scm/DirAwareSVNXMLLogHandler.java
            src/main/java/hudson/scm/SubversionChangeLogBuilder.java
            src/main/java/hudson/scm/SubversionChangeLogSet.java
            src/test/java/hudson/scm/SubversionChangeLogParserTest.java
            src/test/resources/hudson/scm/changelog_relativepath.xml
            http://jenkins-ci.org/commit/subversion-plugin/5a2683ed4a7d8b384577188cb4733345afb14d71
            Log:
            Merge pull request #80 from daniel-beck/JENKINS-18574

            [RFC] FIX JENKINS-18574/JENKINS-16045

            Compare: https://github.com/jenkinsci/subversion-plugin/compare/6e4e39127eb7...5a2683ed4a7d

            Show
            scm_issue_link SCM/JIRA link daemon added a comment - Code changed in jenkins User: Stephen Connolly Path: src/main/java/hudson/scm/DirAwareSVNXMLLogHandler.java src/main/java/hudson/scm/SubversionChangeLogBuilder.java src/main/java/hudson/scm/SubversionChangeLogSet.java src/test/java/hudson/scm/SubversionChangeLogParserTest.java src/test/resources/hudson/scm/changelog_relativepath.xml http://jenkins-ci.org/commit/subversion-plugin/5a2683ed4a7d8b384577188cb4733345afb14d71 Log: Merge pull request #80 from daniel-beck/ JENKINS-18574 [RFC] FIX JENKINS-18574 / JENKINS-16045 Compare: https://github.com/jenkinsci/subversion-plugin/compare/6e4e39127eb7...5a2683ed4a7d
            Hide
            jamesdumay James Dumay added a comment -

            Assuming due to the lack of feedback here that the PR merged has fixed this issue. If that is not the case, please reopen.

            Show
            jamesdumay James Dumay added a comment - Assuming due to the lack of feedback here that the PR merged has fixed this issue. If that is not the case, please reopen.

              People

              Assignee:
              Unassigned Unassigned
              Reporter:
              cpf_ Mathias Teugels
              Votes:
              1 Vote for this issue
              Watchers:
              6 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved: