Skip to content

Commit

Permalink
Fix for JENKINS-22342
Browse files Browse the repository at this point in the history 
Only normalize with slash certain repository types.
Should match behaviour prior to
7abfafc
  • Loading branch information
Alan Clucas committed Mar 26, 2014
1 parent b2a731e commit 962e244
Show file tree
Hide file tree
Showing 15 changed files with 39 additions and 19 deletions.
1 change: 1 addition & 0 deletions src/main/java/hudson/plugins/git/browser/BitbucketWeb.java
View file
Expand Up @@ -24,6 +24,7 @@ public class BitbucketWeb extends GitRepositoryBrowser {
@DataBoundConstructor
public BitbucketWeb(String repoUrl) {
super(repoUrl);
this.normalizeUrl = true;
}

@Override
Expand Down
3 changes: 2 additions & 1 deletion src/main/java/hudson/plugins/git/browser/CGit.java
View file
Expand Up @@ -25,6 +25,7 @@ public class CGit extends GitRepositoryBrowser {
@DataBoundConstructor
public CGit(String repoUrl) {
super(repoUrl);
this.normalizeUrl = true;
}

private QueryBuilder param(URL url) {
Expand Down Expand Up @@ -90,4 +91,4 @@ public CGit newInstance(StaplerRequest req, JSONObject jsonObject) throws FormEx
return req.bindJSON(CGit.class, jsonObject);
}
}
}
}
5 changes: 3 additions & 2 deletions src/main/java/hudson/plugins/git/browser/FisheyeGitRepositoryBrowser.java
View file
Expand Up @@ -25,9 +25,10 @@ public class FisheyeGitRepositoryBrowser extends GitRepositoryBrowser {
private static final long serialVersionUID = 2881872624557203410L;

@DataBoundConstructor
public FisheyeGitRepositoryBrowser(String repoUrl) {
public FisheyeGitRepositoryBrowser(String repoUrl) {
super(repoUrl);
}
this.normalizeUrl = true;
}

@Override
public URL getDiffLink(Path path) throws IOException {
Expand Down
1 change: 1 addition & 0 deletions src/main/java/hudson/plugins/git/browser/GitBlitRepositoryBrowser.java
View file
Expand Up @@ -30,6 +30,7 @@ public class GitBlitRepositoryBrowser extends GitRepositoryBrowser {
public GitBlitRepositoryBrowser(String repoUrl, String projectName) {
super(repoUrl);
this.projectName = projectName;
this.normalizeUrl = true;
}

@Override
Expand Down
9 changes: 5 additions & 4 deletions src/main/java/hudson/plugins/git/browser/GitLab.java
View file
Expand Up @@ -27,6 +27,7 @@ public class GitLab extends GitRepositoryBrowser {
public GitLab(String repoUrl, String version) {
super(repoUrl);
this.version = Double.valueOf(version);
this.normalizeUrl = true;
}

public double getVersion() {
Expand All @@ -50,9 +51,9 @@ public URL getChangeSetLink(GitChangeSet changeSet) throws IOException {

/**
* Creates a link to the commit diff.
*
*
* https://[GitLab URL]/commits/a9182a07750c9a0dfd89a8461adf72ef5ef0885b#[path to file]
*
*
* @param path
* @return diff link
* @throws IOException
Expand All @@ -66,7 +67,7 @@ public URL getDiffLink(Path path) throws IOException {
/**
* Creates a link to the file.
* https://[GitLab URL]/a9182a07750c9a0dfd89a8461adf72ef5ef0885b/tree/pom.xml
*
*
* @param path
* @return file link
* @throws IOException
Expand Down Expand Up @@ -105,6 +106,6 @@ private String calculatePrefix() {
}

return "commits/";
}
}

}
12 changes: 9 additions & 3 deletions src/main/java/hudson/plugins/git/browser/GitRepositoryBrowser.java
View file
Expand Up @@ -14,6 +14,7 @@
public abstract class GitRepositoryBrowser extends RepositoryBrowser<GitChangeSet> {

private /* mostly final */ String url;
protected boolean normalizeUrl;

@Deprecated
protected GitRepositoryBrowser() {
Expand Down Expand Up @@ -43,7 +44,12 @@ public final URL getUrl() throws IOException {
}
}

return normalizeToEndWithSlash(new URL(u));
if (normalizeUrl) {
return normalizeToEndWithSlash(new URL(u));
}
else {
return new URL(u);
}
}

/**
Expand All @@ -56,7 +62,7 @@ public final URL getUrl() throws IOException {
* @throws IOException
*/
public abstract URL getDiffLink(GitChangeSet.Path path) throws IOException;

/**
* Determines the link to a single file under Git.
* This page should display all the past revisions of this file, etc.
Expand All @@ -67,6 +73,6 @@ public final URL getUrl() throws IOException {
* @throws IOException
*/
public abstract URL getFileLink(GitChangeSet.Path path) throws IOException;

private static final long serialVersionUID = 1L;
}
1 change: 1 addition & 0 deletions src/main/java/hudson/plugins/git/browser/GitWeb.java
View file
Expand Up @@ -25,6 +25,7 @@ public class GitWeb extends GitRepositoryBrowser {
@DataBoundConstructor
public GitWeb(String repoUrl) {
super(repoUrl);
this.normalizeUrl = false;
}

@Override
Expand Down
1 change: 1 addition & 0 deletions src/main/java/hudson/plugins/git/browser/GithubWeb.java
View file
Expand Up @@ -33,6 +33,7 @@ public class GithubWeb extends GitRepositoryBrowser {
@DataBoundConstructor
public GithubWeb(String repoUrl) {
super(repoUrl);
this.normalizeUrl = true;
}

@Override
Expand Down
7 changes: 4 additions & 3 deletions src/main/java/hudson/plugins/git/browser/GitoriousWeb.java
View file
Expand Up @@ -24,6 +24,7 @@ public class GitoriousWeb extends GitRepositoryBrowser {
@DataBoundConstructor
public GitoriousWeb(String repoUrl) {
super(repoUrl);
this.normalizeUrl = true;
}

@Override
Expand All @@ -33,9 +34,9 @@ public URL getChangeSetLink(GitChangeSet changeSet) throws IOException {

/**
* Creates a link to the commit diff.
*
*
* https://[Gitorious URL]/commit/a9182a07750c9a0dfd89a8461adf72ef5ef0885b/diffs?diffmode=sidebyside&fragment=1#[path to file]
*
*
* @param path
* @return diff link
* @throws IOException
Expand All @@ -49,7 +50,7 @@ public URL getDiffLink(Path path) throws IOException {
/**
* Creates a link to the file.
* https://[Gitorious URL]/blobs/a9182a07750c9a0dfd89a8461adf72ef5ef0885b/pom.xml
*
*
* @param path
* @return file link
* @throws IOException
Expand Down
1 change: 1 addition & 0 deletions src/main/java/hudson/plugins/git/browser/KilnGit.java
View file
Expand Up @@ -26,6 +26,7 @@ public class KilnGit extends GitRepositoryBrowser {
@DataBoundConstructor
public KilnGit(String repoUrl) {
super(repoUrl);
this.normalizeUrl = true;
}

private QueryBuilder param(URL url) {
Expand Down
1 change: 1 addition & 0 deletions src/main/java/hudson/plugins/git/browser/Phabricator.java
View file
Expand Up @@ -27,6 +27,7 @@ public class Phabricator extends GitRepositoryBrowser {
public Phabricator(String repoUrl, String repo) {
super(repoUrl);
this.repo = repo;
this.normalizeUrl = true;
}

public String getRepo() {
Expand Down
13 changes: 7 additions & 6 deletions src/main/java/hudson/plugins/git/browser/RedmineWeb.java
View file
Expand Up @@ -16,7 +16,7 @@

/**
* Git Browser for <a href="http://www.redmine.org/">Redmine</a>.
*
*
* @author mfriedenhagen
*/
public class RedmineWeb extends GitRepositoryBrowser {
Expand All @@ -26,6 +26,7 @@ public class RedmineWeb extends GitRepositoryBrowser {
@DataBoundConstructor
public RedmineWeb(String repoUrl) {
super(repoUrl);
this.normalizeUrl = true;
}

@Override
Expand All @@ -36,13 +37,13 @@ public URL getChangeSetLink(GitChangeSet changeSet) throws IOException {

/**
* Creates a link to the file diff.
*
*
* https://SERVER/PATH/projects/PROJECT/repository/revisions/a9182a07750c9a0dfd89a8461adf72ef5ef0885b/diff/pom.xml
*
*
* Returns a diff link for {@link EditType#DELETE} and {@link EditType#EDIT}, for {@link EditType#ADD} returns an
* {@link RedmineWeb#getFileLink(Path)}.
*
*
*
*
* @param path
* affected file path
* @return diff link
Expand All @@ -66,7 +67,7 @@ public URL getDiffLink(Path path) throws IOException {
* Creates a link to the file.
* https://SERVER/PATH/projects/PROJECT/repository/revisions/a9182a07750c9a0dfd89a8461adf72ef5ef0885b/entry/pom.xml
* For deleted files just returns a diff link, which will have /dev/null as target file.
*
*
* @param path
* file
* @return file link
Expand Down
1 change: 1 addition & 0 deletions src/main/java/hudson/plugins/git/browser/RhodeCode.java
View file
Expand Up @@ -25,6 +25,7 @@ public class RhodeCode extends GitRepositoryBrowser {
@DataBoundConstructor
public RhodeCode(String repoUrl) {
super(repoUrl);
this.normalizeUrl = true;
}

private QueryBuilder param(URL url) {
Expand Down
1 change: 1 addition & 0 deletions src/main/java/hudson/plugins/git/browser/Stash.java
View file
Expand Up @@ -25,6 +25,7 @@ public class Stash extends GitRepositoryBrowser {
@DataBoundConstructor
public Stash(String repoUrl) {
super(repoUrl);
this.normalizeUrl = true;
}

private QueryBuilder param(URL url) {
Expand Down
1 change: 1 addition & 0 deletions src/main/java/hudson/plugins/git/browser/ViewGitWeb.java
View file
Expand Up @@ -31,6 +31,7 @@ public class ViewGitWeb extends GitRepositoryBrowser {
public ViewGitWeb(String repoUrl, String projectName) {
super(repoUrl);
this.projectName = projectName;
this.normalizeUrl = true;
}

@Override
Expand Down

1 comment on commit 962e244

@MarkEWaite
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alan,

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 fb12fed 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

We can't release a version of the plugin which requires every job to be visited. Some Jenkins servers have hundreds of jobs configured. 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.

Please sign in to comment.