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

fix relative path calculation #1143

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Conversation

neetopia
Copy link
Collaborator

My concern on the startsWith check is true, we have to fallback to try catch for better accuracy.

Open to discussion on better approach other than try catch, hence left one place unchanged, should change to try catch if we deciced to proceed with this approach.

fixes #1079

val gradleRunner = GradleRunner.create().withProjectDir(project.root)

File(project.root, "workload/build.gradle.kts")
.appendText("project.buildDir = File(\"D:/build/\")")
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

note that it does seem like there is only one C drive on github windows runners, therefore this test might not be testable on github runners.

@neetopia neetopia force-pushed the build-dir branch 2 times, most recently from b89e062 to a68d2a7 Compare October 11, 2022 07:43
@@ -154,18 +155,25 @@ class CodeGeneratorImpl(
associate(sources, file)
}

private val File.relativeFile: File
get() =
if (this.startsWith(buildDir))
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I found the reason this fails on Windows.

Windows uses counter slash for directory separator, but in Java API it seems to be intepreting the path as URI, which uses normal slash as directory separator, and this causes lots of issues that will break simple string check.

One workaround is to manually replace counter slash with normal slash when doing check, but counter slash is a valid file name character on unix, uncommon but valid, which messes things up again...

Maybe also put an OS check before checking the strings, but that looks just too bad as a code to me.

@neetopia neetopia force-pushed the build-dir branch 2 times, most recently from 9ff87e5 to 063a0fb Compare October 12, 2022 07:42
neetopia and others added 3 commits October 12, 2022 01:05
The source-to-output map and path converter for lookup tracker needs
to be multiplexed as well.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[ksp] java.lang.IllegalArgumentException: this and base files have different roots
2 participants