Prioritize TASTy files over classfiles on classpath aggregation #19431
Conversation
nicolasstucki
force-pushed
the
prioritze-tasty-over-classfiles
branch
2 times, most recently
from
a0fd162 to
759a71c
Compare
nicolasstucki
added
the
needs-minor-release
nicolasstucki
force-pushed
the
prioritze-tasty-over-classfiles
branch
8 times, most recently
from
e33062c to
78dcb5f
Compare
nicolasstucki
force-pushed
the
prioritze-tasty-over-classfiles
branch
2 times, most recently
from
d0d5fbe to
7d23518
Compare
| * creates an entry containing both of them. If there would be more than one class or source | ||
| * entries for the same class it always would use the first entry of each type found on a classpath. | ||
| * | ||
| * A class entry with a TASTy file will be chosen over one with a class file. Usually the class entries |
There was a problem hiding this comment.
I did not update this documentation correctly after some refactoring. I will update it.
| // Here we do not create a TastyFileEntry(entry.file) because the TASTy and the classfile | ||
| // come from different classpaths. These may not have the same TASTy UUID. |
There was a problem hiding this comment.
This comment is confusing. It seems to directly contradict what the commit is trying to do. On the other hand, if I ignore the comment and only read the code, it makes sense. What is that comment really trying to say? Nothing else in this method creates a TastyFileEntry; why should I expect that we would do that here?
There was a problem hiding this comment.
I did not update this documentation correctly after some refactoring. I will update it.
There was a problem hiding this comment.
The same comment seems to still be here. Perhaps it's meant to say TastyWithClassFileEntry instead of TastyFileEntry? That would make more sense IIUC.
There was a problem hiding this comment.
Yes, it should be TastyWithClassFileEntry. I updated it.
| @@ -52,7 +52,8 @@ sealed trait BinaryFileEntry extends ClassRepresentation { | |||
| object BinaryFileEntry { | |||
| def apply(file: AbstractFile): BinaryFileEntry = | |||
| if file.isTasty then | |||
| TastyFileEntry(file) | |||
| if file.resolveSiblingWithExtension("class") != null then TastyFileEntry(file) | |||
There was a problem hiding this comment.
Should we rename TastyFileEntry into TastyAndClassFileEntry then?
There was a problem hiding this comment.
TastyWithClassFileEntry might be more appropriate.
``` - ClassRepresentation - +- ClassFileEntry - | +- ClassFileEntryImpl(AbstractFile) - +- SourceFileEntry - | +- SourceFileEntryImpl(AbstractFile) - +- ClassAndSourceFilesEntry(AbstractFile, AbstractFile) + ClassRepresentation + +- BinaryFileEntry + | +- ClassFileEntry(AbstractFile) + | +- TastyWithClassFileEntry(AbstractFile) + +- SourceFileEntry(AbstractFile) + +- BinaryAndSourceFilesEntry(BinaryFileEntry, SourceFileEntry) ```
nicolasstucki
force-pushed
the
prioritze-tasty-over-classfiles
branch
from
acede1e to
c48406c
Compare
In most cases the TASTy file is chosen over the classfile in a classpath because they are packaged together. However, for the `scala-library` (Scala 2 compiled library) and `scala2-library-tasty` (Scala 3 compiled Scala 2 library) we have the classfiles in one jar and the TASTy files in another jar. Given that the classpaths order in guaranteed to be deterministic we might end up with the classfile being loaded first and the TASTy file second. The aggregator must be capable of choosing the TASTy file over the classfile in this case as well. The aggregator will only choose the new TASTy over the old classfile if the TASTy file has no classfile in its classpath. In other words, we only use this new behaviour for TASTy only classpaths. This also implies that we can just add the `scala2-library-tasty` as a dependency in any project to use it. Note that this jar is not published yet.
nicolasstucki
force-pushed
the
prioritze-tasty-over-classfiles
branch
from
c48406c to
ebe09ac
Compare
|
oh right I just saw the needs minor release, so does this have to wait until 3.5? |
In most cases the TASTy file is chosen over the classfile in a classpath
because they are packaged together. However, for the
scala-library(Scala 2 compiled library) and
scala2-library-tasty(Scala 3 compiled Scala 2 library)we have the classfiles in one jar and the TASTy files in another jar.
Given that the classpaths order in guaranteed to be deterministic we might
end up with the classfile being loaded first and the TASTy file second.
The aggregator must be capable of choosing the TASTy file over the classfile
in this case as well. The aggregator will only choose the new TASTy over
the old classfile if the TASTy file has no classfile in its classpath. In
other words, we only use this new behaviour for TASTy only classpaths.
This also implies that we can just add the
scala2-library-tastyas adependency in any project to use it. Note that this jar is not published
yet.
This change required a refactoring of
ClassRepresentationimplementations. Here is the change in structure and naming.