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

Support os.proc on Scala Native #257

Merged
merged 6 commits into from
May 30, 2024
Merged

Support os.proc on Scala Native #257

merged 6 commits into from
May 30, 2024

Conversation

lihaoyi
Copy link
Member

@lihaoyi lihaoyi commented Apr 15, 2024

Currently fails with

lihaoyi os-lib$ ./mill -i "os.native[2.12.17].test" test.os.SubprocessTests.lines
[229/229] os.native[2.12.17].test.test
[info] Starting process '/Users/lihaoyi/Github/os-lib/out/os/native/2.12.17/test/nativeLink.dest/out' on port '58844'.
----------------- Running Tests test.os.SubprocessTests.lines -----------------
X test.os.SubprocessTests.lines 70176ms
  java.lang.IllegalArgumentException
    scala.collection.immutable.VectorPointer.gotoNextBlockStartWritable(Unknown)
    scala.collection.immutable.VectorBuilder.gotoNextBlockStartWritable(Unknown)
    scala.collection.immutable.VectorBuilder.$plus$eq(Unknown)
    scala.collection.immutable.VectorBuilder.$plus$eq(Unknown)
    scala.collection.generic.Growable.$anonfun$$plus$plus$eq$1(Unknown)
    scala.collection.generic.Growable$$Lambda$1.apply(Unknown)
    scala.collection.Iterator.foreach(Unknown)
    scala.collection.AbstractIterator.foreach(Unknown)
    scala.collection.generic.Growable.$plus$plus$eq(Unknown)
    scala.collection.immutable.VectorBuilder.$plus$plus$eq(Unknown)
    scala.collection.immutable.VectorBuilder.$plus$plus$eq(Unknown)
    scala.collection.TraversableOnce.to(Unknown)
    scala.collection.AbstractIterator.to(Unknown)
    scala.collection.TraversableOnce.toIndexedSeq(Unknown)
    scala.collection.AbstractIterator.toIndexedSeq(Unknown)
    os.proc.call(Unknown)
    test.os.SubprocessTests$.$anonfun$tests$2(Unknown)
Tests: 1, Passed: 0, Failed: 1
1 targets failed
os.native[2.12.17].test.test 1 tests failed:
  test.os.SubprocessTests test.os.SubprocessTests.lines

@WojciechMazur
Copy link

Scala Native 0.5.1 is out, it should fix this issue

@lihaoyi
Copy link
Member Author

lihaoyi commented Apr 17, 2024

Thanks @WojciechMazur !

I'm now hitting another issue trying to run ./mill -i "os.native[2.13.10].test" test.os.ListingWalkingTests.list.0 on this PR. Seems like Files.walkFileTree behaves differently between JVM and Native, and on Native it fails to enumerate all the files (e.g. out/scratch/ListingWalkingTests/tests/list/0/Multi Line.txt below) in the directory tree?

Native

visitFile out/scratch/ListingWalkingTests/tests/list/0/misc/file-symlink
visitFile out/scratch/ListingWalkingTests/tests/list/0/misc/broken-abs-symlink
visitFile out/scratch/ListingWalkingTests/tests/list/0/misc/broken-symlink
postVisitDirectory out/scratch/ListingWalkingTests/tests/list/0/misc
postVisitDirectory out/scratch/ListingWalkingTests/tests/list/0
X test.os.ListingWalkingTests.list.0 3ms
  java.nio.file.DirectoryNotEmptyException: out/scratch/ListingWalkingTests/tests/list/0
    java.nio.file.PosixException$.apply(Unknown)
    java.nio.file.Files$.$anonfun$unixDeletePath$1(Unknown)
    java.nio.file.Files$$$Lambda$3.apply(Unknown)
    scala.scalanative.unsafe.Zone$.acquire(Unknown)
    java.nio.file.Files$.unixDeletePath(Unknown)
    java.nio.file.Files$.delete(Unknown)
    java.nio.file.Files.delete(Unknown)
    test.os.TestUtil$$anon$1.postVisitDirectory(Unknown)
    test.os.TestUtil$$anon$1.postVisitDirectory(Unknown)
    java.nio.file.Files$._walkFileTree(Unknown)
    java.nio.file.Files$.walkFileTree(Unknown)
    java.nio.file.Files$.walkFileTree(Unknown)
    java.nio.file.Files.walkFileTree(Unknown)
    test.os.TestUtil$.prep(Unknown)
    test.os.ListingWalkingTests$.$anonfun$tests$3(Unknown)

