Skip to content
Permalink
Browse files
[FIXED JENKINS-14629] return null to notify about checkout failure
relates to [JENKINS-12201]
  • Loading branch information
ndeloof committed Jul 30, 2012
1 parent 9f19244 commit 09f17089fdbbfa202735bbd4fc4f0aa241045e64
Showing 2 changed files with 6 additions and 2 deletions.
@@ -684,7 +684,7 @@ public boolean checkout(AbstractBuild build, Launcher launcher, FilePath workspa

List<External> externals = checkout(build,workspace,listener,env);

if(externals==null)
if (externals==null)
return false;

// write out the revision file
@@ -741,6 +741,10 @@ private List<External> checkout(AbstractBuild build, FilePath workspace, TaskLis
List<External> externals = new ArrayList<External>();
for (ModuleLocation location : getLocations(env, build)) {
List<External> externalsFound = workspace.act(new CheckOutTask(build, this, location, build.getTimestamp().getTime(), listener, env));
if (externalsFound == null) {
// An error occurred during checkout
return null;
}
externals.addAll( externalsFound );
// olamy: remove null check at it cause test failure
// see https://github.com/jenkinsci/subversion-plugin/commit/de23a2b781b7b86f41319977ce4c11faee75179b#commitcomment-1551273
@@ -164,7 +164,7 @@ public List<External> perform() throws IOException, InterruptedException {
if (e.getErrorMessage().getErrorCode() == SVNErrorCode.WC_NOT_LOCKED) {
listener.getLogger().println("Polled jobs are " + Hudson.getInstance().getDescriptorByType(SCMTrigger.DescriptorImpl.class).getItemsBeingPolled());
}
return Collections.EMPTY_LIST;
return null;

This comment has been minimized.

Copy link
@kutzi

kutzi Aug 8, 2012

Member

Javadoc of UpdateTask#perform explicitly states: '@return Where svn:external mounting happened. Can be empty but never null.'
So either we change the Javadoc or - as I'd prefer - throw an exception to indicate the error.

}

return externals;

4 comments on commit 09f1708

@ndeloof
Copy link
Contributor Author

@ndeloof ndeloof commented on 09f1708 Aug 9, 2012

Choose a reason for hiding this comment

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

throwing an exception would make things cleaner, but I'm confused how it will be thrown through the remote channel and we can catch it a consistent way.

@kutzi
Copy link
Member

@kutzi kutzi commented on 09f1708 Aug 9, 2012

Choose a reason for hiding this comment

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

Add an unit test for it and you'll know ;-)

@dty
Copy link
Member

@dty dty commented on 09f1708 Aug 16, 2012

Choose a reason for hiding this comment

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

The catch block for SVNCancelException already throws an InterruptedException, so throwing an IOException for the general SVNException seems like it would be more consistent than returning null.

@ndeloof
Copy link
Contributor Author

@ndeloof ndeloof commented on 09f1708 Aug 21, 2012

Choose a reason for hiding this comment

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

fixed at 7f594bc

Please sign in to comment.