Skip to content

Conversation

@rossbacher
Copy link
Contributor

This can otherwise lead to issues if one path is relative and the other is not.

@holgerbrandl
Copy link
Collaborator

Could you provide an example where the current approach is not working? Just to allow me to better understand the intent, which I'd need to maintain this feature/fix later on.

@rossbacher
Copy link
Contributor Author

@holgerbrandl yes, totally, I'll try to synthesize one in a bit, I ran into this with one of our projects.

@rossbacher
Copy link
Contributor Author

Could you provide an example where the current approach is not working? Just to allow me to better understand the intent, which I'd need to maintain this feature/fix later on.

Les say you have this simple script:

PathTest.kts:

#!/usr/bin/env kscript

@file:Include("./subdir/Sub.kts")

println(helloWorld)

and then subdir/Sub.kts:

#!/usr/bin/env kscript

val helloWorld = "Hello World"

now if you do kscript --idea PathTest.kts it actually throws an NPE because AppHelpers line 323 Paths.get(scriptFile.absolutePath).parent is null and that is used in 332. It is null because the unresolved path of scriptFile is empty and thus the parent is null.

If you would have run kscript --idea <fully_qualified_path>/PathTest.kts it would have worked.

Another issue is if you just go to a different directory (e.g one down from where the script is and call kscript --idea ../PathTest.kts this also does not work without this PR, as with this the scriptDir and includeDir have a different root (well one has a root and the other has none). Which also fails. Again if you give the full path of the script it works.

So this PR will always use the full absolute path for both script and include dir to avoid any of those issues.

@holgerbrandl holgerbrandl merged commit faa5388 into kscripting:master Nov 9, 2020
@holgerbrandl
Copy link
Collaborator

Thanks @rossbacher, for the nice explanation and the fix.

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.

2 participants