Merge branch 'main' into scala_update

.github/workflows/release.yml

@@ -1,13 +1,13 @@
 # Copyright (c) 2021 Concurrent Technologies Corporation.
-#                                                                                                               
+#
 # Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance
-# with the License.  You may obtain a copy of the License at                                                    
-#                                                                                                               
-#     http://www.apache.org/licenses/LICENSE-2.0                                                                
-#                                                                                                               
-# Unless required by applicable law or agreed to in writing, software is distributed under the License is       
-# distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or              
-# implied.  See the License for the specific language governing permissions and limitations under the License.  
+# with the License.  You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software is distributed under the License is
+# distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
+# implied.  See the License for the specific language governing permissions and limitations under the License.
 
 ---
 name: Automatic Release
@@ -16,7 +16,7 @@ on:
     tags:
       - 'v*.*.*'
 env:
-  node_version: 16
+  node_version: 18
   export_cmd: |
     GIT_TAG=$(echo ${GITHUB_REF#refs/*/} | cut -d 'v' -f 2)
     PKG_VERSION=$(echo $(node -p "JSON.stringify(require('./package.json').version)") | sed 's|"||g')
@@ -42,7 +42,7 @@ jobs:
           # This check name is defined as the github action job name (in .github/workflows/testing.yaml)
           checkName: "Rat Check 🐀"
           ref: ${{ github.event.pull_request.head.sha || github.sha }}
-      
+
       - name: Check Scala format ✅
         uses: fountainhead/action-wait-for-check@v1.2.0
         id: scala-format
@@ -51,7 +51,7 @@ jobs:
           # This check name is defined as the github action job name (in .github/workflows/testing.yaml)
           checkName: "Scala Code Format"
           ref: ${{ github.event.pull_request.head.sha || github.sha }}
-      
+
       - name: Check TypeScript format ✅
         uses: fountainhead/action-wait-for-check@v1.2.0
         id: ts-format
@@ -60,7 +60,7 @@ jobs:
           # This check name is defined as the github action job name (in .github/workflows/testing.yaml)
           checkName: "TypeScript code is properly formatted"
           ref: ${{ github.event.pull_request.head.sha || github.sha }}
-      
+
       - name: Check tests - macOS 12 ✅
         uses: fountainhead/action-wait-for-check@v1.2.0
         id: macos12-tests
@@ -69,7 +69,7 @@ jobs:
           # This check name is defined as the github action job name (in .github/workflows/testing.yaml)
           checkName: "Build middleware macos-12 🔧"
           ref: ${{ github.event.pull_request.head.sha || github.sha }}
-      
+
       - name: Check tests - macOS 14 ✅
         uses: fountainhead/action-wait-for-check@v1.2.0
         id: macos14-tests
@@ -96,7 +96,7 @@ jobs:
           # This check name is defined as the github action job name (in .github/workflows/testing.yaml)
           checkName: "Build middleware windows-2019 🔧"
           ref: ${{ github.event.pull_request.head.sha || github.sha }}
-        
+
       - name: Check native build arm64 - Linux ✅
         uses: fountainhead/action-wait-for-check@v1.2.0
         id: ubuntu-arm64-native
@@ -178,7 +178,7 @@ jobs:
       - name: Export git tag and package.json version 🚢
         run: ${{ env.export_cmd }}
         shell: bash
-      
+
       - name: Download upload_url 🔻
         uses: actions/download-artifact@v4
         with:
@@ -195,7 +195,7 @@ jobs:
           distribution: temurin
           java-version: 8
           cache: sbt
-      
+
       - name: Make _install directory to store lib files
         run: mkdir -p _install
 
@@ -207,7 +207,7 @@ jobs:
           workflow_conclusion: success
           name: ubuntu-20.04-x64-libomega_edit.so
           path: _install/libomega_edit_linux_amd64.so
-      
+
       - name: Download linux arm64 library file 🔻
         uses: dawidd6/action-download-artifact@v3
         with:
@@ -216,7 +216,7 @@ jobs:
           workflow_conclusion: success
           name: ubuntu-20.04-arm64-libomega_edit.so
           path: _install/libomega_edit_linux_aarch64.so
-      
+
       - name: Download macos-12 library file 🔻
         uses: dawidd6/action-download-artifact@v3
         with:
@@ -234,7 +234,7 @@ jobs:
           workflow_conclusion: success
           name: macos-12-x64-libomega_edit.dylib
           path: _install/libomega_edit_macos_x86_64.dylib
-      
+
       - name: Download windows library file 🔻
         uses: dawidd6/action-download-artifact@v3
         with:

.github/workflows/ts-format.yml

@@ -29,12 +29,12 @@ jobs:
         with:
           fetch-depth: 0
           persist-credentials: false
-      
+
       - name: Setup Node 📐
         uses: actions/setup-node@v4
         with:
           registry-url: 'https://registry.npmjs.org'
-          node-version: 16
+          node-version: 18
 
       - name: yarn lint - all workspaces 🧶
         run: |

package.json

@@ -38,7 +38,7 @@
     "typedoc": "^0.25.13",
     "typescript": "^5.4.5",
     "unzip-stream": "^0.3.4",
-    "webpack": "^5.91.0",
+    "webpack": "^5.95.0",
     "webpack-cli": "^5.1.4"
   },
   "prettier": {

packages/client/package.json

@@ -43,12 +43,12 @@
     "ts-node": "^10.9.2"
   },
   "dependencies": {
-    "@grpc/grpc-js": "1.10.9",
+    "@grpc/grpc-js": "1.12.2",
     "@omega-edit/server": "0.9.82",
     "@types/google-protobuf": "3.15.12",
-    "google-protobuf": "3.21.2",
-    "pid-port": "0.2.0",
-    "pino": "8.20.0",
+    "google-protobuf": "3.21.4",
+    "pid-port": "1.0.0",
+    "pino": "9.5.0",
     "wait-port": "1.1.0"
   }
 }

