Fix file comparison (#2709)

lib/livebook/file_system/file.ex

@@ -77,6 +77,18 @@ defmodule Livebook.FileSystem.File do
     {file.file_system_id, node, file.path}
   end
 
+  @doc """
+  Checks if two files are equal.
+
+  Comparing files with `Kernel.==/2` may result in false-negatives,
+  because the structs hold additional information.
+  """
+  @spec equal?(t(), t()) :: boolean()
+  def equal?(file1, file2) do
+    file1.path == file2.path and file1.file_system_id == file2.file_system_id and
+      file1.file_system_module == file2.file_system_module
+  end
+
   @doc """
   Checks if the given file is within a file system local to its node.
   """

lib/livebook/notebook_manager.ex

@@ -139,7 +139,7 @@ defmodule Livebook.NotebookManager do
 
   @impl true
   def handle_cast({:add_recent_notebook, file, name}, state = prev_state) do
-    recent_notebooks = Enum.reject(state.recent_notebooks, &(&1.file == file))
+    recent_notebooks = Enum.reject(state.recent_notebooks, &FileSystem.File.equal?(&1.file, file))
 
     recent_notebooks = [
       %{file: file, name: name, added_at: DateTime.utc_now()} | recent_notebooks
@@ -158,7 +158,7 @@ defmodule Livebook.NotebookManager do
   end
 
   def handle_cast({:add_starred_notebook, file, name}, state = prev_state) do
-    if Enum.any?(state.starred_notebooks, &(&1.file == file)) do
+    if Enum.any?(state.starred_notebooks, &FileSystem.File.equal?(&1.file, file)) do
       {:noreply, state}
     else
       starred_notebooks = [
@@ -172,14 +172,16 @@ defmodule Livebook.NotebookManager do
   end
 
   def handle_cast({:remove_recent_notebook, file}, state = prev_state) do
-    recent_notebooks = Enum.reject(state.recent_notebooks, &(&1.file == file))
+    recent_notebooks = Enum.reject(state.recent_notebooks, &FileSystem.File.equal?(&1.file, file))
     state = %{state | recent_notebooks: recent_notebooks}
     broadcast_changes(state, prev_state)
     {:noreply, state, {:continue, :dump_state}}
   end
 
   def handle_cast({:remove_starred_notebook, file}, state = prev_state) do
-    starred_notebooks = Enum.reject(state.starred_notebooks, &(&1.file == file))
+    starred_notebooks =
+      Enum.reject(state.starred_notebooks, &FileSystem.File.equal?(&1.file, file))
+
     state = %{state | starred_notebooks: starred_notebooks}
     broadcast_changes(state, prev_state)
     {:noreply, state, {:continue, :dump_state}}

lib/livebook_web/live/file_select_component.ex

@@ -689,11 +689,15 @@ defmodule LivebookWeb.FileSelectComponent do
       unhighlighted: name,
       file: file,
       is_dir: FileSystem.File.dir?(file),
-      is_running: file in running_files,
+      is_running: running?(file, running_files),
       editable: Keyword.get(opts, :editable, true)
     }
   end
 
+  defp running?(file, running_files) do
+    Enum.any?(running_files, &FileSystem.File.equal?(&1, file))
+  end
+
   defp hidden?(filename) do
     String.starts_with?(filename, ".")
   end

lib/livebook_web/live/notebook_cards_component.ex

@@ -1,6 +1,8 @@
 defmodule LivebookWeb.NotebookCardsComponent do
   use LivebookWeb, :live_component
 
+  alias Livebook.FileSystem
+
   @impl true
   def render(assigns) do
     assigns = assign_new(assigns, :card_icon, fn -> nil end)
@@ -72,10 +74,10 @@ defmodule LivebookWeb.NotebookCardsComponent do
   end
 
   defp file_running?(file, sessions) do
-    Enum.any?(sessions, &(&1.file == file))
+    Enum.any?(sessions, &(&1.file && FileSystem.File.equal?(&1.file, file)))
   end
 
   defp session_by_file(file, sessions) do
-    Enum.find(sessions, &(&1.file == file))
+    Enum.find(sessions, &(&1.file && FileSystem.File.equal?(&1.file, file)))
   end
 end

lib/livebook_web/live/open_live.ex

@@ -276,11 +276,11 @@ defmodule LivebookWeb.OpenLive do
   end
 
   defp file_running?(file, sessions) do
-    Enum.any?(sessions, &(&1.file == file))
+    Enum.any?(sessions, &(&1.file && FileSystem.File.equal?(&1.file, file)))
   end
 
   defp session_id_by_file(file, sessions) do
-    session = Enum.find(sessions, &(&1.file == file))
+    session = Enum.find(sessions, &(&1.file && FileSystem.File.equal?(&1.file, file)))
     session.id
   end
 end

lib/livebook_web/live/open_live/file_component.ex

@@ -112,14 +112,14 @@ defmodule LivebookWeb.OpenLive.FileComponent do
   end
 
   defp file_running?(file, sessions) do
-    Enum.any?(sessions, &(&1.file == file))
+    Enum.any?(sessions, &(&1.file && FileSystem.File.equal?(&1.file, file)))
   end
 
   defp files(sessions) do
-    Enum.map(sessions, & &1.file)
+    for session <- sessions, session.file, do: session.file
   end
 
   defp session_by_file(file, sessions) do
-    Enum.find(sessions, &(&1.file == file))
+    Enum.find(sessions, &(&1.file && FileSystem.File.equal?(&1.file, file)))
   end
 end

lib/livebook_web/live/session_live/persistence_component.ex

@@ -6,7 +6,7 @@ defmodule LivebookWeb.SessionLive.PersistenceComponent do
   @impl true
   def mount(socket) do
     sessions = Sessions.list_sessions()
-    running_files = Enum.map(sessions, & &1.file)
+    running_files = for session <- sessions, session.file, do: session.file
     {:ok, assign(socket, running_files: running_files)}
   end
 
@@ -212,7 +212,9 @@ defmodule LivebookWeb.SessionLive.PersistenceComponent do
 
   defp savable?(draft_file, saved_file, running_files) do
     file = normalize_file(draft_file)
-    not FileSystem.File.dir?(draft_file) and (file not in running_files or file == saved_file)
+    running? = Enum.any?(running_files, &FileSystem.File.equal?(&1, file))
+    changed? = saved_file == nil or not FileSystem.File.equal?(file, saved_file)
+    not FileSystem.File.dir?(draft_file) and (running? or changed?)
   end
 
   defp map_diff(left, right) do

lib/livebook_web/live/session_live/render.ex

@@ -1377,7 +1377,7 @@ defmodule LivebookWeb.SessionLive.Render do
 
   defp star_button(assigns) do
     ~H"""
-    <%= if @file in @starred_files do %>
+    <%= if starred?(@file, @starred_files) do %>
       <span class="tooltip left" data-tooltip="Unstar notebook">
         <.icon_button phx-click="unstar_notebook">
           <.remix_icon icon="star-fill" class="text-yellow-600" />
@@ -1392,4 +1392,8 @@ defmodule LivebookWeb.SessionLive.Render do
     <% end %>
     """
   end
+
+  defp starred?(file, starred_files) do
+    Enum.any?(starred_files, &Livebook.FileSystem.File.equal?(&1, file))
+  end
 end

test/livebook/file_system/file_test.exs

@@ -33,6 +33,17 @@ defmodule Livebook.FileSystem.FileTest do
     end
   end
 
+  describe "equal?/2" do
+    test "returns true for matching files built in different processes" do
+      file_system = FileSystem.Local.new()
+
+      file1 = Livebook.FileSystem.File.new(file_system, "/tmp/")
+      file2 = Task.await(Task.async(fn -> Livebook.FileSystem.File.local("/tmp/") end))
+
+      assert FileSystem.File.equal?(file1, file2)
+    end
+  end
+
   describe "local/1" do
     test "uses the globally configured local file system instance" do
       assert FileSystem.File.local(p("/path")).file_system_id ==