Set cwd before executing a project #12618

Akirathan · 2025-03-24T13:16:37Z

Pull Request Description

Set working directory to the parent of PROJECT_ROOT before executing project. This ensures that all the relative paths are resolved as expected.

Important Notes

Implemented via TruffleLanguage.Env.setWorkingDirectory in https://github.com/enso-org/enso/pull/12618/files#diff-00348de86920168e139b84ff0d5c712b1e58065389cc7a0cbd90421a31a2954dR177. At the end, it seems that we won't need to use syscalls as proposed in #12301.

Reverted relevant parts of #10660 and #10928 - see #12618 (comment)

Checklist

Please ensure that the following checklist has been satisfied before submitting the PR:

The documentation has been updated, if necessary.
Screenshots/screencasts have been attached, if there are any visual changes. For interactive or animated visual changes, a screencast is preferred.
All code follows the
Scala,
Java,
TypeScript,
and
Rust
style guides. In case you are using a language not listed above, follow the Rust style guide.
Unit tests have been written where possible.
If meaningful changes were made to logic or tests affecting Enso Cloud integration in the libraries,
or the Snowflake database integration, a run of the Extra Tests has been scheduled.
- If applicable, it is suggested to paste a link to a successful run of the Extra Tests.

This reverts commit 8b74793.

This reverts commit a923150.

And does not use java.nio.Files

Akirathan · 2025-03-26T16:07:17Z

Setting current working directory via System.setProperty("user.dir", path) does not work. Let's try to set the current directory via polyglot context builder - 020c735.

…guage

…ests. Runner_Spec was failing on NI anyway. There is no way of getting path to the currently running binary.

This reverts part of #10660.

This reverts part of #10660

Akirathan · 2025-03-27T10:51:53Z

Relevant parts from #10660 reverted in:

#10928 reverted in c0df438

Akirathan · 2025-03-27T10:53:10Z

Tested:

runEngineDistribution --run /home/pavel/tmp/Proj/src/Main.enso
runEngineDistribution --run test/Base_Tests/src/Data/Vector_Spec.enso

Both works well without any warnings.

JaroslavTulach

trying to avoid usage of JNI and System Java?
I am skeptical we can get this done without it
Please expand the test cases to use java.io.File via host interop

JaroslavTulach · 2025-03-27T12:47:54Z

engine/runtime/src/main/java/org/enso/interpreter/EnsoLanguage.java

+        var parentDirAbs = parentDir.toAbsolutePath().toString();
+        if (!System.getProperty("user.dir").equals(parentDirAbs)) {
+          var truffleFile = env.getPublicTruffleFile(parentDirAbs);
+          env.setCurrentWorkingDirectory(truffleFile);


The javadoc says:

The current working directory is used to resolve non absolute paths in TruffleFile methods.

Is this enough? How does:

polyglot java import java.io.File main = file = File.new "MY_FILE.txt" file.absolute.path

behave?

Unfortunately you are right. The test with java.io.File, added in b6ae534, fails:

[error] Test org.enso.interpreter.test.ContextInitTest.shouldChangeWorkingDir_BeforeExecutingProject_JavaFile failed: java.lang.AssertionError: [error] Expected: is "/tmp/junit14724631386455624624/MY_FILE.txt" [error] but: was "/home/pavel/dev/enso/engine/runtime-integration-tests/MY_FILE.txt", took 4.496 sec [error] at org.enso.interpreter.test.ContextInitTest.lambda$shouldChangeWorkingDir_BeforeExecutingProject_JavaFile$1(ContextInitTest.java:64) [error] at org.enso.test.utils.ProjectUtils.testProjectRun(ProjectUtils.java:108) [error] at org.enso.interpreter.test.ContextInitTest.shouldChangeWorkingDir_BeforeExecutingProject_JavaFile(ContextInitTest.java:59) [error] at jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103) [error] at java.lang.reflect.Method.invoke(Method.java:580) [error] ... [info] Test run org.enso.interpreter.test.ContextInitTest finished: 1 failed, 0 ignored, 2 total, 6.832s [error] Failed: Total 2, Failed 1, Errors 0, Passed 1 [error] Failed tests: [error] org.enso.interpreter.test.ContextInitTest [error] (Test / testOnly) sbt.TestsFailedException: Tests unsuccessful [error] Total time: 113 s (01:53), completed Mar 27, 2025, 6:22:45 PM

I will need to implement this via JNI and System Java after all ...

engine/runtime-integration-tests/src/test/java/org/enso/interpreter/test/ContextInitTest.java

JaroslavTulach · 2025-03-27T13:44:13Z

engine/runtime-integration-tests/src/test/java/org/enso/interpreter/test/ContextInitTest.java

