Merge remote-tracking branch 'origin/master' into api-exec-await

Author: haraldschilly

src/compute/compute/lib/listings.ts

@@ -52,8 +52,8 @@ export async function initListings({
     project_id,
     compute_server_id,
     getListing,
-    createWatcher: (path: string, debounceMs: number) =>
-      new Watcher(path, debounceMs),
+    createWatcher: (path: string, debounce: number) =>
+      new Watcher(path, { debounce }),
     onDeletePath: (path) => {
       logger.debug("onDeletePath -- TODO:", { path });
     },

src/packages/backend/path-watcher.ts

@@ -4,6 +4,9 @@
  */
 
 /*
+Watch A DIRECTORY for changes.  Use ./watcher.ts for a single file.
+
+
 Slightly generalized fs.watch that works even when the directory doesn't exist,
 but also doesn't provide any information about what changed.
 
@@ -76,11 +79,18 @@ export class Watcher extends EventEmitter {
   private exists: boolean;
   private watchContents?: FSWatcher;
   private watchExistence?: FSWatcher;
+  private interval_ms: number;
   private debounce_ms: number;
   private debouncedChange: any;
   private log: Function;
 
-  constructor(path: string, debounce_ms: number) {
+  constructor(
+    path: string,
+    {
+      debounce: debounce_ms = 0,
+      interval: interval_ms,
+    }: { debounce?: number; interval?: number } = {},
+  ) {
     super();
     this.log = logger.extend(path).debug;
     this.log(`initializing: poll=${POLLING}`);
@@ -89,10 +99,13 @@ export class Watcher extends EventEmitter {
     }
     this.path = path.startsWith("/") ? path : join(process.env.HOME, path);
     this.debounce_ms = debounce_ms;
-    this.debouncedChange = debounce(this.change.bind(this), this.debounce_ms, {
-      leading: true,
-      trailing: true,
-    }).bind(this);
+    this.interval_ms = interval_ms ?? DEFAULT_POLL_MS;
+    this.debouncedChange = this.debounce_ms
+      ? debounce(this.change, this.debounce_ms, {
+          leading: true,
+          trailing: true,
+        }).bind(this)
+      : this.change;
     this.init();
   }
 
@@ -109,8 +122,16 @@ export class Watcher extends EventEmitter {
     }
   }
 
+  private chokidarOptions = () => {
+    return {
+      ...ChokidarOpts,
+      interval: this.interval_ms,
+      binaryInterval: this.interval_ms,
+    };
+  };
+
   private initWatchContents(): void {
-    this.watchContents = watch(this.path, ChokidarOpts);
+    this.watchContents = watch(this.path, this.chokidarOptions());
     this.watchContents.on("all", this.debouncedChange);
     this.watchContents.on("error", (err) => {
       this.log(`error watching listings -- ${err}`);
@@ -119,7 +140,7 @@ export class Watcher extends EventEmitter {
 
   private async initWatchExistence(): Promise<void> {
     const containing_path = path_split(this.path).head;
-    this.watchExistence = watch(containing_path, ChokidarOpts);
+    this.watchExistence = watch(containing_path, this.chokidarOptions());
     this.watchExistence.on("all", this.watchExistenceChange(containing_path));
     this.watchExistence.on("error", (err) => {
       this.log(`error watching for existence of ${this.path} -- ${err}`);
@@ -147,9 +168,9 @@ export class Watcher extends EventEmitter {
     }
   };
 
-  private change(): void {
+  private change = (): void => {
     this.emit("change");
-  }
+  };
 
   public close(): void {
     this.watchExistence?.close();

src/packages/backend/watcher.ts

@@ -4,78 +4,78 @@
  */
 
 /*
-Watch a file for changes
+Watch one SINGLE FILE for changes.   Use ./path-watcher.ts for a directory.
 
-Watch for changes to the given file.  Returns obj, which
+Watch for changes to the given file, which means the mtime changes or the
+mode changes (e.g., readonly versus readwrite).  Returns obj, which
 is an event emitter with events:
 
-   - 'change', ctime - when file changes or is created
+   - 'change', ctime, stats - when file changes or is created
    - 'delete' - when file is deleted
 
 and a method .close().
 
 Only fires after the file definitely has not had its
-ctime changed for at least debounce ms (this is the atomic
-option to chokidar).  Does NOT fire when the file first
-has ctime changed.
+ctime changed for at least debounce ms.  Does NOT
+fire when the file first has ctime changed.
+
+NOTE: for directories we use chokidar in path-watcher.  However,
+for a single file using polling, chokidar is horribly buggy and
+lacking in functionality (e.g., https://github.com/paulmillr/chokidar/issues/1132),
+and declared all bugs fixed, so we steer clear.  It had a lot of issues
+with just noticing actual file changes.
+
+We *always* use polling to fully support networked filesystems.
 */
 
 import { EventEmitter } from "node:events";
-import { watch, FSWatcher } from "chokidar";
+import { unwatchFile, watchFile } from "node:fs";
 import { getLogger } from "./logger";
 import { debounce as lodashDebounce } from "lodash";
 
-const L = getLogger("watcher");
+const logger = getLogger("backend:watcher");
 
 export class Watcher extends EventEmitter {
   private path: string;
-  private interval: number;
-  private watcher: FSWatcher;
 
-  constructor(path: string, interval: number = 300, debounce: number = 0) {
+  constructor(
+    path: string,
+    { debounce, interval = 300 }: { debounce?: number; interval?: number } = {},
+  ) {
     super();
     this.path = path;
-    this.interval = interval;
-
-    L.debug(`${path}: interval=${interval}, debounce=${debounce}`);
-    this.watcher = watch(this.path, {
-      interval: this.interval,
-      // polling is critical for network mounted file systems,
-      // and given architecture of cocalc there is no easy way around this.
-      // E.g., on compute servers, everything breaks involving sync or cloudfs,
-      // and in shared project s3/gcsfuse/sshfs would all break. So we
-      // use polling.
-      usePolling: true,
-      persistent: false,
-      alwaysStat: true,
-      atomic: true,
-    });
-    this.watcher.on("unlink", () => {
-      this.emit("delete");
-    });
-    this.watcher.on("unlinkDir", () => {
-      this.emit("delete");
-    });
-
-    const emitChange = lodashDebounce(
-      (ctime) => this.emit("change", ctime),
-      debounce,
-    );
-    this.watcher.on("error", (err) => {
-      L.debug("WATCHER error -- ", err);
-    });
-
-    this.watcher.on("change", (_, stats) => {
-      if (stats == null) {
-        L.debug("WATCHER change with no stats (shouldn't happen)", { path });
-        return;
-      }
-      emitChange(stats.ctime);
-    });
+
+    logger.debug("watchFile", { path, debounce, interval });
+    watchFile(this.path, { persistent: false, interval }, this.handleChange);
+
+    if (debounce) {
+      this.emitChange = lodashDebounce(this.emitChange, debounce);
+    }
   }
 
-  close = async () => {
+  private emitChange = (stats) => {
+    this.emit("change", stats.ctime, stats);
+  };
+
+  private handleChange = (curr, prev) => {
+    const path = this.path;
+    if (!curr.dev) {
+      logger.debug("handleChange: delete", { path });
+      this.emit("delete");
+      return;
+    }
+    if (curr.mtimeMs == prev.mtimeMs && curr.mode == prev.mode) {
+      logger.debug("handleChange: access but no change", { path });
+      // just *accessing* triggers watchFile (really StatWatcher), of course.
+      return;
+    }
+    logger.debug("handleChange: change", { path });
+    this.emitChange(curr);
+  };
+
+  close = () => {
+    logger.debug("close", this.path);
     this.removeAllListeners();
-    await this.watcher.close();
+    unwatchFile(this.path, this.handleChange);
   };
 }

src/packages/frontend/chat/chatroom.tsx

@@ -197,7 +197,7 @@ export const ChatRoom: React.FC<Props> = ({ project_id, path }) => {
           }
           placement="left"
         >
-          <Icon name="arrow-down" /> <VisibleMDLG>New</VisibleMDLG>
+          <Icon name="arrow-down" /> <VisibleMDLG>Newest</VisibleMDLG>
         </Tip>
       </Button>
     );

src/packages/frontend/chat/input.tsx

@@ -92,7 +92,8 @@ export default function ChatInput({
   const lastSavedRef = useRef<string | null>(null);
   const saveChat = useDebouncedCallback(
     (input) => {
-      if (!isMountedRef.current || syncdb == null) return;
+      if (!isMountedRef.current || syncdb == null || !saveOnUnmountRef.current)
+        return;
       onChange(input);
       lastSavedRef.current = input;
       // also save to syncdb, so we have undo, etc.
@@ -129,8 +130,12 @@ export default function ChatInput({
     },
   );
 
+  const saveOnUnmountRef = useRef<boolean>(true);
   useEffect(() => {
     return () => {
+      if (!saveOnUnmountRef.current) {
+        return;
+      }
       // save before unmounting.  This is very important since if a new reply comes in,
       // then the input component gets unmounted, then remounted BELOW the reply.
       // Note: it is still slightly annoying, due to loss of focus... however, data
@@ -148,7 +153,6 @@ export default function ChatInput({
       ) {
         return;
       }
-
       syncdb.set({
         event: "draft",
         sender_id,
@@ -206,6 +210,9 @@ export default function ChatInput({
         saveChat(input);
       }}
       onShiftEnter={(input) => {
+        // no need to save on unmount, since we are saving
+        // the correct state below.
+        saveOnUnmountRef.current = false;
         setInput(input);
         saveChat(input);
         saveChat.cancel();

src/packages/frontend/chat/message.tsx

@@ -610,22 +610,19 @@ export default function Message(props: Readonly<Props>) {
             edited_message_ref.current = value;
           }}
         />
-        <div style={{ marginTop: "10px" }}>
+        <div style={{ marginTop: "10px", display: "flex" }}>
           <Button
-            type="primary"
             style={{ marginRight: "5px" }}
-            onClick={saveEditedMessage}
-          >
-            <Icon name="save" /> Save Edited Message
-          </Button>
-          <Button
             onClick={() => {
               props.actions?.set_editing(message, false);
               props.actions?.delete_draft(date);
             }}
           >
             Cancel
           </Button>
+          <Button type="primary" onClick={saveEditedMessage}>
+            <Icon name="save" /> Save Edited Message
+          </Button>
         </div>
       </div>
     );

src/packages/frontend/editors/task-editor/history-viewer.tsx

@@ -10,29 +10,18 @@ History viewer for Tasks notebooks  --- very similar to same file in jupyter/ di
 import { Checkbox } from "antd";
 import { fromJS, Map } from "immutable";
 import TaskList from "./list";
-import { CSS, React, useState } from "../../app-framework";
+import { useState } from "../../app-framework";
 import { cmp } from "@cocalc/util/misc";
-import { SyncDB } from "@cocalc/sync/editor/db";
 import { Tasks } from "./types";
 
-const SHOW_DONE_STYLE: CSS = {
+const SHOW_DONE_STYLE = {
   fontSize: "12pt",
   color: "#666",
   padding: "5px 15px",
   borderBottom: "1px solid lightgrey",
 } as const;
 
-interface Props {
-  syncdb: SyncDB;
-  version: Date;
-  font_size: number;
-}
-
-export const TasksHistoryViewer: React.FC<Props> = ({
-  syncdb,
-  version,
-  font_size,
-}) => {
+export function TasksHistoryViewer({ doc, project_id, path, font_size }) {
   const [show_done, set_show_done] = useState(false);
 
   function render_task_list(doc) {
@@ -50,8 +39,8 @@ export const TasksHistoryViewer: React.FC<Props> = ({
 
     return (
       <TaskList
-        path={syncdb.get_path()}
-        project_id={syncdb.get_project_id()}
+        path={path}
+        project_id={project_id}
         tasks={tasks}
         visible={visible}
         read_only={true}
@@ -60,7 +49,6 @@ export const TasksHistoryViewer: React.FC<Props> = ({
     );
   }
 
-  const doc = syncdb.version(version);
   return (
     <div
       style={{
@@ -81,4 +69,4 @@ export const TasksHistoryViewer: React.FC<Props> = ({
       {doc == null ? <span>Unknown version</span> : render_task_list(doc)}
     </div>
   );
-};
+}

src/packages/frontend/file-associations.ts

@@ -23,8 +23,8 @@ import videoExtensions from "video-extensions";
 import audioExtensions from "audio-extensions";
 import { filename_extension } from "@cocalc/util/misc";
 
-export function filenameMode(path: string): string {
-  return file_associations[filename_extension(path)]?.opts?.mode ?? "text";
+export function filenameMode(path: string, fallback = "text"): string {
+  return file_associations[filename_extension(path)]?.opts?.mode ?? fallback;
 }
 
 const codemirror_associations: { [ext: string]: string } = {