Skip to content

Commit

Permalink
Merge branch 'master' into upgrade-htmlunit
Browse files Browse the repository at this point in the history
* master:
  commons-compress -> 1.10
  Noting #1788
  Remove hardcode in tar test
  [FIXED JENKINS-10629] Unbroke stream with new tar implementation.
  Revert "Revert "FIXED JENKINS-10629] - Enable BigNumber mode to support archiving of files with size >8Gb""
  Revert "Revert "[JENKINS-10629] - Migrate the Tar archives handling code to commons-compress""
  Fix at-since from PR #1788
  Merge PR #1788: Make plugin manager pluggable
  • Loading branch information
tfennelly committed Aug 26, 2015
2 parents 1625c84 + b5b3779 commit 3f98011
Show file tree
Hide file tree
Showing 11 changed files with 159 additions and 65 deletions.
8 changes: 7 additions & 1 deletion changelog.html
Expand Up @@ -58,6 +58,12 @@
<li class="major bug">
Race condition in triggers could cause various <code>NullPointerException</code>s.
(<a href="https://issues.jenkins-ci.org/browse/JENKINS-29790">issue 29790</a>)
<li class=bug>
Archiving of large artifacts. Tar implementation cannot handle files having a size >8GB.
(<a href="https://issues.jenkins-ci.org/browse/JENKINS-10629">issue 10629</a>)
<li class="rfe">
Allow plugins to augment or replace the plugin manager UI.
(<a href="https://github.com/jenkinsci/jenkins/pull/1788">PR 1788</a>)
</ul>
</div><!--=TRUNK-END=-->
<h3><a name=v1.626>What's new in 1.626</a> (2015/08/23)</h3>
Expand Down Expand Up @@ -289,7 +295,7 @@ <h3><a name=v1.610>What's new in 1.610</a> (2015/04/19)</h3>
Since 1.598 overrides of <code>Descriptor.getId</code> were not correctly handled by form binding, breaking at least the CloudBees Templates plugin.
(<a href="https://issues.jenkins-ci.org/browse/JENKINS-26781">issue 26781</a>)
<li class=bug>
<b>Reverted in 1.611</b>. Archiving of large artifacts. Tar implementation cannot handle files having a size >8GB.
<b>Reverted in 1.611, reimplemented in 1.627</b>. Archiving of large artifacts. Tar implementation cannot handle files having a size >8GB.
(<a href="https://issues.jenkins-ci.org/browse/JENKINS-10629">issue 10629</a>)
<li class=bug>
The queue state was not updated between scheduling builds.
Expand Down
5 changes: 5 additions & 0 deletions core/pom.xml
Expand Up @@ -276,6 +276,11 @@ THE SOFTWARE.
<artifactId>commons-beanutils</artifactId>
<version>1.8.3</version>
</dependency>
<dependency>
<groupId>org.apache.commons</groupId>
<artifactId>commons-compress</artifactId>
<version>1.10</version>
</dependency>
<dependency>
<groupId>javax.mail</groupId>
<artifactId>mail</artifactId>
Expand Down
32 changes: 9 additions & 23 deletions core/src/main/java/hudson/FilePath.java
Expand Up @@ -33,7 +33,6 @@
import hudson.model.Computer;
import hudson.model.Item;
import hudson.model.TaskListener;
import hudson.org.apache.tools.tar.TarInputStream;
import hudson.os.PosixAPI;
import hudson.os.PosixException;
import hudson.remoting.Callable;
Expand Down Expand Up @@ -70,7 +69,6 @@
import org.apache.tools.ant.DirectoryScanner;
import org.apache.tools.ant.Project;
import org.apache.tools.ant.types.FileSet;
import org.apache.tools.tar.TarEntry;
import org.apache.tools.zip.ZipEntry;
import org.apache.tools.zip.ZipFile;
import org.kohsuke.stapler.Stapler;
Expand Down Expand Up @@ -120,6 +118,8 @@
import javax.annotation.Nonnull;
import javax.annotation.Nullable;
import jenkins.security.MasterToSlaveCallable;
import org.apache.commons.compress.archivers.tar.TarArchiveEntry;
import org.apache.commons.compress.archivers.tar.TarArchiveInputStream;
import org.jenkinsci.remoting.RoleChecker;
import org.jenkinsci.remoting.RoleSensitive;

