Skip to content

Conversation

@arturaz
Copy link
Contributor

@arturaz arturaz commented May 7, 2025

  • Remove extraneous printlns in WatchServiceWatcher
  • Add filter: os.Path => Boolean parameter to watch
  • Correctly handle OVERFLOW events.
  • Bump scala versions to allow compilation on newer JDKs.

Comment on lines 173 to 177
object WatchServiceWatcher {
implicit class WatchEventOps[A](private val e: WatchEvent[A]) extends AnyVal {
def contextSafe: Option[A] = Option(e.context())
}
}
Copy link
Member

Choose a reason for hiding this comment

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

@arturaz let's make this a local private helper method rather than a public extension method, as we use extension methods very rarely in the com-lihaoyi projects.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Does making the extension helper private work? I feel that it's better to have .contextSafe pop-up in autocomplete when you are typing evt.context. Helper function doesn't have that discoverability.

Copy link
Member

Choose a reason for hiding this comment

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

let's make it a normal method

Copy link
Member

Choose a reason for hiding this comment

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

I'm aware of the benefits of extension methods, but just stylistically we basically have ~never used them in these projects, and I don't feel like starting now haha.

@lihaoyi lihaoyi merged commit 5443f89 into com-lihaoyi:main May 7, 2025
8 checks passed
@lihaoyi
Copy link
Member

lihaoyi commented May 7, 2025

@lefou lefou added this to the 0.11.5 milestone May 7, 2025
@arturaz
Copy link
Contributor Author

arturaz commented May 7, 2025

@arturaz kicked off a publish here https://github.com/com-lihaoyi/os-lib/actions/runs/14881872790

Unfortunately it failed: Error: Validation Failed: {"resource":"Release","code":"custom","field":"pre_receive","message":"pre_receive Sorry, branch or tag names starting with 'refs/' are not allowed."}, {"resource":"Release","code":"custom","message":"Published releases must have a valid tag"}

@lihaoyi
Copy link
Member

lihaoyi commented May 7, 2025

This run seems to jhave worked https://github.com/com-lihaoyi/os-lib/actions/runs/14882228075

lihaoyi pushed a commit to com-lihaoyi/mill that referenced this pull request May 19, 2025
Currently watching files uses polling which is inefficient with large
codebases. On my laptop `./mill --watch` uses 20% CPU just to watch the
files for changes.

This uses `oslib`'s `watch` module to watch for the file changes instead
of polling them.

We had to update oslib (com-lihaoyi/os-lib#386)
to:
- add a capability of filtering what folders it watches. This is needed
because watching large generated folders like `out` or `.bloop` emits
Java FS watcher `OVERFLOW` events.
- remove the random `println`'s that `oslib.watch` had.

The current approach to watching is to watch the workspace root via FS
watching. FS watching only works with folders and we have to watch
`root/build.mill`, and thus, `root/`, so everything else falls under it.

Mac OS watcher performs watching recursively natively, but on linux
oslib adds recursive watches itself, so we use `oslib.watch` filter to
prevent watching anything that is not an ancestor of watched root. For
example, if we have source at `root/module-a/src/`, we'll watch `root/`,
`root/module-a/` and `root/`.

Finally, events are filtered so that unrelated changes (like creating
`root/random-file.txt`) does not trigger reevaluation.

The fs watching is enabled by default and can be disabled via
`--watch-via-fs-notify=false`.

`0.12.x` version: #5073

---------

Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
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.

3 participants