-
Notifications
You must be signed in to change notification settings - Fork 7.6k
增加更多memorycompiler对jar内容的获取 #3104
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
base: master
Are you sure you want to change the base?
Conversation
|
1luik seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
|
我是使用的idea登录github提交的 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.
| import javax.tools.JavaFileObject; | ||
| import java.io.*; | ||
| import java.net.URI; | ||
| import java.nio.charset.Charset; |
Copilot
AI
Jan 14, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The unused import java.nio.charset.Charset is imported but never used in this class.
| import java.nio.charset.Charset; |
| import java.util.zip.ZipException; | ||
| import java.util.zip.ZipFile; | ||
|
|
||
| public class GetZipFile extends ZipFile { |
Copilot
AI
Jan 14, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The class name GetZipFile is poorly named as it uses a verb ("Get") for a class that represents an extended ZipFile. A better name would be something like CustomZipFile, ExtendedZipFile, or StreamableZipFile to better describe what the class represents.
| import java.net.URI; | ||
| import java.net.URL; | ||
| import java.net.URLDecoder; | ||
| import java.net.*; |
Copilot
AI
Jan 14, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using wildcard import import java.net.*; reduces code clarity. It's better to explicitly import only the classes that are actually used (URLConnection, JarURLConnection, URI, URL, URLDecoder) so readers can easily see the dependencies.
| import java.net.*; | |
| import java.net.JarURLConnection; | |
| import java.net.URI; | |
| import java.net.URL; | |
| import java.net.URLConnection; | |
| import java.net.URLDecoder; |
| @@ -0,0 +1,113 @@ | |||
| package com.taobao.arthas.compiler; | |||
|
|
|||
| import javax.lang.model.element.Modifier; | |||
Copilot
AI
Jan 14, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The unused import javax.lang.model.element.Modifier is imported but never used in this class.
| package com.taobao.arthas.compiler; | ||
|
|
||
| import javax.lang.model.element.Modifier; | ||
| import javax.lang.model.element.NestingKind; |
Copilot
AI
Jan 14, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The unused import javax.lang.model.element.NestingKind is imported but never used in this class.
| } | ||
| } | ||
| } | ||
| }catch (SecurityException e) { |
Copilot
AI
Jan 14, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing a URISyntaxException handler. The new URI(jarUri) call on line 132 can throw URISyntaxException, but the catch block on line 171 only catches SecurityException. This means URISyntaxException will be caught by the outer catch block on line 175, which may not provide the appropriate error message for URI syntax issues.
| }catch (SecurityException e) { | |
| } catch (URISyntaxException e) { | |
| throw new RuntimeException("Invalid URI syntax for jarUri: " + jarUri, e); | |
| } catch (SecurityException e) { |
| }else { | ||
| //是否可以在硬盘上找到对应的文件 | ||
| try { | ||
| URI uri1 = new URI(jarUri); | ||
| // 获取 authority(host:port) + path + query + fragment | ||
| StringBuilder sb = new StringBuilder(); | ||
| if (uri1.getAuthority() != null) { | ||
| sb.append(uri1.getAuthority()); | ||
| } | ||
| if (uri1.getPath() != null) { | ||
| sb.append(uri1.getPath()); | ||
| } | ||
| if (uri1.getQuery() != null) { | ||
| sb.append('?').append(uri1.getQuery()); | ||
| } | ||
| if (uri1.getFragment() != null) { | ||
| sb.append('#').append(uri1.getFragment()); | ||
| } | ||
|
|
||
| Enumeration<JarEntry> entryEnum = jarConn.getJarFile().entries(); | ||
| while (entryEnum.hasMoreElements()) { | ||
| JarEntry jarEntry = entryEnum.nextElement(); | ||
| String name = jarEntry.getName(); | ||
| if (name.startsWith(rootEntryName) && name.indexOf('/', rootEnd) == -1 && name.endsWith(CLASS_FILE_EXTENSION)) { | ||
| URI uri = URI.create(jarUri + "!/" + name); | ||
| String binaryName = name.replaceAll("/", "."); | ||
| binaryName = binaryName.replaceAll(CLASS_FILE_EXTENSION + "$", ""); | ||
| File file = new File(sb.toString()); | ||
| if (file.exists()) { | ||
| //可能是文件或目录 | ||
| if (file.isDirectory()) { | ||
| result.addAll(processDir(packageName, file)); | ||
| } else { | ||
| try { | ||
| GetZipFile zipFile = new GetZipFile(file); | ||
| zipFile.stream().forEach(e -> { | ||
| String name = e.getName(); | ||
| if (name.startsWith(packageName) && name.endsWith(CLASS_FILE_EXTENSION)) { | ||
| URI uri = URI.create(jarUri + "!/" + name); | ||
| String binaryName = name.replaceAll("/", "."); | ||
| binaryName = binaryName.replaceAll(CLASS_FILE_EXTENSION + "$", ""); | ||
|
|
||
| result.add(new CustomJavaFileObject(binaryName, uri)); | ||
| result.add(zipFile.new ZipJavaFileObject(binaryName, uri, e)); | ||
| } | ||
| }); | ||
| }catch (ZipException e) { | ||
| // ignore | ||
| } | ||
| } | ||
| } | ||
| }catch (SecurityException e) { | ||
| throw new RuntimeException("Failed to open " + packageFolderURL + " as a jar file", e); | ||
| } | ||
| } |
Copilot
AI
Jan 14, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The new fallback logic (lines 129-174) for handling non-JarURLConnection cases lacks test coverage. Given the complexity of the URI parsing and file system access logic, and the fact that this addresses a specific issue with Minecraft Forge's custom URL handlers, adding test cases for this code path would help ensure it works correctly and prevent regressions.
| URI uri1 = new URI(jarUri); | ||
| // 获取 authority(host:port) + path + query + fragment | ||
| StringBuilder sb = new StringBuilder(); | ||
| if (uri1.getAuthority() != null) { | ||
| sb.append(uri1.getAuthority()); | ||
| } | ||
| if (uri1.getPath() != null) { | ||
| sb.append(uri1.getPath()); | ||
| } | ||
| if (uri1.getQuery() != null) { | ||
| sb.append('?').append(uri1.getQuery()); | ||
| } | ||
| if (uri1.getFragment() != null) { | ||
| sb.append('#').append(uri1.getFragment()); |
Copilot
AI
Jan 14, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The variable name uri1 is not descriptive. Consider using a more meaningful name like parsedJarUri or jarFileUri to clearly indicate what this URI represents.
| URI uri1 = new URI(jarUri); | |
| // 获取 authority(host:port) + path + query + fragment | |
| StringBuilder sb = new StringBuilder(); | |
| if (uri1.getAuthority() != null) { | |
| sb.append(uri1.getAuthority()); | |
| } | |
| if (uri1.getPath() != null) { | |
| sb.append(uri1.getPath()); | |
| } | |
| if (uri1.getQuery() != null) { | |
| sb.append('?').append(uri1.getQuery()); | |
| } | |
| if (uri1.getFragment() != null) { | |
| sb.append('#').append(uri1.getFragment()); | |
| URI parsedJarUri = new URI(jarUri); | |
| // 获取 authority(host:port) + path + query + fragment | |
| StringBuilder sb = new StringBuilder(); | |
| if (parsedJarUri.getAuthority() != null) { | |
| sb.append(parsedJarUri.getAuthority()); | |
| } | |
| if (parsedJarUri.getPath() != null) { | |
| sb.append(parsedJarUri.getPath()); | |
| } | |
| if (parsedJarUri.getQuery() != null) { | |
| sb.append('?').append(parsedJarUri.getQuery()); | |
| } | |
| if (parsedJarUri.getFragment() != null) { | |
| sb.append('#').append(parsedJarUri.getFragment()); |
| Enumeration<? extends ZipEntry> entries = super.entries(); | ||
| List<ZipEntry> entryList = new ArrayList<>(); | ||
| while (entries.hasMoreElements()) { | ||
| entryList.add(entries.nextElement()); | ||
| } | ||
| return entryList.stream(); |
Copilot
AI
Jan 14, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The stream() method loads all ZipEntry elements into an ArrayList before converting to a stream, which is inefficient for large zip files. This creates unnecessary memory overhead. Consider using Collections.list(entries).stream() or implementing a custom Spliterator for better performance and memory efficiency.
| Enumeration<? extends ZipEntry> entries = super.entries(); | |
| List<ZipEntry> entryList = new ArrayList<>(); | |
| while (entries.hasMoreElements()) { | |
| entryList.add(entries.nextElement()); | |
| } | |
| return entryList.stream(); | |
| return super.stream().map(e -> (ZipEntry) e); |
| GetZipFile zipFile = new GetZipFile(file); | ||
| zipFile.stream().forEach(e -> { | ||
| String name = e.getName(); | ||
| if (name.startsWith(packageName) && name.endsWith(CLASS_FILE_EXTENSION)) { | ||
| URI uri = URI.create(jarUri + "!/" + name); | ||
| String binaryName = name.replaceAll("/", "."); | ||
| binaryName = binaryName.replaceAll(CLASS_FILE_EXTENSION + "$", ""); | ||
|
|
||
| result.add(new CustomJavaFileObject(binaryName, uri)); | ||
| result.add(zipFile.new ZipJavaFileObject(binaryName, uri, e)); | ||
| } | ||
| }); |
Copilot
AI
Jan 14, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The GetZipFile resource is not properly closed. The ZipFile is created but never closed, which can lead to resource leaks and file handle exhaustion. Consider using try-with-resources or ensuring the file is closed after processing.
|
6 |

修复#3103 ,但是获取不到minecraft包还是编译不了