Where communities thrive


  • Join over 1.5M+ people
  • Join over 100K+ communities
  • Free without limits
  • Create your own community
People
Repo info
Activity
    Li Haoyi
    @lihaoyi
    entirely unexpected events sounds a bit tough
    utgheith
    @utgheith
    But we're already in violation of the 3rd assertion (using the strict interpretation). DELETE events are being generated for deleted sub-folders but not for moved ones.
    Li Haoyi
    @lihaoyi
    yeah
    utgheith
    @utgheith

    I apologize for all the spam but here is a simple test that shows that the problem is in WatchService. os.watch is likely masking some of those issues in its implementation but not all.

    https://github.com/utgheith/watch3/blob/master/src/main/scala/main.scala

    utgheith
    @utgheith

    It seems that readme.md is using travis-ci badges instead of the github actions ones. Clicking on the build status goes back to a January build (telling me that the switch occurred around this time). Tried to submit a pull request that fixes that but it gets stuck and never runs the tests. This seems to be related to an issue that I don't fully understand but requires special privileges to deal with anyway. Here is a link:

    https://github.community/t/expected-waiting-for-status-to-be-reported/16727

    Lorenzo Gabriele
    @lolgab
    @utgheith The build now has a name: ci that's why the statuses don't match. Can you please rebase the PR on top of current master?
    utgheith
    @utgheith
    done
    Li Haoyi
    @lihaoyi
    @utgheith I would not be surprised by bugs in the underlying WatchService, would be nice if we could paper over it though. Not sure hwo hard that would be
    utgheith
    @utgheith
    How about using intotify directly? Similar to what happens on MacOS. But first I need to convince myself that the issues in WatchService are not cause by inotify :-)
    utgheith
    @utgheith
    com-lihaoyi/os-lib#69 is an attempt to paper over it but it makes me uncomfortable.
    Li Haoyi
    @lihaoyi
    using inotify directly is possible, I never thought of it because I thought WatchService generally worked for the stuff we used it for (as opposed on on OS-X, where WatchService does not work at all
    on a large folder due to polling)
    utgheith
    @utgheith
    I will write some stress tests and share the results but I have a few concerns regarding os.watch and I want to know your thoughts on them.
    (1) os.watch is the sort of thing that requires stress testing. Unit testing is still important obviously. Do you have your own stress tests? If not, is there value in developing some and running them as part of CI?
    (2) The API doesn't make any statements regarding OVERFLOW (silently drop events, force the caller to re-examine the world, ...). inotify uses a finite system-wide queue for notifications and there is no guarantee that it will not overflow.
    Li Haoyi
    @lihaoyi
    idont have any stress tests directly on the library; only for downstream projects.
    I actualyl dont know what to do about OVERFLOW events
    utgheith
    @utgheith
    In the one project where I used os.watch, I resorted to periodically restarting the application in an orderly manner. I could imagine writing a loop that reinitializes the world when it receives an OVERFLOW event.
    utgheith
    @utgheith

    Same test as before but uses inotify (not directly but by spawning a process running inotifywatch). Works reliably on the same test that was failing before. This strongly hints that using inotify directly will be more reliable. Also inotify is explicit about how it handles moves (move_to, move_from, move_self, etc)

    https://github.com/utgheith/watch4/blob/master/src/main/scala/main.scala

    Li Haoyi
    @lihaoyi
    @utgheith I think com-lihaoyi/os-lib#69 sounds reasonable; am I right to say that this change is linux-only and won't affect the fsevents-based OSX version?
    if someone doesn't care about the inner events, they can just ignore them
    I'm happy to have a raw inotify-based implementation too, if you don't mind wrangling some JNA FFI code. That's what we ended up doing for the OS-X backend anyway so it's not unprecedented
    Li Haoyi
    @lihaoyi
    w.r.t. the extra events, the only worry I can think of is if moving a large folder results in a huge flood of useless events
    hard to guess whether that would be a problem in practice or not; for our use case at work, it'll probably just take a bit longer to realize it doesn't need to do anything, but no big deal
    utgheith
    @utgheith
    Started an intofiy based Watcher implementation (com-lihaoyi/os-lib#70). It's still rough around the edges and didn't have a change to stress-test it but it already passes the existing os.watch tests reliably (even on Docker). Would be great if you took look at the overall design.
    @lihaoyi yes, the changes will be Linux only either way because OSX uses its own Watcher
    Li Haoyi
    @lihaoyi
    OSX has similar bugs :p also only under heavy load, so havent had a chance to investigate too deeply
    utgheith
    @utgheith
    @lihaoyi your assertion is correct. I added a simple randomized test that creates trees with up to 100 things in it then delete them. The bad news: it fails on both OSX and Linux. The good news: it passes for the inotify based Watcher. I think this will be sorted out.
    Do you have any objections to adding scalacheck as a dependency? It makes working with such tests easier.
    Li Haoyi
    @lihaoyi
    go for it
    utgheith
    @utgheith
    com-lihaoyi/os-lib#70 is ready for review. The randomized tests fail with fairly high probability for both OSX and Linux when using the WatchServiceWatcher. They are currently disabled for OSX.
    Li Haoyi
    @lihaoyi
    will take a look
    utgheith
    @utgheith
    I noticed that this pattern repeats in the code. I think having both the overloaded method and the default value is redundant and confusing. Is there some subtle reason for doing this? I could see scenarios in which having the overloaded method would be more efficient. Why have both? Mostly curious.
    def apply(src: Path, sort: Boolean = true) = { ... }
    def apply(src: Path) = apply(src, true)
    Li Haoyi
    @lihaoyi
    @utgheith it's to make things like Seq[os.Path](...).map(thing) work, otherwise we'd need to do .map(thing(_)) which is kinda annoying
    utgheith
    @utgheith
    But does that require the default value in the long form?
    Tobias Roeser
    @lefou
    @lihaoyi, I'd like to tag current master as 0.7.5 to make latest changes available downstream. Any objections?
    I would create a new commit of course, to update the Readme and make CI release process happy.
    Li Haoyi
    @lihaoyi
    go for it
    Tobias Roeser
    @lefou
    Done
    discobaba
    @uncleweirdo_twitter
    how do you do the equivalent of "chown -h <whoever> <file>", i.e. don't follow symlink?
    discobaba
    @uncleweirdo_twitter
    def setLinkUserAndGroup(link: os.Path, usr: String, grp: String) : Unit = {
      val lookupService = FileSystems.getDefault.getUserPrincipalLookupService
      val attrView = Files.getFileAttributeView(Paths.get(link.toString()), classOf[PosixFileAttributeView],
        LinkOption.NOFOLLOW_LINKS)
      attrView.setOwner(lookupService.lookupPrincipalByName(usr))
      attrView.setGroup(lookupService.lookupPrincipalByGroupName(grp))
    }
    or something, I guess. I feel your pain.
    Tobias Roeser
    @lefou
    Hi, I introduced a binary incompatible change in os-lib 0.7.5 which I'm going to fix with PR #80. I'm sorry for the inconveniences. If this PR is merged, I'd like to cut a new os-lib release. Any review appreciated.
    Tobias Roeser
    @lefou
    Tagged 0.7.8
    Jack Koenig
    @jackkoenig
    Does .call(check = false) not work?
    $ amm
    Loading...
    Welcome to the Ammonite Repl 2.4.0 (Scala 2.13.6 Java 11.0.11)
    @ os.proc("foo").call(check = false) 
    java.io.IOException: Cannot run program "foo" (in directory ".../oslib"): error=2, No such file or directory
      java.lang.ProcessBuilder.start(ProcessBuilder.java:1128)
      java.lang.ProcessBuilder.start(ProcessBuilder.java:1071)
      os.proc.proc$lzycompute$1(ProcessOps.scala:128)
      os.proc.proc$1(ProcessOps.scala:122)
      os.proc.spawn(ProcessOps.scala:135)
      os.proc.call(ProcessOps.scala:76)
      ammonite.$sess.cmd0$.<clinit>(cmd0.sc:1)
    java.io.IOException: error=2, No such file or directory
    ...
    Jack Koenig
    @jackkoenig
    My expectation being it would not throw an exception--I'd like to check the exit code. Looking at the code it seems like it's not supposed to ever throw an IOException so something is going on here: https://github.com/com-lihaoyi/os-lib/blob/790b81aba491047b73dd481e290abb36ee1a7de8/os/src-jvm/ProcessOps.scala#L84
    Jack Koenig
    @jackkoenig
    Actually looking at that stack trace, yeah it looks like proc.spawn is accidentally throwing the exception here: https://github.com/com-lihaoyi/os-lib/blob/790b81aba491047b73dd481e290abb36ee1a7de8/os/src-jvm/ProcessOps.scala#L128 I'll file an issue
    jodersky
    @jodersky
    @jackkoenig, does "foo" exist on your path? I think that what you're seeing is expected behavior: check = false will not throw if the exit code is non zero, but it does not influence what happens if the executable doesn't exist in the first place
    here is the result of some tests:
    scala> os.proc("false").call() // false returns 1, so this should throw
    os.SubprocessException: CommandResult 1
    
      at os.SubprocessException$.apply(Model.scala:202)
      at os.proc.call(ProcessOps.scala:85)
      ... 28 elided
    
    scala> os.proc("false").call(check = false) // error is suppressed                                                                                                                                                                                                      
    val res4: os.CommandResult = CommandResult 1
    
    scala> os.proc("nonexistant").call(check = false) // we still get an error, because "nonexistant" is does not exist                                                                                                                                                                                                    
    java.io.IOException: Cannot run program "nonexistant" (in directory "/home/jodersky/p/buildsio"): error=2, No such file or directory
      at java.base/java.lang.ProcessBuilder.start(ProcessBuilder.java:1141)
      at java.base/java.lang.ProcessBuilder.start(ProcessBuilder.java:1072)
      at os.proc.proc$lzyINIT1$7(ProcessOps.scala:128)
      at os.proc.proc$1(ProcessOps.scala:133)
      at os.proc.spawn(ProcessOps.scala:135)
      at os.proc.call(ProcessOps.scala:76)
      ... 28 elided
    Caused by: java.io.IOException: error=2, No such file or directory
      at java.base/java.lang.ProcessImpl.forkAndExec(Native Method)
      at java.base/java.lang.ProcessImpl.<init>(ProcessImpl.java:313)
      at java.base/java.lang.ProcessImpl.start(ProcessImpl.java:244)
      at java.base/java.lang.ProcessBuilder.start(ProcessBuilder.java:1108)
      ... 33 more
    Li Haoyi
    @lihaoyi
    it's arguable whether the check flag should take effect when an executable@is not found; if people think it should we can change the logic
    jodersky
    @jodersky
    What should the command result be in such a case?
    if we set a synthetic exit code (such as 127), then we'd follow the semantics of a classic shell
    IMO, it's more intuitive to think of call and spawn as a system which ends up calling exec rather than following the semantics of a shell
    juskem
    @juskem
    Hi, has anyone had errors with closedbyinterruptexception? More details in https://stackoverflow.com/questions/69403981/scala-closedbyinterruptexception-using-os-lib-watch-service