phoenixframework / phoenix_live_reload

Provides live-reload functionality for Phoenix
MIT License
315 stars 90 forks source link

fs update possible? #53

Closed derailed closed 7 years ago

derailed commented 7 years ago

All,

I am trying to use mix_test_watch as a dependency with my phoenix project and running into version conflicts with fs.

I am getting the following error while updating my dependencies:

Failed to use "fs" (version 0.9.2) because mix_test_watch (version 0.4.0) requires ~> 2.12 phoenix_live_reload (version 1.0.8) requires ~> 0.9.1 mix.lock specifies 0.9.2

Would it be possible to revive the phx_reloader to a more recent fs version or are there underlying issues?

Thank you!

josevalim commented 7 years ago

The new fs version was causing a lot of issues to build on rebar on different machines so we have reverted it. Please see the commit log. At this point it would probably be best for someone to build and maintain a file system watcher version for Elixir.

agustif commented 7 years ago

There is sentix which uses fswatch under the hood, not sure if that's worse than fs which is erl/OTP at least

mveytsman commented 7 years ago

@josevalim It looks like https://github.com/falood/exfswatch might fit the bill. It's derived from a newer version of fs but doesn't depend on it any more (the author packaged in the c shims and the exe). It's a mix project so the rebar issues will go away.

Would you be interested in a PR that switches over to exfswatch?

(cc @falood)

josevalim commented 7 years ago

@mveytsman oh, that looks very interesting. The only question though is why didn't the author consider a functional API? There is no need to have use with a compilation time path, we would definitely need a function that we call at runtime instead.

falood commented 7 years ago

I'm not sure whether it's a good time to talk about filesystem monitor for erlang/elixir here. I researched filesystem monitor solution long time ago, and I think for now we only have 3 backends touse: fswatch, watchman and fs.

fswatch

support most kinds of OS should be installed manually talk with elixir via port

watchman

only support linux and macOS, beta status for windows should be installed manually have great API and docs talk with elixir via socket

fs

