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-24141] Pull ChangeLogSet-related logic out of AbstractBuild #2730

Merged
merged 20 commits into from
May 10, 2017
Merged

[JENKINS-24141] Pull ChangeLogSet-related logic out of AbstractBuild #2730

Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
bd84482
[WIP JENKINS-24141] First work on abstracting out changelogs
abayer Jan 31, 2017
61acefc
Switch to a mixin approach
abayer Jan 31, 2017
d7d4a82
Minor cleanup, fixing AbstractBuild.getChangeSets()
abayer Jan 31, 2017
285e83a
Merge branch 'master' into jenkins-24141
abayer Apr 25, 2017
8ff1dd6
Add SCM.buildEnvVars(Run,Map<String,String>)
abayer Apr 25, 2017
7fe3a4a
Initial work on DependencyGraph
abayer Apr 28, 2017
569c990
Merge branch 'master' into jenkins-24141
abayer Apr 28, 2017
4f3172c
Normalize versioning because that was unneeded
abayer May 1, 2017
e4b84a2
Revert "Initial work on DependencyGraph"
abayer May 1, 2017
e6f6064
Code review responses
abayer May 2, 2017
3913680
Rework to solo RunWithSCM interface
abayer May 2, 2017
977f922
Refactor for supporting on-demand calculation of culprits
abayer May 2, 2017
28dce4c
Get rid of asRun().
abayer May 2, 2017
31bc417
Review comments.
abayer May 2, 2017
66460f9
Get rid of isBuilding() check on previous build's culprits.
abayer May 2, 2017
99baf7a
Forgot a bit of javadoc
abayer May 2, 2017
aa5ff54
Forgot to remove SCMTriggerItem everywhere in View
abayer May 2, 2017
71a4614
Javadoc cleanup
abayer May 2, 2017
b6e1ae5
Adding a @since TODO for Job.doRssChangelog
abayer May 8, 2017
914963c
Get rid of superfluous Queue.Task/Executable
abayer May 8, 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
106 changes: 31 additions & 75 deletions core/src/main/java/hudson/model/AbstractBuild.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import hudson.FilePath;
import hudson.Functions;
import hudson.Launcher;
import jenkins.scm.RunWithSCM;
import jenkins.util.SystemProperties;
import hudson.console.ModelHyperlinkNote;
import hudson.model.Fingerprint.BuildPtr;
Expand Down Expand Up @@ -70,7 +71,6 @@
import java.io.IOException;
import java.io.InterruptedIOException;
import java.lang.ref.WeakReference;
import java.util.AbstractSet;
import java.util.ArrayList;
import java.util.Calendar;
import java.util.Collection;
Expand All @@ -91,8 +91,6 @@

import jenkins.model.lazy.BuildReference;
import jenkins.model.lazy.LazyBuildMixIn;
import org.kohsuke.accmod.Restricted;
import org.kohsuke.accmod.restrictions.DoNotUse;

