Reimplement path file management system. #33
Conversation
ascopes
force-pushed
the
task/reimplement-paths
branch
from
7c05e1c to
4aaf5be
Compare
|
This pull request fixes 1 alert when merging 4aaf5be into 8d0d8f8 - view on LGTM.com fixed alerts:
|
Codecov Report
@@ Coverage Diff @@
## main #33 +/- ##
===========================================
- Coverage 70.61% 58.51% -12.11%
===========================================
Files 42 63 +21
Lines 1746 2114 +368
Branches 108 155 +47
===========================================
+ Hits 1233 1237 +4
- Misses 474 817 +343
- Partials 39 60 +21
Continue to review full report at Codecov.
|
|
This pull request fixes 1 alert when merging f0c1ba0 into 8d0d8f8 - view on LGTM.com fixed alerts:
|
|
This pull request fixes 1 alert when merging 025cc76 into 8d0d8f8 - view on LGTM.com fixed alerts:
|
ascopes
force-pushed
the
task/reimplement-paths
branch
from
025cc76 to
0d868aa
Compare
|
This pull request fixes 1 alert when merging 0d868aa into 8d0d8f8 - view on LGTM.com fixed alerts:
|
|
This pull request fixes 1 alert when merging 004f479 into 8d0d8f8 - view on LGTM.com fixed alerts:
|
|
This pull request fixes 1 alert when merging 74b77b0 into 8d0d8f8 - view on LGTM.com fixed alerts:
|
|
This pull request fixes 1 alert when merging b06d4fd into 8d0d8f8 - view on LGTM.com fixed alerts:
|
It appears the file management for modules in ECJ is totally bypassing the mechanism we implement to read files and query the compilation unit file system. Instead, it is using org.eclipse.jdt.internal.compiler.batch.FileSystem#initializeKnownFileNames and using java.io.File, which will not work with non-default file systems. As for why this works without modules, I have no idea.
|
This pull request fixes 1 alert when merging 2b7e79e into 8d0d8f8 - view on LGTM.com fixed alerts:
|
|
This pull request fixes 1 alert when merging 46c819c into 8d0d8f8 - view on LGTM.com fixed alerts:
|
ascopes
force-pushed
the
task/reimplement-paths
branch
from
967c7a2 to
367670d
Compare
ascopes
force-pushed
the
task/reimplement-paths
branch
from
367670d to
69941a2
Compare
![sonatype-lift[bot]](https://avatars.githubusercontent.com/in/9843?s=60&v=4){kind=small}
| export M2_HOME="${HOME:-$HOMEDIR}/.m2" | ||
| fi | ||
|
|
||
| workspace_dir="$(realpath $(dirname ${BASH_SOURCE[0]:-$0})/..)" |
There was a problem hiding this comment.
SC2046: Quote this to prevent word splitting.
(at-me in a reply with help or ignore)
Was this a good recommendation?
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]
| -w "${container_workspace_dir}" \ | ||
| --rm \ | ||
| "${image}" \ | ||
| ${command} \ |
There was a problem hiding this comment.
SC2086: Double quote to prevent globbing and word splitting.
(at-me in a reply with help or ignore)
Was this a good recommendation?
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]
| command="${default_command}" | ||
|
|
||
| while getopts ":c:v:h" opt; do | ||
| case "${opt}" in |
There was a problem hiding this comment.
SC2220: Invalid flags are not handled. Add a *) case.
(at-me in a reply with help or ignore)
Was this a good recommendation?
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]
| export M2_HOME="${HOME:-$HOMEDIR}/.m2" | ||
| fi | ||
|
|
||
| workspace_dir="$(realpath $(dirname ${BASH_SOURCE[0]:-$0})/..)" |
There was a problem hiding this comment.
SC2086: Double quote to prevent globbing and word splitting.
(at-me in a reply with help or ignore)
Was this a good recommendation?
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]
|
|
||
| mkdir target 2>&1 || true | ||
|
|
||
| for version in $(seq ${first_version} ${last_version}); do |
There was a problem hiding this comment.
SC2086: Double quote to prevent globbing and word splitting.
(at-me in a reply with help or ignore)
Was this a good recommendation?
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]
| "${image}" \ | ||
| ${command} \ | ||
| 2>&1 | | ||
| while read line; do |
There was a problem hiding this comment.
SC2162: read without -r will mangle backslashes.
(at-me in a reply with help or ignore)
Was this a good recommendation?
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]
|
|
||
| mkdir target 2>&1 || true | ||
|
|
||
| for version in $(seq ${first_version} ${last_version}); do |
There was a problem hiding this comment.
SC2086: Double quote to prevent globbing and word splitting.
(at-me in a reply with help or ignore)
Was this a good recommendation?
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]
ascopes
force-pushed
the
task/reimplement-paths
branch
from
4d59370 to
cf3769a
Compare
|
This pull request introduces 1 alert and fixes 1 when merging ee28ada into 8d0d8f8 - view on LGTM.com new alerts:
fixed alerts:
|
Fix Windows runner issue that was propagating through assertj
ascopes
force-pushed
the
task/reimplement-paths
branch
from
ee28ada to
3240e7b
Compare
|
This pull request introduces 1 alert and fixes 1 when merging 3240e7b into 8d0d8f8 - view on LGTM.com new alerts:
fixed alerts:
|
This rewrites the path file management system from scratch to use a mechanism that is closer to how OpenJDK Javac works under the hood.
The implementation works by storing a map of locations to container groups in the file manager.
The file manager (part of the JSR-199 compiler API in the JDK) has a one-to-many relationship with container groups. Each container group has a one-to-many relationship with containers. Each container has a 1-to-1 relationship with some form of Path-like object.
Each container group can supply a classloader, and provides some form of access to "containers". The container group implementations fall into three categories:
Each container group holds containers. Containers are wrappers around single path-like objects, and provide an interface for consistent read, write, and list operations on the file tree associated with the path. There are currently three implementations of container:
Paththat is on some file system.FileSysteminternally that gets closed once the enclosing RamPath is garbage collected.Multi-Releaseconfiguration enabled. The purpose here is to allow reading a JAR or WAR of files. JAR containers do not support opening files for output, and are "mounted" as read-only. A JarFileSystem is not used for this since the OpenJDK JAR filesystem provider implementation can not open more than one copy of the same ZIP-like archive at once, which prevents tests using JCT from running safely in parallel. Conversely the ZipFileSystem provider that JarFileSystem extends does not appear to have this limitation. This entire mechanism replaces the need to create temporary symbolic links in the default system FileSystem to work around the JarFileSystem constraints explained here.A side effect of supporting
Multi-Releasein the MANIFEST.MF is that we have to hold the target compiler version number string, which effectively prevents this entire mechanism from being constructed until this release version is known. This means that the existing process for adding new paths to a Compiler implementation will have to store all paths and locations in a separate structure, only registering them fully once a FileManager is created in the SimpleCompilationFactory.The majority of the "old" paths implementation will be removed entirely as part of this PR.