+    var ctxBuilder = ContextUtils.defaultContextBuilder().out(out).err(out);
+    var expectedWorkingDir = projDir.getParent();
+    var expectedPath = expectedWorkingDir.resolve("MY_FILE.txt").toAbsolutePath().toString();
+    ProjectUtils.testProjectRun(


Better to turn this (also) into integration test that launches enso with and without --jvm flag. Such tests are traditionally placed into Example_Tests - like

enso/test/Examples_Tests/src/Python_Examples_Spec.enso

Line 91 in ed63362

res = Process.run enso_bin.path [ "--run", prj.path ]

As noted in the commit message of ae2b06c, I have already tried to test it that way, but the test was failing in NI, as enso_bin could not be found. If you could think about a way how to launch something like enso --new New_Project from within enso running in NI, I will reintroduce that test once again.

There is no way of getting path to the currently running binary

There is a way. I was working with @sdedic to get such functionality into NI few years ago...

how to launch something like enso --new New_Project from within enso running in NI

..one can getClass().getProtectioDomain().getCodeSource().getLocation() to get "some URL" - probably the URL should point to the location of enso executable. Invoking that from a .enso test may need further changes:

either some builtin

or reflective config

This is not sufficient. See #12618 (comment).

JaroslavTulach · 2025-03-28T07:56:48Z

engine/common/src/main/java/org/enso/common/Platform.java

  private Platform() {}

-  private static Platform detectOperatingSystem() {
+  public static Platform detectOperatingSystem() {


We probably want to make this a compilation constant during NI build time, so I suggest:

Suggested change

public static Platform detectOperatingSystem() {

private static Platform detectOperatingSystem() {

and rather make OPERATING_SYSTEM publicly visible. Possibly rename the field. Maybe CURRENT?

JaroslavTulach · 2025-03-28T07:59:16Z

lib/java/desktop-environment/src/main/java/org/enso/desktopenvironment/Directories.java


 /** Provides information about user directories. */
 public sealed interface Directories permits LinuxDirectories, MacOsDirectories, WindowsDirectories {
+  static Directories getForCurrentPlatform() {


Here we may also benefit from CURRENT being a compilation constant during NI build time. Make it a field:

Suggested change

static Directories getForCurrentPlatform() {

public static final Directories CURRENT = switch (...) {}

JaroslavTulach · 2025-03-28T08:00:54Z

lib/java/desktop-environment/src/main/java/org/enso/desktopenvironment/LinuxDirectories.java

+
+  private LinuxDirectories() {}
+
+  static LinuxDirectories getInstance() {


Lazy initialization makes no sense. Either:

private static final LinuxDirectories INSTANCE = new LinuxDirectories()

or just new LinuxDirectories() in the CURRENT = switch.

JaroslavTulach · 2025-03-28T08:03:19Z

lib/java/desktop-environment/src/main/java/org/enso/desktopenvironment/LinuxTrashBin.java

  private static final String XDG_DATA_HOME = "XDG_DATA_HOME";
  private static final String PATH_TRASH = "Trash";
  private static final String PATH_FILES = "files";
  private static final String PATH_INFO = "info";

-  private final LinuxDirectories directories = new LinuxDirectories();
+  public static LinuxTrashBin getInstance() {


At least encapsulate:

Suggested change

public static LinuxTrashBin getInstance() {

static LinuxTrashBin getInstance() {

but probably do way more. I really don't think this lazy initialization pattern is good for anything!

JaroslavTulach · 2025-03-28T08:04:03Z

lib/java/desktop-environment/src/main/java/org/enso/desktopenvironment/MacOsDirectories.java

@@ -4,6 +4,16 @@
 import java.nio.file.Path;

 final class MacOsDirectories implements Directories {
+  private static MacOsDirectories instance;


Just use new MacOSDirectories() in https://github.com/enso-org/enso/pull/12618/files#r2018098336

JaroslavTulach · 2025-03-28T08:04:33Z

lib/java/desktop-environment/src/main/java/org/enso/desktopenvironment/TrashBin.java


 /** Operations with system trash */
 public sealed interface TrashBin permits LinuxTrashBin, WindowsTrashBin, MacTrashBin {
+  static TrashBin getForCurrentPlatform() {


Make public static final field.

Akirathan · 2025-03-31T14:10:08Z

I attempted to implement and test working directory changing in repo https://github.com/Akirathan/chdir-native. Linux part is OK, but Windows tests on GH hosted runners were still failing. Last fail is in https://github.com/Akirathan/chdir-native/actions/runs/14173427574/job/39702423890#step:6:1876. Failing in linker. I think this is an issue with how I set up the Windows environment in those jobs. I will continue the work here.

GitHub
GitHub - Akirathan/chdir-native: Does java.io in NI respect cwd?
Does java.io in NI respect cwd? Contribute to Akirathan/chdir-native development by creating an account on GitHub.

GitHub
attempt to fix linking error on windows during NI build - shell32 · Akirathan/chdir-native@c1e6107
Does java.io in NI respect cwd? Contribute to Akirathan/chdir-native development by creating an account on GitHub.

With NativeImage tests.

Akirathan · 2025-03-31T18:55:46Z

On fe862e1, we can run sbt:change-directory/test which builds NativeImage and runs TestRunner in native image. The native image is lightweight, the build on my machine takes just 18 seconds. If we want to have some unit test for this feature, this is the easiest and proper way to do that - refactor it into a separate project, and build test NativeImage.

The test passes for Linux at the moment, I will add more platforms.

In the future, we can implement our own junit-platform-native inspired by graalvm/native-build-tools/junit-platform-native.

Add failing Runner_Spec test

9b67773

Akirathan added the CI: No changelog needed Do not require a changelog entry for this PR. label Mar 24, 2025

Akirathan self-assigned this Mar 24, 2025

Akirathan added 8 commits March 24, 2025 17:12

Update docs

eacfbf1

Move Platform from desktop-environment to engine-common

a923150

Fix compilation of project-manager

8b74793

Change working dir in Context.Builder

020c735

Revert "Fix compilation of project-manager"

3d79722

This reverts commit 8b74793.

Revert "Move Platform from desktop-environment to engine-common"

191ac45

This reverts commit a923150.

Change working dir if parent of project root exists

a20a0e9

Runner_Spec does not create any files.

1128b5e

And does not use java.nio.Files

enso-bot bot mentioned this pull request Mar 26, 2025

Fix CWD before executing libraries code #12301

Open

7 tasks

Akirathan added 2 commits March 27, 2025 11:27

Move setting working dir functionality from COntextFactory to EnsoLan…

3eb80b0

…guage

Remove Runner_Spec and replace it with test in runtinme-integration-t…

ae2b06c

…ests. Runner_Spec was failing on NI anyway. There is no way of getting path to the currently running binary.

Akirathan marked this pull request as ready for review March 27, 2025 10:29

Akirathan requested review from jdunkerley, radeusgd, GregoryTravis, AdRiley, marthasharkey, 4e6, JaroslavTulach and hubertp as code owners March 27, 2025 10:29

Akirathan added 3 commits March 27, 2025 11:36

Do not explicitly set working dir in build_tools.

e5c35f2

This reverts part of #10660.

Resolve relative paths test in File_Spec is not ignored.

4032f8a

This reverts part of #10660

Revert #10928

c0df438

JaroslavTulach reviewed Mar 27, 2025

View reviewed changes

GregoryTravis approved these changes Mar 27, 2025

View reviewed changes

Akirathan added 4 commits March 27, 2025 18:23

Duplicate test with java.io.File

b6ae534

Move Platform from desktop-environment to engine-common

7b89619

Fix compilation of project-manager

edf884f

Do not attempt to change dir via Language.Env.currentDirectory.

04242a0

This is not sufficient. See #12618 (comment).

JaroslavTulach reviewed Mar 28, 2025

View reviewed changes

Akirathan added 2 commits March 31, 2025 20:05

Create separate change-directory project.

94372e6

With NativeImage tests.

Use TestCollectorFeature instead of manually created reflect-config.json

fe862e1

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Set cwd before executing a project #12618

Set cwd before executing a project #12618

Akirathan commented Mar 24, 2025 •

edited

Loading

Akirathan commented Mar 26, 2025

Akirathan commented Mar 27, 2025

Akirathan commented Mar 27, 2025

JaroslavTulach left a comment

JaroslavTulach Mar 27, 2025

Akirathan Mar 27, 2025

JaroslavTulach Mar 27, 2025

Akirathan Mar 27, 2025

JaroslavTulach Mar 28, 2025

JaroslavTulach Mar 28, 2025

JaroslavTulach Mar 28, 2025

JaroslavTulach Mar 28, 2025

JaroslavTulach Mar 28, 2025

JaroslavTulach Mar 28, 2025

JaroslavTulach Mar 28, 2025

Akirathan commented Mar 31, 2025 •

edited by unfurl-links bot

Loading

Akirathan commented Mar 31, 2025

	public static Platform detectOperatingSystem() {
	private static Platform detectOperatingSystem() {

	static Directories getForCurrentPlatform() {
	public static final Directories CURRENT = switch (...) {}


		private LinuxDirectories() {}

		static LinuxDirectories getInstance() {

	public static LinuxTrashBin getInstance() {
	static LinuxTrashBin getInstance() {

Set cwd before executing a project #12618

Are you sure you want to change the base?

Set cwd before executing a project #12618

Conversation

Akirathan commented Mar 24, 2025 • edited Loading

Pull Request Description

Important Notes

Checklist

Akirathan commented Mar 26, 2025

Akirathan commented Mar 27, 2025

Akirathan commented Mar 27, 2025

JaroslavTulach left a 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

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

Choose a reason for hiding this comment

Akirathan commented Mar 31, 2025 • edited by unfurl-links bot Loading

Akirathan commented Mar 31, 2025

Akirathan commented Mar 24, 2025 •

edited

Loading

Akirathan commented Mar 31, 2025 •

edited by unfurl-links bot

Loading