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

Change names of files with the same path - this time by Transformer #102

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -142,47 +142,47 @@ public class ShadowCopyAction implements CopyAction {
return visitedFiles.add(path.pathString)
}

private void visitFile(FileCopyDetails fileDetails) {
if (!isArchive(fileDetails)) {
private void visitFile(FileCopyDetails mergedJarFileDetails) {
if (!isArchive(mergedJarFileDetails)) {
try {
boolean isClass = (FilenameUtils.getExtension(fileDetails.path) == 'class')
boolean isClass = (FilenameUtils.getExtension(mergedJarFileDetails.path) == 'class')
if (!remapper.hasRelocators() || !isClass) {
if (!isTransformable(fileDetails)) {
String path = fileDetails.relativePath.pathString
if (!isTransformable(mergedJarFileDetails)) {
String path = mergedJarFileDetails.relativePath.pathString
ZipEntry archiveEntry = new ZipEntry(path)
archiveEntry.setTime(fileDetails.lastModified)
archiveEntry.unixMode = (UnixStat.FILE_FLAG | fileDetails.mode)
archiveEntry.setTime(mergedJarFileDetails.lastModified)
archiveEntry.unixMode = (UnixStat.FILE_FLAG | mergedJarFileDetails.mode)
zipOutStr.putNextEntry(archiveEntry)
fileDetails.copyTo(zipOutStr)
mergedJarFileDetails.copyTo(zipOutStr)
zipOutStr.closeEntry()
} else {
transform(fileDetails)
transform(mergedJarFileDetails)
}
} else {
remapClass(fileDetails)
remapClass(mergedJarFileDetails)
}
recordVisit(fileDetails.relativePath)
recordVisit(mergedJarFileDetails.relativePath)
} catch (Exception e) {
throw new GradleException(String.format("Could not add %s to ZIP '%s'.", fileDetails, zipFile), e)
throw new GradleException(String.format("Could not add %s to ZIP '%s'.", mergedJarFileDetails, zipFile), e)
}
} else {
processArchive(fileDetails)
processArchive(mergedJarFileDetails)
}
}

private void processArchive(FileCopyDetails fileDetails) {
private void processArchive(FileCopyDetails mergedJarFileDetails) {
stats.startJar()
ZipFile archive = new ZipFile(fileDetails.file)
ZipFile archive = new ZipFile(mergedJarFileDetails.file)
List<ArchiveFileTreeElement> archiveElements = archive.entries.collect {
new ArchiveFileTreeElement(new RelativeArchivePath(it, fileDetails))
new ArchiveFileTreeElement(new RelativeArchivePath(it, mergedJarFileDetails))
}
Spec<FileTreeElement> patternSpec = patternSet.getAsSpec()
List<ArchiveFileTreeElement> filteredArchiveElements = archiveElements.findAll { ArchiveFileTreeElement archiveElement ->
patternSpec.isSatisfiedBy(archiveElement)
}
filteredArchiveElements.each { ArchiveFileTreeElement archiveElement ->
if (archiveElement.relativePath.file) {
visitArchiveFile(archiveElement, archive)
visitArchiveFile(archiveElement, mergedJarFileDetails.file)
}
}
archive.close()
Expand All @@ -196,18 +196,19 @@ public class ShadowCopyAction implements CopyAction {
}
}

private void visitArchiveFile(ArchiveFileTreeElement archiveFile, ZipFile archive) {
private void visitArchiveFile(ArchiveFileTreeElement archiveFile, File mergedJarFile) {
ZipFile mergedJar = new ZipFile(mergedJarFile)
def archiveFilePath = archiveFile.relativePath
if (archiveFile.classFile || !isTransformable(archiveFile)) {
if (recordVisit(archiveFilePath)) {
if (!remapper.hasRelocators() || !archiveFile.classFile) {
copyArchiveEntry(archiveFilePath, archive)
copyArchiveEntry(archiveFilePath, mergedJar)
} else {
remapClass(archiveFilePath, archive)
remapClass(archiveFilePath, mergedJar)
}
}
} else {
transform(archiveFile, archive)
transform(mergedJarFile.name, archiveFile, mergedJar)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Won't mergedJar.name work fine for this? Seems like we've made a bunch of changes that aren't really need as the information is available from the existing API.

Copy link
Collaborator

Choose a reason for hiding this comment

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

On that note, I'd prefer if you kept the changes to a minimum. The variable renames makes this a lot harder to review because it's harder to see what the actually change was.

Copy link
Author

Choose a reason for hiding this comment

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

Ok, thanks for looking at this! WRT variables naming, I was really lost with "archive", it was never clear to me if it is the .jar we are merging into fat jar, or far jar itself.
WRT redundancy, I'll look at this, but AFAIR there was no jar file path in mergedJar

Copy link
Collaborator

Choose a reason for hiding this comment

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

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'd prefer to name the variable sourceJar then.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Or sourceArchive

}
}

Expand Down Expand Up @@ -289,17 +290,17 @@ public class ShadowCopyAction implements CopyAction {
}
}

private void transform(ArchiveFileTreeElement element, ZipFile archive) {
transform(element, archive.getInputStream(element.relativePath.entry))
private void transform(String jarName, ArchiveFileTreeElement element, ZipFile archive) {
transform(jarName, element, archive.getInputStream(element.relativePath.entry))
}

private void transform(FileCopyDetails details) {
transform(details, details.file.newInputStream())
transform(details.name, details, details.file.newInputStream())
}

private void transform(FileTreeElement element, InputStream is) {
private void transform(String jarName, FileTreeElement element, InputStream is) {
String mappedPath = remapper.map(element.relativePath.pathString)
transformers.find { it.canTransformResource(element) }.transform(mappedPath, is, relocators)
transformers.find { it.canTransformResource(element) }.transform(jarName, mappedPath, is, relocators)
}

private boolean isTransformable(FileTreeElement element) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ class ApacheLicenseResourceTransformer implements Transformer {
LICENSE_TXT_PATH.regionMatches(true, 0, path, 0, LICENSE_TXT_PATH.length())
}

void transform(String path, InputStream is, List<Relocator> relocators) {
void transform(String jarName, String path, InputStream is, List<Relocator> relocators) {

}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ class ApacheNoticeResourceTransformer implements Transformer {
return false
}

void transform(String path, InputStream is, List<Relocator> relocators) {
void transform(String jarName, String path, InputStream is, List<Relocator> relocators) {
if (entries.isEmpty()) {
String year = new SimpleDateFormat("yyyy").format(new Date())
if (!inceptionYear.equals(year)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ class AppendingTransformer implements Transformer {
return false
}

void transform(String path, InputStream is, List<Relocator> relocators) {
void transform(String jarName, String path, InputStream is, List<Relocator> relocators) {
IOUtil.copy(is, data)
data.write('\n'.bytes)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ class ComponentsXmlResourceTransformer implements Transformer {
return COMPONENTS_XML_PATH.equals(path)
}

void transform(String path, InputStream is, List<Relocator> relocators) {
void transform(String jarName, String path, InputStream is, List<Relocator> relocators) {
Xpp3Dom newDom

try {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ class DontIncludeResourceTransformer implements Transformer {
return false
}

void transform(String path, InputStream is, List<Relocator> relocators) {
void transform(String jarName, String path, InputStream is, List<Relocator> relocators) {
// no op
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ class GroovyExtensionModuleTransformer implements Transformer {
}

@Override
void transform(String path, InputStream is, List<Relocator> relocators) {
void transform(String jarName, String path, InputStream is, List<Relocator> relocators) {
def props = new Properties()
props.load(is)
props.each { String key, String value ->
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ public class IncludeResourceTransformer implements Transformer {
return false
}

public void transform(String path, InputStream is, List<Relocator> relocators) {
public void transform(String jarName, String path, InputStream is, List<Relocator> relocators) {
// no op
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ class ManifestResourceTransformer implements Transformer {
return false
}

void transform(String path, InputStream is, List<Relocator> relocators) {
void transform(String jarName, String path, InputStream is, List<Relocator> relocators) {
// We just want to take the first manifest we come across as that's our project's manifest. This is the behavior
// now which is situational at best. Right now there is no context passed in with the processing so we cannot
// tell what artifact is being processed.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ class PropertiesFileTransformer implements Transformer {
}

@Override
void transform(String path, InputStream is, List<Relocator> relocators) {
void transform(String jarName, String path, InputStream is, List<Relocator> relocators) {
Properties props = propertiesEntries[path]
if (props == null) {
props = new Properties()
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,204 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

package com.github.jengelman.gradle.plugins.shadow.transformers
import com.github.jengelman.gradle.plugins.shadow.relocation.Relocator
import org.apache.commons.collections.map.MultiValueMap
import org.apache.commons.io.FilenameUtils
import org.apache.commons.lang.StringUtils
import org.apache.tools.zip.ZipEntry
import org.apache.tools.zip.ZipOutputStream
import org.gradle.api.file.FileTreeElement
import org.gradle.api.specs.Spec
import org.gradle.api.tasks.util.PatternFilterable
import org.gradle.api.tasks.util.PatternSet
import org.gradle.mvn3.org.codehaus.plexus.util.IOUtil

class SamePathFilesTransformer implements Transformer, PatternFilterable {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Need a better name for this class. Also javadoc describing what it does.


private final PatternSet patternSet = new PatternSet()

private final def serviceEntires = new MultiValueMap()

@Override
boolean canTransformResource(FileTreeElement element) {
return patternSet.asSpec.isSatisfiedBy(element)
}

@Override
void transform(String jarName, String path, InputStream is, List<Relocator> relocators) {
serviceEntires.put(path, new InputStreamWithJarName(is, jarName))
}

@Override
boolean hasTransformedResource() {
return true;
}

@Override
void modifyOutputStream(ZipOutputStream os) {
serviceEntires.each { String path, List<InputStreamWithJarName> streams ->
if (streams.size() == 1) {
os.putNextEntry(new ZipEntry(path))
IOUtil.copy(streams[0].is, os)
os.closeEntry()
} else {
streams.each {
def newPath = createFilePathWithMergedJarName(path, it.jarName)
os.putNextEntry(new ZipEntry(newPath))
IOUtil.copy(it.is, os)
os.closeEntry()
}
}
}
}

private String createFilePathWithMergedJarName(String path, String mergedJarName) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this needs to be configurable.

def extension = FilenameUtils.getExtension(path)
def pathWithoutExtension = path
if (StringUtils.isNotEmpty(extension)) {
pathWithoutExtension = path.substring(0, path.length() - extension.length() - 1)
extension = ".$extension"
}

def mergedJarExtension = FilenameUtils.getExtension(mergedJarName)
if (StringUtils.isNotEmpty(mergedJarExtension)) {
mergedJarName = mergedJarName.substring(0, mergedJarName.length() - mergedJarExtension.length() - 1)
}

return "${pathWithoutExtension}_$mergedJarName$extension"
}

/**
* {@inheritDoc}
*/
@Override
SamePathFilesTransformer include(String... includes) {
patternSet.include(includes)
return this
}

/**
* {@inheritDoc}
*/
@Override
SamePathFilesTransformer include(Iterable<String> includes) {
patternSet.include(includes)
return this
}

/**
* {@inheritDoc}
*/
@Override
SamePathFilesTransformer include(Spec<FileTreeElement> includeSpec) {
patternSet.include(includeSpec)
return this
}

/**
* {@inheritDoc}
*/
@Override
SamePathFilesTransformer include(Closure includeSpec) {
patternSet.include(includeSpec)
return this
}

/**
* {@inheritDoc}
*/
@Override
SamePathFilesTransformer exclude(String... excludes) {
patternSet.exclude(excludes)
return this
}

/**
* {@inheritDoc}
*/
@Override
SamePathFilesTransformer exclude(Iterable<String> excludes) {
patternSet.exclude(excludes)
return this
}

/**
* {@inheritDoc}
*/
@Override
SamePathFilesTransformer exclude(Spec<FileTreeElement> excludeSpec) {
patternSet.exclude(excludeSpec)
return this
}

/**
* {@inheritDoc}
*/
@Override
SamePathFilesTransformer exclude(Closure excludeSpec) {
patternSet.exclude(excludeSpec)
return this
}

/**
* {@inheritDoc}
*/
@Override
Set<String> getIncludes() {
return patternSet.includes
}

/**
* {@inheritDoc}
*/
@Override
SamePathFilesTransformer setIncludes(Iterable<String> includes) {
patternSet.includes = includes
return this
}

/**
* {@inheritDoc}
*/
@Override
Set<String> getExcludes() {
return patternSet.excludes
}

/**
* {@inheritDoc}
*/
@Override
SamePathFilesTransformer setExcludes(Iterable<String> excludes) {
patternSet.excludes = excludes
return this
}

private class InputStreamWithJarName {

private InputStream is;
private String jarName;

InputStreamWithJarName(InputStream is, String jarName) {
this.is = is
this.jarName = jarName
}
}
}
Loading