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.

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

walkmod execution to remove dead code #1957

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
30 changes: 30 additions & 0 deletions core/pom.xml
Expand Up @@ -149,27 +149,57 @@ THE SOFTWARE.
<artifactId>windows-package-checker</artifactId>
<version>1.0</version>
</dependency>
<dependency>
<groupId>org.zeroturnaround</groupId>
<artifactId>javarebel-sdk</artifactId>
<version>2.0.2</version>
<scope>runtime</scope>
</dependency>
Copy link
Member

Choose a reason for hiding this comment

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

<dependency>
<groupId>org.kohsuke.stapler</groupId>
<artifactId>stapler-adjunct-zeroclipboard</artifactId>
<version>1.3.5-1</version>
<exclusions>
<exclusion>
<groupId>org.kohsuke.stapler</groupId>
<artifactId>stapler</artifactId>
</exclusion>
</exclusions>
</dependency>
<dependency>
<groupId>org.kohsuke.stapler</groupId>
<artifactId>stapler-adjunct-timeline</artifactId>
<version>1.4</version>
<exclusions>
<exclusion>
<groupId>org.kohsuke.stapler</groupId>
<artifactId>stapler</artifactId>
</exclusion>
</exclusions>
</dependency>
<dependency>
<groupId>org.kohsuke.stapler</groupId>
<artifactId>stapler-adjunct-codemirror</artifactId>
<version>1.3</version>
<exclusions>
<exclusion>
<groupId>org.kohsuke.stapler</groupId>
<artifactId>stapler</artifactId>
</exclusion>
</exclusions>
</dependency>
<dependency><!-- this helps us see the source code of the control while we edit Jenkins. -->
<groupId>org.kohsuke.stapler</groupId>
<artifactId>stapler-adjunct-timeline</artifactId>
<version>1.4</version>
<classifier>tests</classifier>
<scope>test</scope>
<exclusions>
<exclusion>
<groupId>org.kohsuke.stapler</groupId>
<artifactId>stapler</artifactId>
</exclusion>
</exclusions>
</dependency>

<dependency>
Expand Down
2 changes: 1 addition & 1 deletion core/src/main/java/hudson/FilePath.java
Expand Up @@ -1601,7 +1601,7 @@ private static void _chmod(File f, int mask) throws IOException {
PosixAPI.jnr().chmod(f.getAbsolutePath(),mask);
}

private static boolean CHMOD_WARNED = false;


/**
* Gets the file permission bit mask.
Expand Down
1 change: 0 additions & 1 deletion core/src/main/java/hudson/cli/handlers/package-info.java
Expand Up @@ -3,4 +3,3 @@
*/
package hudson.cli.handlers;

import org.kohsuke.args4j.spi.OptionHandler;
1 change: 0 additions & 1 deletion core/src/main/java/hudson/init/package-info.java
Expand Up @@ -44,4 +44,3 @@
*/
package hudson.init;