packages/client/src/server.ts

@@ -33,6 +33,11 @@ import {
   ServerControlResponse,
   ServerInfoResponse,
 } from './omega_edit_pb'
+import { execFile } from 'child_process'
+import { promisify } from 'util'
+
+// Convert execFile to a promise-based function
+const execFilePromise = promisify(execFile)
 
 const DEFAULT_PORT = 9000 // default port for the server
 const DEFAULT_HOST = '127.0.0.1' // default host for the server
@@ -271,6 +276,36 @@ export async function stopProcessUsingPID(
   return true
 }
 
+/**
+ * Get the process id using the port
+ * @param port port to check
+ * @returns process id or undefined if the port is not in use
+ */
+async function getPidByPort(port: number): Promise<number | undefined> {
+  try {
+    // Try to get the PID using `lsof`
+    const { stdout } = await execFilePromise('lsof', [
+      '-iTCP:' + port,
+      '-sTCP:LISTEN',
+      '-n',
+      '-P',
+    ])
+    const lines = stdout.trim().split('\n')
+    if (lines.length > 1) {
+      const [_, pid] = lines[1].trim().split(/\s+/)
+      return parseInt(pid, 10)
+    }
+    return undefined
+  } catch (error) {
+    // Fallback to `portToPid` if `lsof` fails
+    try {
+      return await portToPid(port)
+    } catch (portToPidError) {
+      return undefined
+    }
+  }
+}
+
 /**
  * Stop the service running on a port
  * @param port port
@@ -288,21 +323,42 @@ export async function stopServiceOnPort(
     signal,
   }
   log.debug(logMetadata)
+
   try {
-    const pid = await portToPid(port)
-    return pid ? stopProcessUsingPID(pid as number, signal) : true
+    // Attempt to get the PID for the given port
+    const pid = await getPidByPort(port)
+
+    if (pid) {
+      log.debug({ ...logMetadata, msg: `Found PID ${pid} for port ${port}` })
+
+      // Attempt to stop the process using the PID
+      const result = await stopProcessUsingPID(pid, signal)
+      log.debug({
+        ...logMetadata,
+        msg: `stopProcessUsingPID result: ${result}`,
+      })
+      return result
+    } else {
+      log.debug({
+        ...logMetadata,
+        stopped: true,
+        msg: 'No process found using the port',
+      })
+      return true // No process was using the port, so consider it as stopped
+    }
   } catch (err) {
+    // Handle case where `portToPid` cannot find a process for the port
     if (err instanceof Error) {
       if (err.message.startsWith('Could not find a process that uses port')) {
         log.debug({
           ...logMetadata,
           stopped: true,
           msg: err.message,
         })
-        // if the port is not in use, return true
-        return true
+        return true // No process using the port, so we consider it stopped
       }
-      log.debug({
+      // Log other types of errors that occur
+      log.error({
         ...logMetadata,
         stopped: false,
         err: {
@@ -317,8 +373,8 @@ export async function stopServiceOnPort(
         err: { msg: String(err) },
       })
     }
+    return false // Return false for any errors that occur
   }
-  return false
 }
 
 /**
@@ -601,7 +657,10 @@ export function pidIsRunning(pid) {
     process.kill(pid, 0)
     return true
   } catch (e) {
-    return false
+    if ((e as NodeJS.ErrnoException).code === 'ESRCH') {
+      return false
+    }
+    throw e
   }
 }
 

packages/client/tests/specs/common.ts

@@ -171,10 +171,13 @@ export async function subscribeViewport(
           : 1
       )
       const event = viewportEvent.getViewportEventKind()
+      const viewport_id_from_event = viewportEvent.getViewportId()
       if (ViewportEventKind.VIEWPORT_EVT_EDIT == event) {
         log_info(
           'viewport_id: ' +
             viewport_id +
+            ', viewport_id_from_event: ' +
+            viewport_id_from_event +
             ', event: ' +
             event +
             ', serial: ' +
@@ -188,6 +191,15 @@ export async function subscribeViewport(
             '", callbacks: ' +
             viewport_callbacks.get(viewport_id)
         )
+        if (viewport_id_from_event !== viewport_id) {
+          log_error(
+            'viewport ID mismatch: "' +
+              viewport_id +
+              '" not equal to "' +
+              viewport_id_from_event +
+              '"'
+          )
+        }
       } else {
         log_info(
           'viewport: ' +

packages/client/tests/specs/viewport.spec.ts

@@ -176,12 +176,15 @@ describe('Viewports', () => {
     await checkCallbackCount(viewport_callbacks, viewport_1_id, 1)
     await checkCallbackCount(viewport_callbacks, viewport_2_id, 3)
     log_info(viewport_callbacks)
+
     // Unsubscribe all viewporta
-    await unsubscribeViewport(viewport_1_id)
-    await unsubscribeViewport(viewport_2_id)
+    expect(await unsubscribeViewport(viewport_1_id)).to.equal(viewport_1_id)
+    expect(await unsubscribeViewport(viewport_2_id)).to.equal(viewport_2_id)
+
     // Note the viewports are not destroyed, just unsubscribed
     expect(await getViewportCount(session_id)).to.equal(1)
-    await destroyViewport(viewport_1_id)
+
+    expect(await destroyViewport(viewport_1_id)).to.equal(viewport_1_id)
     expect(await getViewportCount(session_id)).to.equal(0)
   }).timeout(8000)
 
@@ -304,6 +307,7 @@ describe('Viewports', () => {
       false
     )
     const viewport_id = viewport_response.getViewportId()
+    expect(viewport_id).to.equal(session_id + ':' + desired_viewport_id)
     expect(await viewportHasChanges(viewport_id)).to.be.false
     expect(viewport_response.getData_asU8()).to.deep.equal(Buffer.from(''))
     expect(viewport_response.getLength()).to.equal(0)
@@ -414,7 +418,9 @@ describe('Viewports', () => {
     )
     expect(await getViewportCount(session_id)).to.equal(1)
     // Unsubscribe the viewport
-    expect(await unsubscribeViewport(viewport_id)).to.equal(desired_viewport_id)
+    expect(await unsubscribeViewport(viewport_id)).to.equal(
+      session_id + ':' + desired_viewport_id
+    )
     // Note the viewport is not destroyed, just unsubscribed
     expect(await getViewportCount(session_id)).to.equal(1)
     await destroyViewport(viewport_id)

packages/server/src/index.ts

@@ -90,6 +90,7 @@ async function executeServer(args: string[]): Promise<ChildProcess> {
     cwd: path.dirname(serverScript),
     stdio: 'ignore',
     detached: true,
+    shell: os.platform().startsWith('win'), // use shell on Windows because it can't execute scripts directly
   })
 
   serverProcess.on('error', (err: Error) => {

server/scala/api/src/main/scala/com/ctc/omega_edit/ViewportImpl.scala

@@ -67,6 +67,7 @@ private[omega_edit] class ViewportImpl(p: Pointer, i: FFI) extends Viewport {
 
   def modify(offset: Long, capacity: Long, isFloating: Boolean): Boolean =
     i.omega_viewport_modify(p, offset, capacity, if (isFloating) 1 else 0) == 0
+
   override def toString: String =
     data.mkString // TODO: probably render instead as hex
 

server/scala/serv/src/main/scala/com/ctc/omega_edit/grpc/EditorService.scala

@@ -444,7 +444,7 @@ class EditorService(implicit val system: ActorSystem) extends Editor {
     ObjectId(in.id) match {
       case Viewport.Id(sid, vid) =>
         (editors ? ViewportOp(sid, vid, Viewport.Unwatch)).mapTo[Result].map {
-          case Ok(id) => ObjectId(id)
+          case Ok(_) => in
           case Err(c) => throw grpcFailure(c)
         }
       case _ =>

server/scala/serv/src/main/scala/com/ctc/omega_edit/grpc/Session.scala

@@ -194,7 +194,7 @@ class Session(
       val vid = id.getOrElse(Viewport.Id.uuid())
       val fqid = s"$sessionId:$vid"
 
-      context.child(fqid) match {
+      context.child(vid) match {
         case Some(_) =>
           sender() ! Err(Status.ALREADY_EXISTS)
         case None =>
@@ -204,8 +204,8 @@ class Session(
           val cb = ViewportCallback { (v, e, c) =>
             input.queue.offer(
               ViewportEvent(
-                sessionId = fqid,
-                viewportId = vid,
+                sessionId = sessionId,
+                viewportId = fqid,
                 serial = c.map(_.id),
                 data = Option(ByteString.copyFrom(v.data)),
                 length = Some(v.data.size.toLong),