Jvm

[188/188] os.jvm[2.13.10].test.test
--------------- Running Tests test.os.ListingWalkingTests.list.0 ---------------
visitFile out/scratch/ListingWalkingTests/tests/list/0/misc/file-symlink
visitFile out/scratch/ListingWalkingTests/tests/list/0/misc/broken-abs-symlink
visitFile out/scratch/ListingWalkingTests/tests/list/0/misc/broken-symlink
postVisitDirectory out/scratch/ListingWalkingTests/tests/list/0/misc
visitFile out/scratch/ListingWalkingTests/tests/list/0/Multi Line.txt
postVisitDirectory out/scratch/ListingWalkingTests/tests/list/0
+ test.os.ListingWalkingTests.list.0 1

@lihaoyi
Copy link
Member Author

lihaoyi commented Apr 17, 2024

Minimal repro
scala-native/scala-native#3878

@lefou
Copy link
Member

lefou commented May 7, 2024

The issue should be fixed by:

@WojciechMazur
Copy link

It seems to pass using 0.5.2-SNAPSHOT, but there is a few other failures. I'll try to minimize and fix them later this week

@lihaoyi
Copy link
Member Author

lihaoyi commented May 8, 2024

Thanks @WojciechMazur !

@WojciechMazur
Copy link

Update on the SN issues: The latest 0.5.2-SNAPSHOT is good enough to successfully run the tests thanks to:

There is however 1 ongoing issue related to Process Input/Output - the thread dedicated to processing stdout/stderr of process can fail with exception after process is finished due to bad file descriptor. I'm working on a fix to this problem. Anyway, if there is an urgent need we can publish a 0.5.2 this week with prepared fixes.

@lihaoyi
Copy link
Member Author

lihaoyi commented May 16, 2024

Thanks @WojciechMazur! I think there's no urgent-urgent need at the moment. Looking forward to your fixes to scala-native so I can update the tests in the os-lib repo

@lolgab lolgab changed the title [WIP] try to get subprocess tests passing on scala-native Support os.proc on Scala Native May 29, 2024
@lolgab lolgab marked this pull request as ready for review May 29, 2024 16:38
@lolgab
Copy link
Member

lolgab commented May 29, 2024

Scala Native 0.5.2 was just released which makes all tests pass 🎉

@lolgab lolgab marked this pull request as draft May 29, 2024 17:25
@lolgab lolgab marked this pull request as draft May 29, 2024 17:25
@lolgab lolgab marked this pull request as draft May 29, 2024 17:25
@lolgab lolgab marked this pull request as draft May 29, 2024 17:25
@lolgab lolgab marked this pull request as draft May 29, 2024 17:25
@lolgab lolgab marked this pull request as draft May 29, 2024 17:25
@lihaoyi lihaoyi marked this pull request as ready for review May 30, 2024 10:34
@lihaoyi lihaoyi merged commit c984ddf into main May 30, 2024
18 checks passed
@lihaoyi lihaoyi deleted the native-subprocesses branch May 30, 2024 10:34
@lihaoyi
Copy link
Member Author

lihaoyi commented May 30, 2024

Thanks @lolgab @WojciechMazur ! I guess this can be used without a new os-lib release, just by bumping the Scala-Native version right?

@lefou
Copy link
Member

lefou commented May 30, 2024

Thanks @lolgab @WojciechMazur ! I guess this can be used without a new os-lib release, just by bumping the Scala-Native version right?

I guess a release is needed, since you moved some source files from src-jvm to src.

@lefou lefou added this to the after 0.10.1 milestone May 30, 2024
@lihaoyi
Copy link
Member Author

lihaoyi commented May 30, 2024

@lefou good point, I'll tag one

@lihaoyi
Copy link
Member Author

lihaoyi commented May 30, 2024

I tagged OS-Lib 0.10.2, which should be out soon

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.

None yet

4 participants