import org.jvnet.hudson.reactor.Task;
2 changes: 1 addition & 1 deletion core/src/main/java/hudson/model/Job.java
Expand Up @@ -674,7 +674,7 @@ public void renameTo(String newName) throws IOException {

@Override
public void movedTo(DirectlyModifiableTopLevelItemGroup destination, AbstractItem newItem, File destDir) throws IOException {
Job newJob = (Job) newItem; // Missing covariant parameters type here.
// Missing covariant parameters type here.
Copy link
Member

Choose a reason for hiding this comment

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

I thought this comment was related to removed line. I guess it used to check types even if it not used.

Copy link
Author

Choose a reason for hiding this comment

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

But do you think that is makes sense if the parameter is not used?

Copy link
Member

Choose a reason for hiding this comment

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

I don't see any reason to keep this line

Copy link
Member

Choose a reason for hiding this comment

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

Or to be on the safe side assert newItem instanceof Job

File oldBuildDir = getBuildDir();
super.movedTo(destination, newItem, destDir);
File newBuildDir = getBuildDir();
Expand Down
12 changes: 2 additions & 10 deletions core/src/main/java/hudson/model/LoadStatistics.java
Expand Up @@ -27,7 +27,7 @@
import hudson.model.MultiStageTimeSeries.TimeScale;
import hudson.model.MultiStageTimeSeries.TrendChart;
import hudson.model.queue.SubTask;
import hudson.model.queue.Tasks;

import hudson.util.ColorPalette;
import hudson.util.NoOverlapCategoryAxis;
import jenkins.model.Jenkins;
Expand Down Expand Up @@ -401,15 +401,7 @@ protected void doRun() {
j.overallLoad.updateCounts(j.overallLoad.computeSnapshot(bis));
}

private int count(List<Queue.BuildableItem> bis, Label l) {
int q=0;
for (Queue.BuildableItem bi : bis) {
for (SubTask st : Tasks.getSubTasksOf(bi.task))
if (bi.getAssignedLabelFor(st)==l)
q++;
}
return q;
}

}

/**
Expand Down
2 changes: 1 addition & 1 deletion core/src/main/java/hudson/model/Queue.java
Expand Up @@ -65,7 +65,7 @@
import hudson.model.queue.CauseOfBlockage.BecauseNodeIsBusy;
import hudson.model.queue.WorkUnitContext;
import hudson.security.AccessControlled;
import hudson.security.Permission;

import jenkins.security.QueueItemAuthenticatorProvider;
import jenkins.util.Timer;
import hudson.triggers.SafeTimerTask;
Expand Down
4 changes: 2 additions & 2 deletions core/src/main/java/hudson/model/Slave.java
Expand Up @@ -52,10 +52,10 @@
import java.net.URL;
import java.net.URLConnection;
import java.util.ArrayList;
import java.util.Arrays;

import java.util.List;
import java.util.Set;
import java.util.TreeSet;


import javax.servlet.ServletException;

Expand Down
Expand Up @@ -26,7 +26,7 @@
import hudson.Extension;
import hudson.ExtensionList;
import hudson.ExtensionPoint;
import jenkins.model.Jenkins;

import java.util.Collection;
import java.util.Collections;

Expand Down Expand Up @@ -57,4 +57,4 @@ public Collection<? extends Action> createFor(User target) {
public static ExtensionList<TransientUserActionFactory> all() {
return ExtensionList.lookup(TransientUserActionFactory.class);
}
}
}
6 changes: 3 additions & 3 deletions core/src/main/java/hudson/model/UpdateCenter.java
Expand Up @@ -62,19 +62,19 @@
import org.kohsuke.stapler.StaplerRequest;
import org.kohsuke.stapler.StaplerResponse;

import javax.annotation.Nonnull;

import javax.net.ssl.SSLHandshakeException;
import javax.servlet.ServletException;
import java.io.File;
import java.io.FileInputStream;

import java.io.FileOutputStream;
import java.io.IOException;
import java.io.OutputStream;
import java.net.MalformedURLException;
import java.net.URL;
import java.net.URLConnection;
import java.net.UnknownHostException;
import java.security.DigestInputStream;

import java.security.DigestOutputStream;
import java.security.MessageDigest;
import java.security.NoSuchAlgorithmException;
Expand Down
Expand Up @@ -175,12 +175,7 @@ public T get(Computer c) {
return record.data.get(c);
}

/**
* Is the monitoring activity currently in progress?
*/
private synchronized boolean isInProgress() {
return inProgress !=null && inProgress.isAlive();
}


/**
* The timestamp that indicates when the last round of the monitoring has completed.
Expand Down
Expand Up @@ -23,7 +23,7 @@
*/
package hudson.node_monitors;

import hudson.Util;

Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't these lines just be removed?

Copy link
Author

Choose a reason for hiding this comment

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

Yes

import hudson.Extension;
import hudson.model.Computer;
import hudson.remoting.Callable;
Expand Down
6 changes: 3 additions & 3 deletions core/src/main/java/hudson/util/LRUStringConverter.java
@@ -1,11 +1,11 @@
package hudson.util;

import com.thoughtworks.xstream.converters.basic.AbstractSingleValueConverter;
import com.thoughtworks.xstream.converters.basic.StringConverter;

import org.apache.commons.collections.map.LRUMap;

import java.util.Collections;
import java.util.HashMap;

import java.util.Map;

public class LRUStringConverter extends AbstractSingleValueConverter {
Expand Down Expand Up @@ -38,4 +38,4 @@ public Object fromString(final String str) {

return s;
}
}
}
Expand Up @@ -4,7 +4,7 @@
import hudson.ExtensionList;
import hudson.ExtensionPoint;
import hudson.model.ModelObject;
import hudson.security.*;

import hudson.security.Messages;

/**
Expand Down
14 changes: 14 additions & 0 deletions core/walkmod.xml
@@ -0,0 +1,14 @@
<!DOCTYPE walkmod PUBLIC "-//WALKMOD//DTD" "http://www.walkmod.com/dtd/walkmod-1.1.dtd">
<walkmod>
Copy link
Member

Choose a reason for hiding this comment

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

Would it be possible to keep this file somewhere outside the top level? E.g. src/walkmod

Copy link
Author

Choose a reason for hiding this comment

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

walkmod is like maven - it uses the execution directory to find the configuration. If it is a strict requirement, I can add an option for this.

Copy link
Member

Choose a reason for hiding this comment

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

No strict requirement

<conf-providers>
<conf-provider type="maven"/>
</conf-providers>
<chain name="default">
<transformation type="dead-code-cleaner">
<param name="ignoreSerializableMethods">true</param>
<param name="excludedMethods">["hudson.FilePath#_syncIO()"]</param>
<param name="excludedFields">["hudson.model.Slave#labels", "hudson.model.JenkinsLocationConfiguration#charset", "jenkins.model.JenkinsLocationConfiguration#useSsl"]</param>
Copy link
Member

Choose a reason for hiding this comment

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

Keeping configuraiton in xml... afair @jglick is strongly against such things.

Copy link
Author

Choose a reason for hiding this comment

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

I can support yml configurations. Is it better?

Copy link
Member

Choose a reason for hiding this comment

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

XML is fine for me...

</transformation>
<writer path="src/main/java" type="org.walkmod:walkmod-javalang-plugin:string-writer"/>
</chain>
</walkmod>