Skip to content
Permalink
Browse files
[FIXED JENKINS-11877] Use AbortException rather than returning false …
…to indicate checkout failure
  • Loading branch information
davidmc24 committed Dec 4, 2011
1 parent 5146c97 commit e75bd4df0bc492542b7235e7584243e048b41632
Showing with 30 additions and 33 deletions.
  1. +30 −33 src/main/java/hudson/plugins/mercurial/MercurialSCM.java
@@ -395,26 +395,23 @@ public boolean checkout(AbstractBuild<?,?> build, Launcher launcher, FilePath wo
} else {
e.printStackTrace(listener.error("Failed to determine whether workspace can be reused"));
}
return false;
throw new AbortException("Failed to determine whether workspace can be reused");

This comment has been minimized.

Copy link
@KostyaSha

KostyaSha Sep 9, 2021

Member

@jglick

that logic seems to be broken
If we get that canReuseExistingWorkspace false, then we shouldn't throw abortException, because later there should be full checkout

 if (canReuseExistingWorkspace) {
            update(build, launcher, repository, listener);
        } else {
            clone(build, launcher, repository, listener);
        }

I guess throw should be only in if (causedByMissingHg(e)) { branch

This comment has been minimized.

Copy link
@KostyaSha

This comment has been minimized.

Copy link
@jglick

jglick Sep 9, 2021

Member

Hard to say. Other unclassified errors may be critical too. Probably something more important is wrong if you get here.

}

boolean success;
if (canReuseExistingWorkspace) {
success = update(build, launcher, repository, listener);
update(build, launcher, repository, listener);
} else {
success = clone(build, launcher, repository, listener);
clone(build, launcher, repository, listener);
}

if (success) {
try {
determineChanges(build, launcher, listener, changelogFile, repository);
} catch (IOException e) {
listener.error("Failed to capture change log");
e.printStackTrace(listener.getLogger());
success = false;
}
try {
determineChanges(build, launcher, listener, changelogFile, repository);
} catch (IOException e) {
listener.error("Failed to capture change log");
e.printStackTrace(listener.getLogger());
throw new AbortException("Failed to capture change log");
}
return success;
return true;
}

private boolean canReuseWorkspace(FilePath repo,
@@ -514,7 +511,7 @@ private void determineChanges(AbstractBuild<?, ?> build, Launcher launcher, Buil
/*
* Updates the current repository.
*/
private boolean update(AbstractBuild<?, ?> build, Launcher launcher, FilePath repository, BuildListener listener)
private void update(AbstractBuild<?, ?> build, Launcher launcher, FilePath repository, BuildListener listener)
throws InterruptedException, IOException {
EnvVars env = build.getEnvironment(listener);

@@ -528,45 +525,45 @@ private boolean update(AbstractBuild<?, ?> build, Launcher launcher, FilePath re
} else {
e.printStackTrace(listener.error("Failed to pull"));
}
return false;
throw new AbortException("Failed to pull");
}

int updateExitCode;
try {
if(hg.run("update", "--clean", "--rev", getBranch(env)).pwd(repository).join()!=0) {
listener.error("Failed to update");
return false;
}
updateExitCode = hg.run("update", "--clean", "--rev", getBranch(env)).pwd(repository).join();
} catch (IOException e) {
listener.error("Failed to update");
e.printStackTrace(listener.getLogger());
return false;
throw new AbortException("Failed to update");
}
if (updateExitCode != 0) {
listener.error("Failed to update");
throw new AbortException("Failed to update");
}

if(clean) {
if (hg.cleanAll().pwd(repository).join() != 0) {
listener.error("Failed to clean unversioned files");
return false;
throw new AbortException("Failed to clean unversioned files");
}
}

String tip = hg.tip(repository);
if (tip != null) {
build.addAction(new MercurialTagAction(tip));
}

return true;
}

/**
* Start from scratch and clone the whole repository.
*/
private boolean clone(AbstractBuild<?,?> build, Launcher launcher, FilePath repository, BuildListener listener)
private void clone(AbstractBuild<?,?> build, Launcher launcher, FilePath repository, BuildListener listener)
throws InterruptedException, IOException {
try {
repository.deleteRecursive();
} catch (IOException e) {
e.printStackTrace(listener.error("Failed to clean the repository checkout"));
return false;
throw new AbortException("Failed to clean the repository checkout");
}

EnvVars env = build.getEnvironment(listener);
@@ -593,19 +590,21 @@ private boolean clone(AbstractBuild<?,?> build, Launcher launcher, FilePath repo
args.add(source);
}
args.add(repository.getRemote());
int cloneExitCode;
try {
if(hg.run(args).join()!=0) {
listener.error("Failed to clone "+source);
return false;
}
cloneExitCode = hg.run(args).join();
} catch (IOException e) {
if (causedByMissingHg(e)) {
listener.error("Failed to clone " + source + " because hg could not be found;" +
" check that you've properly configured your Mercurial installation");
} else {
e.printStackTrace(listener.error("Failed to clone "+source));
}
return false;
throw new AbortException("Failed to clone "+source);
}
if(cloneExitCode!=0) {
listener.error("Failed to clone "+source);
throw new AbortException("Failed to clone "+source);
}

if (cachedSource != null && cachedSource.isUseCaches() && !cachedSource.isUseSharing()) {
@@ -614,7 +613,7 @@ private boolean clone(AbstractBuild<?,?> build, Launcher launcher, FilePath repo
String hgrcText = hgrc.readToString();
if (!hgrcText.contains(cachedSource.getRepoLocation())) {
listener.error(".hg/hgrc did not contain " + cachedSource.getRepoLocation() + " as expected:\n" + hgrcText);
return false;
throw new AbortException(".hg/hgrc did not contain " + cachedSource.getRepoLocation() + " as expected:\n" + hgrcText);
}
hgrc.write(hgrcText.replace(cachedSource.getRepoLocation(), source), null);
}
@@ -632,8 +631,6 @@ private boolean clone(AbstractBuild<?,?> build, Launcher launcher, FilePath repo
if (tip != null) {
build.addAction(new MercurialTagAction(tip));
}

return true;
}

@Override

0 comments on commit e75bd4d

Please sign in to comment.