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

Git plugin no longer updates submodules with spaces in the name

    Details

    • Similar Issues:

      Description

      If a submodule has a space in its name then Jenkins will fail to update that submodule. It looks like the reason is that the regex search has changed from:

      git config -f .gitmodules --get-regexp ^submodule\.(.*)\.url

      to:

      git config -f .gitmodules --get-regexp ^submodule\.([^ ].*)\.url

      This now ignores a submodule with a name something like:

      Stingray/Stingray Studio 12.2

        Attachments

          Issue Links

            Activity

            Hide
            markewaite Mark Waite added a comment -

            Sorry about that!

            In the short term, you'll need to switch from git client 2.7.0 back to git client 2.6.0. That will also force you to switch from git plugin 3.7.0 back to git plugin 3.6.4.

            At first glance, I don't see a solution which will resolve both JENKINS-46054 and this bug. The parsing of the .gitmodules file will likely need to switch from using a regular expression to some other technique.

            I'd love to have an additional test proposed which shows this problem in the unit tests of the plugin. Even better would be a test followed by a proposed fix. You can refer to the unit tests I created, the integration test I created, and the change which satisfied those tests.

            Ultimately, JENKINS-43977, JENKINS-41553, and JENKINS-38860 will force a different method of submodule management in the plugin.

            I was surprised that an embedded space is allowed in a submodule name. Sorry that I missed that case when writing the original change!

            Show
            markewaite Mark Waite added a comment - Sorry about that! In the short term, you'll need to switch from git client 2.7.0 back to git client 2.6.0. That will also force you to switch from git plugin 3.7.0 back to git plugin 3.6.4. At first glance, I don't see a solution which will resolve both JENKINS-46054 and this bug. The parsing of the .gitmodules file will likely need to switch from using a regular expression to some other technique. I'd love to have an additional test proposed which shows this problem in the unit tests of the plugin. Even better would be a test followed by a proposed fix. You can refer to the unit tests I created , the integration test I created , and the change which satisfied those tests . Ultimately, JENKINS-43977 , JENKINS-41553 , and JENKINS-38860 will force a different method of submodule management in the plugin. I was surprised that an embedded space is allowed in a submodule name. Sorry that I missed that case when writing the original change!
            Hide
            stevebaxter Steve Baxter added a comment -

            Thanks for the investigation!

            Show
            stevebaxter Steve Baxter added a comment - Thanks for the investigation!
            Hide
            stevebaxter Steve Baxter added a comment -

            Would the second idea proposed in JENKINS-46054 fix both, e.g.:

            "^submodule\\.(.*)
            .url " (with a space at the end)

            It's not ideal but might be a short-term fix?

            Show
            stevebaxter Steve Baxter added a comment - Would the second idea proposed in JENKINS-46054 fix both, e.g.: "^submodule\\.(.*) .url " (with a space at the end) It's not ideal but might be a short-term fix?
            Hide
            flabrie Francis Labrie added a comment -

            I have this issue too. The best would be a patter like this one: 

            git config -f .gitmodules --get-regexp '^submodule\.(.+)\.url$'
            

            Only the final .url is matched then.

             

            Show
            flabrie Francis Labrie added a comment - I have this issue too. The best would be a patter like this one:  git config -f .gitmodules --get-regexp '^submodule\.(.+)\.url$' Only the final .url is matched then.  
            Hide
            markewaite Mark Waite added a comment -

            Francis Labrie, the proposed regular expression seems to reintroduce JENKINS-46054. If I'm reintroducing JENKINS-46054, I'd prefer to revert the change which caused the problem. I would keep the tests which show that there is a problem, and could use them for later evaluation of alternatives.

            I think that failure to support a module name that ends with '.url' (JENKINS-46054) seems much less severe than failure to support a module name that includes embedded spaces.

            Does that revert plan seem reasonable to you?

            Show
            markewaite Mark Waite added a comment - Francis Labrie , the proposed regular expression seems to reintroduce JENKINS-46054 . If I'm reintroducing JENKINS-46054 , I'd prefer to revert the change which caused the problem. I would keep the tests which show that there is a problem, and could use them for later evaluation of alternatives. I think that failure to support a module name that ends with '.url' ( JENKINS-46054 ) seems much less severe than failure to support a module name that includes embedded spaces. Does that revert plan seem reasonable to you?
            Hide
            flabrie Francis Labrie added a comment -

            For me, yes it seems reasonable. Thanks!

            Show
            flabrie Francis Labrie added a comment - For me, yes it seems reasonable. Thanks!
            Hide
            markewaite Mark Waite added a comment - - edited

            I submitted a git client plugin pull request to improve the tests and provide a better fix that resolves JENKINS-46054 and this bug.

            The pull request creates an evaluation build of the plugin.

            Francis Labrie or Steve Baxter, could you test that evaluation build in your environment to confirm it fixes the problem? I'd love to also have you review the pull request if you're comfortable with code review. It uses the technique suggested by Francis Labrie with an additional extension to handle additional cases covered in the automated tests.

            It passes all the automated tests and passes the previously failing test case and the second test case that I added to my regression test setup.

            Show
            markewaite Mark Waite added a comment - - edited I submitted a git client plugin pull request to improve the tests and provide a better fix that resolves JENKINS-46054 and this bug. The pull request creates an evaluation build of the plugin . Francis Labrie or Steve Baxter , could you test that evaluation build in your environment to confirm it fixes the problem? I'd love to also have you review the pull request if you're comfortable with code review. It uses the technique suggested by Francis Labrie with an additional extension to handle additional cases covered in the automated tests. It passes all the automated tests and passes the previously failing test case and the second test case that I added to my regression test setup.
            Hide
            flabrie Francis Labrie added a comment -

            I've tried to install the evaluation build of the plugin but it fails got install every time, I don't understand why:

            org.apache.commons.fileupload.FileUploadBase$IOFileUploadException: Processing of multipart/form-data request failed. java.util.concurrent.TimeoutException: Idle timeout expired: 5084/5000 ms
            	at org.apache.commons.fileupload.FileUploadBase.parseRequest(FileUploadBase.java:351)
            	...

            Any hint to avoid this error?

            Show
            flabrie Francis Labrie added a comment - I've tried to install the  evaluation build of the plugin  but it fails got install every time, I don't understand why: org.apache.commons.fileupload.FileUploadBase$IOFileUploadException: Processing of multipart/form-data request failed. java.util.concurrent.TimeoutException: Idle timeout expired: 5084/5000 ms at org.apache.commons.fileupload.FileUploadBase.parseRequest(FileUploadBase.java:351) ... Any hint to avoid this error?
            Hide
            markewaite Mark Waite added a comment -

            Francis Labrie I've never seen that error. Are you attempting to upload the evaluation build from the "Manage Jenkins" -> "Manage Plugins" -> "Advanced" tab, where there is a "Choose File" box?

            Show
            markewaite Mark Waite added a comment - Francis Labrie I've never seen that error. Are you attempting to upload the evaluation build from the "Manage Jenkins" -> "Manage Plugins" -> "Advanced" tab, where there is a "Choose File" box?
            Hide
            flabrie Francis Labrie added a comment -

            Yes!

            Show
            flabrie Francis Labrie added a comment - Yes!
            Hide
            markewaite Mark Waite added a comment - - edited

            Is there a proxy between you and the Jenkins server which might be causing the upload data to be damaged in transit?

            Are you running under Tomcat or some other web container process which might be disrupting the upload process?

            Does the output of "jar -tvf git-client.hpi" output content like this:

                 0 Sat Jan 20 22:31:28 MST 2018 META-INF/
               783 Sat Jan 20 22:31:26 MST 2018 META-INF/MANIFEST.MF
                 0 Sat Jan 20 22:31:28 MST 2018 WEB-INF/
                 0 Sat Jan 20 22:31:28 MST 2018 WEB-INF/lib/
            212158 Sat Jan 20 22:31:28 MST 2018 WEB-INF/lib/git-client.jar
            125146 Sat Jan 20 22:31:28 MST 2018 WEB-INF/lib/JavaEWAH-0.7.9.jar
            2384093 Sat Jan 20 22:31:28 MST 2018 WEB-INF/lib/org.eclipse.jgit-4.5.4.201711221230-r.jar
             21793 Sat Jan 20 22:31:28 MST 2018 WEB-INF/lib/org.eclipse.jgit.http.apache-4.5.4.201711221230-r.jar
             97098 Sat Jan 20 22:31:28 MST 2018 WEB-INF/lib/org.eclipse.jgit.http.server-4.5.4.201711221230-r.jar
              2548 Sat Jan 20 22:31:28 MST 2018 WEB-INF/licenses.xml
                 0 Sat Jan 20 22:31:28 MST 2018 META-INF/maven/
                 0 Sat Jan 20 22:31:28 MST 2018 META-INF/maven/org.jenkins-ci.plugins/
                 0 Sat Jan 20 22:31:28 MST 2018 META-INF/maven/org.jenkins-ci.plugins/git-client/
             10027 Sat Jan 20 22:17:04 MST 2018 META-INF/maven/org.jenkins-ci.plugins/git-client/pom.xml
               131 Sat Jan 20 22:31:28 MST 2018 META-INF/maven/org.jenkins-ci.plugins/git-client/pom.properties
            

            With the explanatory note that the date stamps in your archive will be displayed differently than mine, unless you're in the US Mountain time zone.

            Show
            markewaite Mark Waite added a comment - - edited Is there a proxy between you and the Jenkins server which might be causing the upload data to be damaged in transit? Are you running under Tomcat or some other web container process which might be disrupting the upload process? Does the output of "jar -tvf git-client.hpi" output content like this: 0 Sat Jan 20 22:31:28 MST 2018 META-INF/ 783 Sat Jan 20 22:31:26 MST 2018 META-INF/MANIFEST.MF 0 Sat Jan 20 22:31:28 MST 2018 WEB-INF/ 0 Sat Jan 20 22:31:28 MST 2018 WEB-INF/lib/ 212158 Sat Jan 20 22:31:28 MST 2018 WEB-INF/lib/git-client.jar 125146 Sat Jan 20 22:31:28 MST 2018 WEB-INF/lib/JavaEWAH-0.7.9.jar 2384093 Sat Jan 20 22:31:28 MST 2018 WEB-INF/lib/org.eclipse.jgit-4.5.4.201711221230-r.jar 21793 Sat Jan 20 22:31:28 MST 2018 WEB-INF/lib/org.eclipse.jgit.http.apache-4.5.4.201711221230-r.jar 97098 Sat Jan 20 22:31:28 MST 2018 WEB-INF/lib/org.eclipse.jgit.http.server-4.5.4.201711221230-r.jar 2548 Sat Jan 20 22:31:28 MST 2018 WEB-INF/licenses.xml 0 Sat Jan 20 22:31:28 MST 2018 META-INF/maven/ 0 Sat Jan 20 22:31:28 MST 2018 META-INF/maven/org.jenkins-ci.plugins/ 0 Sat Jan 20 22:31:28 MST 2018 META-INF/maven/org.jenkins-ci.plugins/git-client/ 10027 Sat Jan 20 22:17:04 MST 2018 META-INF/maven/org.jenkins-ci.plugins/git-client/pom.xml 131 Sat Jan 20 22:31:28 MST 2018 META-INF/maven/org.jenkins-ci.plugins/git-client/pom.properties With the explanatory note that the date stamps in your archive will be displayed differently than mine, unless you're in the US Mountain time zone.
            Hide
            flabrie Francis Labrie added a comment -

            The file was correct, like the one above. I was using a VPN. So I tried to use another network access to reach the Jenkins server, and the installation did work that time. Thank you very much!

            I've tested this evaluation build, and it works well with submodules having spaces in their names. So far so good, thanks Mark.

            Show
            flabrie Francis Labrie added a comment - The file was correct, like the one above. I was using a VPN. So I tried to use another network access to reach the Jenkins server, and the installation did work that time. Thank you very much! I've tested this evaluation build, and it works well with submodules having spaces in their names. So far so good, thanks Mark.
            Hide
            stevebaxter Steve Baxter added a comment -

            Unfortunately I don't have admin access to the Jenkins server where we are seeing this problem so it's not easy for me to test the fix. Glad to hear it works though, thanks for looking into this so quickly!

            Show
            stevebaxter Steve Baxter added a comment - Unfortunately I don't have admin access to the Jenkins server where we are seeing this problem so it's not easy for me to test the fix. Glad to hear it works though, thanks for looking into this so quickly!
            Hide
            markewaite Mark Waite added a comment - - edited

            Thanks to both of you. I need a code review of the pull request, then I can release a new version. Francis Labrie, since you suggested the regular expression change, would you be willing to review the change?

            Show
            markewaite Mark Waite added a comment - - edited Thanks to both of you. I need a code review of the pull request , then I can release a new version. Francis Labrie , since you suggested the regular expression change, would you be willing to review the change?
            Hide
            flabrie Francis Labrie added a comment -

            Done

            Show
            flabrie Francis Labrie added a comment - Done
            Hide
            scm_issue_link SCM/JIRA link daemon added a comment -

            Code changed in jenkins
            User: Mark Waite
            Path:
            src/main/java/org/jenkinsci/plugins/gitclient/CliGitAPIImpl.java
            src/test/java/org/jenkinsci/plugins/gitclient/GitClientTest.java
            src/test/java/org/jenkinsci/plugins/gitclient/SubmodulePatternStringTest.java
            http://jenkins-ci.org/commit/git-client-plugin/69297552b3c36e084bea336448c6ad82763b303a
            Log:
            Fix both JENKINS-48818 and JENKINS-46054

            Add JENKINS-48818 test - space character in submodule remote name

            Submodule remote name which contains a space character was supported by
            git client plugin versions prior to 2.7.0. The bug fix for JENKINS-46054
            (support submodule update when the remote name or the remote URL end with
            '.url') broke support for submodule remote names which contain a space.

            This includes a test to show that a remote name containing a space
            character is not correctly handled by the 2.7.0 change.

            Show
            scm_issue_link SCM/JIRA link daemon added a comment - Code changed in jenkins User: Mark Waite Path: src/main/java/org/jenkinsci/plugins/gitclient/CliGitAPIImpl.java src/test/java/org/jenkinsci/plugins/gitclient/GitClientTest.java src/test/java/org/jenkinsci/plugins/gitclient/SubmodulePatternStringTest.java http://jenkins-ci.org/commit/git-client-plugin/69297552b3c36e084bea336448c6ad82763b303a Log: Fix both JENKINS-48818 and JENKINS-46054 Add JENKINS-48818 test - space character in submodule remote name Submodule remote name which contains a space character was supported by git client plugin versions prior to 2.7.0. The bug fix for JENKINS-46054 (support submodule update when the remote name or the remote URL end with '.url') broke support for submodule remote names which contain a space. This includes a test to show that a remote name containing a space character is not correctly handled by the 2.7.0 change.
            Hide
            scm_issue_link SCM/JIRA link daemon added a comment -

            Code changed in jenkins
            User: Mark Waite
            Path:
            src/main/java/org/jenkinsci/plugins/gitclient/CliGitAPIImpl.java
            src/test/java/org/jenkinsci/plugins/gitclient/SubmodulePatternStringTest.java
            http://jenkins-ci.org/commit/git-client-plugin/2dcbb2d62a980468d339e52f4f8dd6653473d5cd
            Log:
            Merge pull request #296 from MarkEWaite/explore-JENKINS-46054

            Fix JENKINS-48818 and keep JENKINS-46054 fixed

            Compare: https://github.com/jenkinsci/git-client-plugin/compare/21914aaf97f0...2dcbb2d62a98

            Show
            scm_issue_link SCM/JIRA link daemon added a comment - Code changed in jenkins User: Mark Waite Path: src/main/java/org/jenkinsci/plugins/gitclient/CliGitAPIImpl.java src/test/java/org/jenkinsci/plugins/gitclient/SubmodulePatternStringTest.java http://jenkins-ci.org/commit/git-client-plugin/2dcbb2d62a980468d339e52f4f8dd6653473d5cd Log: Merge pull request #296 from MarkEWaite/explore- JENKINS-46054 Fix JENKINS-48818 and keep JENKINS-46054 fixed Compare: https://github.com/jenkinsci/git-client-plugin/compare/21914aaf97f0...2dcbb2d62a98
            Hide
            markewaite Mark Waite added a comment -

            Fixed in git client plugin 2.7.1

            Show
            markewaite Mark Waite added a comment - Fixed in git client plugin 2.7.1

              People

              • Assignee:
                markewaite Mark Waite
                Reporter:
                stevebaxter Steve Baxter
              • Votes:
                1 Vote for this issue
                Watchers:
                4 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved: