Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[JENKINS-26100] Switch buildEnvVars and others to Run #492

Merged
merged 12 commits into from
Jun 21, 2017
Merged

[JENKINS-26100] Switch buildEnvVars and others to Run #492

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
47f738c
[JENKINS-26100] Switch buildEnvVars and others to Run
abayer May 1, 2017
6ee22da
Make findbugs happy.
abayer May 1, 2017
803d0cb
Update to buildEnvironment.
abayer May 2, 2017
e9dbe68
Switch to release
abayer May 16, 2017
3c125fb
Reverting back to 1.625.3 baseline
abayer May 18, 2017
b409573
Another bit of review comments
abayer May 19, 2017
6d158dc
Clarification
abayer May 19, 2017
3681660
Merge remote-tracking branch 'origin' into jenkins-26100
abayer May 20, 2017
74cd756
Switch to modern parameterized-trigger dependency.
abayer May 20, 2017
f4bb730
Merge remote-tracking branch 'origin/master' into jenkins-26100
abayer Jun 6, 2017
e38f561
Whoops, fixing merge bork.
abayer Jun 16, 2017
133609e
Merge remote-tracking branch 'origin/master' into jenkins-26100
abayer Jun 16, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
<parent>
<groupId>org.jenkins-ci.plugins</groupId>
<artifactId>plugin</artifactId>
<version>2.26</version>
<version>2.27</version>
<relativePath />
</parent>

Expand All @@ -24,8 +24,9 @@
<inceptionYear>2007</inceptionYear>

<properties>
<jenkins.version>1.625.3</jenkins.version>
<java.level>7</java.level>
<jenkins-core.version>2.58-20170502.192524-8</jenkins-core.version> <!-- TODO: Switch to release once https://github.com/jenkinsci/jenkins/pull/2730 is merged and released -->
<jenkins-war.version>2.58-20170502.192544-8</jenkins-war.version> <!-- TODO: Switch to release once https://github.com/jenkinsci/jenkins/pull/2730 is merged and released -->
Copy link
Member

Choose a reason for hiding this comment

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

2.61

Copy link
Member

Choose a reason for hiding this comment

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

2.60 I meant.

<java.level>8</java.level>
<no-test-jar>false</no-test-jar>
<concurrency>1C</concurrency>
<findbugs.failOnError>true</findbugs.failOnError>
Expand Down
14 changes: 9 additions & 5 deletions src/main/java/hudson/plugins/git/GitChangeSet.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import hudson.scm.ChangeLogSet;
import hudson.scm.ChangeLogSet.AffectedFile;
import hudson.scm.EditType;
import jenkins.model.Jenkins;
import org.apache.commons.lang.math.NumberUtils;
import org.kohsuke.stapler.export.Exported;
import org.kohsuke.stapler.export.ExportedBean;
Expand Down Expand Up @@ -420,13 +421,16 @@ private boolean hasHudsonTasksMailer() {
}

private boolean isCreateAccountBasedOnEmail() {
Hudson hudson = Hudson.getInstance();
if (hudson == null) {
Jenkins jenkins = Jenkins.getInstanceOrNull();
if (jenkins == null) {
Copy link
Member

Choose a reason for hiding this comment

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

Maybe just revert changes in this file?

Copy link
Member Author

Choose a reason for hiding this comment

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

'k

return false;
}
DescriptorImpl descriptor = (DescriptorImpl) jenkins.getDescriptor(GitSCM.class);
if (descriptor != null) {
return descriptor.isCreateAccountBasedOnEmail();
} else {
return false;
}
DescriptorImpl descriptor = (DescriptorImpl) hudson.getDescriptor(GitSCM.class);

return descriptor.isCreateAccountBasedOnEmail();
}

@Override
Expand Down
12 changes: 6 additions & 6 deletions src/main/java/hudson/plugins/git/GitSCM.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1255,8 +1255,8 @@ private void computeChangeLog(GitClient git, Revision revToBuild, TaskListener l
}
}

