From bb99b01d8c2683d93a62a1c4dccd6e0f6f23f2d7 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Fri, 17 Nov 2023 08:03:33 +0000 Subject: [PATCH 01/16] s --- .github/workflows/build.yml | 2 +- .github/workflows/codeql-analysis.yml | 68 --- .github/workflows/pr-check.yml | 475 +----------------- .github/workflows/pr-labels.yml | 21 - src/client/common/process/proc.ts | 9 +- src/client/common/process/types.ts | 3 +- src/client/common/process/worker/main.ts | 27 + .../common/process/worker/plainExecWorker.ts | 18 + .../process/worker/rawProcessApiWrapper.ts | 26 + .../common/process/worker/shellExecWorker.ts | 18 + src/client/common/process/worker/types.ts | 38 ++ .../process/worker/workerRawProcessApis.ts | 213 ++++++++ src/test/common/process/proc.exec.test.ts | 22 + 13 files changed, 374 insertions(+), 566 deletions(-) delete mode 100644 .github/workflows/codeql-analysis.yml delete mode 100644 .github/workflows/pr-labels.yml create mode 100644 src/client/common/process/worker/main.ts create mode 100644 src/client/common/process/worker/plainExecWorker.ts create mode 100644 src/client/common/process/worker/rawProcessApiWrapper.ts create mode 100644 src/client/common/process/worker/shellExecWorker.ts create mode 100644 src/client/common/process/worker/types.ts create mode 100644 src/client/common/process/worker/workerRawProcessApis.ts diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index d17d5fff4b92..2a155b5ebf98 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -282,7 +282,7 @@ jobs: shell: pwsh if: matrix.test-suite == 'venv' run: | - # 1. For `terminalActivation.testvirtualenvs.test.ts` + # 1. For `*.testvirtualenvs.test.ts` if ('${{ matrix.os }}' -match 'windows-latest') { $condaPythonPath = Join-Path -Path $Env:CONDA -ChildPath python.exe $condaExecPath = Join-Path -Path $Env:CONDA -ChildPath Scripts | Join-Path -ChildPath conda diff --git a/.github/workflows/codeql-analysis.yml b/.github/workflows/codeql-analysis.yml deleted file mode 100644 index 5b037d5a1d0b..000000000000 --- a/.github/workflows/codeql-analysis.yml +++ /dev/null @@ -1,68 +0,0 @@ -# For most projects, this workflow file will not need changing; you simply need -# to commit it to your repository. -# -# You may wish to alter this file to override the set of languages analyzed, -# or to provide custom queries or build logic. -name: 'CodeQL' - -on: - push: - branches: - - main - - release-* - - release/* - pull_request: - # The branches below must be a subset of the branches above - branches: [main] - schedule: - - cron: '0 3 * * 0' - -permissions: - security-events: write - -jobs: - analyze: - name: Analyze - runs-on: ubuntu-latest - - strategy: - fail-fast: false - matrix: - # Override automatic language detection by changing the below list - # Supported options are ['csharp', 'cpp', 'go', 'java', 'javascript', 'python'] - language: ['javascript', 'python'] - # Learn more... - # https://docs.github.com/en/github/finding-security-vulnerabilities-and-errors-in-your-code/configuring-code-scanning#overriding-automatic-language-detection - - steps: - - name: Checkout repository - uses: actions/checkout@v4 - - # Initializes the CodeQL tools for scanning. - - name: Initialize CodeQL - uses: github/codeql-action/init@v2 - with: - languages: ${{ matrix.language }} - # If you wish to specify custom queries, you can do so here or in a config file. - # By default, queries listed here will override any specified in a config file. - # Prefix the list here with "+" to use these queries and those in the config file. - # queries: ./path/to/local/query, your-org/your-repo/queries@main - - # Autobuild attempts to build any compiled languages (C/C++, C#, or Java). - # If this step fails, then you should remove it and run the build manually (see below) - #- name: Autobuild - # uses: github/codeql-action/autobuild@v1 - - # â„šī¸ Command-line programs to run using the OS shell. - # 📚 https://git.io/JvXDl - - # âœī¸ If the Autobuild fails above, remove it and uncomment the following three lines - # and modify them (or add more) to build your code if your project - # uses a compiled language - - #- run: | - # make bootstrap - # make release - - - name: Perform CodeQL Analysis - uses: github/codeql-action/analyze@v2 diff --git a/.github/workflows/pr-check.yml b/.github/workflows/pr-check.yml index 3283d3281e5b..f5a15df2a0eb 100644 --- a/.github/workflows/pr-check.yml +++ b/.github/workflows/pr-check.yml @@ -2,7 +2,6 @@ name: PR/CI Check on: pull_request: - push: branches-ignore: - main - release* @@ -34,290 +33,6 @@ jobs: vsix_name: ${{ env.VSIX_NAME }} artifact_name: ${{ env.ARTIFACT_NAME_VSIX }} - lint: - name: Lint - runs-on: ubuntu-latest - steps: - - name: Checkout - uses: actions/checkout@v4 - - - name: Lint - uses: ./.github/actions/lint - with: - node_version: ${{ env.NODE_VERSION }} - - check-types: - name: Check Python types - runs-on: ubuntu-latest - steps: - - name: Use Python ${{ env.PYTHON_VERSION }} - uses: actions/setup-python@v4 - with: - python-version: ${{ env.PYTHON_VERSION }} - - - name: Checkout - uses: actions/checkout@v4 - - - name: Install base Python requirements - uses: brettcannon/pip-secure-install@v1 - with: - options: '-t ./pythonFiles/lib/python --no-cache-dir --implementation py' - - - name: Install Jedi requirements - uses: brettcannon/pip-secure-install@v1 - with: - requirements-file: './pythonFiles/jedilsp_requirements/requirements.txt' - options: '-t ./pythonFiles/lib/jedilsp --no-cache-dir --implementation py' - - - name: Install other Python requirements - run: | - python -m pip --disable-pip-version-check install -t ./pythonFiles/lib/python --no-cache-dir --implementation py --no-deps --upgrade --pre debugpy - python -m pip install --upgrade -r build/test-requirements.txt - - - name: Run Pyright - uses: jakebailey/pyright-action@v1 - with: - version: 1.1.308 - working-directory: 'pythonFiles' - - python-tests: - name: Python Tests - # The value of runs-on is the OS of the current job (specified in the strategy matrix below) instead of being hardcoded. - runs-on: ${{ matrix.os }} - defaults: - run: - working-directory: ${{ env.special-working-directory }} - strategy: - fail-fast: false - matrix: - # We're not running CI on macOS for now because it's one less matrix entry to lower the number of runners used, - # macOS runners are expensive, and we assume that Ubuntu is enough to cover the Unix case. - os: [ubuntu-latest, windows-latest] - # Run the tests on the oldest and most recent versions of Python. - python: ['3.8', '3.x', '3.12-dev'] - - steps: - - name: Checkout - uses: actions/checkout@v4 - with: - path: ${{ env.special-working-directory-relative }} - - - name: Use Python ${{ matrix.python }} - uses: actions/setup-python@v4 - with: - python-version: ${{ matrix.python }} - - - name: Install base Python requirements - uses: brettcannon/pip-secure-install@v1 - with: - requirements-file: '"${{ env.special-working-directory-relative }}/requirements.txt"' - options: '-t "${{ env.special-working-directory-relative }}/pythonFiles/lib/python" --no-cache-dir --implementation py' - - - name: Install test requirements - run: python -m pip install --upgrade -r build/test-requirements.txt - - - name: Run Python unit tests - run: python pythonFiles/tests/run_all.py - - tests: - name: Tests - # The value of runs-on is the OS of the current job (specified in the strategy matrix below) instead of being hardcoded. - runs-on: ${{ matrix.os }} - defaults: - run: - working-directory: ${{ env.special-working-directory }} - strategy: - fail-fast: false - matrix: - # We're not running CI on macOS for now because it's one less matrix entry to lower the number of runners used, - # macOS runners are expensive, and we assume that Ubuntu is enough to cover the Unix case. - os: [ubuntu-latest, windows-latest] - # Run the tests on the oldest and most recent versions of Python. - python: ['3.x'] - test-suite: [ts-unit, venv, single-workspace, debugger, functional] - - steps: - - name: Checkout - uses: actions/checkout@v4 - with: - path: ${{ env.special-working-directory-relative }} - - - name: Install Node - uses: actions/setup-node@v4 - with: - node-version: ${{ env.NODE_VERSION }} - cache: 'npm' - cache-dependency-path: ${{ env.special-working-directory-relative }}/package-lock.json - - - name: Install dependencies (npm ci) - run: npm ci - - - name: Compile - run: npx gulp prePublishNonBundle - - - name: Use Python ${{ matrix.python }} - uses: actions/setup-python@v4 - with: - python-version: ${{ matrix.python }} - - - name: Install debugpy - run: | - # We need to have debugpy so that tests relying on it keep passing, but we don't need install_debugpy's logic in the test phase. - python -m pip --disable-pip-version-check install -t ./pythonFiles/lib/python --no-cache-dir --implementation py --no-deps --upgrade --pre debugpy - - - name: Download get-pip.py - run: | - python -m pip install wheel - python -m pip install -r build/build-install-requirements.txt - python ./pythonFiles/download_get_pip.py - shell: bash - - - name: Install base Python requirements - uses: brettcannon/pip-secure-install@v1 - with: - requirements-file: '"${{ env.special-working-directory-relative }}/requirements.txt"' - options: '-t "${{ env.special-working-directory-relative }}/pythonFiles/lib/python" --no-cache-dir --implementation py' - - - name: Install Jedi requirements - uses: brettcannon/pip-secure-install@v1 - with: - requirements-file: '"${{ env.special-working-directory-relative }}/pythonFiles/jedilsp_requirements/requirements.txt"' - options: '-t "${{ env.special-working-directory-relative }}/pythonFiles/lib/jedilsp" --no-cache-dir --implementation py' - - - name: Install test requirements - run: python -m pip install --upgrade -r build/test-requirements.txt - - - name: Install debugpy wheels (Python ${{ matrix.python }}) - run: | - python -m pip install wheel - python -m pip --disable-pip-version-check install -r build/build-install-requirements.txt - python ./pythonFiles/install_debugpy.py - shell: bash - if: matrix.test-suite == 'debugger' - - - name: Install functional test requirements - run: python -m pip install --upgrade -r ./build/functional-test-requirements.txt - if: matrix.test-suite == 'functional' - - - name: Prepare pipenv for venv tests - env: - TEST_FILES_SUFFIX: testvirtualenvs - PYTHON_VIRTUAL_ENVS_LOCATION: './src/tmp/envPaths.json' - shell: pwsh - if: matrix.test-suite == 'venv' - run: | - python -m pip install pipenv - python -m pipenv run python ./build/ci/addEnvPath.py ${{ env.PYTHON_VIRTUAL_ENVS_LOCATION }} pipenvPath - - - name: Prepare poetry for venv tests - env: - TEST_FILES_SUFFIX: testvirtualenvs - shell: pwsh - if: matrix.test-suite == 'venv' - run: | - python -m pip install poetry - Move-Item -Path ".\build\ci\pyproject.toml" -Destination . - poetry env use python - - - name: Prepare virtualenv for venv tests - env: - TEST_FILES_SUFFIX: testvirtualenvs - PYTHON_VIRTUAL_ENVS_LOCATION: './src/tmp/envPaths.json' - shell: pwsh - if: matrix.test-suite == 'venv' - run: | - python -m pip install virtualenv - python -m virtualenv .virtualenv/ - if ('${{ matrix.os }}' -match 'windows-latest') { - & ".virtualenv/Scripts/python.exe" ./build/ci/addEnvPath.py ${{ env.PYTHON_VIRTUAL_ENVS_LOCATION }} virtualEnvPath - } else { - & ".virtualenv/bin/python" ./build/ci/addEnvPath.py ${{ env.PYTHON_VIRTUAL_ENVS_LOCATION }} virtualEnvPath - } - - - name: Prepare venv for venv tests - env: - TEST_FILES_SUFFIX: testvirtualenvs - PYTHON_VIRTUAL_ENVS_LOCATION: './src/tmp/envPaths.json' - shell: pwsh - if: matrix.test-suite == 'venv' && startsWith(matrix.python, 3.) - run: | - python -m venv .venv - if ('${{ matrix.os }}' -match 'windows-latest') { - & ".venv/Scripts/python.exe" ./build/ci/addEnvPath.py ${{ env.PYTHON_VIRTUAL_ENVS_LOCATION }} venvPath - } else { - & ".venv/bin/python" ./build/ci/addEnvPath.py ${{ env.PYTHON_VIRTUAL_ENVS_LOCATION }} venvPath - } - - - name: Prepare conda for venv tests - env: - TEST_FILES_SUFFIX: testvirtualenvs - PYTHON_VIRTUAL_ENVS_LOCATION: './src/tmp/envPaths.json' - shell: pwsh - if: matrix.test-suite == 'venv' - run: | - # 1. For `terminalActivation.testvirtualenvs.test.ts` - if ('${{ matrix.os }}' -match 'windows-latest') { - $condaPythonPath = Join-Path -Path $Env:CONDA -ChildPath python.exe - $condaExecPath = Join-Path -Path $Env:CONDA -ChildPath Scripts | Join-Path -ChildPath conda - } else{ - $condaPythonPath = Join-Path -Path $Env:CONDA -ChildPath bin | Join-Path -ChildPath python - $condaExecPath = Join-Path -Path $Env:CONDA -ChildPath bin | Join-Path -ChildPath conda - } - & $condaPythonPath ./build/ci/addEnvPath.py ${{ env.PYTHON_VIRTUAL_ENVS_LOCATION }} condaExecPath $condaExecPath - & $condaPythonPath ./build/ci/addEnvPath.py ${{ env.PYTHON_VIRTUAL_ENVS_LOCATION }} condaPath - & $condaExecPath init --all - - - name: Set CI_PYTHON_PATH and CI_DISABLE_AUTO_SELECTION - run: | - echo "CI_PYTHON_PATH=python" >> $GITHUB_ENV - echo "CI_DISABLE_AUTO_SELECTION=1" >> $GITHUB_ENV - shell: bash - if: matrix.test-suite != 'ts-unit' - - # Run TypeScript unit tests only for Python 3.X. - - name: Run TypeScript unit tests - run: npm run test:unittests - if: matrix.test-suite == 'ts-unit' && startsWith(matrix.python, 3.) - - # The virtual environment based tests use the `testSingleWorkspace` set of tests - # with the environment variable `TEST_FILES_SUFFIX` set to `testvirtualenvs`, - # which is set in the "Prepare environment for venv tests" step. - # We also use a third-party GitHub Action to install xvfb on Linux, - # run tests and then clean up the process once the tests ran. - # See https://github.com/GabrielBB/xvfb-action - - name: Run venv tests - env: - TEST_FILES_SUFFIX: testvirtualenvs - CI_PYTHON_VERSION: ${{ matrix.python }} - uses: GabrielBB/xvfb-action@v1.6 - with: - run: npm run testSingleWorkspace - working-directory: ${{ env.special-working-directory }} - if: matrix.test-suite == 'venv' - - - name: Run single-workspace tests - env: - CI_PYTHON_VERSION: ${{ matrix.python }} - uses: GabrielBB/xvfb-action@v1.6 - with: - run: npm run testSingleWorkspace - working-directory: ${{ env.special-working-directory }} - if: matrix.test-suite == 'single-workspace' - - - name: Run debugger tests - env: - CI_PYTHON_VERSION: ${{ matrix.python }} - uses: GabrielBB/xvfb-action@v1.6 - with: - run: npm run testDebugger - working-directory: ${{ env.special-working-directory }} - if: matrix.test-suite == 'debugger' - - # Run TypeScript functional tests - - name: Run TypeScript functional tests - run: npm run test:functional - if: matrix.test-suite == 'functional' - smoke-tests: name: Smoke tests # The value of runs-on is the OS of the current job (specified in the strategy matrix below) instead of being hardcoded. @@ -328,7 +43,7 @@ jobs: matrix: # We're not running CI on macOS for now because it's one less matrix entry to lower the number of runners used, # macOS runners are expensive, and we assume that Ubuntu is enough to cover the UNIX case. - os: [ubuntu-latest, windows-latest] + os: [ubuntu-latest] steps: # Need the source to have the tests available. - name: Checkout @@ -339,191 +54,3 @@ jobs: with: node_version: ${{ env.NODE_VERSION }} artifact_name: ${{ env.ARTIFACT_NAME_VSIX }} - - ### Coverage run - coverage: - name: Coverage - # The value of runs-on is the OS of the current job (specified in the strategy matrix below) instead of being hardcoded. - runs-on: ${{ matrix.os }} - strategy: - fail-fast: false - matrix: - # Only run coverage on linux for PRs - os: [ubuntu-latest] - - steps: - - name: Checkout - uses: actions/checkout@v4 - - - name: Install Node - uses: actions/setup-node@v4 - with: - node-version: ${{ env.NODE_VERSION }} - cache: 'npm' - - - name: Install dependencies (npm ci) - run: npm ci - - - name: Compile - run: npx gulp prePublishNonBundle - - - name: Use Python ${{ env.PYTHON_VERSION }} - uses: actions/setup-python@v4 - with: - python-version: ${{ env.PYTHON_VERSION }} - cache: 'pip' - cache-dependency-path: | - requirements.txt - pythonFiles/jedilsp_requirements/requirements.txt - build/test-requirements.txt - build/functional-test-requirements.txt - - - name: Install base Python requirements - uses: brettcannon/pip-secure-install@v1 - with: - options: '-t ./pythonFiles/lib/python --implementation py' - - - name: Install Jedi requirements - uses: brettcannon/pip-secure-install@v1 - with: - requirements-file: './pythonFiles/jedilsp_requirements/requirements.txt' - options: '-t ./pythonFiles/lib/jedilsp --implementation py' - - - name: Install debugpy - run: | - # We need to have debugpy so that tests relying on it keep passing, but we don't need install_debugpy's logic in the test phase. - python -m pip --disable-pip-version-check install -t ./pythonFiles/lib/python --implementation py --no-deps --upgrade --pre debugpy - - - name: Install test requirements - run: python -m pip install --upgrade -r build/test-requirements.txt - - - name: Install functional test requirements - run: python -m pip install --upgrade -r ./build/functional-test-requirements.txt - - - name: Prepare pipenv for venv tests - env: - TEST_FILES_SUFFIX: testvirtualenvs - PYTHON_VIRTUAL_ENVS_LOCATION: './src/tmp/envPaths.json' - shell: pwsh - run: | - python -m pip install pipenv - python -m pipenv run python ./build/ci/addEnvPath.py ${{ env.PYTHON_VIRTUAL_ENVS_LOCATION }} pipenvPath - - - name: Prepare poetry for venv tests - env: - TEST_FILES_SUFFIX: testvirtualenvs - shell: pwsh - run: | - python -m pip install poetry - Move-Item -Path ".\build\ci\pyproject.toml" -Destination . - poetry env use python - - - name: Prepare virtualenv for venv tests - env: - TEST_FILES_SUFFIX: testvirtualenvs - PYTHON_VIRTUAL_ENVS_LOCATION: './src/tmp/envPaths.json' - shell: pwsh - run: | - python -m pip install virtualenv - python -m virtualenv .virtualenv/ - if ('${{ matrix.os }}' -match 'windows-latest') { - & ".virtualenv/Scripts/python.exe" ./build/ci/addEnvPath.py ${{ env.PYTHON_VIRTUAL_ENVS_LOCATION }} virtualEnvPath - } else { - & ".virtualenv/bin/python" ./build/ci/addEnvPath.py ${{ env.PYTHON_VIRTUAL_ENVS_LOCATION }} virtualEnvPath - } - - - name: Prepare venv for venv tests - env: - TEST_FILES_SUFFIX: testvirtualenvs - PYTHON_VIRTUAL_ENVS_LOCATION: './src/tmp/envPaths.json' - shell: pwsh - run: | - python -m venv .venv - if ('${{ matrix.os }}' -match 'windows-latest') { - & ".venv/Scripts/python.exe" ./build/ci/addEnvPath.py ${{ env.PYTHON_VIRTUAL_ENVS_LOCATION }} venvPath - } else { - & ".venv/bin/python" ./build/ci/addEnvPath.py ${{ env.PYTHON_VIRTUAL_ENVS_LOCATION }} venvPath - } - - - name: Prepare conda for venv tests - env: - TEST_FILES_SUFFIX: testvirtualenvs - PYTHON_VIRTUAL_ENVS_LOCATION: './src/tmp/envPaths.json' - shell: pwsh - run: | - # 1. For `terminalActivation.testvirtualenvs.test.ts` - if ('${{ matrix.os }}' -match 'windows-latest') { - $condaPythonPath = Join-Path -Path $Env:CONDA -ChildPath python.exe - $condaExecPath = Join-Path -Path $Env:CONDA -ChildPath Scripts | Join-Path -ChildPath conda - } else{ - $condaPythonPath = Join-Path -Path $Env:CONDA -ChildPath bin | Join-Path -ChildPath python - $condaExecPath = Join-Path -Path $Env:CONDA -ChildPath bin | Join-Path -ChildPath conda - } - & $condaPythonPath ./build/ci/addEnvPath.py ${{ env.PYTHON_VIRTUAL_ENVS_LOCATION }} condaExecPath $condaExecPath - & $condaPythonPath ./build/ci/addEnvPath.py ${{ env.PYTHON_VIRTUAL_ENVS_LOCATION }} condaPath - & $condaExecPath init --all - - - name: Run TypeScript unit tests - run: npm run test:unittests:cover - - - name: Run Python unit tests - run: | - python pythonFiles/tests/run_all.py - - # The virtual environment based tests use the `testSingleWorkspace` set of tests - # with the environment variable `TEST_FILES_SUFFIX` set to `testvirtualenvs`, - # which is set in the "Prepare environment for venv tests" step. - # We also use a third-party GitHub Action to install xvfb on Linux, - # run tests and then clean up the process once the tests ran. - # See https://github.com/GabrielBB/xvfb-action - - name: Run venv tests - env: - TEST_FILES_SUFFIX: testvirtualenvs - CI_PYTHON_VERSION: ${{ env.PYTHON_VERSION }} - CI_DISABLE_AUTO_SELECTION: 1 - uses: GabrielBB/xvfb-action@v1.6 - with: - run: npm run testSingleWorkspace:cover - - - name: Run single-workspace tests - env: - CI_PYTHON_VERSION: ${{ env.PYTHON_VERSION }} - CI_DISABLE_AUTO_SELECTION: 1 - uses: GabrielBB/xvfb-action@v1.6 - with: - run: npm run testSingleWorkspace:cover - - # Enable these tests when coverage is setup for multiroot workspace tests - # - name: Run multi-workspace tests - # env: - # CI_PYTHON_VERSION: ${{ env.PYTHON_VERSION }} - # CI_DISABLE_AUTO_SELECTION: 1 - # uses: GabrielBB/xvfb-action@v1.6 - # with: - # run: npm run testMultiWorkspace:cover - - # Enable these tests when coverage is setup for debugger tests - # - name: Run debugger tests - # env: - # CI_PYTHON_VERSION: ${{ env.PYTHON_VERSION }} - # CI_DISABLE_AUTO_SELECTION: 1 - # uses: GabrielBB/xvfb-action@v1.6 - # with: - # run: npm run testDebugger:cover - - # Run TypeScript functional tests - - name: Run TypeScript functional tests - env: - CI_PYTHON_VERSION: ${{ env.PYTHON_VERSION }} - CI_DISABLE_AUTO_SELECTION: 1 - run: npm run test:functional:cover - - - name: Generate coverage reports - run: npm run test:cover:report - - - name: Upload HTML report - uses: actions/upload-artifact@v3 - with: - name: ${{ runner.os }}-coverage-report-html - path: ./coverage - retention-days: 1 diff --git a/.github/workflows/pr-labels.yml b/.github/workflows/pr-labels.yml deleted file mode 100644 index 730b8e5c5832..000000000000 --- a/.github/workflows/pr-labels.yml +++ /dev/null @@ -1,21 +0,0 @@ -name: 'PR labels' -on: - pull_request: - types: - - 'opened' - - 'reopened' - - 'labeled' - - 'unlabeled' - - 'synchronize' - -jobs: - classify: - name: 'Classify PR' - runs-on: ubuntu-latest - steps: - - name: 'PR impact specified' - uses: mheap/github-action-required-labels@v5 - with: - mode: exactly - count: 1 - labels: 'bug, debt, feature-request, no-changelog' diff --git a/src/client/common/process/proc.ts b/src/client/common/process/proc.ts index 18add7daf6fa..4a5aa984fa44 100644 --- a/src/client/common/process/proc.ts +++ b/src/client/common/process/proc.ts @@ -7,6 +7,7 @@ import { IDisposable } from '../types'; import { EnvironmentVariables } from '../variables/types'; import { execObservable, killPid, plainExec, shellExec } from './rawProcessApis'; import { ExecutionResult, IProcessService, ObservableExecutionResult, ShellOptions, SpawnOptions } from './types'; +import { workerPlainExec, workerShellExec } from './worker/rawProcessApiWrapper'; export class ProcessService extends EventEmitter implements IProcessService { private processesToKill = new Set(); @@ -47,14 +48,20 @@ export class ProcessService extends EventEmitter implements IProcessService { } public exec(file: string, args: string[], options: SpawnOptions = {}): Promise> { + this.emit('exec', file, args, options); + if (options.useWorker) { + return workerPlainExec(file, args, options); + } const execOptions = { ...options, doNotLog: true }; const promise = plainExec(file, args, execOptions, this.env, this.processesToKill); - this.emit('exec', file, args, options); return promise; } public shellExec(command: string, options: ShellOptions = {}): Promise> { this.emit('exec', command, undefined, options); + if (options.useWorker) { + return workerShellExec(command, options); + } const disposables = new Set(); const shellOptions = { ...options, doNotLog: true }; return shellExec(command, shellOptions, this.env, disposables).finally(() => { diff --git a/src/client/common/process/types.ts b/src/client/common/process/types.ts index d4b742718e36..9263e69cbe21 100644 --- a/src/client/common/process/types.ts +++ b/src/client/common/process/types.ts @@ -26,9 +26,10 @@ export type SpawnOptions = ChildProcessSpawnOptions & { extraVariables?: NodeJS.ProcessEnv; outputChannel?: OutputChannel; stdinStr?: string; + useWorker?: boolean; }; -export type ShellOptions = ExecOptions & { throwOnStdErr?: boolean }; +export type ShellOptions = ExecOptions & { throwOnStdErr?: boolean; useWorker?: boolean }; export type ExecutionResult = { stdout: T; diff --git a/src/client/common/process/worker/main.ts b/src/client/common/process/worker/main.ts new file mode 100644 index 000000000000..f6a61b766e38 --- /dev/null +++ b/src/client/common/process/worker/main.ts @@ -0,0 +1,27 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +import { Worker } from 'worker_threads'; +import { traceError } from '../../../logging'; + +// eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/explicit-module-boundary-types +export async function executeWorkerFile(workerFileName: string, workerData: any): Promise { + return new Promise((resolve, reject) => { + const worker = new Worker(workerFileName, { workerData }); + worker.on('message', (msg: { err: Error; res: unknown }) => { + if (msg.err) { + reject(msg.err); + } + resolve(msg.res); + }); + worker.on('error', (ex: Error) => { + traceError(`Error in worker ${workerFileName}`, ex); + reject(ex); + }); + worker.on('exit', (code) => { + if (code !== 0) { + reject(new Error(`Worker ${workerFileName} stopped with exit code ${code}`)); + } + }); + }); +} diff --git a/src/client/common/process/worker/plainExecWorker.ts b/src/client/common/process/worker/plainExecWorker.ts new file mode 100644 index 000000000000..baf2d508def0 --- /dev/null +++ b/src/client/common/process/worker/plainExecWorker.ts @@ -0,0 +1,18 @@ +import { isMainThread, parentPort, workerData } from 'worker_threads'; +import { _workerPlainExecImpl } from './workerRawProcessApis'; + +if (!isMainThread) { + _workerPlainExecImpl(workerData.file, workerData.args, workerData.options) + .then((res) => { + if (!parentPort) { + throw new Error('Not in a worker thread'); + } + parentPort.postMessage({ res }); + }) + .catch((err) => { + if (!parentPort) { + throw new Error('Not in a worker thread'); + } + parentPort.postMessage({ err }); + }); +} diff --git a/src/client/common/process/worker/rawProcessApiWrapper.ts b/src/client/common/process/worker/rawProcessApiWrapper.ts new file mode 100644 index 000000000000..b4fa2ab97db0 --- /dev/null +++ b/src/client/common/process/worker/rawProcessApiWrapper.ts @@ -0,0 +1,26 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +import { SpawnOptions } from 'child_process'; +import * as path from 'path'; +import { executeWorkerFile } from './main'; +import { ExecutionResult, ShellOptions } from './types'; + +export function workerShellExec(command: string, options: ShellOptions): Promise> { + return executeWorkerFile(path.join(__dirname, 'shellExecWorker.js'), { + command, + options, + }); +} + +export function workerPlainExec( + file: string, + args: string[], + options: SpawnOptions = {}, +): Promise> { + return executeWorkerFile(path.join(__dirname, 'plainExecWorker.js'), { + file, + args, + options, + }); +} diff --git a/src/client/common/process/worker/shellExecWorker.ts b/src/client/common/process/worker/shellExecWorker.ts new file mode 100644 index 000000000000..d6132a80db58 --- /dev/null +++ b/src/client/common/process/worker/shellExecWorker.ts @@ -0,0 +1,18 @@ +import { isMainThread, parentPort, workerData } from 'worker_threads'; +import { _workerShellExecImpl } from './workerRawProcessApis'; + +if (!isMainThread) { + _workerShellExecImpl(workerData.command, workerData.options, workerData.defaultEnv) + .then((res) => { + if (!parentPort) { + throw new Error('Not in a worker thread'); + } + parentPort.postMessage({ res }); + }) + .catch((ex) => { + if (!parentPort) { + throw new Error('Not in a worker thread'); + } + parentPort.postMessage({ ex }); + }); +} diff --git a/src/client/common/process/worker/types.ts b/src/client/common/process/worker/types.ts new file mode 100644 index 000000000000..5c58aec10214 --- /dev/null +++ b/src/client/common/process/worker/types.ts @@ -0,0 +1,38 @@ +/* eslint-disable @typescript-eslint/no-empty-function */ +/* eslint-disable @typescript-eslint/explicit-module-boundary-types */ +import { ExecOptions, SpawnOptions as ChildProcessSpawnOptions } from 'child_process'; + +export function noop() {} +export interface IDisposable { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + dispose(): void | undefined | Promise; +} +export type EnvironmentVariables = Record; +export class StdErrError extends Error { + // eslint-disable-next-line @typescript-eslint/no-useless-constructor + constructor(message: string) { + super(message); + } +} + +export type SpawnOptions = ChildProcessSpawnOptions & { + encoding?: string; + // /** + // * Can't use `CancellationToken` here as it comes from vscode which is not available in worker threads. + // */ + // token?: CancellationToken; + mergeStdOutErr?: boolean; + throwOnStdErr?: boolean; + extraVariables?: NodeJS.ProcessEnv; + // /** + // * Can't use `OutputChannel` here as it comes from vscode which is not available in worker threads. + // */ + // outputChannel?: OutputChannel; + stdinStr?: string; +}; +export type ShellOptions = ExecOptions & { throwOnStdErr?: boolean }; + +export type ExecutionResult = { + stdout: T; + stderr?: T; +}; diff --git a/src/client/common/process/worker/workerRawProcessApis.ts b/src/client/common/process/worker/workerRawProcessApis.ts new file mode 100644 index 000000000000..5b04aaa40b0a --- /dev/null +++ b/src/client/common/process/worker/workerRawProcessApis.ts @@ -0,0 +1,213 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +// !!!! IMPORTANT: DO NOT IMPORT FROM VSCODE MODULE AS IT IS NOT AVAILABLE INSIDE WORKER THREADS !!!! + +import { exec, execSync, spawn } from 'child_process'; +import { Readable } from 'stream'; +import { createDeferred } from '../../utils/async'; +import { DEFAULT_ENCODING } from '../constants'; +import { decodeBuffer } from '../decoder'; +import { + ShellOptions, + SpawnOptions, + EnvironmentVariables, + IDisposable, + noop, + StdErrError, + ExecutionResult, +} from './types'; + +const PS_ERROR_SCREEN_BOGUS = /your [0-9]+x[0-9]+ screen size is bogus\. expect trouble/; + +function getDefaultOptions(options: T, defaultEnv?: EnvironmentVariables): T { + const defaultOptions = { ...options }; + const execOptions = defaultOptions as SpawnOptions; + if (execOptions) { + execOptions.encoding = + typeof execOptions.encoding === 'string' && execOptions.encoding.length > 0 + ? execOptions.encoding + : DEFAULT_ENCODING; + const { encoding } = execOptions; + delete execOptions.encoding; + execOptions.encoding = encoding; + } + if (!defaultOptions.env || Object.keys(defaultOptions.env).length === 0) { + const env = defaultEnv || process.env; + defaultOptions.env = { ...env }; + } else { + defaultOptions.env = { ...defaultOptions.env }; + } + + if (execOptions && execOptions.extraVariables) { + defaultOptions.env = { ...defaultOptions.env, ...execOptions.extraVariables }; + } + + // Always ensure we have unbuffered output. + defaultOptions.env.PYTHONUNBUFFERED = '1'; + if (!defaultOptions.env.PYTHONIOENCODING) { + defaultOptions.env.PYTHONIOENCODING = 'utf-8'; + } + + return defaultOptions; +} + +export function _workerShellExecImpl( + command: string, + options: ShellOptions, + defaultEnv?: EnvironmentVariables, + disposables?: Set, +): Promise> { + const shellOptions = getDefaultOptions(options, defaultEnv); + return new Promise((resolve, reject) => { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const callback = (e: any, stdout: any, stderr: any) => { + if (e && e !== null) { + reject(e); + } else if (shellOptions.throwOnStdErr && stderr && stderr.length) { + reject(new Error(stderr)); + } else { + stdout = filterOutputUsingCondaRunMarkers(stdout); + // Make sure stderr is undefined if we actually had none. This is checked + // elsewhere because that's how exec behaves. + resolve({ stderr: stderr && stderr.length > 0 ? stderr : undefined, stdout }); + } + }; + let procExited = false; + const proc = exec(command, shellOptions, callback); // NOSONAR + proc.once('close', () => { + procExited = true; + }); + proc.once('exit', () => { + procExited = true; + }); + proc.once('error', () => { + procExited = true; + }); + const disposable: IDisposable = { + dispose: () => { + // If process has not exited nor killed, force kill it. + if (!procExited && !proc.killed) { + if (proc.pid) { + killPid(proc.pid); + } else { + proc.kill(); + } + } + }, + }; + if (disposables) { + disposables.add(disposable); + } + }); +} + +export function _workerPlainExecImpl( + file: string, + args: string[], + options: SpawnOptions & { doNotLog?: boolean } = {}, + defaultEnv?: EnvironmentVariables, + disposables?: Set, +): Promise> { + const spawnOptions = getDefaultOptions(options, defaultEnv); + const encoding = spawnOptions.encoding ? spawnOptions.encoding : 'utf8'; + const proc = spawn(file, args, spawnOptions); + // Listen to these errors (unhandled errors in streams tears down the process). + // Errors will be bubbled up to the `error` event in `proc`, hence no need to log. + proc.stdout?.on('error', noop); + proc.stderr?.on('error', noop); + const deferred = createDeferred>(); + const disposable: IDisposable = { + dispose: () => { + // If process has not exited nor killed, force kill it. + if (!proc.killed && !deferred.completed) { + if (proc.pid) { + killPid(proc.pid); + } else { + proc.kill(); + } + } + }, + }; + disposables?.add(disposable); + const internalDisposables: IDisposable[] = []; + + // eslint-disable-next-line @typescript-eslint/ban-types + const on = (ee: Readable | null, name: string, fn: Function) => { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + ee?.on(name, fn as any); + // eslint-disable-next-line @typescript-eslint/no-explicit-any + internalDisposables.push({ dispose: () => ee?.removeListener(name, fn as any) as any }); + }; + + // Tokens not supported yet as they come from vscode module which is not available. + // if (options.token) { + // internalDisposables.push(options.token.onCancellationRequested(disposable.dispose)); + // } + + const stdoutBuffers: Buffer[] = []; + on(proc.stdout, 'data', (data: Buffer) => { + stdoutBuffers.push(data); + }); + const stderrBuffers: Buffer[] = []; + on(proc.stderr, 'data', (data: Buffer) => { + if (options.mergeStdOutErr) { + stdoutBuffers.push(data); + stderrBuffers.push(data); + } else { + stderrBuffers.push(data); + } + }); + + proc.once('close', () => { + if (deferred.completed) { + return; + } + const stderr: string | undefined = + stderrBuffers.length === 0 ? undefined : decodeBuffer(stderrBuffers, encoding); + if ( + stderr && + stderr.length > 0 && + options.throwOnStdErr && + // ignore this specific error silently; see this issue for context: https://github.com/microsoft/vscode/issues/75932 + !(PS_ERROR_SCREEN_BOGUS.test(stderr) && stderr.replace(PS_ERROR_SCREEN_BOGUS, '').trim().length === 0) + ) { + deferred.reject(new StdErrError(stderr)); + } else { + let stdout = decodeBuffer(stdoutBuffers, encoding); + stdout = filterOutputUsingCondaRunMarkers(stdout); + deferred.resolve({ stdout, stderr }); + } + internalDisposables.forEach((d) => d.dispose()); + disposable.dispose(); + }); + proc.once('error', (ex) => { + deferred.reject(ex); + internalDisposables.forEach((d) => d.dispose()); + disposable.dispose(); + }); + + return deferred.promise; +} + +function filterOutputUsingCondaRunMarkers(stdout: string) { + // These markers are added if conda run is used or `interpreterInfo.py` is + // run, see `get_output_via_markers.py`. + const regex = />>>PYTHON-EXEC-OUTPUT([\s\S]*)<<= 2 ? match[1].trim() : undefined; + return filteredOut !== undefined ? filteredOut : stdout; +} + +function killPid(pid: number): void { + try { + if (process.platform === 'win32') { + // Windows doesn't support SIGTERM, so execute taskkill to kill the process + execSync(`taskkill /pid ${pid} /T /F`); // NOSONAR + } else { + process.kill(pid); + } + } catch { + console.warn('Unable to kill process with pid', pid); + } +} diff --git a/src/test/common/process/proc.exec.test.ts b/src/test/common/process/proc.exec.test.ts index c193df95d080..7e771e884b82 100644 --- a/src/test/common/process/proc.exec.test.ts +++ b/src/test/common/process/proc.exec.test.ts @@ -34,6 +34,16 @@ suite('ProcessService Observable', () => { expect(result.stderr).to.equal(undefined, 'stderr not undefined'); }); + test('When using worker threads, exec should output print statements', async () => { + const procService = new ProcessService(); + const printOutput = '1234'; + const result = await procService.exec(pythonPath, ['-c', `print("${printOutput}")`], { useWorker: true }); + + expect(result).not.to.be.an('undefined', 'result is undefined'); + expect(result.stdout.trim()).to.be.equal(printOutput, 'Invalid output'); + expect(result.stderr).to.equal(undefined, 'stderr not undefined'); + }); + test('exec should output print unicode characters', async function () { // This test has not been working for many months in Python 2.7 under // Windows. Tracked by #2546. (unicode under Py2.7 is tough!) @@ -241,6 +251,18 @@ suite('ProcessService Observable', () => { expect(result.stderr).to.equal(undefined, 'stderr not empty'); expect(result.stdout.trim()).to.be.equal(printOutput, 'Invalid output'); }); + test('When using worker threads, shellExec should be able to run python and filter output using conda related markers', async () => { + const procService = new ProcessService(); + const printOutput = '1234'; + const result = await procService.shellExec( + `"${pythonPath}" -c "print('>>>PYTHON-EXEC-OUTPUT');print('${printOutput}');print('<< { const procService = new ProcessService(); const result = procService.shellExec('invalid command'); From 5583a5bef09bd6bf1e76f932682abf086b90acbd Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Fri, 17 Nov 2023 08:07:02 +0000 Subject: [PATCH 02/16] s --- .github/workflows/pr-check.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/pr-check.yml b/.github/workflows/pr-check.yml index f5a15df2a0eb..c51e51d65724 100644 --- a/.github/workflows/pr-check.yml +++ b/.github/workflows/pr-check.yml @@ -2,6 +2,7 @@ name: PR/CI Check on: pull_request: + push: branches-ignore: - main - release* From 2e025153656d1aa83ae27d351339a8dbddfe3e62 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Fri, 17 Nov 2023 08:07:43 +0000 Subject: [PATCH 03/16] s --- .github/workflows/pr-check.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/.github/workflows/pr-check.yml b/.github/workflows/pr-check.yml index c51e51d65724..86501cf11ed5 100644 --- a/.github/workflows/pr-check.yml +++ b/.github/workflows/pr-check.yml @@ -1,7 +1,6 @@ name: PR/CI Check on: - pull_request: push: branches-ignore: - main From 49758ea10aadfa5dc58bdbfcdb02e6604893536a Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Fri, 17 Nov 2023 08:58:57 +0000 Subject: [PATCH 04/16] console. logging --- src/client/logging/outputChannelLogger.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/client/logging/outputChannelLogger.ts b/src/client/logging/outputChannelLogger.ts index 40505d33a735..c52ef6325703 100644 --- a/src/client/logging/outputChannelLogger.ts +++ b/src/client/logging/outputChannelLogger.ts @@ -9,22 +9,27 @@ export class OutputChannelLogger implements ILogging { constructor(private readonly channel: LogOutputChannel) {} public traceLog(...data: Arguments): void { + console.log(util.format(...data)); this.channel.appendLine(util.format(...data)); } public traceError(...data: Arguments): void { + console.error(util.format(...data)); this.channel.error(util.format(...data)); } public traceWarn(...data: Arguments): void { + console.warn(util.format(...data)); this.channel.warn(util.format(...data)); } public traceInfo(...data: Arguments): void { + console.info(util.format(...data)); this.channel.info(util.format(...data)); } public traceVerbose(...data: Arguments): void { + console.debug(util.format(...data)); this.channel.debug(util.format(...data)); } } From c794fc86ed6546384e22f317d4f64ed552e4b73d Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Fri, 17 Nov 2023 08:59:48 +0000 Subject: [PATCH 05/16] ss --- src/client/pythonEnvironments/base/locator.ts | 2 +- .../base/locators/lowLevel/condaLocator.ts | 9 +++-- .../lowLevel/windowsRegistryLocator.ts | 7 +++- .../base/locators/wrappers.ts | 11 +----- .../common/environmentManagers/conda.ts | 26 +++++++++----- .../common/externalDependencies.ts | 34 ++++++------------- .../common/windowsRegistry.ts | 2 +- 7 files changed, 44 insertions(+), 47 deletions(-) diff --git a/src/client/pythonEnvironments/base/locator.ts b/src/client/pythonEnvironments/base/locator.ts index 8fefe8e2d0dd..ab3b17629bc5 100644 --- a/src/client/pythonEnvironments/base/locator.ts +++ b/src/client/pythonEnvironments/base/locator.ts @@ -178,7 +178,7 @@ export interface ILocator extends * @param query - if provided, the locator will limit results to match * @returns - the fast async iterator of Python envs, which may have incomplete info */ - iterEnvs(query?: QueryForEvent, useWorkerThreads?: boolean): IPythonEnvsIterator; + iterEnvs(query?: QueryForEvent): IPythonEnvsIterator; } export type ICompositeLocator = Omit, 'providerId'>; diff --git a/src/client/pythonEnvironments/base/locators/lowLevel/condaLocator.ts b/src/client/pythonEnvironments/base/locators/lowLevel/condaLocator.ts index 7cac0cb7df90..651a43ff8868 100644 --- a/src/client/pythonEnvironments/base/locators/lowLevel/condaLocator.ts +++ b/src/client/pythonEnvironments/base/locators/lowLevel/condaLocator.ts @@ -6,6 +6,8 @@ import { BasicEnvInfo, IPythonEnvsIterator } from '../../locator'; import { Conda, getCondaEnvironmentsTxt } from '../../../common/environmentManagers/conda'; import { traceError, traceVerbose } from '../../../../logging'; import { FSWatchingLocator } from './fsWatchingLocator'; +import { DiscoveryUsingWorkers } from '../../../../common/experiments/groups'; +import { inExperiment } from '../../../common/externalDependencies'; export class CondaEnvironmentLocator extends FSWatchingLocator { public readonly providerId: string = 'conda-envs'; @@ -19,8 +21,11 @@ export class CondaEnvironmentLocator extends FSWatchingLocator { } // eslint-disable-next-line class-methods-use-this - public async *doIterEnvs(): IPythonEnvsIterator { - const conda = await Conda.getConda(); + public async *doIterEnvs( + _: unknown, + useWorkerThreads = inExperiment(DiscoveryUsingWorkers.experiment), + ): IPythonEnvsIterator { + const conda = await Conda.getConda(undefined, useWorkerThreads); if (conda === undefined) { traceVerbose(`Couldn't locate the conda binary.`); return; diff --git a/src/client/pythonEnvironments/base/locators/lowLevel/windowsRegistryLocator.ts b/src/client/pythonEnvironments/base/locators/lowLevel/windowsRegistryLocator.ts index 947d90ee9cb5..16b2167021db 100644 --- a/src/client/pythonEnvironments/base/locators/lowLevel/windowsRegistryLocator.ts +++ b/src/client/pythonEnvironments/base/locators/lowLevel/windowsRegistryLocator.ts @@ -7,12 +7,17 @@ import { BasicEnvInfo, IPythonEnvsIterator, Locator } from '../../locator'; import { getRegistryInterpreters } from '../../../common/windowsUtils'; import { traceError, traceVerbose } from '../../../../logging'; import { isMicrosoftStoreDir } from '../../../common/environmentManagers/microsoftStoreEnv'; +import { inExperiment } from '../../../common/externalDependencies'; +import { DiscoveryUsingWorkers } from '../../../../common/experiments/groups'; export class WindowsRegistryLocator extends Locator { public readonly providerId: string = 'windows-registry'; // eslint-disable-next-line class-methods-use-this - public iterEnvs(_?: unknown, useWorkerThreads = true): IPythonEnvsIterator { + public iterEnvs( + _?: unknown, + useWorkerThreads = inExperiment(DiscoveryUsingWorkers.experiment), + ): IPythonEnvsIterator { const iterator = async function* () { traceVerbose('Searching for windows registry interpreters'); const interpreters = await getRegistryInterpreters(useWorkerThreads); diff --git a/src/client/pythonEnvironments/base/locators/wrappers.ts b/src/client/pythonEnvironments/base/locators/wrappers.ts index 1334c60fa2e0..bfaede584f6f 100644 --- a/src/client/pythonEnvironments/base/locators/wrappers.ts +++ b/src/client/pythonEnvironments/base/locators/wrappers.ts @@ -3,13 +3,10 @@ // eslint-disable-next-line max-classes-per-file import { Uri } from 'vscode'; -import { DiscoveryUsingWorkers } from '../../../common/experiments/groups'; import { IDisposable } from '../../../common/types'; import { iterEmpty } from '../../../common/utils/async'; import { getURIFilter } from '../../../common/utils/misc'; import { Disposables } from '../../../common/utils/resourceLifecycle'; -import { traceLog } from '../../../logging'; -import { inExperiment } from '../../common/externalDependencies'; import { PythonEnvInfo } from '../info'; import { BasicEnvInfo, ILocator, IPythonEnvsIterator, PythonLocatorQuery } from '../locator'; import { combineIterators, Locators } from '../locators'; @@ -20,8 +17,6 @@ import { LazyResourceBasedLocator } from './common/resourceBasedLocator'; */ export class ExtensionLocators extends Locators { - private readonly useWorkerThreads: boolean; - constructor( // These are expected to be low-level locators (e.g. system). private readonly nonWorkspace: ILocator[], @@ -30,10 +25,6 @@ export class ExtensionLocators extends Locators { private readonly workspace: ILocator, ) { super([...nonWorkspace, workspace]); - this.useWorkerThreads = inExperiment(DiscoveryUsingWorkers.experiment); - if (this.useWorkerThreads) { - traceLog('Using worker threads for discovery...'); - } } public iterEnvs(query?: PythonLocatorQuery): IPythonEnvsIterator { @@ -42,7 +33,7 @@ export class ExtensionLocators extends Locators { const nonWorkspace = query?.providerId ? this.nonWorkspace.filter((locator) => query.providerId === locator.providerId) : this.nonWorkspace; - iterators.push(...nonWorkspace.map((loc) => loc.iterEnvs(query, this.useWorkerThreads))); + iterators.push(...nonWorkspace.map((loc) => loc.iterEnvs(query))); } return combineIterators(iterators); } diff --git a/src/client/pythonEnvironments/common/environmentManagers/conda.ts b/src/client/pythonEnvironments/common/environmentManagers/conda.ts index 842f81b99a4e..bc98e57829e9 100644 --- a/src/client/pythonEnvironments/common/environmentManagers/conda.ts +++ b/src/client/pythonEnvironments/common/environmentManagers/conda.ts @@ -265,16 +265,21 @@ export class Conda { * @param command - Command used to spawn conda. This has the same meaning as the * first argument of spawn() - i.e. it can be a full path, or just a binary name. */ - constructor(readonly command: string, shellCommand?: string, private readonly shellPath?: string) { + constructor( + readonly command: string, + shellCommand?: string, + private readonly shellPath?: string, + private readonly useWorkerThreads = true, + ) { this.shellCommand = shellCommand ?? command; onDidChangePythonSetting(CONDAPATH_SETTING_KEY, () => { Conda.condaPromise = new Map>(); }); } - public static async getConda(shellPath?: string): Promise { + public static async getConda(shellPath?: string, useWorkerThreads?: boolean): Promise { if (Conda.condaPromise.get(shellPath) === undefined || isTestExecution()) { - Conda.condaPromise.set(shellPath, Conda.locate(shellPath)); + Conda.condaPromise.set(shellPath, Conda.locate(shellPath, useWorkerThreads)); } return Conda.condaPromise.get(shellPath); } @@ -285,10 +290,15 @@ export class Conda { * * @return A Conda instance corresponding to the binary, if successful; otherwise, undefined. */ - private static async locate(shellPath?: string): Promise { + private static async locate(shellPath?: string, useWorkerThreads = true): Promise { traceVerbose(`Searching for conda.`); const home = getUserHomeDir(); - const customCondaPath = getPythonSetting(CONDAPATH_SETTING_KEY); + let customCondaPath: string | undefined = 'conda'; + try { + customCondaPath = getPythonSetting(CONDAPATH_SETTING_KEY); + } catch (ex) { + traceError(`Failed to get conda path setting, ${ex}`); + } const suffix = getOSType() === OSType.Windows ? 'Scripts\\conda.exe' : 'bin/conda'; // Produce a list of candidate binaries to be probed by exec'ing them. @@ -307,7 +317,7 @@ export class Conda { } async function* getCandidatesFromRegistry() { - const interps = await getRegistryInterpreters(true); + const interps = await getRegistryInterpreters(useWorkerThreads); const candidates = interps .filter((interp) => interp.interpreterPath && interp.distroOrgName === 'ContinuumAnalytics') .map((interp) => path.join(path.win32.dirname(interp.interpreterPath), suffix)); @@ -385,7 +395,7 @@ export class Conda { // Probe the candidates, and pick the first one that exists and does what we need. for await (const condaPath of getCandidates()) { traceVerbose(`Probing conda binary: ${condaPath}`); - let conda = new Conda(condaPath, undefined, shellPath); + let conda = new Conda(condaPath, undefined, shellPath, useWorkerThreads); try { await conda.getInfo(); if (getOSType() === OSType.Windows && (isTestExecution() || condaPath !== customCondaPath)) { @@ -440,7 +450,7 @@ export class Conda { if (shellPath) { options.shell = shellPath; } - const resultPromise = exec(command, ['info', '--json'], options); + const resultPromise = exec(command, ['info', '--json'], options, this.useWorkerThreads); // It has been observed that specifying a timeout is still not reliable to terminate the Conda process, see #27915. // Hence explicitly continue execution after timeout has been reached. const success = await Promise.race([ diff --git a/src/client/pythonEnvironments/common/externalDependencies.ts b/src/client/pythonEnvironments/common/externalDependencies.ts index 357967236c9e..2ecbbdf0cb11 100644 --- a/src/client/pythonEnvironments/common/externalDependencies.ts +++ b/src/client/pythonEnvironments/common/externalDependencies.ts @@ -3,7 +3,6 @@ import * as fsapi from 'fs-extra'; import * as path from 'path'; -import { Worker } from 'worker_threads'; import * as vscode from 'vscode'; import { IWorkspaceService } from '../../common/application/types'; import { ExecutionResult, IProcessServiceFactory, ShellOptions, SpawnOptions } from '../../common/process/types'; @@ -12,6 +11,7 @@ import { chain, iterable } from '../../common/utils/async'; import { getOSType, OSType } from '../../common/utils/platform'; import { IServiceContainer } from '../../ioc/types'; import { traceError, traceVerbose } from '../../logging'; +import { DiscoveryUsingWorkers } from '../../common/experiments/groups'; let internalServiceContainer: IServiceContainer; export function initializeExternalDependencies(serviceContainer: IServiceContainer): void { @@ -21,12 +21,20 @@ export function initializeExternalDependencies(serviceContainer: IServiceContain // processes export async function shellExecute(command: string, options: ShellOptions = {}): Promise> { + const useWorker = inExperiment(DiscoveryUsingWorkers.experiment); const service = await internalServiceContainer.get(IProcessServiceFactory).create(); + options = { ...options, useWorker }; return service.shellExec(command, options); } -export async function exec(file: string, args: string[], options: SpawnOptions = {}): Promise> { +export async function exec( + file: string, + args: string[], + options: SpawnOptions = {}, + useWorker = inExperiment(DiscoveryUsingWorkers.experiment), +): Promise> { const service = await internalServiceContainer.get(IProcessServiceFactory).create(); + options = { ...options, useWorker }; return service.exec(file, args, options); } @@ -201,25 +209,3 @@ export function onDidChangePythonSetting(name: string, callback: () => void, roo } }); } - -// eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/explicit-module-boundary-types -export async function executeWorkerFile(workerFileName: string, workerData: any): Promise { - return new Promise((resolve, reject) => { - const worker = new Worker(workerFileName, { workerData }); - worker.on('message', (res: { err: Error; res: unknown }) => { - if (res.err) { - reject(res.err); - } - resolve(res.res); - }); - worker.on('error', (ex: Error) => { - traceError(`Error in worker ${workerFileName}`, ex); - reject(ex); - }); - worker.on('exit', (code) => { - if (code !== 0) { - reject(new Error(`Worker ${workerFileName} stopped with exit code ${code}`)); - } - }); - }); -} diff --git a/src/client/pythonEnvironments/common/windowsRegistry.ts b/src/client/pythonEnvironments/common/windowsRegistry.ts index efac5bb3209f..ac58963a6c14 100644 --- a/src/client/pythonEnvironments/common/windowsRegistry.ts +++ b/src/client/pythonEnvironments/common/windowsRegistry.ts @@ -5,7 +5,7 @@ import { HKCU, HKLM, Options, REG_SZ, Registry, RegistryItem } from 'winreg'; import * as path from 'path'; import { createDeferred } from '../../common/utils/async'; -import { executeWorkerFile } from './externalDependencies'; +import { executeWorkerFile } from '../../common/process/worker/main'; export { HKCU, HKLM, REG_SZ, Options }; From 90df1b37770dffb58a40f5c449c09c29a5b118a7 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Fri, 17 Nov 2023 09:00:50 +0000 Subject: [PATCH 06/16] Ignore envVars.txt in git --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index 046d01588573..07b01e4224cf 100644 --- a/.gitignore +++ b/.gitignore @@ -3,6 +3,7 @@ out log.log **/node_modules +envVars.txt *.pyc *.vsix **/.vscode/.ropeproject/** From 1f16650c7397457fd20cc1da769401127d8f52e7 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Fri, 17 Nov 2023 09:21:54 +0000 Subject: [PATCH 07/16] s --- src/test/smoke/runInTerminal.smoke.test.ts | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/test/smoke/runInTerminal.smoke.test.ts b/src/test/smoke/runInTerminal.smoke.test.ts index 43b53e4480e0..3c01a8ec4e25 100644 --- a/src/test/smoke/runInTerminal.smoke.test.ts +++ b/src/test/smoke/runInTerminal.smoke.test.ts @@ -24,6 +24,17 @@ suite('Smoke Test: Run Python File In Terminal', () => { teardown(closeActiveWindows); test('Exec', async () => { + const directoryPath = + '/home/runner/work/vscode-python/vscode-python/tmp/ext/smokeTestExtensionsFolder/out/client'; + console.log('Check contents of directory:', directoryPath); + fs.readdir(directoryPath, (err, files) => { + if (err) { + return console.error(`Unable to scan directory: ${err}`); + } + files.forEach((file) => { + console.log(file); + }); + }); const file = path.join( EXTENSION_ROOT_DIR_FOR_TESTS, 'src', From 739f65f769f64c8403ee17923f3ac9091e853bf8 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Fri, 17 Nov 2023 10:00:38 +0000 Subject: [PATCH 08/16] s --- src/client/common/process/worker/main.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/client/common/process/worker/main.ts b/src/client/common/process/worker/main.ts index f6a61b766e38..ae0852eff56b 100644 --- a/src/client/common/process/worker/main.ts +++ b/src/client/common/process/worker/main.ts @@ -2,13 +2,16 @@ // Licensed under the MIT License. import { Worker } from 'worker_threads'; -import { traceError } from '../../../logging'; +import { traceError, traceVerbose } from '../../../logging'; // eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/explicit-module-boundary-types export async function executeWorkerFile(workerFileName: string, workerData: any): Promise { return new Promise((resolve, reject) => { + traceVerbose(`Starting worker ${workerFileName} with data ${JSON.stringify(workerData)}`); const worker = new Worker(workerFileName, { workerData }); + traceVerbose(`Started worker ${workerFileName}`); worker.on('message', (msg: { err: Error; res: unknown }) => { + traceVerbose(`Worker ${workerFileName} sent message ${JSON.stringify(msg)}`); if (msg.err) { reject(msg.err); } @@ -19,6 +22,7 @@ export async function executeWorkerFile(workerFileName: string, workerData: any) reject(ex); }); worker.on('exit', (code) => { + traceVerbose(`Worker ${workerFileName} exited with code ${code}`); if (code !== 0) { reject(new Error(`Worker ${workerFileName} stopped with exit code ${code}`)); } From 39718c87ab5a238aeb32db42ea3ad59d4cab4118 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Fri, 17 Nov 2023 10:12:43 +0000 Subject: [PATCH 09/16] s --- build/webpack/webpack.extension.config.js | 4 ++++ src/client/common/process/worker/main.ts | 9 +++++++++ .../worker/{plainExecWorker.ts => plainExec.worker.ts} | 0 src/client/common/process/worker/rawProcessApiWrapper.ts | 4 ++-- .../worker/{shellExecWorker.ts => shellExec.worker.ts} | 0 5 files changed, 15 insertions(+), 2 deletions(-) rename src/client/common/process/worker/{plainExecWorker.ts => plainExec.worker.ts} (100%) rename src/client/common/process/worker/{shellExecWorker.ts => shellExec.worker.ts} (100%) diff --git a/build/webpack/webpack.extension.config.js b/build/webpack/webpack.extension.config.js index a33508e5d96a..19eb3e0bea9e 100644 --- a/build/webpack/webpack.extension.config.js +++ b/build/webpack/webpack.extension.config.js @@ -51,6 +51,10 @@ const config = { }, ], }, + { + test: /\.worker\.js$/, + use: { loader: 'worker-loader' }, + }, ], }, externals: [ diff --git a/src/client/common/process/worker/main.ts b/src/client/common/process/worker/main.ts index ae0852eff56b..f9ab72f1b46e 100644 --- a/src/client/common/process/worker/main.ts +++ b/src/client/common/process/worker/main.ts @@ -4,8 +4,17 @@ import { Worker } from 'worker_threads'; import { traceError, traceVerbose } from '../../../logging'; +/** + * Executes a worker file. + * @param workerFileName Filename of the worker file to execute, it has to end with ".worker.js" for webpack to bundle it. + * @param workerData Arguments to the worker file. + * @returns + */ // eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/explicit-module-boundary-types export async function executeWorkerFile(workerFileName: string, workerData: any): Promise { + if (!workerFileName.endsWith('.worker.js')) { + throw new Error('Worker file must end with ".worker.js" for webpack to bundle webworkers'); + } return new Promise((resolve, reject) => { traceVerbose(`Starting worker ${workerFileName} with data ${JSON.stringify(workerData)}`); const worker = new Worker(workerFileName, { workerData }); diff --git a/src/client/common/process/worker/plainExecWorker.ts b/src/client/common/process/worker/plainExec.worker.ts similarity index 100% rename from src/client/common/process/worker/plainExecWorker.ts rename to src/client/common/process/worker/plainExec.worker.ts diff --git a/src/client/common/process/worker/rawProcessApiWrapper.ts b/src/client/common/process/worker/rawProcessApiWrapper.ts index b4fa2ab97db0..e6476df5d8fa 100644 --- a/src/client/common/process/worker/rawProcessApiWrapper.ts +++ b/src/client/common/process/worker/rawProcessApiWrapper.ts @@ -7,7 +7,7 @@ import { executeWorkerFile } from './main'; import { ExecutionResult, ShellOptions } from './types'; export function workerShellExec(command: string, options: ShellOptions): Promise> { - return executeWorkerFile(path.join(__dirname, 'shellExecWorker.js'), { + return executeWorkerFile(path.join(__dirname, 'shellExec.worker.js'), { command, options, }); @@ -18,7 +18,7 @@ export function workerPlainExec( args: string[], options: SpawnOptions = {}, ): Promise> { - return executeWorkerFile(path.join(__dirname, 'plainExecWorker.js'), { + return executeWorkerFile(path.join(__dirname, 'plainExec.worker.js'), { file, args, options, diff --git a/src/client/common/process/worker/shellExecWorker.ts b/src/client/common/process/worker/shellExec.worker.ts similarity index 100% rename from src/client/common/process/worker/shellExecWorker.ts rename to src/client/common/process/worker/shellExec.worker.ts From 53b0faaafecad959ad59ceca97809406ceeca1ca Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Fri, 17 Nov 2023 11:07:35 +0000 Subject: [PATCH 10/16] attempt to use worker loader --- package-lock.json | 51 ++++++++++++++---------- package.json | 1 + src/client/common/process/worker/main.ts | 2 +- 3 files changed, 33 insertions(+), 21 deletions(-) diff --git a/package-lock.json b/package-lock.json index 250ba6866f28..dacf9be4e84f 100644 --- a/package-lock.json +++ b/package-lock.json @@ -106,7 +106,6 @@ "typemoq": "^2.1.0", "typescript": "4.5.5", "uuid": "^8.3.2", - "vscode-debugadapter-testsupport": "^1.27.0", "webpack": "^5.76.0", "webpack-bundle-analyzer": "^4.5.0", "webpack-cli": "^4.9.2", @@ -114,6 +113,7 @@ "webpack-merge": "^5.8.0", "webpack-node-externals": "^3.0.0", "webpack-require-from": "^1.8.6", + "worker-loader": "^3.0.8", "yargs": "^15.3.1" }, "engines": { @@ -14513,16 +14513,6 @@ "integrity": "sha512-2ham8XPWTONajOR0ohOKOHXkm3+gaBmGut3SRuu75xLd/RRaY6vqgh8NBYYk7+RW3u5AtzPQZG8F10LHkl0lAQ==", "dev": true }, - "node_modules/vscode-debugadapter-testsupport": { - "version": "1.35.0", - "resolved": "https://registry.npmjs.org/vscode-debugadapter-testsupport/-/vscode-debugadapter-testsupport-1.35.0.tgz", - "integrity": "sha512-4emLt6JOk4iKqC2aWNJupOtrK6JwYAZ6KppqvKASN6B1s063VoqI18QhUB1CeoKwNaN1LIG3VPv2xM8HKOjyDA==", - "deprecated": "This package has been renamed to @vscode/debugadapter-testsupport, please update to the new name", - "dev": true, - "dependencies": { - "vscode-debugprotocol": "1.35.0" - } - }, "node_modules/vscode-debugprotocol": { "version": "1.35.0", "resolved": "https://registry.npmjs.org/vscode-debugprotocol/-/vscode-debugprotocol-1.35.0.tgz", @@ -14970,6 +14960,26 @@ "wipe-node-cache": "^2.1.0" } }, + "node_modules/worker-loader": { + "version": "3.0.8", + "resolved": "https://registry.npmjs.org/worker-loader/-/worker-loader-3.0.8.tgz", + "integrity": "sha512-XQyQkIFeRVC7f7uRhFdNMe/iJOdO6zxAaR3EWbDp45v3mDhrTi+++oswKNxShUNjPC/1xUp5DB29YKLhFo129g==", + "dev": true, + "dependencies": { + "loader-utils": "^2.0.0", + "schema-utils": "^3.0.0" + }, + "engines": { + "node": ">= 10.13.0" + }, + "funding": { + "type": "opencollective", + "url": "https://opencollective.com/webpack" + }, + "peerDependencies": { + "webpack": "^4.0.0 || ^5.0.0" + } + }, "node_modules/workerpool": { "version": "6.2.0", "resolved": "https://registry.npmjs.org/workerpool/-/workerpool-6.2.0.tgz", @@ -26646,15 +26656,6 @@ "integrity": "sha512-2ham8XPWTONajOR0ohOKOHXkm3+gaBmGut3SRuu75xLd/RRaY6vqgh8NBYYk7+RW3u5AtzPQZG8F10LHkl0lAQ==", "dev": true }, - "vscode-debugadapter-testsupport": { - "version": "1.35.0", - "resolved": "https://registry.npmjs.org/vscode-debugadapter-testsupport/-/vscode-debugadapter-testsupport-1.35.0.tgz", - "integrity": "sha512-4emLt6JOk4iKqC2aWNJupOtrK6JwYAZ6KppqvKASN6B1s063VoqI18QhUB1CeoKwNaN1LIG3VPv2xM8HKOjyDA==", - "dev": true, - "requires": { - "vscode-debugprotocol": "1.35.0" - } - }, "vscode-debugprotocol": { "version": "1.35.0", "resolved": "https://registry.npmjs.org/vscode-debugprotocol/-/vscode-debugprotocol-1.35.0.tgz", @@ -26980,6 +26981,16 @@ "wipe-node-cache": "^2.1.0" } }, + "worker-loader": { + "version": "3.0.8", + "resolved": "https://registry.npmjs.org/worker-loader/-/worker-loader-3.0.8.tgz", + "integrity": "sha512-XQyQkIFeRVC7f7uRhFdNMe/iJOdO6zxAaR3EWbDp45v3mDhrTi+++oswKNxShUNjPC/1xUp5DB29YKLhFo129g==", + "dev": true, + "requires": { + "loader-utils": "^2.0.0", + "schema-utils": "^3.0.0" + } + }, "workerpool": { "version": "6.2.0", "resolved": "https://registry.npmjs.org/workerpool/-/workerpool-6.2.0.tgz", diff --git a/package.json b/package.json index 068f04114cae..f009f198faf0 100644 --- a/package.json +++ b/package.json @@ -1659,6 +1659,7 @@ "typescript": "4.5.5", "uuid": "^8.3.2", "webpack": "^5.76.0", + "worker-loader": "^3.0.8", "webpack-bundle-analyzer": "^4.5.0", "webpack-cli": "^4.9.2", "webpack-fix-default-import-plugin": "^1.0.3", diff --git a/src/client/common/process/worker/main.ts b/src/client/common/process/worker/main.ts index f9ab72f1b46e..045fca9f5934 100644 --- a/src/client/common/process/worker/main.ts +++ b/src/client/common/process/worker/main.ts @@ -2,7 +2,7 @@ // Licensed under the MIT License. import { Worker } from 'worker_threads'; -import { traceError, traceVerbose } from '../../../logging'; +import { traceVerbose, traceError } from '../../../logging/index'; /** * Executes a worker file. From a6f9f673ca03b0119b90af10d9c9dbf777ff8959 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Fri, 17 Nov 2023 19:27:49 +0000 Subject: [PATCH 11/16] z --- build/webpack/webpack.extension.config.js | 4 --- src/client/common/process/proc.ts | 3 ++- src/client/common/process/worker/main.ts | 17 ++---------- .../process/worker/rawProcessApiWrapper.ts | 26 ------------------- ...plainExec.worker.ts => workerPlainExec.ts} | 21 +++++++++++++++ ...shellExec.worker.ts => workerShellExec.ts} | 14 ++++++++++ 6 files changed, 39 insertions(+), 46 deletions(-) delete mode 100644 src/client/common/process/worker/rawProcessApiWrapper.ts rename src/client/common/process/worker/{plainExec.worker.ts => workerPlainExec.ts} (51%) rename src/client/common/process/worker/{shellExec.worker.ts => workerShellExec.ts} (55%) diff --git a/build/webpack/webpack.extension.config.js b/build/webpack/webpack.extension.config.js index 19eb3e0bea9e..a33508e5d96a 100644 --- a/build/webpack/webpack.extension.config.js +++ b/build/webpack/webpack.extension.config.js @@ -51,10 +51,6 @@ const config = { }, ], }, - { - test: /\.worker\.js$/, - use: { loader: 'worker-loader' }, - }, ], }, externals: [ diff --git a/src/client/common/process/proc.ts b/src/client/common/process/proc.ts index 4a5aa984fa44..4d30d911888c 100644 --- a/src/client/common/process/proc.ts +++ b/src/client/common/process/proc.ts @@ -7,7 +7,8 @@ import { IDisposable } from '../types'; import { EnvironmentVariables } from '../variables/types'; import { execObservable, killPid, plainExec, shellExec } from './rawProcessApis'; import { ExecutionResult, IProcessService, ObservableExecutionResult, ShellOptions, SpawnOptions } from './types'; -import { workerPlainExec, workerShellExec } from './worker/rawProcessApiWrapper'; +import { workerPlainExec } from './worker/workerPlainExec'; +import { workerShellExec } from './worker/workerShellExec'; export class ProcessService extends EventEmitter implements IProcessService { private processesToKill = new Set(); diff --git a/src/client/common/process/worker/main.ts b/src/client/common/process/worker/main.ts index 045fca9f5934..0547a3fd43d2 100644 --- a/src/client/common/process/worker/main.ts +++ b/src/client/common/process/worker/main.ts @@ -1,37 +1,24 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. +// !!!! IMPORTANT: DO NOT IMPORT FROM VSCODE MODULE AS IT IS NOT AVAILABLE INSIDE WORKER THREADS !!!! + import { Worker } from 'worker_threads'; -import { traceVerbose, traceError } from '../../../logging/index'; -/** - * Executes a worker file. - * @param workerFileName Filename of the worker file to execute, it has to end with ".worker.js" for webpack to bundle it. - * @param workerData Arguments to the worker file. - * @returns - */ // eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/explicit-module-boundary-types export async function executeWorkerFile(workerFileName: string, workerData: any): Promise { - if (!workerFileName.endsWith('.worker.js')) { - throw new Error('Worker file must end with ".worker.js" for webpack to bundle webworkers'); - } return new Promise((resolve, reject) => { - traceVerbose(`Starting worker ${workerFileName} with data ${JSON.stringify(workerData)}`); const worker = new Worker(workerFileName, { workerData }); - traceVerbose(`Started worker ${workerFileName}`); worker.on('message', (msg: { err: Error; res: unknown }) => { - traceVerbose(`Worker ${workerFileName} sent message ${JSON.stringify(msg)}`); if (msg.err) { reject(msg.err); } resolve(msg.res); }); worker.on('error', (ex: Error) => { - traceError(`Error in worker ${workerFileName}`, ex); reject(ex); }); worker.on('exit', (code) => { - traceVerbose(`Worker ${workerFileName} exited with code ${code}`); if (code !== 0) { reject(new Error(`Worker ${workerFileName} stopped with exit code ${code}`)); } diff --git a/src/client/common/process/worker/rawProcessApiWrapper.ts b/src/client/common/process/worker/rawProcessApiWrapper.ts deleted file mode 100644 index e6476df5d8fa..000000000000 --- a/src/client/common/process/worker/rawProcessApiWrapper.ts +++ /dev/null @@ -1,26 +0,0 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. -// Licensed under the MIT License. - -import { SpawnOptions } from 'child_process'; -import * as path from 'path'; -import { executeWorkerFile } from './main'; -import { ExecutionResult, ShellOptions } from './types'; - -export function workerShellExec(command: string, options: ShellOptions): Promise> { - return executeWorkerFile(path.join(__dirname, 'shellExec.worker.js'), { - command, - options, - }); -} - -export function workerPlainExec( - file: string, - args: string[], - options: SpawnOptions = {}, -): Promise> { - return executeWorkerFile(path.join(__dirname, 'plainExec.worker.js'), { - file, - args, - options, - }); -} diff --git a/src/client/common/process/worker/plainExec.worker.ts b/src/client/common/process/worker/workerPlainExec.ts similarity index 51% rename from src/client/common/process/worker/plainExec.worker.ts rename to src/client/common/process/worker/workerPlainExec.ts index baf2d508def0..705245ff6d1e 100644 --- a/src/client/common/process/worker/plainExec.worker.ts +++ b/src/client/common/process/worker/workerPlainExec.ts @@ -1,6 +1,27 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +// !!!! IMPORTANT: DO NOT IMPORT FROM VSCODE MODULE AS IT IS NOT AVAILABLE INSIDE WORKER THREADS !!!! + +import { SpawnOptions } from 'child_process'; import { isMainThread, parentPort, workerData } from 'worker_threads'; +import { executeWorkerFile } from './main'; +import { ExecutionResult } from './types'; + import { _workerPlainExecImpl } from './workerRawProcessApis'; +export function workerPlainExec( + file: string, + args: string[], + options: SpawnOptions = {}, +): Promise> { + return executeWorkerFile(__filename, { + file, + args, + options, + }); +} + if (!isMainThread) { _workerPlainExecImpl(workerData.file, workerData.args, workerData.options) .then((res) => { diff --git a/src/client/common/process/worker/shellExec.worker.ts b/src/client/common/process/worker/workerShellExec.ts similarity index 55% rename from src/client/common/process/worker/shellExec.worker.ts rename to src/client/common/process/worker/workerShellExec.ts index d6132a80db58..6ba94825f11e 100644 --- a/src/client/common/process/worker/shellExec.worker.ts +++ b/src/client/common/process/worker/workerShellExec.ts @@ -1,6 +1,20 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +// !!!! IMPORTANT: DO NOT IMPORT FROM VSCODE MODULE AS IT IS NOT AVAILABLE INSIDE WORKER THREADS !!!! + import { isMainThread, parentPort, workerData } from 'worker_threads'; +import { executeWorkerFile } from './main'; +import { ExecutionResult, ShellOptions } from './types'; import { _workerShellExecImpl } from './workerRawProcessApis'; +export function workerShellExec(command: string, options: ShellOptions): Promise> { + return executeWorkerFile(__filename, { + command, + options, + }); +} + if (!isMainThread) { _workerShellExecImpl(workerData.command, workerData.options, workerData.defaultEnv) .then((res) => { From 40d4f93aa8ee6a97d7805a0bf6d851223ab4ad8b Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Fri, 17 Nov 2023 20:54:43 +0000 Subject: [PATCH 12/16] Revert "z" This reverts commit a6f9f673ca03b0119b90af10d9c9dbf777ff8959. --- build/webpack/webpack.extension.config.js | 4 +++ src/client/common/process/proc.ts | 3 +-- src/client/common/process/worker/main.ts | 17 ++++++++++-- ...workerPlainExec.ts => plainExec.worker.ts} | 21 --------------- .../process/worker/rawProcessApiWrapper.ts | 26 +++++++++++++++++++ ...workerShellExec.ts => shellExec.worker.ts} | 14 ---------- 6 files changed, 46 insertions(+), 39 deletions(-) rename src/client/common/process/worker/{workerPlainExec.ts => plainExec.worker.ts} (51%) create mode 100644 src/client/common/process/worker/rawProcessApiWrapper.ts rename src/client/common/process/worker/{workerShellExec.ts => shellExec.worker.ts} (55%) diff --git a/build/webpack/webpack.extension.config.js b/build/webpack/webpack.extension.config.js index a33508e5d96a..19eb3e0bea9e 100644 --- a/build/webpack/webpack.extension.config.js +++ b/build/webpack/webpack.extension.config.js @@ -51,6 +51,10 @@ const config = { }, ], }, + { + test: /\.worker\.js$/, + use: { loader: 'worker-loader' }, + }, ], }, externals: [ diff --git a/src/client/common/process/proc.ts b/src/client/common/process/proc.ts index 4d30d911888c..4a5aa984fa44 100644 --- a/src/client/common/process/proc.ts +++ b/src/client/common/process/proc.ts @@ -7,8 +7,7 @@ import { IDisposable } from '../types'; import { EnvironmentVariables } from '../variables/types'; import { execObservable, killPid, plainExec, shellExec } from './rawProcessApis'; import { ExecutionResult, IProcessService, ObservableExecutionResult, ShellOptions, SpawnOptions } from './types'; -import { workerPlainExec } from './worker/workerPlainExec'; -import { workerShellExec } from './worker/workerShellExec'; +import { workerPlainExec, workerShellExec } from './worker/rawProcessApiWrapper'; export class ProcessService extends EventEmitter implements IProcessService { private processesToKill = new Set(); diff --git a/src/client/common/process/worker/main.ts b/src/client/common/process/worker/main.ts index 0547a3fd43d2..045fca9f5934 100644 --- a/src/client/common/process/worker/main.ts +++ b/src/client/common/process/worker/main.ts @@ -1,24 +1,37 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. -// !!!! IMPORTANT: DO NOT IMPORT FROM VSCODE MODULE AS IT IS NOT AVAILABLE INSIDE WORKER THREADS !!!! - import { Worker } from 'worker_threads'; +import { traceVerbose, traceError } from '../../../logging/index'; +/** + * Executes a worker file. + * @param workerFileName Filename of the worker file to execute, it has to end with ".worker.js" for webpack to bundle it. + * @param workerData Arguments to the worker file. + * @returns + */ // eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/explicit-module-boundary-types export async function executeWorkerFile(workerFileName: string, workerData: any): Promise { + if (!workerFileName.endsWith('.worker.js')) { + throw new Error('Worker file must end with ".worker.js" for webpack to bundle webworkers'); + } return new Promise((resolve, reject) => { + traceVerbose(`Starting worker ${workerFileName} with data ${JSON.stringify(workerData)}`); const worker = new Worker(workerFileName, { workerData }); + traceVerbose(`Started worker ${workerFileName}`); worker.on('message', (msg: { err: Error; res: unknown }) => { + traceVerbose(`Worker ${workerFileName} sent message ${JSON.stringify(msg)}`); if (msg.err) { reject(msg.err); } resolve(msg.res); }); worker.on('error', (ex: Error) => { + traceError(`Error in worker ${workerFileName}`, ex); reject(ex); }); worker.on('exit', (code) => { + traceVerbose(`Worker ${workerFileName} exited with code ${code}`); if (code !== 0) { reject(new Error(`Worker ${workerFileName} stopped with exit code ${code}`)); } diff --git a/src/client/common/process/worker/workerPlainExec.ts b/src/client/common/process/worker/plainExec.worker.ts similarity index 51% rename from src/client/common/process/worker/workerPlainExec.ts rename to src/client/common/process/worker/plainExec.worker.ts index 705245ff6d1e..baf2d508def0 100644 --- a/src/client/common/process/worker/workerPlainExec.ts +++ b/src/client/common/process/worker/plainExec.worker.ts @@ -1,27 +1,6 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. -// Licensed under the MIT License. - -// !!!! IMPORTANT: DO NOT IMPORT FROM VSCODE MODULE AS IT IS NOT AVAILABLE INSIDE WORKER THREADS !!!! - -import { SpawnOptions } from 'child_process'; import { isMainThread, parentPort, workerData } from 'worker_threads'; -import { executeWorkerFile } from './main'; -import { ExecutionResult } from './types'; - import { _workerPlainExecImpl } from './workerRawProcessApis'; -export function workerPlainExec( - file: string, - args: string[], - options: SpawnOptions = {}, -): Promise> { - return executeWorkerFile(__filename, { - file, - args, - options, - }); -} - if (!isMainThread) { _workerPlainExecImpl(workerData.file, workerData.args, workerData.options) .then((res) => { diff --git a/src/client/common/process/worker/rawProcessApiWrapper.ts b/src/client/common/process/worker/rawProcessApiWrapper.ts new file mode 100644 index 000000000000..e6476df5d8fa --- /dev/null +++ b/src/client/common/process/worker/rawProcessApiWrapper.ts @@ -0,0 +1,26 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +import { SpawnOptions } from 'child_process'; +import * as path from 'path'; +import { executeWorkerFile } from './main'; +import { ExecutionResult, ShellOptions } from './types'; + +export function workerShellExec(command: string, options: ShellOptions): Promise> { + return executeWorkerFile(path.join(__dirname, 'shellExec.worker.js'), { + command, + options, + }); +} + +export function workerPlainExec( + file: string, + args: string[], + options: SpawnOptions = {}, +): Promise> { + return executeWorkerFile(path.join(__dirname, 'plainExec.worker.js'), { + file, + args, + options, + }); +} diff --git a/src/client/common/process/worker/workerShellExec.ts b/src/client/common/process/worker/shellExec.worker.ts similarity index 55% rename from src/client/common/process/worker/workerShellExec.ts rename to src/client/common/process/worker/shellExec.worker.ts index 6ba94825f11e..d6132a80db58 100644 --- a/src/client/common/process/worker/workerShellExec.ts +++ b/src/client/common/process/worker/shellExec.worker.ts @@ -1,20 +1,6 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. -// Licensed under the MIT License. - -// !!!! IMPORTANT: DO NOT IMPORT FROM VSCODE MODULE AS IT IS NOT AVAILABLE INSIDE WORKER THREADS !!!! - import { isMainThread, parentPort, workerData } from 'worker_threads'; -import { executeWorkerFile } from './main'; -import { ExecutionResult, ShellOptions } from './types'; import { _workerShellExecImpl } from './workerRawProcessApis'; -export function workerShellExec(command: string, options: ShellOptions): Promise> { - return executeWorkerFile(__filename, { - command, - options, - }); -} - if (!isMainThread) { _workerShellExecImpl(workerData.command, workerData.options, workerData.defaultEnv) .then((res) => { From eb312c4750bb82c047ed63092c5c8a8c1a18c797 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Fri, 17 Nov 2023 22:12:38 +0000 Subject: [PATCH 13/16] z --- build/webpack/webpack.extension.config.js | 1 + 1 file changed, 1 insertion(+) diff --git a/build/webpack/webpack.extension.config.js b/build/webpack/webpack.extension.config.js index 19eb3e0bea9e..c700a9a893bc 100644 --- a/build/webpack/webpack.extension.config.js +++ b/build/webpack/webpack.extension.config.js @@ -19,6 +19,7 @@ const config = { target: 'node', entry: { extension: './src/client/extension.ts', + shellExecWorker: './src/client/common/process/worker/shellExec.worker.ts', }, devtool: 'source-map', node: { From 54bbba9c79f5a427c0f00de21f3b4f48f324eecd Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Fri, 17 Nov 2023 23:59:57 +0000 Subject: [PATCH 14/16] s --- build/webpack/webpack.extension.config.js | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/build/webpack/webpack.extension.config.js b/build/webpack/webpack.extension.config.js index c700a9a893bc..24f43671ceca 100644 --- a/build/webpack/webpack.extension.config.js +++ b/build/webpack/webpack.extension.config.js @@ -17,10 +17,11 @@ const existingModulesInOutDir = common.getListOfExistingModulesInOutDir(); const config = { mode: 'production', target: 'node', - entry: { - extension: './src/client/extension.ts', - shellExecWorker: './src/client/common/process/worker/shellExec.worker.ts', - }, + entry: [ + './src/client/extension.ts', + './src/client/common/process/worker/shellExec.worker.ts', + './src/client/common/process/worker/plainExec.worker.ts', + ], devtool: 'source-map', node: { __dirname: false, From e30356c80a3ca8cb11e9c30d15f5700796389155 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Sat, 18 Nov 2023 00:08:26 +0000 Subject: [PATCH 15/16] z --- build/webpack/webpack.extension.config.js | 12 ++++++------ .../{plainExec.worker.ts => plainExecWorker.ts} | 0 .../common/process/worker/rawProcessApiWrapper.ts | 4 ++-- .../{shellExec.worker.ts => shellExecWorker.ts} | 0 4 files changed, 8 insertions(+), 8 deletions(-) rename src/client/common/process/worker/{plainExec.worker.ts => plainExecWorker.ts} (100%) rename src/client/common/process/worker/{shellExec.worker.ts => shellExecWorker.ts} (100%) diff --git a/build/webpack/webpack.extension.config.js b/build/webpack/webpack.extension.config.js index 24f43671ceca..ac915bcde27c 100644 --- a/build/webpack/webpack.extension.config.js +++ b/build/webpack/webpack.extension.config.js @@ -17,11 +17,11 @@ const existingModulesInOutDir = common.getListOfExistingModulesInOutDir(); const config = { mode: 'production', target: 'node', - entry: [ - './src/client/extension.ts', - './src/client/common/process/worker/shellExec.worker.ts', - './src/client/common/process/worker/plainExec.worker.ts', - ], + entry: { + extension: './src/client/extension.ts', + shellExecWorker: './src/client/common/process/worker/shellExecWorker.ts', + plainExecWorker: './src/client/common/process/worker/plainExecWorker.ts', + }, devtool: 'source-map', node: { __dirname: false, @@ -54,7 +54,7 @@ const config = { ], }, { - test: /\.worker\.js$/, + test: /\Worker\.js$/, use: { loader: 'worker-loader' }, }, ], diff --git a/src/client/common/process/worker/plainExec.worker.ts b/src/client/common/process/worker/plainExecWorker.ts similarity index 100% rename from src/client/common/process/worker/plainExec.worker.ts rename to src/client/common/process/worker/plainExecWorker.ts diff --git a/src/client/common/process/worker/rawProcessApiWrapper.ts b/src/client/common/process/worker/rawProcessApiWrapper.ts index e6476df5d8fa..b4fa2ab97db0 100644 --- a/src/client/common/process/worker/rawProcessApiWrapper.ts +++ b/src/client/common/process/worker/rawProcessApiWrapper.ts @@ -7,7 +7,7 @@ import { executeWorkerFile } from './main'; import { ExecutionResult, ShellOptions } from './types'; export function workerShellExec(command: string, options: ShellOptions): Promise> { - return executeWorkerFile(path.join(__dirname, 'shellExec.worker.js'), { + return executeWorkerFile(path.join(__dirname, 'shellExecWorker.js'), { command, options, }); @@ -18,7 +18,7 @@ export function workerPlainExec( args: string[], options: SpawnOptions = {}, ): Promise> { - return executeWorkerFile(path.join(__dirname, 'plainExec.worker.js'), { + return executeWorkerFile(path.join(__dirname, 'plainExecWorker.js'), { file, args, options, diff --git a/src/client/common/process/worker/shellExec.worker.ts b/src/client/common/process/worker/shellExecWorker.ts similarity index 100% rename from src/client/common/process/worker/shellExec.worker.ts rename to src/client/common/process/worker/shellExecWorker.ts From d1c0049a39a5335b2bb0ab388d3f059f9585527d Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Sat, 18 Nov 2023 00:18:47 +0000 Subject: [PATCH 16/16] s --- build/webpack/webpack.extension.config.js | 6 +++--- .../worker/{plainExecWorker.ts => plainExec.worker.ts} | 0 src/client/common/process/worker/rawProcessApiWrapper.ts | 4 ++-- .../worker/{shellExecWorker.ts => shellExec.worker.ts} | 0 4 files changed, 5 insertions(+), 5 deletions(-) rename src/client/common/process/worker/{plainExecWorker.ts => plainExec.worker.ts} (100%) rename src/client/common/process/worker/{shellExecWorker.ts => shellExec.worker.ts} (100%) diff --git a/build/webpack/webpack.extension.config.js b/build/webpack/webpack.extension.config.js index ac915bcde27c..f85d9ea7b2d2 100644 --- a/build/webpack/webpack.extension.config.js +++ b/build/webpack/webpack.extension.config.js @@ -19,8 +19,8 @@ const config = { target: 'node', entry: { extension: './src/client/extension.ts', - shellExecWorker: './src/client/common/process/worker/shellExecWorker.ts', - plainExecWorker: './src/client/common/process/worker/plainExecWorker.ts', + 'shellExec.worker': './src/client/common/process/worker/shellExec.worker.ts', + 'plainExec.worker': './src/client/common/process/worker/plainExec.worker.ts', }, devtool: 'source-map', node: { @@ -54,7 +54,7 @@ const config = { ], }, { - test: /\Worker\.js$/, + test: /\.worker\.js$/, use: { loader: 'worker-loader' }, }, ], diff --git a/src/client/common/process/worker/plainExecWorker.ts b/src/client/common/process/worker/plainExec.worker.ts similarity index 100% rename from src/client/common/process/worker/plainExecWorker.ts rename to src/client/common/process/worker/plainExec.worker.ts diff --git a/src/client/common/process/worker/rawProcessApiWrapper.ts b/src/client/common/process/worker/rawProcessApiWrapper.ts index b4fa2ab97db0..e6476df5d8fa 100644 --- a/src/client/common/process/worker/rawProcessApiWrapper.ts +++ b/src/client/common/process/worker/rawProcessApiWrapper.ts @@ -7,7 +7,7 @@ import { executeWorkerFile } from './main'; import { ExecutionResult, ShellOptions } from './types'; export function workerShellExec(command: string, options: ShellOptions): Promise> { - return executeWorkerFile(path.join(__dirname, 'shellExecWorker.js'), { + return executeWorkerFile(path.join(__dirname, 'shellExec.worker.js'), { command, options, }); @@ -18,7 +18,7 @@ export function workerPlainExec( args: string[], options: SpawnOptions = {}, ): Promise> { - return executeWorkerFile(path.join(__dirname, 'plainExecWorker.js'), { + return executeWorkerFile(path.join(__dirname, 'plainExec.worker.js'), { file, args, options, diff --git a/src/client/common/process/worker/shellExecWorker.ts b/src/client/common/process/worker/shellExec.worker.ts similarity index 100% rename from src/client/common/process/worker/shellExecWorker.ts rename to src/client/common/process/worker/shellExec.worker.ts