/**
* Base implementation of {@link Run}s that build software.
Expand All @@ -102,7 +100,7 @@
* @author Kohsuke Kawaguchi
* @see AbstractProject
*/
public abstract class AbstractBuild<P extends AbstractProject<P,R>,R extends AbstractBuild<P,R>> extends Run<P,R> implements Queue.Executable, LazyBuildMixIn.LazyLoadingRun<P,R> {
public abstract class AbstractBuild<P extends AbstractProject<P,R>,R extends AbstractBuild<P,R>> extends Run<P,R> implements Queue.Executable, LazyBuildMixIn.LazyLoadingRun<P,R>, RunWithSCM<P,R> {

/**
* Set if we want the blame information to flow from upstream to downstream build.
Expand Down Expand Up @@ -321,80 +319,38 @@ public FilePath[] getModuleRoots() {
return getParent().getScm().getModuleRoots(ws, this);
}

/**
* List of users who committed a change since the last non-broken build till now.
*
* <p>
* This list at least always include people who made changes in this build, but
* if the previous build was a failure it also includes the culprit list from there.
*
* @return
* can be empty but never null.
*/
@Exported
public Set<User> getCulprits() {
if (culprits==null) {
Set<User> r = new HashSet<User>();
R p = getPreviousCompletedBuild();
if (p !=null && isBuilding()) {
Result pr = p.getResult();
if (pr!=null && pr.isWorseThan(Result.SUCCESS)) {
// we are still building, so this is just the current latest information,
// but we seems to be failing so far, so inherit culprits from the previous build.
// isBuilding() check is to avoid recursion when loading data from old Hudson, which doesn't record
// this information
r.addAll(p.getCulprits());
}
}
for (Entry e : getChangeSet())
r.add(e.getAuthor());
@Override
@CheckForNull public Set<String> getCulpritIds() {
return culprits;
}

if (upstreamCulprits) {
// If we have dependencies since the last successful build, add their authors to our list
if (getPreviousNotFailedBuild() != null) {
Map <AbstractProject,DependencyChange> depmap = getDependencyChanges(getPreviousSuccessfulBuild());
for (DependencyChange dep : depmap.values()) {
for (AbstractBuild<?,?> b : dep.getBuilds()) {
for (Entry entry : b.getChangeSet()) {
r.add(entry.getAuthor());
}
@Override
public boolean shouldCalculateCulprits() {
return getCulpritIds() == null;
}

@Override
@Nonnull
public Set<User> calculateCulprits() {
Set<User> c = RunWithSCM.super.calculateCulprits();

AbstractBuild<P,R> p = getPreviousCompletedBuild();
Copy link
Member

Choose a reason for hiding this comment

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

If this implements caching, where does it consider parallel builds that complete out of order?

Copy link
Member Author

Choose a reason for hiding this comment

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

shrug? It's the same logic as had previously existed for populating AbstractBuild.culprits, just organized differently.

if (upstreamCulprits) {
// If we have dependencies since the last successful build, add their authors to our list
if (p.getPreviousNotFailedBuild() != null) {
Map<AbstractProject, AbstractBuild.DependencyChange> depmap =
p.getDependencyChanges(p.getPreviousSuccessfulBuild());
for (AbstractBuild.DependencyChange dep : depmap.values()) {
for (AbstractBuild<?, ?> b : dep.getBuilds()) {
for (ChangeLogSet.Entry entry : b.getChangeSet()) {
c.add(entry.getAuthor());
}
}
}
}

return r;
}

return new AbstractSet<User>() {
public Iterator<User> iterator() {
return new AdaptedIterator<String,User>(culprits.iterator()) {
protected User adapt(String id) {
return User.get(id);
}
};
}

public int size() {
return culprits.size();
}
};
}

/**
* Returns true if this user has made a commit to this build.
*
* @since 1.191
*/
public boolean hasParticipant(User user) {
for (ChangeLogSet.Entry e : getChangeSet())
try{
if (e.getAuthor()==user)
return true;
} catch (RuntimeException re) {
LOGGER.log(Level.INFO, "Failed to determine author of changelog " + e.getCommitId() + "for " + getParent().getDisplayName() + ", " + getDisplayName(), re);
}
return false;
return c;
}

/**
Expand Down Expand Up @@ -863,7 +819,7 @@ public Collection<Fingerprint> getBuildFingerprints() {
* @return never null.
*/
@Exported
public ChangeLogSet<? extends Entry> getChangeSet() {
@Nonnull public ChangeLogSet<? extends Entry> getChangeSet() {
synchronized (changeSetLock) {
if (scm==null) {
scm = NullChangeLogParser.INSTANCE;
Expand All @@ -887,10 +843,10 @@ public ChangeLogSet<? extends Entry> getChangeSet() {
return cs;
}

@Restricted(DoNotUse.class) // for project-changes.jelly
public List<ChangeLogSet<? extends ChangeLogSet.Entry>> getChangeSets() {
@Override
@Nonnull public List<ChangeLogSet<? extends ChangeLogSet.Entry>> getChangeSets() {
ChangeLogSet<? extends Entry> cs = getChangeSet();
return cs.isEmptySet() ? Collections.<ChangeLogSet<? extends ChangeLogSet.Entry>>emptyList() : Collections.<ChangeLogSet<? extends ChangeLogSet.Entry>>singletonList(cs);
return cs.isEmptySet() ? Collections.emptyList() : Collections.singletonList(cs);
}

/**
Expand Down
65 changes: 0 additions & 65 deletions core/src/main/java/hudson/model/AbstractProject.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@
import hudson.EnvVars;
import hudson.ExtensionList;
import hudson.ExtensionPoint;
import hudson.FeedAdapter;
import hudson.FilePath;
import hudson.Functions;
import hudson.Launcher;
Expand All @@ -55,8 +54,6 @@
import hudson.model.queue.QueueTaskFuture;
import hudson.model.queue.SubTask;
import hudson.model.queue.SubTaskContributor;
import hudson.scm.ChangeLogSet;
import hudson.scm.ChangeLogSet.Entry;
import hudson.scm.NullSCM;
import hudson.scm.PollingResult;

Expand Down Expand Up @@ -87,7 +84,6 @@
import java.io.IOException;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Calendar;
import java.util.Collection;
import java.util.Collections;
import java.util.Comparator;
Expand All @@ -107,7 +103,6 @@
import javax.servlet.ServletException;
import jenkins.model.BlockedBecauseOfBuildInProgress;
import jenkins.model.Jenkins;
import jenkins.model.JenkinsLocationConfiguration;
import jenkins.model.ParameterizedJobMixIn;
import jenkins.model.Uptime;
import jenkins.model.lazy.LazyBuildMixIn;
Expand Down Expand Up @@ -1974,66 +1969,6 @@ public HttpResponse doEnable() throws IOException, ServletException {
}


/**
* RSS feed for changes in this project.
*/
public void doRssChangelog( StaplerRequest req, StaplerResponse rsp ) throws IOException, ServletException {
class FeedItem {
ChangeLogSet.Entry e;
int idx;

public FeedItem(Entry e, int idx) {
this.e = e;
this.idx = idx;
}

AbstractBuild<?,?> getBuild() {
return e.getParent().build;
}
}

List<FeedItem> entries = new ArrayList<FeedItem>();

for(R r=getLastBuild(); r!=null; r=r.getPreviousBuild()) {
int idx=0;
for( ChangeLogSet.Entry e : r.getChangeSet())
entries.add(new FeedItem(e,idx++));
}

RSS.forwardToRss(
getDisplayName()+' '+getScm().getDescriptor().getDisplayName()+" changes",
getUrl()+"changes",
entries, new FeedAdapter<FeedItem>() {
public String getEntryTitle(FeedItem item) {
return "#"+item.getBuild().number+' '+item.e.getMsg()+" ("+item.e.getAuthor()+")";
}

public String getEntryUrl(FeedItem item) {
return item.getBuild().getUrl()+"changes#detail"+item.idx;
}

public String getEntryID(FeedItem item) {
return getEntryUrl(item);
}

public String getEntryDescription(FeedItem item) {
StringBuilder buf = new StringBuilder();
for(String path : item.e.getAffectedPaths())
buf.append(path).append('\n');
return buf.toString();
}

public Calendar getEntryTimestamp(FeedItem item) {
return item.getBuild().getTimestamp();
}

public String getEntryAuthor(FeedItem entry) {
return JenkinsLocationConfiguration.get().getAdminAddress();
}
},
req, rsp );
}

/**
* {@link AbstractProject} subtypes should implement this base class as a descriptor.
*
Expand Down
84 changes: 82 additions & 2 deletions core/src/main/java/hudson/model/Job.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import hudson.EnvVars;
import hudson.Extension;
import hudson.ExtensionPoint;
import hudson.FeedAdapter;
import hudson.PermalinkList;
import hudson.Util;
import hudson.cli.declarative.CLIResolver;
Expand All @@ -36,6 +37,8 @@
import hudson.model.Fingerprint.RangeSet;
import hudson.model.PermalinkProjectAction.Permalink;
import hudson.model.listeners.ItemListener;
import hudson.scm.ChangeLogSet;
import hudson.scm.SCM;
import hudson.search.QuickSilver;
import hudson.search.SearchIndex;
import hudson.search.SearchIndexBuilder;
Expand Down Expand Up @@ -83,12 +86,14 @@
import jenkins.model.BuildDiscarderProperty;
import jenkins.model.DirectlyModifiableTopLevelItemGroup;
import jenkins.model.Jenkins;
import jenkins.model.JenkinsLocationConfiguration;
import jenkins.model.ModelObjectWithChildren;
import jenkins.model.ProjectNamingStrategy;
import jenkins.model.RunIdMigrator;
import jenkins.model.lazy.LazyBuildMixIn;
import jenkins.scm.RunWithSCM;
import jenkins.security.HexStringConfidentialKey;
import jenkins.util.io.OnMaster;
import jenkins.triggers.SCMTriggerItem;
import net.sf.json.JSONException;
import net.sf.json.JSONObject;
import org.apache.commons.io.FileUtils;
Expand Down Expand Up @@ -1042,7 +1047,82 @@ public PermalinkList getPermalinks() {
}
return permalinks;
}


/**
* RSS feed for changes in this project.
Copy link
Member

Choose a reason for hiding this comment

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

Should note that @since TODO it was pulled up into job.

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 do.

*/
public void doRssChangelog(StaplerRequest req, StaplerResponse rsp) throws IOException, ServletException {
class FeedItem {
ChangeLogSet.Entry e;
int idx;

public FeedItem(ChangeLogSet.Entry e, int idx) {
this.e = e;
this.idx = idx;
}

Run<?, ?> getBuild() {
return e.getParent().build;
}
}

List<FeedItem> entries = new ArrayList<FeedItem>();
String scmDisplayName = "";
if (this instanceof SCMTriggerItem) {
SCMTriggerItem scmItem = (SCMTriggerItem) this;
List<String> scmNames = new ArrayList<>();
for (SCM s : scmItem.getSCMs()) {
scmNames.add(s.getDescriptor().getDisplayName());
}
scmDisplayName = " " + Util.join(scmNames, ", ");
}

for (RunT r = getLastBuild(); r != null; r = r.getPreviousBuild()) {
int idx = 0;
if (r instanceof RunWithSCM) {
for (ChangeLogSet<? extends ChangeLogSet.Entry> c : ((RunWithSCM<?,?>) r).getChangeSets()) {
for (ChangeLogSet.Entry e : c) {
entries.add(new FeedItem(e, idx++));
}
}
}
}
RSS.forwardToRss(
getDisplayName() + scmDisplayName + " changes",
getUrl() + "changes",
entries, new FeedAdapter<FeedItem>() {
public String getEntryTitle(FeedItem item) {
return "#" + item.getBuild().number + ' ' + item.e.getMsg() + " (" + item.e.getAuthor() + ")";
}

public String getEntryUrl(FeedItem item) {
return item.getBuild().getUrl() + "changes#detail" + item.idx;
}

public String getEntryID(FeedItem item) {
return getEntryUrl(item);
}

public String getEntryDescription(FeedItem item) {
StringBuilder buf = new StringBuilder();
for (String path : item.e.getAffectedPaths())
buf.append(path).append('\n');
return buf.toString();
}

public Calendar getEntryTimestamp(FeedItem item) {
return item.getBuild().getTimestamp();
}

public String getEntryAuthor(FeedItem entry) {
return JenkinsLocationConfiguration.get().getAdminAddress();
}
},
req, rsp);
}



@Override public ContextMenu doChildrenContextMenu(StaplerRequest request, StaplerResponse response) throws Exception {
// not sure what would be really useful here. This needs more thoughts.
// for the time being, I'm starting with permalinks
Expand Down
Loading