Expand Down Expand Up @@ -2268,12 +2268,15 @@ private Integer writeToTar(File baseDir, DirScanner scanner, OutputStream out) t

/**
* Reads from a tar stream and stores obtained files to the base dir.
* @since TODO supports large files > 10 GB, migration to commons-compress
*/
private void readFromTar(String name, File baseDir, InputStream in) throws IOException {
TarInputStream t = new TarInputStream(in);
TarArchiveInputStream t = new TarArchiveInputStream(in);

// TarInputStream t = new TarInputStream(in);
try {
TarEntry te;
while ((te = t.getNextEntry()) != null) {
TarArchiveEntry te;
while ((te = t.getNextTarEntry()) != null) {
File f = new File(baseDir,te.getName());
if(te.isDirectory()) {
mkdirs(f);
Expand All @@ -2282,8 +2285,7 @@ private void readFromTar(String name, File baseDir, InputStream in) throws IOExc
if (parent != null) mkdirs(parent);
writing(f);

byte linkFlag = (Byte) LINKFLAG_FIELD.get(te);
if (linkFlag==TarEntry.LF_SYMLINK) {
if (te.isSymbolicLink()) {
new FilePath(f).symlinkTo(te.getLinkName(), TaskListener.NULL);
} else {
IOUtils.copy(t,f);
Expand All @@ -2300,8 +2302,6 @@ private void readFromTar(String name, File baseDir, InputStream in) throws IOExc
} catch (InterruptedException e) {
Thread.currentThread().interrupt(); // process this later
throw new IOException("Failed to extract "+name,e);
} catch (IllegalAccessException e) {
throw new IOException("Failed to extract "+name,e);
} finally {
t.close();
}
Expand Down Expand Up @@ -2725,20 +2725,6 @@ public int compare(String o1, String o2) {
}
};

private static final Field LINKFLAG_FIELD = getTarEntryLinkFlagField();

private static Field getTarEntryLinkFlagField() {
try {
Field f = TarEntry.class.getDeclaredField("linkFlag");
f.setAccessible(true);
return f;
} catch (SecurityException e) {
throw new AssertionError(e);
} catch (NoSuchFieldException e) {
throw new AssertionError(e);
}
}

/**
* Gets the {@link FilePath} representation of the "~" directory
* (User's home directory in the Unix sense) of the given channel.
Expand Down
13 changes: 12 additions & 1 deletion core/src/main/java/hudson/PluginManager.java
Expand Up @@ -71,6 +71,7 @@
import org.kohsuke.stapler.HttpResponse;
import org.kohsuke.stapler.HttpResponses;
import org.kohsuke.stapler.QueryParameter;
import org.kohsuke.stapler.StaplerOverridable;
import org.kohsuke.stapler.StaplerRequest;
import org.kohsuke.stapler.StaplerResponse;
import org.kohsuke.stapler.export.Exported;
Expand Down Expand Up @@ -128,7 +129,7 @@
* @author Kohsuke Kawaguchi
*/
@ExportedBean
public abstract class PluginManager extends AbstractModelObject implements OnMaster {
public abstract class PluginManager extends AbstractModelObject implements OnMaster, StaplerOverridable {
/**
* All discovered plugins.
*/
Expand Down Expand Up @@ -215,6 +216,16 @@ public Api getApi() {
return new Api(this);
}

/**
* Find all registered overrides (intended to allow overriding/adding views)
* @return List of extensions
* @since 1.627
*/
@Override
public Collection<PluginManagerStaplerOverride> getOverrides() {
return PluginManagerStaplerOverride.all();
}

/**
* Called immediately after the construction.
* This is a separate method so that code executed from here will see a valid value in
Expand Down
28 changes: 28 additions & 0 deletions core/src/main/java/hudson/PluginManagerStaplerOverride.java
@@ -0,0 +1,28 @@
package hudson;

import jenkins.model.Jenkins;

import javax.annotation.CheckForNull;
import javax.annotation.Nonnull;

/**
* Extension point for selectively overriding parts of the {@link PluginManager} views
* Anything extending this and registered with an @Extension can replace existing views and define new views.
*
* It is also possible to add/modify API calls coming via Stapler, but this requires caution.
*
* In both cases, this is simply done by defining a resource or method that matches the existing one
*
* @author Sam Van Oort
* @since 1.627
*/
public abstract class PluginManagerStaplerOverride implements ExtensionPoint {

/**
* Return all implementations of this extension point
* @return All implementations of this extension point
*/
public static @Nonnull ExtensionList<PluginManagerStaplerOverride> all() {
return ExtensionList.lookup(PluginManagerStaplerOverride.class);
}
}
Expand Up @@ -37,8 +37,9 @@
* methods are provided to position at each successive entry in
* the archive, and the read each entry as a normal input stream
* using read().
*
* @deprecated Use {@link org.apache.commons.compress.archivers.tar.TarArchiveInputStream} instead
*/
@Deprecated
public class TarInputStream extends FilterInputStream {

// CheckStyle:VisibilityModifier OFF - bc
Expand Down
Expand Up @@ -35,8 +35,11 @@
* The TarOutputStream writes a UNIX tar archive as an OutputStream.
* Methods are provided to put entries, and then write their contents
* by writing to this stream using write().
*
* @deprecated Use {@link org.apache.commons.compress.archivers.tar.TarArchiveOutputStream} instead
*
*/
@Deprecated
public class TarOutputStream extends FilterOutputStream {
/** Fail if a long file name is required in the archive. */
public static final int LONGFILE_ERROR = 0;
Expand Down
52 changes: 14 additions & 38 deletions core/src/main/java/hudson/util/io/TarArchiver.java
Expand Up @@ -37,6 +37,8 @@
import java.io.IOException;
import java.io.OutputStream;
import java.lang.reflect.Field;
import org.apache.commons.compress.archivers.tar.TarArchiveEntry;
import org.apache.commons.compress.archivers.tar.TarArchiveOutputStream;

import static org.apache.tools.tar.TarConstants.LF_SYMLINK;

Expand All @@ -47,24 +49,17 @@
*/
final class TarArchiver extends Archiver {
private final byte[] buf = new byte[8192];
private final TarOutputStream tar;
private final TarArchiveOutputStream tar;

TarArchiver(OutputStream out) {
tar = new TarOutputStream(new BufferedOutputStream(out) {
// TarOutputStream uses TarBuffer internally,
// which flushes the stream for each block. this creates unnecessary
// data stream fragmentation, and flush request to a remote, which slows things down.
@Override
public void flush() throws IOException {
// so don't do anything in flush
}
});
tar.setLongFileMode(TarOutputStream.LONGFILE_GNU);
tar = new TarArchiveOutputStream(out);
tar.setBigNumberMode(TarArchiveOutputStream.BIGNUMBER_STAR);
tar.setLongFileMode(TarArchiveOutputStream.LONGFILE_GNU);
}

@Override
public void visitSymlink(File link, String target, String relativePath) throws IOException {
TarEntry e = new TarEntry(relativePath, LF_SYMLINK);
TarArchiveEntry e = new TarArchiveEntry(relativePath, LF_SYMLINK);
try {
int mode = IOUtils.mode(link);
if (mode != -1) {
Expand All @@ -73,16 +68,11 @@ public void visitSymlink(File link, String target, String relativePath) throws I
} catch (PosixException x) {
// ignore
}

e.setLinkName(target);

try {
StringBuffer linkName = (StringBuffer) LINKNAME_FIELD.get(e);
linkName.setLength(0);
linkName.append(target);
} catch (IllegalAccessException x) {
throw new IOException("Failed to set linkName", x);
}

tar.putNextEntry(e);
tar.putArchiveEntry(e);
tar.closeArchiveEntry();
entriesWritten++;
}

Expand All @@ -97,14 +87,14 @@ public void visit(File file, String relativePath) throws IOException {

if(file.isDirectory())
relativePath+='/';
TarEntry te = new TarEntry(relativePath);
TarArchiveEntry te = new TarArchiveEntry(relativePath);
int mode = IOUtils.mode(file);
if (mode!=-1) te.setMode(mode);
te.setModTime(file.lastModified());
if(!file.isDirectory())
te.setSize(file.length());

tar.putNextEntry(te);
tar.putArchiveEntry(te);

if (!file.isDirectory()) {
FileInputStream in = new FileInputStream(file);
Expand All @@ -117,25 +107,11 @@ public void visit(File file, String relativePath) throws IOException {
}
}

tar.closeEntry();
tar.closeArchiveEntry();
entriesWritten++;
}

public void close() throws IOException {
tar.close();
}

private static final Field LINKNAME_FIELD = getTarEntryLinkNameField();

private static Field getTarEntryLinkNameField() {
try {
Field f = TarEntry.class.getDeclaredField("linkName");
f.setAccessible(true);
return f;
} catch (SecurityException e) {
throw new AssertionError(e);
} catch (NoSuchFieldException e) {
throw new AssertionError(e);
}
}
}
33 changes: 32 additions & 1 deletion core/src/test/java/hudson/FilePathTest.java
Expand Up @@ -371,7 +371,7 @@ private void checkTarUntarRoundTrip(String filePrefix, long fileSize) throws Exc
// Decompress
FilePath outDir = new FilePath(temp.newFolder(filePrefix + "_out"));
final FilePath outFile = outDir.child(tempFile.getName());
tmpDirPath.child( filePrefix + ".tar").untar(outDir, TarCompression.NONE);
tmpDirPath.child(tarFile.getName()).untar(outDir, TarCompression.NONE);
assertEquals("Result file after the roundtrip differs from the initial file",
new FilePath(tempFile).digest(), outFile.digest());
}
Expand Down Expand Up @@ -659,4 +659,35 @@ private InputStream someZippedContent() throws IOException {
// test conflict subdir
src.moveAllChildrenTo(dst);
}

@Issue("JENKINS-10629")
@Test
public void testEOFbrokenFlush() throws IOException, InterruptedException {
final File srcFolder = temp.newFolder("src");
// simulate magic structure with magic sizes:
// |- dir/pom.xml (2049)
// |- pom.xml (2049)
// \- small.tar (1537)
final File smallTar = new File(srcFolder, "small.tar");
givenSomeContentInFile(smallTar, 1537);
final File dir = new File(srcFolder, "dir");
dir.mkdirs();
final File pomFile = new File(dir, "pom.xml");
givenSomeContentInFile(pomFile, 2049);
FileUtils.copyFileToDirectory(pomFile, srcFolder);

final File archive = temp.newFile("archive.tar");

// Compress archive
final FilePath tmpDirPath = new FilePath(srcFolder);
int tarred = tmpDirPath.tar(new FileOutputStream(archive), "**");
assertEquals("One file should have been compressed", 3, tarred);

// Decompress
final File dstFolder = temp.newFolder("dst");
dstFolder.mkdirs();
FilePath outDir = new FilePath(dstFolder);
// and now fail when flush is bad!
tmpDirPath.child("../" + archive.getName()).untar(outDir, TarCompression.NONE);
}
}
41 changes: 41 additions & 0 deletions test/src/main/java/hudson/core/PluginManagerOverrideTest.java
@@ -0,0 +1,41 @@
package hudson.core;

import com.gargoylesoftware.htmlunit.html.HtmlPage;
import hudson.PluginManagerStaplerOverride;
import org.junit.Rule;
import org.junit.Test;
import org.jvnet.hudson.test.JenkinsRule;
import org.jvnet.hudson.test.TestExtension;

import static org.junit.Assert.*;


/**
* Verify that the PluginManagerStaplerOverride extensions register and allow safely modifying PluginManager views
* @author Sam Van Oort
*/
public class PluginManagerOverrideTest {

@Rule
public JenkinsRule j = new JenkinsRule();

@Test
public void testViewOverrides() throws Exception {
// Verify extension registered correctly and comes back in overrides
assertEquals(1,PluginManagerStaplerOverride.all().size());
assertTrue(PluginManagerStaplerOverride.all().get(0) instanceof BasicPluginManagerOverride);

// Verify we can load untouched resources
JenkinsRule.WebClient client = j.createWebClient();
assertEquals(200, client.goTo("self/pluginManager/available").getWebResponse().getStatusCode());

// Verify new view loads
HtmlPage p = j.createWebClient().goTo("self/pluginManager/newview");
assertEquals("LoremIpsum", p.getElementById("dummyElement").getTextContent());
}

/** Micro-implementation simply to allow adding a view resource */
@TestExtension("testViewOverrides")
public static class BasicPluginManagerOverride extends PluginManagerStaplerOverride {
}
}

0 comments on commit 3f98011

Please sign in to comment.