Skip to content
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
1 change: 0 additions & 1 deletion pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,6 @@
<groupId>org.jenkins-ci.plugins</groupId>
<artifactId>branch-api</artifactId>
<version>2.1.2</version>
<scope>test</scope>
Copy link
Member

Choose a reason for hiding this comment

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

That you need to add a hard dependency on branch-api suggests this change should be an extension plugin and not in the core plugin. SCM implementations shouldn't be tied to orthogonal consumer concepts like branch-api

</dependency>
<dependency>
<groupId>org.jenkins-ci.plugins</groupId>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@
import java.util.logging.Level;
import java.util.logging.Logger;
import jenkins.model.Jenkins;
import jenkins.plugins.git.AbstractGitSCMSource;
import jenkins.plugins.git.AbstractGitSCMSource.SCMRevisionImpl;
import jenkins.scm.api.SCMHead;
import jenkins.scm.api.SCMHeadObserver;
Expand Down Expand Up @@ -73,6 +74,9 @@ private static void createBuildCommitStatus(Run<?, ?> build, TaskListener listen
SCMSource src = SCMSource.SourceByItem.findSource(build.getParent());
SCMRevision revision = src != null ? SCMRevisionAction.getRevision(src, build) : null;
if (revision != null) { // only notify if we have a revision to notify
GitHubEnvAction gitHubEnvAction = getGitHubEnvAction(revision);
Copy link
Member

Choose a reason for hiding this comment

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

Unnecessary to add this action.

https://github.com/jenkinsci/branch-api-plugin/blob/b875b2641d60e39f2b6074838f034a1b921269f6/src/main/java/jenkins/branch/BranchNameContributor.java#L55-L58

Followed by a call to https://github.com/jenkinsci/branch-api-plugin/blob/0e8545955a5ded2f482f77055ab064786267c710/src/main/java/jenkins/branch/BranchProjectFactory.java#L162 will get you the revision and then you can use the exact same logic for testing the instance types to extract the information you need.

A single class extension plugin with just an EnvironmentContributor implementation will do exactly what you are attempting without requiring changes to the core plugin

Copy link
Member Author

Choose a reason for hiding this comment

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

Will take a look thanks

build.getParent().addAction(gitHubEnvAction);

try {
GitHub gitHub = lookUpGitHub(build.getParent());
try {
Expand Down Expand Up @@ -127,6 +131,31 @@ private static void createBuildCommitStatus(Run<?, ?> build, TaskListener listen
}
}

private static GitHubEnvAction getGitHubEnvAction(SCMRevision revision) {
Copy link
Member

Choose a reason for hiding this comment

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

I would suggest moving this logic to the environment contributor

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 don't have access to the revision there as far as I know.
It's possible to look it up, but that requires an API call from memory, and this plugin is very focused on minimising API calls as it's easy to go over the hourly limit

Copy link
Member

Choose a reason for hiding this comment

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

Not seeing any API calls in this method. All just accessing fields in the SCMRevision instance which you can get anyway from branch-api

Copy link
Member

Choose a reason for hiding this comment

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

GitHubEnvAction action;
if (revision instanceof PullRequestSCMRevision) {
PullRequestSCMRevision pullRequestSCMRevision = (PullRequestSCMRevision) revision;

action = new GitHubEnvAction(
pullRequestSCMRevision.getPullHash(),
pullRequestSCMRevision.getMergeHash(),
pullRequestSCMRevision.getBaseHash()
);


} else if (revision instanceof SCMRevisionImpl) {
SCMRevisionImpl gitRevision = (SCMRevisionImpl) revision;

action = new GitHubEnvAction(
gitRevision.getHash(),
revision.getHead().getName()
);
} else {
throw new IllegalArgumentException("did not recognize " + revision);
}
return action;
}

/**
* Returns the GitHub Repository associated to a Job.
*
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
/*
* The MIT License
*
* Copyright 2019 Tim Jacomb
*
* Permission is hereby granted, free of charge, to any person obtaining a copy
* of this software and associated documentation files (the "Software"), to deal
* in the Software without restriction, including without limitation the rights
* to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
* copies of the Software, and to permit persons to whom the Software is
* furnished to do so, subject to the following conditions:
*
* The above copyright notice and this permission notice shall be included in
* all copies or substantial portions of the Software.
*
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
* AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
* THE SOFTWARE.
*/
package org.jenkinsci.plugins.github_branch_source;

import hudson.model.InvisibleAction;
import java.util.HashMap;
import java.util.Map;
import java.util.Objects;
import org.kohsuke.accmod.Restricted;
import org.kohsuke.accmod.restrictions.NoExternalUse;

/**
* Caches SCM values for a Run
*/
@Restricted(NoExternalUse.class)
public class GitHubEnvAction extends InvisibleAction {
Copy link
Member

Choose a reason for hiding this comment

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

Redundant class. Information is already retained the in the SCMRevision assoicated with the branch

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes but the javadoc for EnvironmentContributor says to use an Action to cache data that can be expensive to calculate, and it's expensive due to API calls required if you have to look it up

Copy link
Member

Choose a reason for hiding this comment

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

But none of the info it is looking for requires an API call. https://github.com/jenkinsci/branch-api-plugin/blob/0e8545955a5ded2f482f77055ab064786267c710/src/main/java/jenkins/branch/BranchProjectFactory.java#L162 will give you the revision of the branch without looking outside of Jenkins


private static final String CHANGE_SOURCE_COMMIT_ID = "CHANGE_SOURCE_COMMIT_ID";
private static final String CHANGE_MERGE_COMMIT_ID = "CHANGE_MERGE_COMMIT_ID";
private static final String CHANGE_BASE_COMMIT_ID = "CHANGE_BASE_COMMIT_ID";
private static final String CHANGE_BRANCH = "CHANGE_BRANCH";


private final Map<String, String> values;

public GitHubEnvAction(String sourceCommit, String branch) {
HashMap<String, String> theValues = new HashMap<>();
theValues.put(CHANGE_SOURCE_COMMIT_ID, sourceCommit);
theValues.put(CHANGE_BRANCH, branch);

this.values = theValues;
}

public GitHubEnvAction(String sourceCommit, String mergeCommit, String baseCommit) {
Map<String, String> theValues = new HashMap<>();
theValues.put(CHANGE_SOURCE_COMMIT_ID, sourceCommit);
theValues.put(CHANGE_MERGE_COMMIT_ID, mergeCommit);
theValues.put(CHANGE_BASE_COMMIT_ID, baseCommit);

this.values = theValues;
}

public Map<String, String> getValues() {
return values;
}

@Override
public boolean equals(Object o) {
if (this == o) return true;
if (o == null || getClass() != o.getClass()) return false;
GitHubEnvAction action = (GitHubEnvAction) o;
return Objects.equals(values, action.values);
}

@Override
public int hashCode() {
return Objects.hash(values);
}

@Override
public String toString() {
return String.format("GitHubEnvAction{values=%s}", values);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
/*
* The MIT License
*
* Copyright 2019 Tim Jacomb
*
* Permission is hereby granted, free of charge, to any person obtaining a copy
* of this software and associated documentation files (the "Software"), to deal
* in the Software without restriction, including without limitation the rights
* to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
* copies of the Software, and to permit persons to whom the Software is
* furnished to do so, subject to the following conditions:
*
* The above copyright notice and this permission notice shall be included in
* all copies or substantial portions of the Software.
*
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
* AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
* THE SOFTWARE.
*/
package org.jenkinsci.plugins.github_branch_source;

import hudson.EnvVars;
import hudson.Extension;
import hudson.model.EnvironmentContributor;
import hudson.model.ItemGroup;
import hudson.model.Job;
import hudson.model.TaskListener;
import javax.annotation.Nonnull;
import jenkins.branch.MultiBranchProject;
import org.kohsuke.accmod.Restricted;
import org.kohsuke.accmod.restrictions.NoExternalUse;

@Extension
@Restricted(NoExternalUse.class)
public class GitHubEnvContributor extends EnvironmentContributor {

public void buildEnvironmentFor(Job job, @Nonnull EnvVars envs, @Nonnull TaskListener listener) {
ItemGroup parent = job.getParent();
if (parent instanceof MultiBranchProject) {
GitHubEnvAction action = job.getAction(GitHubEnvAction.class);
if (action != null) {
envs.putAll(action.getValues());
}
}
}
}