microsoft / genaiscript

Generative AI Scripting
https://microsoft.github.io/genaiscript/
MIT License
81 stars 22 forks source link

container copy #557

Closed pelikhan closed 2 weeks ago

pelikhan commented 2 weeks ago

generated by pr-describe

github-actions[bot] commented 2 weeks ago

The changes in the GIT_DIFF seem to be introducing a new copy function into the ContainerHost interface and its implementations (DockerManager and VSCodeHost). This function is designed to copy files from the host to the container.

In DockerManager, the copy function uses host.findFiles(from) to get the files to copy, host.path.resolve(file) to get the absolute path of each file, and copyFile(source, target) to actually copy the files.

In VSCodeHost, the copy function uses this.findFiles(from) to get the files to copy, vscode.workspace.fs.copy(source, target, { overwrite: true, }) to actually copy the files.

However, there are some potential issues:

  1. Both copy implementations are not handling errors. If a file cannot be copied for any reason (e.g. it does not exist, it is locked by another process, there are insufficient permissions, etc.), the error will not be caught and may cause unexpected behavior in the application.

  2. The copy function in VSCodeHost does not ensure that the target directory exists before attempting to copy the file. If the directory does not exist, the copy operation may fail.

Given the above, here are some suggested changes:

@@ -248,6 +249,19 @@ export class DockerManager {
                 const files = await host.findFiles(from)
                 for (const file of files) {
                   try {
+                    const source = host.path.resolve(file)
+                    const target = host.path.resolve(hostPath, file)
+                    logVerbose(`copy ${source} -> ${target}`)
+                    await ensureDir(host.path.dirname(target))
+                    await copyFile(source, target)
+                  } catch (error) {
+                    logError(`Failed to copy file: ${error}`)
+                  }
+                }
+            }

@@ -87,6 +87,20 @@ export class VSCodeHost extends EventTarget implements Host {
                 const files = await this.findFiles(from)
                 for (const file of files) {
                   try {
+                    const source = Utils.joinPath(prj, file)
+                    const target = Utils.joinPath(prj, hostPath, file)
+                    if (!(await vscode.workspace.fs.stat(target).exists)) {
+                      await vscode.workspace.fs.createDirectory(target)
+                    }
+                    vscode.workspace.fs.copy(source, target, { overwrite: true, })
+                  } catch (error) {
+                    vscode.window.showErrorMessage(`Failed to copy file: ${error}`)
+                  }
+                }
+            }

With the above changes, we will catch and handle any errors that occur during the copy operations, and ensure that the target directory exists before attempting to copy a file.

generated by pr-review

pelikhan commented 2 weeks ago

/genai-describe