Add VirtualFile / VirtualFileRef #712

eed3si9n · 2019-10-30T03:35:39Z

This uses rootPaths to encode VirtualFileRef id. As a relatively simple method of abstracting out absolute paths, VirtualFileUtil.mapSources takes a list of root paths, and substitutes them with positional index like ${0}.

stephenjudkins · 2019-12-10T21:57:11Z

Is there any way I could help here? I could definitely assist with any rote conversions from java.io.File to xsbti.VirtualFile

eed3si9n · 2019-12-11T01:16:48Z

@stephenjudkins I guess you could help test it or port it to Bazel once it's ready.

stephenjudkins · 2019-12-11T17:22:28Z

Any expectations on when this might land in a release?

eed3si9n · 2019-12-11T17:39:28Z

I'm still working on other parts, so maybe in a month or so?

stephenjudkins · 2019-12-11T22:41:59Z

Great, thanks for the estimate. Really appreciate your work on this.

eatkins · 2020-01-09T21:03:18Z

internal/compiler-interface/src/main/java/xsbti/BasicVirtualFileRef.java

@@ -11,6 +11,9 @@

 package xsbti;

+import java.util.Arrays;
+import java.util.ArrayList;
+
 public class BasicVirtualFileRef implements VirtualFileRef {
    private String parent;


Could parent and name be marked final and possibly annotated @NotNull? Below it isn't obvious to me in the names method that a null check isn't necessary on parent.

Sounds like a good idea.

Is there a NotNull that's standard in JDK 8 and 11?

Oh, good point. There might not be.

eatkins · 2020-01-09T21:03:21Z

internal/compiler-interface/src/main/java/xsbti/BasicVirtualFileRef.java

+        if (parent == "") {
+            return new String[] { name };
+        }
+        ArrayList<String> xs = new ArrayList<String>(Arrays.asList(parent.split("/")));


Not a big deal at all, but I kinda prefer an imperative approach here just to minimize the extra copying (it also isn't obvious from the code alone that you need to wrap the result of Arrays.asList because otherwise xs.add will throw an unsupported operation exception). Maybe that's a premature optimization though:

String[] parts = parent.split("/"); String[] results = new String[parts.length + 1]; int i = 0; for (i = 0; i < parts.length; i++) results[i] = parts[i]; results[i] = name; return results;

eed3si9n · 2020-01-17T23:22:25Z

About 90% of the scripted tests are passing.

At least one of the failure is related to the virtual file position vs concrete position.

[error] (run-main-0) sbt.internal.inc.ScriptedRunnerImpl$ScriptedFailure$1: 11 (out of 143) scripted tests failed:
[error] 
[error] 	source-dependencies / check-products
[error] 	source-dependencies / compactify
[error] 	source-dependencies / default-arguments-separate-compilation
[error] 	source-dependencies / empty-package
[error] 	source-dependencies / java-basic
[error] 	source-dependencies / java-inner
[error] 	source-dependencies / recorded-products
[error] 	source-dependencies / remove-test-b
[error] 	source-dependencies / restore-classes
[error] 	source-dependencies / typeref-only
[error] 	reporter / warnings-reported

typesafe-tools · 2020-02-09T21:34:23Z

A validation involving this pull request is in progress...

typesafe-tools · 2020-02-09T22:23:13Z

The validator has checked the following projects, tested using sources.sh.

Project	Reference	Commit
sbt	pull/5417/head	sbt/sbt@`bd98dcc`
zinc	pull/712/head	`e732a05`
io	develop	sbt/io@`fcedb90`
librarymanagement	develop	sbt/librarymanagement@`3132a3e`
util	develop	sbt/util@`098796c`

✅ The result is: SUCCESS
(restart)

eed3si9n · 2020-02-13T23:07:34Z

For the interest of review and/or porting, squashed all the commits into one.

build.sbt

Currently this repo lacks validation beyond compilation. This improves the situation slightly by adding a "dummy-bridge" implementation, which is an actual bridge proposed in sbt/zinc#712. The test currently tests that the bridge is capable of building an in-memory VirtualFile.

eed3si9n · 2020-02-21T22:04:48Z

For the record, I've started to move xsbti data types in this PR including VirtualFile to https://github.com/scala/compiler-interface/, which likely will be under some other package like scala.tools.sci.

retronym · 2020-02-25T07:18:36Z

internal/compiler-interface/src/main/java/xsbti/VirtualFileRef.java

+package xsbti;
+
+/*
+ * Represents a reference to a file-like object.


How does one dereference to get to the underlying content or the underlying physical file? Why is a reference required? Some of these design notes could live here.

How are VirtualFileRef-s expected to behave for equality -- is it just based on .id?

Added docs in 5137b10

retronym · 2020-02-25T07:20:13Z

internal/compiler-interface/src/main/java/xsbti/VirtualFileRef.java

+   * Returns unique identifier for the file.
+   * For Java compilation
+   * it needs to contain the path structure matching
+   * the package name.


"... for example com/acme/Foo.java." (?)

retronym

I'm happy with this direction.

Ref sbt/zinc#712

smarter · 2020-05-02T16:02:30Z

internal/compiler-interface/src/main/java/xsbti/VirtualFileRef.java

+ * Given the sequence of Scala and Java source files,
+ * <code>MappedFileConverter</code> will create a sequence of
+ * <code>MappedVirtualFile</code> whose <code>id</code> would look like
+ * <code>${0}/src/main/example/A.scala</code>.


I don't undestand what this ${0} thing is. Why isn't the id src/main/example/A.scala ?

MappedFileConverter can calculate the id based on multiple root paths, including for example Coursier cache directory.

if toPath on a MappedVirtualFile does not resolve the id what should you substitute?

smarter · 2020-05-02T16:06:31Z

internal/compiler-interface/src/main/java/xsbti/VirtualFileRef.java

+   * Java files must end with ".java",
+   * and Scala files must end with ".scala".
+   */
+  public String[] names();


This has the exact same documentation as name, but I guess it does something different? How can a file have multiple names?

Names return the path elements like id.split("/").

smarter · 2020-05-03T11:38:12Z

internal/zinc-core/src/main/scala/sbt/internal/inc/MappedVirtualFile.scala

+      if (u.startsWith("file:///var/folders/"))
+        Seq(
+          idx -> u,
+          idx -> u.replaceAllLiterally("file:///var/folders/", "file:///private/var/folders/")
+        )


What is this special case for? It's missing documentation

On macOS /tmp/ and /var/ seems to be symlinks to /private/tmp/ and /private/var/ respectively.

lrwxr-xr-x@ 1 root wheel 20xx tmp@ -> private/tmp lrwxr-xr-x@ 1 root wheel 20xx var@ -> private/var

When temporary directories are created under /var/folders/ Zinc and compilers sometimes end up reporting different paths for *.scala, *.java, *.class etc. Maybe there's a better solution than this, but the above is a workaround for that.

I suggest using getCanonicalPath.

(or Path#toRealPath which seems to be the equivalent for Path, though this one will throw an exception if the file doesn't actually exist on disk)

Yea. I have to figure out the right point to dealias symlinks so that a build can be created under /tmp/ (or compiler bridge can be compiled in temp etc) and things continue to work.

mernst-github · 2022-10-24T07:44:12Z

internal/zinc-core/src/main/scala/sbt/internal/inc/IncrementalCommon.scala

+          if (classpathEntry.id.endsWith(".jar") &&
+              (converter.toPath(classpathEntry).toString != converter.toPath(file).toString))
+            invalidateBinary(s"${className} is now provided by ${classpathEntry}")
+          else compareStamps(file, classpathEntry)


This line seems to contain a bug: if classpathEntry turns out to be a directory, this comparison unsurprisingly always fails:

Invalidating '.../build/classes/scala/main/.../Foo.class' because .../build/classes/scala/main/....class (farm(77c283a23fed88f2)) != .../build/classes/scala/main (absent) ^^^ RHS is a ***directory***.

The resolve step was completely dropped in this PR.

I've just found out this bug and I am going to fix it.

eed3si9n mentioned this pull request Nov 1, 2019

Make Analysis machine-independent #707

Closed

eed3si9n mentioned this pull request Dec 10, 2019

Allow operation without a filesystem #717

Closed

stephenjudkins mentioned this pull request Dec 13, 2019

Faster incremental compilation options higherkindness/rules_scala#256

Merged

eatkins reviewed Jan 9, 2020

View reviewed changes

eed3si9n force-pushed the wip/machine_vf branch 3 times, most recently from d5ca70b to 0e941b5 Compare January 16, 2020 23:39

eed3si9n marked this pull request as ready for review January 16, 2020 23:40

eed3si9n force-pushed the wip/machine_vf branch from 0e941b5 to cb99df7 Compare January 17, 2020 03:32

eed3si9n force-pushed the wip/machine_vf branch 2 times, most recently from 13c2331 to 554fec1 Compare January 23, 2020 18:53

eed3si9n force-pushed the wip/machine_vf branch from 0600b7e to 621d89f Compare February 3, 2020 17:15

This comment has been minimized.

Sign in to view

typesafe-tools mentioned this pull request Feb 4, 2020

integrate with VirtualFile changes sbt/sbt#5417

Merged

This comment has been minimized.

Sign in to view

eed3si9n force-pushed the wip/machine_vf branch from d79dcf1 to db028c3 Compare February 13, 2020 23:01

dwijnand reviewed Feb 14, 2020

View reviewed changes

build.sbt Outdated Show resolved Hide resolved

Bring back some mima filters

b8cab8a

eed3si9n mentioned this pull request Feb 19, 2020

Format and add a test scala/compiler-interface#11

Merged

retronym reviewed Feb 25, 2020

View reviewed changes

retronym approved these changes Feb 25, 2020

View reviewed changes

Add some documentation on VirtualFileRef

d86bc5a

eed3si9n force-pushed the wip/machine_vf branch from 5137b10 to d86bc5a Compare February 26, 2020 15:55

eed3si9n merged commit fc58a39 into sbt:develop Feb 26, 2020

eed3si9n deleted the wip/machine_vf branch February 26, 2020 16:21

eed3si9n added a commit to eed3si9n/sbt that referenced this pull request Apr 19, 2020

integrate with VirtualFile changes

dabd725

Ref sbt/zinc#712

eed3si9n mentioned this pull request Apr 22, 2020

Test on AppVeyor + Fix the regression around < on Windows #759

Merged

eed3si9n added a commit to eed3si9n/sbt that referenced this pull request Apr 24, 2020

integrate with VirtualFile changes

3ce4d22

Ref sbt/zinc#712

smarter reviewed May 2, 2020

View reviewed changes

smarter reviewed May 3, 2020

View reviewed changes

eed3si9n mentioned this pull request May 3, 2020

Dealias symlinks (via getCanonicalPath etc) somewhere #776

Open

This was referenced May 4, 2020

Epic: Integrate changes in mspnf/zinc #777

Closed

Unbreak sbt-dotty #779

Closed

eed3si9n mentioned this pull request May 7, 2020

implement RemoteCache sbt/sbt#5534

Merged

adpi2 mentioned this pull request May 26, 2020

The semanticdb plugin is incompatible with future Zinc 1.4.0 scalameta/scalameta#2053

Closed

eed3si9n mentioned this pull request Oct 21, 2020

Incremental compilation inside docker container broken since SBT 1.1.0 sbt/sbt#4168

Closed

1 task

eed3si9n mentioned this pull request Dec 10, 2020

AnalsysisCallback handler in Zinc is no longer thread-safe sbt/sbt#6198

Closed

ikaakkola mentioned this pull request Jan 20, 2022

Unable to use custom doclet in sbt doc build #1023

Open

mernst-github reviewed Oct 24, 2022

View reviewed changes

eed3si9n mentioned this pull request Oct 24, 2022

This line seems to contain a bug: if classpathEntry turns out to be a directory, this comparison unsurprisingly always fails: #1140

Closed

Add VirtualFile / VirtualFileRef #712

Add VirtualFile / VirtualFileRef #712

Conversation

eed3si9n commented Oct 30, 2019

stephenjudkins commented Dec 10, 2019

eed3si9n commented Dec 11, 2019

stephenjudkins commented Dec 11, 2019

eed3si9n commented Dec 11, 2019

stephenjudkins commented Dec 11, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eed3si9n commented Jan 17, 2020

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

typesafe-tools commented Feb 9, 2020

typesafe-tools commented Feb 9, 2020

eed3si9n commented Feb 13, 2020

eed3si9n commented Feb 21, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

retronym Feb 25, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

retronym left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bishabosha May 26, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

smarter May 3, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mernst-github Oct 24, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

retronym Feb 25, 2020 •

edited

Loading

bishabosha May 26, 2020 •

edited

Loading

smarter May 3, 2020 •

edited

Loading

mernst-github Oct 24, 2022 •

edited

Loading