public void buildEnvVars(AbstractBuild<?, ?> build, java.util.Map<String, String> env) {
super.buildEnvVars(build, env);
@Override
public void buildEnvironment(Run<?, ?> build, java.util.Map<String, String> env) {
Copy link
Member

Choose a reason for hiding this comment

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

Put in the usual TODOs.

Copy link
Member Author

Choose a reason for hiding this comment

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

'k

Revision rev = fixNull(getBuildData(build)).getLastBuiltRevision();
if (rev!=null) {
Branch branch = Iterables.getFirst(rev.getBranches(), null);
Expand Down Expand Up @@ -1319,7 +1319,7 @@ private String getBranchName(Branch branch)
return name;
}

private String getLastBuiltCommitOfBranch(AbstractBuild<?, ?> build, Branch branch) {
private String getLastBuiltCommitOfBranch(Run<?, ?> build, Branch branch) {
String prevCommit = null;
if (build.getPreviousBuiltBuild() != null) {
final Build lastBuildOfBranch = fixNull(getBuildData(build.getPreviousBuiltBuild())).getLastBuildOfBranch(branch.getName());
Expand All @@ -1333,7 +1333,7 @@ private String getLastBuiltCommitOfBranch(AbstractBuild<?, ?> build, Branch bran
return prevCommit;
}

private String getLastSuccessfulBuiltCommitOfBranch(AbstractBuild<?, ?> build, Branch branch) {
private String getLastSuccessfulBuiltCommitOfBranch(Run<?, ?> build, Branch branch) {
String prevCommit = null;
if (build.getPreviousSuccessfulBuild() != null) {
final Build lastSuccessfulBuildOfBranch = fixNull(getBuildData(build.getPreviousSuccessfulBuild())).getLastBuildOfBranch(branch.getName());
Expand Down Expand Up @@ -1772,9 +1772,9 @@ private boolean isRevExcluded(GitClient git, Revision r, TaskListener listener,

@Initializer(after=PLUGINS_STARTED)
public static void onLoaded() {
Jenkins jenkins = Jenkins.getInstance();
Jenkins jenkins = Jenkins.getInstanceOrNull();
if (jenkins == null) {
LOGGER.severe("Jenkins.getInstance is null in GitSCM.onLoaded");
LOGGER.severe("Jenkins.getInstanceOrNull is null in GitSCM.onLoaded");
return;
}
DescriptorImpl desc = jenkins.getDescriptorByType(DescriptorImpl.class);
Expand Down
8 changes: 4 additions & 4 deletions src/main/java/hudson/plugins/git/GitStatus.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,9 +152,9 @@ public HttpResponse doNotifyCommit(HttpServletRequest request, @QueryParameter(r
}

final List<ResponseContributor> contributors = new ArrayList<>();
Jenkins jenkins = Jenkins.getInstance();
Jenkins jenkins = Jenkins.getInstanceOrNull();
if (jenkins == null) {
return HttpResponses.error(SC_BAD_REQUEST, new Exception("Jenkins.getInstance() null for : " + url));
return HttpResponses.error(SC_BAD_REQUEST, new Exception("Jenkins.getInstanceOrNull() null for : " + url));
}
String origin = SCMEvent.originOf(request);
for (Listener listener : jenkins.getExtensionList(Listener.class)) {
Expand Down Expand Up @@ -339,9 +339,9 @@ public List<ResponseContributor> onNotifyCommit(String origin, URIish uri, Strin

boolean scmFound = false,
urlFound = false;
Jenkins jenkins = Jenkins.getInstance();
Jenkins jenkins = Jenkins.getInstanceOrNull();
if (jenkins == null) {
LOGGER.severe("Jenkins.getInstance() is null in GitStatus.onNotifyCommit");
LOGGER.severe("Jenkins.getInstanceOrNull() is null in GitStatus.onNotifyCommit");
return result;
}
for (final Item project : Jenkins.getInstance().getAllItems()) {
Expand Down
4 changes: 2 additions & 2 deletions src/main/java/hudson/plugins/git/GitTagAction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,9 @@ protected GitTagAction(Run build, FilePath workspace, Revision revision) {
private static final Logger LOGGER = Logger.getLogger(GitTagAction.class.getName());

public Descriptor<GitTagAction> getDescriptor() {
Jenkins jenkins = Jenkins.getInstance();
Jenkins jenkins = Jenkins.getInstanceOrNull();
if (jenkins == null) {
LOGGER.severe("Jenkins.getInstance() null in GitTagAction.getDescriptor");
LOGGER.severe("Jenkins.getInstanceOrNull() null in GitTagAction.getDescriptor");
return null;
}
return jenkins.getDescriptorOrDie(getClass());
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/hudson/plugins/git/util/BuildChooserDescriptor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ public String getLegacyId() {
}

public static DescriptorExtensionList<BuildChooser,BuildChooserDescriptor> all() {
Jenkins jenkins = Jenkins.getInstance();
Jenkins jenkins = Jenkins.getInstanceOrNull();
if (jenkins == null) {
LOGGER.severe("Jenkins instance is null in BuildChooserDescriptor");
return null;
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/jenkins/plugins/git/AbstractGitSCMSource.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -470,7 +470,7 @@ protected String getCacheEntry() {
}

protected static File getCacheDir(String cacheEntry) {
Jenkins jenkins = Jenkins.getInstance();
Jenkins jenkins = Jenkins.getInstanceOrNull();
Copy link
Member

Choose a reason for hiding this comment

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

Most of these should actually be getInstance() with no null check, but good enough for now.

Copy link
Member Author

Choose a reason for hiding this comment

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

I went for the most stupidly non-logic-altering Findbugs fix I could. =) Wasn't 100% sure there weren't some cases where they might be expecting the existing behavior with a possibly null jenkins instance, so hey, better safe than sorry.

if (jenkins == null) {
LOGGER.severe("Jenkins instance is null in AbstractGitSCMSource.getCacheDir");
return null;
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/jenkins/plugins/git/GitSCMSource.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -329,7 +329,7 @@ public List<GitStatus.ResponseContributor> onNotifyCommit(String origin,
// run in high privilege to see all the projects anonymous users don't see.
// this is safe because when we actually schedule a build, it's a build that can
// happen at some random time anyway.
Jenkins jenkins = Jenkins.getInstance();
Jenkins jenkins = Jenkins.getInstanceOrNull();
if (jenkins == null) {
LOGGER.severe("Jenkins instance is null in GitSCMSource.onNotifyCommit");
return result;
Expand Down