support windows, linux, macOS and freebsd, (but I can't run it successfully on FreeBSD) it's easy to make it an internal elixir package talk with elixir via port

So, what do you guys think is most important? OS supported, beautiful and consistent API, or easy to use?

For the exfswatch repo, my first plan is to make fswatch NIF or Port Driver for elixir. but I found it's so hard and I meet fs, then I change my mind to make it an elixir wrapper for fs. Finally copy the c files after talk with the author link. That's why exfswatch don't have a functional API.

I'll add functional API for exfswatch and make it syncup with fs if you guys want to use it. I also have an unpublicized stuff about the watchman wrapper, but I only finished JSON Protocol part. for the fswatch wrapper, I think sentix is a very good project.

BTW, here's an erlang ticket about filesystem monitor, link

josevalim commented 7 years ago

@falood thanks for the overview. :heart: The most important, IMO, is the number of OS supported and the out of the box experience.

I don't mind depending on fswatch as long as the project provides a Mix task to download the executable and make it locally available. So for example, if you try to run the project for the first time, it could say: "fswatch is not available, you may either install one in your system or run mix local.sentrix to install a precompiled local copy". That's how Mix handles rebar, for example.

Regarding the API, it can be as simple as possible. For example, a function to start a file watcher process and another to subscribe to the process. For example:

Each backend is simply a GenServer that implements the subscribe API. Here is how the functions above would likely work:

def start_link(opts) do
  case :os.type do
    {:win32, _} -> FSWatcher.WinNotify.start_link(...)
    {:unix, :darwin} -> FSWatcher.X.start_link(...)
    ... ->
    _ ->
      IO.puts "Unsupported OS"
      :ignore
  end
end

def subscribe(fswatcher, pid) when is_pid(pid) do
  GenServer.call(fswatcher, pid)
end

Here is how a simple backend would look like:

defmodule FSWatcher.X do
  use GenServer

  def start_link(opts) do
    GenServer.start_link(__MODULE__, opts, opts)
  end

  def init(opts) do
    {:ok, %{subscribers: %{}, port: start_port(opts)}
  end

  def handle_call(:subscribe, {pid, _}, state) do
    ref = Process.monitor(pid)
    state = put_in(state.subscribers[ref], pid)
    {:reply, :ok, state}
  end

  def handle_info({:DOWN, _pid, _, ref, _reason}, state) do
    {_, state} = pop_in state.subscribers[ref]
    {:noreply, state}
  end
end

Once you receive an event from the port, you can broadcast it by traversing the values in the subscribers map and sending each of them a message.

The approach above has the benefit that multiple paths can be watched in a system.

But to sum it up, we can survive a not so great API, for example, the :fs API is quite limited. However I am afraid the use ExFSWatch approach won't work, because it imposes a process shape which hurts composition with things such as Phoenix channels.

/cc @whitfin

mveytsman commented 7 years ago

@falood if you'd like, I'd be happy to take a crack at adding a functional API to exfswatch similar to what @josevalim described.

Let me know!

whitfin commented 7 years ago

@josevalim I would be more than happy to add mix tasks to Sentix for installing fswatch if missing. Where would you recommend we place it; does it make sense to throw something like that just in priv?

The current API is pretty similar to what you described:

Sentix.start_link(:name, [ paths ], [ fswatch_opts ])
Sentix.subscribe(:name, pid \\ self())

If it needs it, totally happy to adjust the API as needed in a major bump - existing API was thrown together quite quickly for a use case at work.

Edit: @mveytsman totally happy to accept your suggestions/improvements on Sentix too, if you have any!

josevalim commented 7 years ago

@whitfin the mix tasks can be part of lib. The precompiled stuff can be hosted on S3 or even on GitHub. Alternatively you can fetch fswatch release file and attempt to compile it on the user machine. Anyway, the idea is to make everything flowing with as little user input as possible.

I do like the Sentix API, so you should definitely stick with it. :+1:

My personal preference would be something that interacts with Windows/Linux/Mac directly - fswatch is quite large - but I understand that the former also requires more effort.

josevalim commented 7 years ago

I realize I may have sent mixed signals, so let me try to clarify my position.

First, thank you @whitfin and @falood for the great projects! And thanks everyone who joined the discussion so far.

I would love to get rid of the fs dependency from Phoenix. My preferred approach is the one in exfswatch because it provides a better out of the box experience. My only concern is the API. If we can provide a nicer API, something similar to Sentix or to what I sketched above, it would definitely be my first choice.

Sentix looks great but it requires more work from users. Getting started with Phoenix already requires installing node, postgres, etc and we want to minimize that as much as possible. I copied @whitfin since we were discussing his library but, even if we improve the flow with mix tasks, I think it is hard to beat exfswatch if it just works.

Apologies if there was any confusion. :)

falood commented 7 years ago

thank you @josevalim

For a long term plan, do you think it's better to have an elixir-level interface? The responses and events are different from different backends, at least we can define the event list, something like this. Is it a good idea to have a middleware project like plug, and we'll migrate current project as a backend like cowboy?

josevalim commented 7 years ago

For a long term plan, do you think it's better to have an elixir-level interface?

Yes.

The responses and events are different from different backends, at least we can define the event list, something like this.

Each backend implementation should normalize the events into a common format. The good news is that we don't need to support all of them, start small with a handful of events. You can add more when people request it.

Is it a good idea to have a middleware project like plug, and we'll migrate current project as a backend like cowboy?

I don't think such is necessary. The contract between backends is much simpler than the HTTP spec abstracted by Plug. If you really want to, you and @whitfin can define a format together that you would both support and leave it simply as a document.

mveytsman commented 7 years ago

If you're looking to define a format like that, the design doc for Go's fsnotify.

For now, I'm going to work on getting exfswatch to have an api similar to fs so we can use it in phoenix. And I'll move the conversation over to exfswatch so we don't hijack this thread :)

chrismccord commented 7 years ago

We have released 1.1.0 with :file_system for change monitoring