JenkinsIcon indicating the project type

Jenkins

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

Gitweb URL can be mangled (if it doesn't contain a path?)

    Details

    • Similar Issues:

      Description

      With git plugin at b2a731e62272c93a60f7f45d9d3c5d3b1e57fa11 (so that repository browser URLs are saved) then the URL to our repository https://gerrit.company.local/gitweb?p=repo.git gets mangled to https://gerrit.company.local/gitweb/?p=repo.git. Note the extra slash after gitweb.

      I believe this is introduced by the call to normalizeToEndWithSlash() which was done for some repository types prior to https://github.com/jenkinsci/git-plugin/commit/7abfafcf58addbce2c268dbfed1910a90d4fe2a3 but is now done for all repository types in GitRepositoryBrowser. This is wrong for our particular usage. I'm happy to create a patch to fix this for all repo types that didn't formerly do this if some knows that this is the right way forward.

        Attachments

          Activity

          Ascending order - Click to sort in descending order
          Hide
          Permalink
          markewaite Mark Waite added a comment -

          Can you explain why the trailing slash does not work in your case? It works for me with gitweb. Is the problem that it is now done for all repository types?

          If that is the problem, then it seems reasonable that you create a patch with a test that asserts the failing case is fixed.

          Show
          markewaite Mark Waite added a comment - Can you explain why the trailing slash does not work in your case? It works for me with gitweb. Is the problem that it is now done for all repository types? If that is the problem, then it seems reasonable that you create a patch with a test that asserts the failing case is fixed.
          Hide
          Permalink
          joibel Alan Clucas added a comment -

          There is no directory (or virtual directory) called gitweb, it is just a cgi script that is only invoked by gitweb with no trailing slash on our config of apache. I'm sure I could fix this on the apache side.

          From a higher standpoint - the URL is getting changed unnecessarily, and used to work in version 2.0.1, hence thinking the fix is really something that should be done on the jenkins plugin side.

          I'm working on getting a presentable fix, but it won't be done for a week as I'm away now.

          Show
          joibel Alan Clucas added a comment - There is no directory (or virtual directory) called gitweb, it is just a cgi script that is only invoked by gitweb with no trailing slash on our config of apache. I'm sure I could fix this on the apache side. From a higher standpoint - the URL is getting changed unnecessarily, and used to work in version 2.0.1, hence thinking the fix is really something that should be done on the jenkins plugin side. I'm working on getting a presentable fix, but it won't be done for a week as I'm away now.
          Hide
          Permalink
          joibel Alan Clucas added a comment -

          Did get enough time to do this today, so have submitted a pull request for review https://github.com/jenkinsci/git-plugin/pull/217

          Show
          joibel Alan Clucas added a comment - Did get enough time to do this today, so have submitted a pull request for review https://github.com/jenkinsci/git-plugin/pull/217
          Hide
          Permalink
          joibel Alan Clucas added a comment -

          Can I persuade someone to look at this please?

          Show
          joibel Alan Clucas added a comment - Can I persuade someone to look at this please?
          Hide
          Permalink
          markewaite Mark Waite added a comment -

          I hope to look at it within the next 7 days. I'd prefer that Nicolas be the one to evaluate it, since he made the most recent code changes in that area, but he's been busy in other areas.

          Show
          markewaite Mark Waite added a comment - I hope to look at it within the next 7 days. I'd prefer that Nicolas be the one to evaluate it, since he made the most recent code changes in that area, but he's been busy in other areas.
          Hide
          Permalink
          apryce Adam Pryce added a comment -

          We are having the same problem. Our gitweb (through Gerrit with an Apache proxy) fails to find the resource with the additional slash.

          Show
          apryce Adam Pryce added a comment - We are having the same problem. Our gitweb (through Gerrit with an Apache proxy) fails to find the resource with the additional slash.
          Hide
          Permalink
          scm_issue_link SCM/JIRA link daemon added a comment -

          Code changed in jenkins
          User: Alan Clucas
          Path:
          src/main/java/hudson/plugins/git/browser/BitbucketWeb.java
          src/main/java/hudson/plugins/git/browser/CGit.java
          src/main/java/hudson/plugins/git/browser/FisheyeGitRepositoryBrowser.java
          src/main/java/hudson/plugins/git/browser/GitBlitRepositoryBrowser.java
          src/main/java/hudson/plugins/git/browser/GitLab.java
          src/main/java/hudson/plugins/git/browser/GitRepositoryBrowser.java
          src/main/java/hudson/plugins/git/browser/GitWeb.java
          src/main/java/hudson/plugins/git/browser/GithubWeb.java
          src/main/java/hudson/plugins/git/browser/GitoriousWeb.java
          src/main/java/hudson/plugins/git/browser/KilnGit.java
          src/main/java/hudson/plugins/git/browser/Phabricator.java
          src/main/java/hudson/plugins/git/browser/RedmineWeb.java
          src/main/java/hudson/plugins/git/browser/RhodeCode.java
          src/main/java/hudson/plugins/git/browser/Stash.java
          src/main/java/hudson/plugins/git/browser/ViewGitWeb.java
          http://jenkins-ci.org/commit/git-plugin/962e244a72dff3a79d1643d5107faa33f0ff74d8
          Log:
          Fix for JENKINS-22342

          Only normalize with slash certain repository types.
          Should match behaviour prior to
          7abfafcf58addbce2c268dbfed1910a90d4fe2a3

          Show
          scm_issue_link SCM/JIRA link daemon added a comment - Code changed in jenkins User: Alan Clucas Path: src/main/java/hudson/plugins/git/browser/BitbucketWeb.java src/main/java/hudson/plugins/git/browser/CGit.java src/main/java/hudson/plugins/git/browser/FisheyeGitRepositoryBrowser.java src/main/java/hudson/plugins/git/browser/GitBlitRepositoryBrowser.java src/main/java/hudson/plugins/git/browser/GitLab.java src/main/java/hudson/plugins/git/browser/GitRepositoryBrowser.java src/main/java/hudson/plugins/git/browser/GitWeb.java src/main/java/hudson/plugins/git/browser/GithubWeb.java src/main/java/hudson/plugins/git/browser/GitoriousWeb.java src/main/java/hudson/plugins/git/browser/KilnGit.java src/main/java/hudson/plugins/git/browser/Phabricator.java src/main/java/hudson/plugins/git/browser/RedmineWeb.java src/main/java/hudson/plugins/git/browser/RhodeCode.java src/main/java/hudson/plugins/git/browser/Stash.java src/main/java/hudson/plugins/git/browser/ViewGitWeb.java http://jenkins-ci.org/commit/git-plugin/962e244a72dff3a79d1643d5107faa33f0ff74d8 Log: Fix for JENKINS-22342 Only normalize with slash certain repository types. Should match behaviour prior to 7abfafcf58addbce2c268dbfed1910a90d4fe2a3
          Hide
          Permalink
          scm_issue_link SCM/JIRA link daemon added a comment -

          Code changed in jenkins
          User: Alan Clucas
          Path:
          src/test/java/hudson/plugins/git/browser/GitWebTest.java
          http://jenkins-ci.org/commit/git-plugin/1d2d879b56c8a2a1962c53273799a31c41ae2730
          Log:
          Add tests for gitweb repo broswer

          Add tests for gitweb, which include cover for JENKINS-22342

          Show
          scm_issue_link SCM/JIRA link daemon added a comment - Code changed in jenkins User: Alan Clucas Path: src/test/java/hudson/plugins/git/browser/GitWebTest.java http://jenkins-ci.org/commit/git-plugin/1d2d879b56c8a2a1962c53273799a31c41ae2730 Log: Add tests for gitweb repo broswer Add tests for gitweb, which include cover for JENKINS-22342
          Hide
          Permalink
          markewaite Mark Waite added a comment -

          Will be available in the next git-plugin release after 2.2.0

          Show
          markewaite Mark Waite added a comment - Will be available in the next git-plugin release after 2.2.0
          Hide
          Permalink
          markewaite Mark Waite added a comment -

          The change which is now on the tip of the git-plugin also breaks the URL for githubweb. It is important that the githubweb URL (and other URLs) work before the next release of the git-plugin.

          Show
          markewaite Mark Waite added a comment - The change which is now on the tip of the git-plugin also breaks the URL for githubweb. It is important that the githubweb URL (and other URLs) work before the next release of the git-plugin.
          Hide
          Permalink
          joibel Alan Clucas added a comment -

          What kind of URLs does it break - do you have any examples? It's passing the test cases, so I'm not sure what needs fixing.

          Show
          joibel Alan Clucas added a comment - What kind of URLs does it break - do you have any examples? It's passing the test cases, so I'm not sure what needs fixing.
          Hide
          Permalink
          markewaite Mark Waite added a comment - - edited

          It appears this change breaks the previously working githubweb links until the next time the job is saved. We can't release the plugin with that behavior because we can't force our users to save the job definition in order to restore something that was working previously.

          The steps I took to show the problem were:

          1. Define a job using GitHub URL https://github.com/jenkinsci/git-plugin and GitHub repository browser URL https://github.com/jenkinsci/git-plugin
          2. Save that job
          3. Let the job run when changes arrive on that GitHub project
          4. Upgrade the git plugin to the latest unreleased version
          5. Open the job and click one of the recent changes links to githubweb. A slash will be missing from the githubweb URL. Instead of https://github.com/jenkinsci/git-plugin/commit/fb12fed4c1c8af11db68ec236961401d3d11eb7b it will be https://github.com/jenkinsci/git-plugincommit/fb12fed4c1c8af11db68ec236961401d3d11eb7b
          6. Configure the job
          7. Save that configuration without making any changes
          8. Click one of the recent change links to githubweb. A slash will be inserted correctly into the githubweb URL. The link fb12fed works

          Can you propose a change which will fix that case? If you don't have time for that, I'll reopen the bug and revert this change so that plugin releases of other fixes and improvements are not blocked.

          Show
          markewaite Mark Waite added a comment - - edited It appears this change breaks the previously working githubweb links until the next time the job is saved. We can't release the plugin with that behavior because we can't force our users to save the job definition in order to restore something that was working previously. The steps I took to show the problem were: Define a job using GitHub URL https://github.com/jenkinsci/git-plugin and GitHub repository browser URL https://github.com/jenkinsci/git-plugin Save that job Let the job run when changes arrive on that GitHub project Upgrade the git plugin to the latest unreleased version Open the job and click one of the recent changes links to githubweb. A slash will be missing from the githubweb URL. Instead of https://github.com/jenkinsci/git-plugin/commit/fb12fed4c1c8af11db68ec236961401d3d11eb7b it will be https://github.com/jenkinsci/git-plugincommit/fb12fed4c1c8af11db68ec236961401d3d11eb7b Configure the job Save that configuration without making any changes Click one of the recent change links to githubweb. A slash will be inserted correctly into the githubweb URL. The link fb12fed works Can you propose a change which will fix that case? If you don't have time for that, I'll reopen the bug and revert this change so that plugin releases of other fixes and improvements are not blocked.
          Hide
          Permalink
          markewaite Mark Waite added a comment -

          When I compare the job definitions, one point that may be relevant is that the jobs with the failing URL's do not have any normalizeUrl tag inserted into the job definition, while those jobs which have been edited get a normalizeUrl tag with the value "true".

          Show
          markewaite Mark Waite added a comment - When I compare the job definitions, one point that may be relevant is that the jobs with the failing URL's do not have any normalizeUrl tag inserted into the job definition, while those jobs which have been edited get a normalizeUrl tag with the value "true".
          Hide
          Permalink
          joibel Alan Clucas added a comment -

          Ok, that's me not understanding how all this stuff works. I can confirm that I'm sure my change breaks it. I'll have a look at fixing it today

          Show
          joibel Alan Clucas added a comment - Ok, that's me not understanding how all this stuff works. I can confirm that I'm sure my change breaks it. I'll have a look at fixing it today
          Hide
          Permalink
          markewaite Mark Waite added a comment -

          Thanks very much. I've alerted Nicolas to not release a new version of the plugin until we either have a fix or we have reverted the change.

          Show
          markewaite Mark Waite added a comment - Thanks very much. I've alerted Nicolas to not release a new version of the plugin until we either have a fix or we have reverted the change.
          Hide
          Permalink
          joibel Alan Clucas added a comment -

          Sent a pull request with a fix. Tested against Github and Gitweb. Now understand more than I want to about java serialization.

          Show
          joibel Alan Clucas added a comment - Sent a pull request with a fix. Tested against Github and Gitweb. Now understand more than I want to about java serialization.
          Hide
          Permalink
          markewaite Mark Waite added a comment -

          Thanks. I've started the automated evaluation of the fix in my various environments. Would you consider modifying your changes to minimize your diffs from git-2.2.0? I've submitted a pull request to your repository with my proposed changes for you to evaluate.

          Show
          markewaite Mark Waite added a comment - Thanks. I've started the automated evaluation of the fix in my various environments. Would you consider modifying your changes to minimize your diffs from git-2.2.0? I've submitted a pull request to your repository with my proposed changes for you to evaluate.
          Hide
          Permalink
          joibel Alan Clucas added a comment -

          I think I've done the right thing to accept them.

          Show
          joibel Alan Clucas added a comment - I think I've done the right thing to accept them.
          Hide
          Permalink
          joibel Alan Clucas added a comment -

          Just wondered if you wanted them squashing down?

          Show
          joibel Alan Clucas added a comment - Just wondered if you wanted them squashing down?
          Hide
          Permalink
          markewaite Mark Waite added a comment -

          I would prefer if you squashed them down so that there isn't an intermediate state where the constructor is missing.

          Show
          markewaite Mark Waite added a comment - I would prefer if you squashed them down so that there isn't an intermediate state where the constructor is missing.
          Hide
          Permalink
          scm_issue_link SCM/JIRA link daemon added a comment -

          Code changed in jenkins
          User: Alan Clucas
          Path:
          src/main/java/hudson/plugins/git/browser/BitbucketWeb.java
          src/main/java/hudson/plugins/git/browser/CGit.java
          src/main/java/hudson/plugins/git/browser/FisheyeGitRepositoryBrowser.java
          src/main/java/hudson/plugins/git/browser/GitBlitRepositoryBrowser.java
          src/main/java/hudson/plugins/git/browser/GitLab.java
          src/main/java/hudson/plugins/git/browser/GitRepositoryBrowser.java
          src/main/java/hudson/plugins/git/browser/GitWeb.java
          src/main/java/hudson/plugins/git/browser/GithubWeb.java
          src/main/java/hudson/plugins/git/browser/GitoriousWeb.java
          src/main/java/hudson/plugins/git/browser/KilnGit.java
          src/main/java/hudson/plugins/git/browser/Phabricator.java
          src/main/java/hudson/plugins/git/browser/RedmineWeb.java
          src/main/java/hudson/plugins/git/browser/RhodeCode.java
          src/main/java/hudson/plugins/git/browser/Stash.java
          src/main/java/hudson/plugins/git/browser/ViewGitWeb.java
          http://jenkins-ci.org/commit/git-plugin/510337b35a4665f1d938d35215801bd9d354554e
          Log:
          JENKINS-22342 ensure URL normalization correct

          Previous attempts to fix this stored the normalization flag
          in the serialized repository browser. Moved to be a function
          call to avoid the deserializer getting involved

          Show
          scm_issue_link SCM/JIRA link daemon added a comment - Code changed in jenkins User: Alan Clucas Path: src/main/java/hudson/plugins/git/browser/BitbucketWeb.java src/main/java/hudson/plugins/git/browser/CGit.java src/main/java/hudson/plugins/git/browser/FisheyeGitRepositoryBrowser.java src/main/java/hudson/plugins/git/browser/GitBlitRepositoryBrowser.java src/main/java/hudson/plugins/git/browser/GitLab.java src/main/java/hudson/plugins/git/browser/GitRepositoryBrowser.java src/main/java/hudson/plugins/git/browser/GitWeb.java src/main/java/hudson/plugins/git/browser/GithubWeb.java src/main/java/hudson/plugins/git/browser/GitoriousWeb.java src/main/java/hudson/plugins/git/browser/KilnGit.java src/main/java/hudson/plugins/git/browser/Phabricator.java src/main/java/hudson/plugins/git/browser/RedmineWeb.java src/main/java/hudson/plugins/git/browser/RhodeCode.java src/main/java/hudson/plugins/git/browser/Stash.java src/main/java/hudson/plugins/git/browser/ViewGitWeb.java http://jenkins-ci.org/commit/git-plugin/510337b35a4665f1d938d35215801bd9d354554e Log: JENKINS-22342 ensure URL normalization correct Previous attempts to fix this stored the normalization flag in the serialized repository browser. Moved to be a function call to avoid the deserializer getting involved
          Hide
          Permalink
          scm_issue_link SCM/JIRA link daemon added a comment -

          Code changed in jenkins
          User: Mark Waite
          Path:
          src/main/java/hudson/plugins/git/browser/BitbucketWeb.java
          src/main/java/hudson/plugins/git/browser/CGit.java
          src/main/java/hudson/plugins/git/browser/FisheyeGitRepositoryBrowser.java
          src/main/java/hudson/plugins/git/browser/GitBlitRepositoryBrowser.java
          src/main/java/hudson/plugins/git/browser/GitLab.java
          src/main/java/hudson/plugins/git/browser/GitRepositoryBrowser.java
          src/main/java/hudson/plugins/git/browser/GitWeb.java
          src/main/java/hudson/plugins/git/browser/GithubWeb.java
          src/main/java/hudson/plugins/git/browser/GitoriousWeb.java
          src/main/java/hudson/plugins/git/browser/KilnGit.java
          src/main/java/hudson/plugins/git/browser/Phabricator.java
          src/main/java/hudson/plugins/git/browser/RedmineWeb.java
          src/main/java/hudson/plugins/git/browser/RhodeCode.java
          src/main/java/hudson/plugins/git/browser/Stash.java
          src/main/java/hudson/plugins/git/browser/ViewGitWeb.java
          http://jenkins-ci.org/commit/git-plugin/737e25272e598fa3786ab2dd961cbcf0ebe2eceb
          Log:
          Merge pull request #224 from Joibel/master

          [Fixed JENKINS-22342] ensure URL normalization correct, retain compatibility

          Compare: https://github.com/jenkinsci/git-plugin/compare/743d715f2118...737e25272e59

          Show
          scm_issue_link SCM/JIRA link daemon added a comment - Code changed in jenkins User: Mark Waite Path: src/main/java/hudson/plugins/git/browser/BitbucketWeb.java src/main/java/hudson/plugins/git/browser/CGit.java src/main/java/hudson/plugins/git/browser/FisheyeGitRepositoryBrowser.java src/main/java/hudson/plugins/git/browser/GitBlitRepositoryBrowser.java src/main/java/hudson/plugins/git/browser/GitLab.java src/main/java/hudson/plugins/git/browser/GitRepositoryBrowser.java src/main/java/hudson/plugins/git/browser/GitWeb.java src/main/java/hudson/plugins/git/browser/GithubWeb.java src/main/java/hudson/plugins/git/browser/GitoriousWeb.java src/main/java/hudson/plugins/git/browser/KilnGit.java src/main/java/hudson/plugins/git/browser/Phabricator.java src/main/java/hudson/plugins/git/browser/RedmineWeb.java src/main/java/hudson/plugins/git/browser/RhodeCode.java src/main/java/hudson/plugins/git/browser/Stash.java src/main/java/hudson/plugins/git/browser/ViewGitWeb.java http://jenkins-ci.org/commit/git-plugin/737e25272e598fa3786ab2dd961cbcf0ebe2eceb Log: Merge pull request #224 from Joibel/master [Fixed JENKINS-22342] ensure URL normalization correct, retain compatibility Compare: https://github.com/jenkinsci/git-plugin/compare/743d715f2118...737e25272e59
          Hide
          Permalink
          markewaite Mark Waite added a comment -

          Fixed in git-plugin 2.2.1 release 12 Apr 2014

          Show
          markewaite Mark Waite added a comment - Fixed in git-plugin 2.2.1 release 12 Apr 2014

            People

            • Assignee:
              markewaite Mark Waite
              Reporter:
              joibel Alan Clucas
            • Votes:
              1 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: