address feedback 2

Author: StellaHuang95

src/common/lockfile.apis.ts

@@ -5,6 +5,19 @@ import * as crypto from 'crypto';
 import * as fsapi from 'fs-extra';
 import * as path from 'path';
 
+export interface AcquireFileLockOptions {
+    readonly timeoutMs: number;
+    readonly retryIntervalMs: number;
+}
+
+export interface AcquiredFileLock {
+    readonly release: () => Promise<void>;
+    /** Keep the lock and make later acquisition attempts fail immediately. */
+    readonly retain: () => Promise<void>;
+}
+
+type LockState = 'held' | 'released' | 'retained';
+
 /** Acquire an atomic lock released only explicitly; interrupted operations remain locked. */
 export async function acquireFileLock(filePath: string, options: AcquireFileLockOptions): Promise<AcquiredFileLock> {
     const lockPath = `${path.resolve(filePath)}.lock`;
@@ -40,13 +53,13 @@ export async function acquireFileLock(filePath: string, options: AcquireFileLock
                     try {
                         await fsapi.writeFile(retainedMarker, '', { flag: 'wx' });
                     } catch (error) {
-                        if (isAlreadyExistsError(error)) {
+                        if (hasErrorCode(error, 'EEXIST')) {
                             return;
                         }
                         try {
                             await fsapi.rename(ownerMarker, retainedMarker);
                         } catch (renameError) {
-                            if (!isAlreadyExistsError(renameError)) {
+                            if (!hasErrorCode(renameError, 'EEXIST')) {
                                 throw createLockError(
                                     'Failed to mark the lock as retained',
                                     'ERETAINFAILED',
@@ -64,7 +77,7 @@ export async function acquireFileLock(filePath: string, options: AcquireFileLock
                     try {
                         await fsapi.unlink(ownerMarker);
                     } catch (error) {
-                        if (isFileNotFoundError(error)) {
+                        if (hasErrorCode(error, 'ENOENT')) {
                             throw createLockError('Lock ownership was compromised', 'ECOMPROMISED', lockPath);
                         }
                         throw error;
@@ -73,7 +86,7 @@ export async function acquireFileLock(filePath: string, options: AcquireFileLock
                 },
             };
         } catch (error) {
-            if (!isAlreadyExistsError(error)) {
+            if (!hasErrorCode(error, 'EEXIST')) {
                 throw error;
             }
             if (await isRetainedLock(lockPath)) {
@@ -87,20 +100,12 @@ export async function acquireFileLock(filePath: string, options: AcquireFileLock
     }
 }
 
-function isAlreadyExistsError(error: unknown): boolean {
-    return hasErrorCode(error, 'EEXIST');
-}
-
-function isFileNotFoundError(error: unknown): boolean {
-    return hasErrorCode(error, 'ENOENT');
-}
-
 async function isRetainedLock(lockPath: string): Promise<boolean> {
     try {
         await fsapi.lstat(path.join(lockPath, 'retained'));
         return true;
     } catch (error) {
-        if (isFileNotFoundError(error)) {
+        if (hasErrorCode(error, 'ENOENT')) {
             return false;
         }
         throw error;
@@ -120,16 +125,3 @@ function createLockError(message: string, code: string, lockPath: string): NodeJ
 async function delay(milliseconds: number): Promise<void> {
     return new Promise((resolve) => setTimeout(resolve, milliseconds));
 }
-
-export interface AcquireFileLockOptions {
-    readonly timeoutMs: number;
-    readonly retryIntervalMs: number;
-}
-
-export interface AcquiredFileLock {
-    readonly release: () => Promise<void>;
-    /** Keep the lock and make later acquisition attempts fail immediately. */
-    readonly retain: () => Promise<void>;
-}
-
-type LockState = 'held' | 'released' | 'retained';

src/managers/builtin/venvUtils.ts

@@ -67,11 +67,6 @@ export interface CreateEnvironmentResult {
     pkgInstallationCancelled?: boolean;
 }
 
-export interface CreateWithProgressOptions {
-    /** Whether to record uv-created environments in workspace state. Defaults to true. */
-    readonly trackUvEnvironment?: boolean;
-}
-
 export async function clearVenvCache(): Promise<void> {
     const keys = [VENV_WORKSPACE_KEY, VENV_GLOBAL_KEY, UV_ENVS_KEY];
     const state = await getWorkspacePersistentState();
@@ -358,7 +353,7 @@ export async function createWithProgress(
     venvRoot: Uri,
     envPath: string,
     packages?: PipPackages,
-    options?: CreateWithProgressOptions,
+    trackUvEnvironment = true,
 ): Promise<CreateEnvironmentResult | undefined> {
     const pythonPath = getVenvPythonPath(envPath);
 
@@ -401,7 +396,7 @@ export async function createWithProgress(
                 const env = api.createPythonEnvironmentItem(await getPythonInfo(resolved), manager);
 
                 if (
-                    options?.trackUvEnvironment !== false &&
+                    trackUvEnvironment &&
                     useUv &&
                     (resolved.kind === NativePythonEnvironmentKind.venvUv ||
                         resolved.kind === NativePythonEnvironmentKind.uvWorkspace)

src/test/managers/builtin/venvUtils.createWithProgress.unit.test.ts

@@ -16,7 +16,7 @@ import { createWithProgress } from '../../../managers/builtin/venvUtils';
 import { NativePythonEnvironmentKind, NativePythonFinder } from '../../../managers/common/nativePythonFinder';
 import * as managerUtils from '../../../managers/common/utils';
 
-suite('createWithProgress uv tracking options', () => {
+suite('createWithProgress uv tracking', () => {
     let addUvEnvironmentStub: sinon.SinonStub;
     let api: PythonEnvironmentApi;
     let baseEnvironment: PythonEnvironment;
@@ -105,7 +105,7 @@ suite('createWithProgress uv tracking options', () => {
             Uri.file(tempRoot),
             envPath,
             undefined,
-            { trackUvEnvironment: false },
+            false, // trackUvEnvironment
         );
 
         assert.ok(result?.environment);
@@ -124,7 +124,7 @@ suite('createWithProgress uv tracking options', () => {
             Uri.file(tempRoot),
             envPath,
             { install: ['requests'], uninstall: [] },
-            { trackUvEnvironment: false },
+            false, // trackUvEnvironment
         );
 
         assert.ok(result?.environment);