diff --git a/.config/nextest.toml b/.config/nextest.toml index f2537ce580356..22609e1d7b27d 100644 --- a/.config/nextest.toml +++ b/.config/nextest.toml @@ -22,3 +22,19 @@ slow-timeout = { period = "1s", terminate-after = 60 } # Show slow jobs in the final summary final-status-level = "slow" + +[profile.ci-short] +# Print out output for failing tests as soon as they fail, and also at the end +# of the run (for easy scrollability). +failure-output = "immediate-final" +# Do not cancel the test run on the first failure. +fail-fast = false + +status-level = "pass" + +# Mark tests that take longer than 1s as slow. +# Terminate after 60s as a stop-gap measure to terminate on deadlock. +slow-timeout = { period = "1s", terminate-after = 60 } + +# Show slow jobs in the final summary +final-status-level = "slow" diff --git a/.github/actions/cache-wsl-distro/action.yml b/.github/actions/cache-wsl-distro/action.yml new file mode 100644 index 0000000000000..100ede2a211d3 --- /dev/null +++ b/.github/actions/cache-wsl-distro/action.yml @@ -0,0 +1,89 @@ +name: "Create and cache WSL vhdx" + +inputs: + cache_key: + description: "The cache key to use with actions/cache" + required: true + cache_path: + description: "The path to use with actions/cache" + required: true + vhdx_file: + description: "The full path to the vhdx file restored by actions/cache" + required: true + distro: + description: "The distro name to use with wsl --import-in-place and Vampire/setup-wsl" + required: true + ext4_workspace: + description: "The path to the workspace directory on the wsl ext4 filesystem" + required: true + +runs: + using: "composite" + steps: + - name: "Install ${{ inputs.distro }} with build-essential & mold" + uses: Vampire/setup-wsl@f40fb59d850112c9a292b0218bca8271305b9127 # v5.0.0 + with: + distribution: ${{ inputs.distro }} + use-cache: "false" + additional-packages: build-essential mold + - name: "Validate Installation" + uses: ubuntu/WSL/.github/actions/wsl-bash@dd06b19f5acdd0d085ab539427980e49bb5e8143 # 2024-02-01 + with: + distro: ${{ inputs.distro }} + exec: | + id + uname -a + df -T + - name: "Install Toolchain" + uses: ubuntu/WSL/.github/actions/wsl-bash@dd06b19f5acdd0d085ab539427980e49bb5e8143 # 2024-02-01 + with: + distro: ${{ inputs.distro }} + exec: | + set -ex + export CI=1 + echo "::group::Rustup" + curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh -s -- -y + echo "::endgroup::" + echo "::group::Insta" + curl -LsSf https://insta.rs/install.sh | sh + echo "::endgroup::" + echo "::group::Nextest" + source $HOME/.cargo/env + curl -L --proto '=https' --tlsv1.2 -sSf https://raw.githubusercontent.com/cargo-bins/cargo-binstall/main/install-from-binstall-release.sh | bash + cargo binstall cargo-nextest --secure -y + echo "::endgroup::" + - name: "Create dir for ext4 workspace" + uses: ubuntu/WSL/.github/actions/wsl-bash@dd06b19f5acdd0d085ab539427980e49bb5e8143 # 2024-02-01 + with: + distro: ${{ inputs.distro }} + exec: | + mkdir --parents ${{ inputs.ext4_workspace }} + - name: "Export distro for cache" + shell: pwsh + env: + CACHE_PATH: ${{ inputs.cache_path }} + VHDX_FILE: ${{ inputs.vhdx_file }} + run: | + $ErrorActionPreference = 'Stop' + wsl --list + wsl --shutdown + New-Item -ItemType Directory -Path $env:CACHE_PATH -Force + wsl --export --vhd $env:DISTRO $env:VHDX_FILE + - name: "Check vhdx file exists" + id: check + shell: pwsh + env: + VHDX_FILE: ${{ inputs.vhdx_file }} + run: | + $ErrorActionPreference = 'Stop' + if (Test-Path $env:VHDX_FILE) { + exit 0 + } else { + Write-Error "File not found: $env:VHDX_FILE" + exit 1 + } + - name: "Write cache" + uses: actions/cache/save@v4 + with: + path: ${{ inputs.cache_path }} + key: ${{ inputs.cache_key }} diff --git a/.github/workflows/test_flake8_executable_wsl.yml b/.github/workflows/test_flake8_executable_wsl.yml new file mode 100644 index 0000000000000..1f99c116367a0 --- /dev/null +++ b/.github/workflows/test_flake8_executable_wsl.yml @@ -0,0 +1,216 @@ +# This workflow will check flake8_executable lints on: +# - Native Linux +# - WSL, using an ext4 filesystem +# - WSL, using a windows filesystem +# +# This is to ensure that EXE001 & EXE002 are automatically disabled +# where appropriate. +# +# It will run when: +# - this file is changed or +# - any of the flake8_executable code, tests, fixtures, ... are changed. +# +# On first run it will cache the installed WSL distro, as the initial +# installation is time-consuming. + +name: Check flake8_executable lints + +on: + push: + paths: + [ + ".github/workflows/test_flake8_executable_wsl.yml", + ".github/actions/cache-wsl-distro/action.yml", + "**/flake8_executable/**", + ] + workflow_dispatch: + +permissions: {} + +jobs: + find_wsl_cache: + name: "Check/Create WSL vhdx" + runs-on: windows-latest + timeout-minutes: 15 + env: + CACHE_FILE: "ubuntu_ruff.vhdx" + CACHE_DIR: ".wsl_cache" + DISTRO: "Ubuntu-24.04" + EXT4_WORKSPACE: "/workspaces/${{ github.repository }}" + outputs: + cache_key: ${{ steps.calc_vars.outputs.cache_key }} + cache_path: ${{ steps.calc_vars.outputs.cache_path }} + vhdx_file: ${{ steps.calc_vars.outputs.vhdx_file }} + distro: ${{ env.DISTRO }} + ext4_workspace: ${{ env.EXT4_WORKSPACE }} + steps: + # Using version numbers, not SHAs, throughout to avoid dependabot triggering this workflow often + - uses: actions/checkout@8e8c483db84b4bee98b60c0593521ed34d9990e8 # v6.0.1 + with: + persist-credentials: false + - id: calc_vars + name: "Calculate paths & key" + shell: pwsh + run: | + $ErrorActionPreference = 'Stop' + $cachePath = Join-Path ${{ runner.temp }} $env:CACHE_DIR + Write-Output "cache_path=$cachePath" + Write-Output "cache_path=$cachePath" >> $env:GITHUB_OUTPUT + $vhdxPath = Join-Path $cachePath $env:CACHE_FILE + Write-Output "vhdx_file=$vhdxPath" + Write-Output "vhdx_file=$vhdxPath" >> $env:GITHUB_OUTPUT + $workflowHash = (Get-FileHash .github/actions/cache-wsl-distro/action.yml).Hash + Write-Output "cache_key=wsl_distro_cache-$env:DISTRO-$workflowHash" + Write-Output "cache_key=wsl_distro_cache-$env:DISTRO-$workflowHash" >> $env:GITHUB_OUTPUT + - id: check-wsl-cache + uses: actions/cache/restore@9255dc7a253b0ccc959486e2bca901246202afeb # v5.0.1 + with: + key: ${{ steps.calc_vars.outputs.cache_key }} + path: ${{ steps.calc_vars.outputs.cache_path}} + lookup-only: true + - name: "Create and cache distro" + if: ${{ steps.check-wsl-cache.outputs.cache-hit != 'true'}} + uses: ./.github/actions/cache-wsl-distro + with: + cache_key: ${{ steps.calc_vars.outputs.cache_key }} + cache_path: ${{ steps.calc_vars.outputs.cache_path }} + vhdx_file: ${{ steps.calc_vars.outputs.vhdx_file }} + distro: ${{ env.DISTRO }} + ext4_workspace: ${{ env.EXT4_WORKSPACE }} + + test_wsl_ntfs: + needs: find_wsl_cache + name: "Run tests on WSL - mounted NTFS filesystem" + runs-on: windows-latest + timeout-minutes: 10 + steps: + - uses: actions/checkout@8e8c483db84b4bee98b60c0593521ed34d9990e8 # v6.0.1 + with: + persist-credentials: false + - name: "Get wsl distro from cache" + uses: actions/cache/restore@9255dc7a253b0ccc959486e2bca901246202afeb # v5.0.1 + with: + key: ${{ needs.find_wsl_cache.outputs.cache_key }} + path: ${{ needs.find_wsl_cache.outputs.cache_path }} + fail-on-cache-miss: true + - name: "Import cached Distro" + id: import-wsl + shell: pwsh + env: + VHDX_FILE: ${{ needs.find_wsl_cache.outputs.vhdx_file }} + DISTRO: ${{ needs.find_wsl_cache.outputs.distro }} + run: | + $ErrorActionPreference = 'Stop' + Write-Output "Updating WSL" + wsl --update + Write-Output "Importing $env:VHDX_FILE as $env:DISTRO" + wsl --import-in-place $env:DISTRO $env:VHDX_FILE + + # We use this --cd trick otherwise bash gets confused with the \ path separators + $workspacePath = wsl.exe -d $env:DISTRO --cd $env:GITHUB_WORKSPACE -- wslpath -ua . + Write-Output "WSL Path to Workspace: $workspacePath" + Write-Output "workspace_path=$workspacePath" >> $env:GITHUB_OUTPUT + # Using the wsl-native filesystem for storing build artifacts reduces build from 10 mins to 3 mins + - name: "Use wsl-native file system for ./target" + uses: ubuntu/WSL/.github/actions/wsl-bash@dd06b19f5acdd0d085ab539427980e49bb5e8143 # 2024-02-01 + with: + distro: ${{ needs.find_wsl_cache.outputs.distro }} + working-dir: ${{ steps.import-wsl.outputs.workspace_path }} + exec: | + set -ex + mkdir /tmp/target + mkdir target || true + mount --bind /tmp/target target + - name: "Run tests" + uses: ubuntu/WSL/.github/actions/wsl-bash@dd06b19f5acdd0d085ab539427980e49bb5e8143 # 2024-02-01 + with: + distro: ${{ needs.find_wsl_cache.outputs.distro }} + working-dir: ${{ steps.import-wsl.outputs.workspace_path }} + exec: | + set -ex + source $HOME/.cargo/env + export NEXTEST_PROFILE="ci-short" + RUFF_TEST_ENVIRONMENT="ntfs" cargo insta test --check --test-runner nextest --package ruff_linter -- flake8_executable + + test_wsl_ext4: + needs: find_wsl_cache + name: "Run tests on WSL Native filesystem" + runs-on: windows-latest + timeout-minutes: 10 + steps: + - name: "Get wsl distro from cache" + uses: actions/cache/restore@9255dc7a253b0ccc959486e2bca901246202afeb # v5.0.1 + with: + key: ${{ needs.find_wsl_cache.outputs.cache_key }} + path: ${{ needs.find_wsl_cache.outputs.cache_path }} + fail-on-cache-miss: true + - name: "Import cached Distro" + shell: pwsh + env: + VHDX_FILE: ${{ needs.find_wsl_cache.outputs.vhdx_file }} + DISTRO: ${{ needs.find_wsl_cache.outputs.distro }} + run: | + $ErrorActionPreference = 'Stop' + Write-Output "Updating WSL" + wsl --update + Write-Output "Importing $env:VHDX_FILE as $env:DISTRO" + wsl --import-in-place $env:DISTRO $env:VHDX_FILE + - name: Checkout + uses: ubuntu/WSL/.github/actions/wsl-checkout@0cbf324d47c733b1b5026a07357a9ec15ab11408 # 2023-05-30 + with: + distro: ${{ needs.find_wsl_cache.outputs.distro }} + working-dir: ${{ needs.find_wsl_cache.outputs.ext4_workspace }} + - name: "Run tests" + uses: ubuntu/WSL/.github/actions/wsl-bash@dd06b19f5acdd0d085ab539427980e49bb5e8143 # 2024-02-01 + with: + distro: ${{ needs.find_wsl_cache.outputs.distro }} + working-dir: ${{ needs.find_wsl_cache.outputs.ext4_workspace }} + exec: | + set -ex + source "$HOME/.cargo/env" + export NEXTEST_PROFILE="ci-short" + cargo insta test --check --test-runner nextest --package ruff_linter -- flake8_executable + + test_non_wsl: + strategy: + matrix: + os: [ubuntu-latest, windows-latest] + runs-on: ${{ matrix.os }} + name: "Test on ${{ matrix.os }}" + timeout-minutes: ${{ matrix.os == 'windows-latest' && 10 || 5 }} + steps: + - uses: actions/checkout@8e8c483db84b4bee98b60c0593521ed34d9990e8 # v6.0.1 + with: + persist-credentials: false + - uses: Swatinem/rust-cache@779680da715d629ac1d338a641029a2f4372abb5 # v2.8.2 + if: runner.os != 'Windows' + # Known performance problems windows - see https://github.com/Swatinem/rust-cache/issues/169 + - uses: actions/cache@9255dc7a253b0ccc959486e2bca901246202afeb # v5.0.1 + if: runner.os == 'Windows' + with: + path: | + ~/.cargo/bin/ + ~/.cargo/registry/index/ + ~/.cargo/registry/cache/ + ~/.cargo/git/db/ + target/ + key: ${{ runner.os }}-cargo-${{ hashFiles('**/Cargo.lock') }} + restore-keys: ${{ runner.os }}-cargo- + - name: "Install Rust toolchain" + run: rustup show + - name: "Install mold" + if: runner.os != 'Windows' + uses: rui314/setup-mold@e16410e7f8d9e167b74ad5697a9089a35126eb50 # v1 + - name: "Install cargo nextest" + uses: taiki-e/install-action@90558ad1e179036f31467972b00dec6cb80701fa # v2.66.3 + with: + tool: cargo-nextest + - name: "Install cargo insta" + uses: taiki-e/install-action@90558ad1e179036f31467972b00dec6cb80701fa # v2.66.3 + with: + tool: cargo-insta + - name: "Run tests" + shell: bash + env: + NEXTEST_PROFILE: "ci-short" + run: cargo insta test --check --test-runner nextest --package ruff_linter -- flake8_executable diff --git a/Cargo.lock b/Cargo.lock index cfc7eb8bf2f2a..4a8537a3c8e11 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1697,15 +1697,6 @@ dependencies = [ "rustversion", ] -[[package]] -name = "is-docker" -version = "0.2.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "928bae27f42bc99b60d9ac7334e3a21d10ad8f1835a4e12ec3ec0464765ed1b3" -dependencies = [ - "once_cell", -] - [[package]] name = "is-macro" version = "0.3.7" @@ -1729,16 +1720,6 @@ dependencies = [ "windows-sys 0.52.0", ] -[[package]] -name = "is-wsl" -version = "0.4.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "173609498df190136aa7dea1a91db051746d339e18476eed5ca40521f02d7aa5" -dependencies = [ - "is-docker", - "once_cell", -] - [[package]] name = "is_terminal_polyfill" version = "1.70.1" @@ -3194,7 +3175,6 @@ dependencies = [ "imperative", "insta", "is-macro", - "is-wsl", "itertools 0.14.0", "jiff", "libcst", diff --git a/Cargo.toml b/Cargo.toml index 3bf0d54e2b0c8..029450aef0770 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -117,7 +117,6 @@ indoc = { version = "2.0.4" } insta = { version = "1.35.1" } insta-cmd = { version = "0.6.0" } is-macro = { version = "0.3.5" } -is-wsl = { version = "0.4.0" } itertools = { version = "0.14.0" } jiff = { version = "0.2.0" } jod-thread = { version = "1.0.0" } diff --git a/crates/ruff_linter/Cargo.toml b/crates/ruff_linter/Cargo.toml index 91880d6e4e418..b7da0845648f5 100644 --- a/crates/ruff_linter/Cargo.toml +++ b/crates/ruff_linter/Cargo.toml @@ -42,7 +42,6 @@ globset = { workspace = true } hashbrown = { workspace = true } imperative = { workspace = true } is-macro = { workspace = true } -is-wsl = { workspace = true } itertools = { workspace = true } jiff = { workspace = true } libcst = { workspace = true } @@ -64,6 +63,7 @@ similar = { workspace = true } smallvec = { workspace = true } strum = { workspace = true } strum_macros = { workspace = true } +tempfile = { workspace = true } thiserror = { workspace = true } toml = { workspace = true } typed-arena = { workspace = true } diff --git a/crates/ruff_linter/build.rs b/crates/ruff_linter/build.rs new file mode 100644 index 0000000000000..ff2de1d5be86d --- /dev/null +++ b/crates/ruff_linter/build.rs @@ -0,0 +1,19 @@ +fn main() { + // Allow e.g. `#[cfg(test_environment="ntfs")]` to be used to select specific tests to run on different environments, + // when the environment cannot be easily identified with standard cfg flags + // + // This is currently used to validate special handling of EXE001 & EXE002 on mounted windows filesystems + // + // Set `RUFF_TEST_ENVIRONMENT` before running tests, to specify the execution environment + // Format is a list of values, separated using the OS-specific path separator (`:` on WSL/*nix, ";" on Windows) + // e.g. `RUFF_TEST_ENVIRONMENT="ntfs" cargo insta ...` + println!("cargo::rustc-check-cfg=cfg(test_environment, values(\"ntfs\"))"); + if let Some(list_of_values) = option_env!("RUFF_TEST_ENVIRONMENT") { + for value in std::env::split_paths(list_of_values) { + println!( + "cargo::rustc-cfg=test_environment=\"{}\"", + value.to_string_lossy() + ); + } + } +} diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_executable/EXE001_1.py b/crates/ruff_linter/resources/test/fixtures/flake8_executable/EXE001_1.py index 6e9bbae5790f8..5bc6458bf6e9c 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_executable/EXE001_1.py +++ b/crates/ruff_linter/resources/test/fixtures/flake8_executable/EXE001_1.py @@ -1,4 +1,8 @@ #!/usr/bin/python if __name__ == '__main__': - print('I should be executable.') + #EXE001 Checks for a shebang directive in a file that is not executable. + executable = False + shebang = True + rule_should = "fail" + print(f'EXE001 should {rule_should}: Executable: {executable}, shebang present: {shebang}') diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_executable/EXE001_1_ntfs.py b/crates/ruff_linter/resources/test/fixtures/flake8_executable/EXE001_1_ntfs.py new file mode 100644 index 0000000000000..88ec3984c4ac2 --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/flake8_executable/EXE001_1_ntfs.py @@ -0,0 +1,8 @@ +#!/usr/bin/python + +if __name__ == '__main__': + #EXE001 Checks for a shebang directive in a file that is not executable. + executable = False + shebang = True + rule_should = "pass" # Rule is not executed on file systems which do not support the executable bit + print(f'EXE001 should {rule_should}: Executable: {executable}, shebang present: {shebang}') diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_executable/EXE001_2.py b/crates/ruff_linter/resources/test/fixtures/flake8_executable/EXE001_2.py index 21c3d83e41be5..e6d94bf4cb154 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_executable/EXE001_2.py +++ b/crates/ruff_linter/resources/test/fixtures/flake8_executable/EXE001_2.py @@ -1,2 +1,6 @@ if __name__ == '__main__': - print('I should be executable.') + #EXE001 Checks for a shebang directive in a file that is not executable. + executable = False + shebang = False + rule_should = "pass" + print(f'EXE001 should {rule_should}: Executable: {executable}, shebang present: {shebang}') diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_executable/EXE001_3.py b/crates/ruff_linter/resources/test/fixtures/flake8_executable/EXE001_3.py index 6e9bbae5790f8..493e6eb30b7be 100755 --- a/crates/ruff_linter/resources/test/fixtures/flake8_executable/EXE001_3.py +++ b/crates/ruff_linter/resources/test/fixtures/flake8_executable/EXE001_3.py @@ -1,4 +1,8 @@ #!/usr/bin/python if __name__ == '__main__': - print('I should be executable.') + #EXE001 Checks for a shebang directive in a file that is not executable. + executable = True + shebang = True + rule_should = "pass" + print(f'EXE001 should {rule_should}: Executable: {executable}, shebang present: {shebang}') diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_executable/EXE002_1.py b/crates/ruff_linter/resources/test/fixtures/flake8_executable/EXE002_1.py index 21c3d83e41be5..477eeb72e1684 100755 --- a/crates/ruff_linter/resources/test/fixtures/flake8_executable/EXE002_1.py +++ b/crates/ruff_linter/resources/test/fixtures/flake8_executable/EXE002_1.py @@ -1,2 +1,6 @@ if __name__ == '__main__': - print('I should be executable.') + #EXE002 Checks for executable .py files that do not have a shebang. + executable = True + shebang = False + rule_should = "fail" + print(f'EXE002 should {rule_should}: Executable: {executable}, shebang present: {shebang}') diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_executable/EXE002_1_ntfs.py b/crates/ruff_linter/resources/test/fixtures/flake8_executable/EXE002_1_ntfs.py new file mode 100755 index 0000000000000..29ae2c3becb32 --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/flake8_executable/EXE002_1_ntfs.py @@ -0,0 +1,6 @@ +if __name__ == '__main__': + #EXE002 Checks for executable .py files that do not have a shebang. + executable = True + shebang = False + rule_should = "pass" # Rule is not executed on file systems which do not support the executable bit + print(f'EXE002 should {rule_should}: Executable: {executable}, shebang present: {shebang}') diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_executable/EXE002_2.py b/crates/ruff_linter/resources/test/fixtures/flake8_executable/EXE002_2.py index 21c3d83e41be5..3abf6fb6dddb7 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_executable/EXE002_2.py +++ b/crates/ruff_linter/resources/test/fixtures/flake8_executable/EXE002_2.py @@ -1,2 +1,6 @@ if __name__ == '__main__': - print('I should be executable.') + #EXE002 Checks for executable .py files that do not have a shebang. + executable = False + shebang = False + rule_should = "pass" + print(f'EXE002 should {rule_should}: Executable: {executable}, shebang present: {shebang}') diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_executable/EXE002_3.py b/crates/ruff_linter/resources/test/fixtures/flake8_executable/EXE002_3.py index 6e9bbae5790f8..9ed3ab6e17918 100755 --- a/crates/ruff_linter/resources/test/fixtures/flake8_executable/EXE002_3.py +++ b/crates/ruff_linter/resources/test/fixtures/flake8_executable/EXE002_3.py @@ -1,4 +1,8 @@ #!/usr/bin/python if __name__ == '__main__': - print('I should be executable.') + #EXE002 Checks for executable .py files that do not have a shebang. + executable = True + shebang = True + rule_should = "pass" + print(f'EXE002 should {rule_should}: Executable: {executable}, shebang present: {shebang}') diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_executable/pyproject.toml b/crates/ruff_linter/resources/test/fixtures/flake8_executable/pyproject.toml new file mode 100644 index 0000000000000..e69de29bb2d1d diff --git a/crates/ruff_linter/src/rules/flake8_executable/helpers.rs b/crates/ruff_linter/src/rules/flake8_executable/helpers.rs index be200dfb2c7be..34489e0f6a732 100644 --- a/crates/ruff_linter/src/rules/flake8_executable/helpers.rs +++ b/crates/ruff_linter/src/rules/flake8_executable/helpers.rs @@ -2,11 +2,33 @@ use std::os::unix::fs::PermissionsExt; use std::path::Path; +use std::sync::OnceLock; use anyhow::Result; +use tempfile::NamedTempFile; + +use crate::settings::LinterSettings; pub(super) fn is_executable(filepath: &Path) -> Result { let metadata = filepath.metadata()?; let permissions = metadata.permissions(); Ok(permissions.mode() & 0o111 != 0) } + +// Some file systems do not support executable bits. Instead, everything is executable. +// See #3110, #5445, #10084, #12941 +// +// Benchmarking shows better performance vs previous, incorrect approach of checking `is_wsl()` +// as long as we use a `OnceLock` and a simple test first (filemode of pyproject.toml). +static EXECUTABLE_BY_DEFAULT: OnceLock = OnceLock::new(); + +pub(super) fn executable_by_default(settings: &LinterSettings) -> bool { + *EXECUTABLE_BY_DEFAULT.get_or_init(|| { + is_executable(&settings.project_root.join("pyproject.toml")).unwrap_or(true) + // if pyproject.toml is executable or doesn't exist, run a slower check too: + && NamedTempFile::new_in(&settings.project_root) + .map_err(std::convert::Into::into) + .and_then(|tmpfile| is_executable(tmpfile.path())) + .unwrap_or(false) // Assume a normal filesystem in case of read-only, IOError, ... + }) +} diff --git a/crates/ruff_linter/src/rules/flake8_executable/mod.rs b/crates/ruff_linter/src/rules/flake8_executable/mod.rs index 765810d4103b7..88780b00b4238 100644 --- a/crates/ruff_linter/src/rules/flake8_executable/mod.rs +++ b/crates/ruff_linter/src/rules/flake8_executable/mod.rs @@ -2,7 +2,7 @@ pub(crate) mod helpers; pub(crate) mod rules; -#[cfg(unix)] +#[cfg(any(unix, windows))] #[cfg(test)] mod tests { use std::path::Path; @@ -11,13 +11,27 @@ mod tests { use test_case::test_case; use crate::registry::Rule; - use crate::test::test_path; + use crate::test::{test_path, test_resource_path}; use crate::{assert_diagnostics, settings}; - #[test_case(Path::new("EXE001_1.py"))] + #[cfg_attr( + all(unix, not(test_environment = "ntfs")), + test_case(Path::new("EXE001_1.py")) + )] + #[cfg_attr( + any(windows, test_environment = "ntfs"), + test_case(Path::new("EXE001_1_ntfs.py")) + )] #[test_case(Path::new("EXE001_2.py"))] #[test_case(Path::new("EXE001_3.py"))] - #[test_case(Path::new("EXE002_1.py"))] + #[cfg_attr( + all(unix, not(test_environment = "ntfs")), + test_case(Path::new("EXE002_1.py")) + )] + #[cfg_attr( + any(windows, test_environment = "ntfs"), + test_case(Path::new("EXE002_1_ntfs.py")) + )] #[test_case(Path::new("EXE002_2.py"))] #[test_case(Path::new("EXE002_3.py"))] #[test_case(Path::new("EXE003.py"))] @@ -31,25 +45,72 @@ mod tests { #[test_case(Path::new("EXE005_1.py"))] #[test_case(Path::new("EXE005_2.py"))] #[test_case(Path::new("EXE005_3.py"))] - fn rules(path: &Path) -> Result<()> { - if is_wsl::is_wsl() { - // these rules are always ignored on WSL, so skip testing them in a WSL environment - // see https://github.com/astral-sh/ruff/pull/21724 for latest discussion - return Ok(()); - } - + fn rules_no_pyproject_toml(path: &Path) -> Result<()> { let snapshot = path.to_string_lossy().into_owned(); + let settings = settings::LinterSettings::for_rules(vec![ + Rule::ShebangNotExecutable, + Rule::ShebangMissingExecutableFile, + Rule::ShebangLeadingWhitespace, + Rule::ShebangNotFirstLine, + Rule::ShebangMissingPython, + ]); + assert!(!&settings.project_root.join("pyproject.toml").exists(), "unexpected pyproject.toml found: {}", &settings.project_root.join("pyproject.toml").to_string_lossy()); let diagnostics = test_path( Path::new("flake8_executable").join(path).as_path(), - &settings::LinterSettings::for_rules(vec![ + &settings, + )?; + assert_diagnostics!(snapshot, diagnostics); + Ok(()) + } + + #[cfg_attr( + all(unix, not(test_environment = "ntfs")), + test_case(Path::new("EXE001_1.py")) + )] + #[cfg_attr( + any(windows, test_environment = "ntfs"), + test_case(Path::new("EXE001_1_ntfs.py")) + )] + #[test_case(Path::new("EXE001_2.py"))] + #[test_case(Path::new("EXE001_3.py"))] + #[cfg_attr( + all(unix, not(test_environment = "ntfs")), + test_case(Path::new("EXE002_1.py")) + )] + #[cfg_attr( + any(windows, test_environment = "ntfs"), + test_case(Path::new("EXE002_1_ntfs.py")) + )] + #[test_case(Path::new("EXE002_2.py"))] + #[test_case(Path::new("EXE002_3.py"))] + #[test_case(Path::new("EXE003.py"))] + #[test_case(Path::new("EXE003_uv.py"))] + #[test_case(Path::new("EXE003_uv_tool.py"))] + #[test_case(Path::new("EXE003_uvx.py"))] + #[test_case(Path::new("EXE004_1.py"))] + #[test_case(Path::new("EXE004_2.py"))] + #[test_case(Path::new("EXE004_3.py"))] + #[test_case(Path::new("EXE004_4.py"))] + #[test_case(Path::new("EXE005_1.py"))] + #[test_case(Path::new("EXE005_2.py"))] + #[test_case(Path::new("EXE005_3.py"))] + fn rules_with_pyproject_toml(path: &Path) -> Result<()> { + let snapshot = path.to_string_lossy().into_owned(); + let mut settings = settings::LinterSettings::for_rules(vec![ Rule::ShebangNotExecutable, Rule::ShebangMissingExecutableFile, Rule::ShebangLeadingWhitespace, Rule::ShebangNotFirstLine, Rule::ShebangMissingPython, - ]), + ]); + settings.project_root = test_resource_path("fixtures").join("flake8_executable"); + assert!(&settings.project_root.join("pyproject.toml").exists(), "{} not found", &settings.project_root.join("pyproject.toml").to_string_lossy()); + let diagnostics = test_path( + Path::new("flake8_executable").join(path).as_path(), + &settings, )?; assert_diagnostics!(snapshot, diagnostics); Ok(()) } + } diff --git a/crates/ruff_linter/src/rules/flake8_executable/rules/shebang_missing_executable_file.rs b/crates/ruff_linter/src/rules/flake8_executable/rules/shebang_missing_executable_file.rs index 301ea999d08a0..f749a428eb9c3 100644 --- a/crates/ruff_linter/src/rules/flake8_executable/rules/shebang_missing_executable_file.rs +++ b/crates/ruff_linter/src/rules/flake8_executable/rules/shebang_missing_executable_file.rs @@ -5,7 +5,7 @@ use ruff_macros::{ViolationMetadata, derive_message_formats}; use crate::Violation; use crate::checkers::ast::LintContext; #[cfg(target_family = "unix")] -use crate::rules::flake8_executable::helpers::is_executable; +use crate::rules::flake8_executable::helpers::{executable_by_default, is_executable}; /// ## What it does /// Checks for executable `.py` files that do not have a shebang. @@ -26,13 +26,20 @@ use crate::rules::flake8_executable::helpers::is_executable; /// Otherwise, remove the executable bit from the file /// (e.g., `chmod -x __main__.py` or `git update-index --chmod=-x __main__.py`). /// +/// ## Filesystem considerations +/// /// A file is considered executable if it has the executable bit set (i.e., its /// permissions mode intersects with `0o111`). As such, _this rule is only -/// available on Unix-like systems_, and is not enforced on Windows or WSL. +/// available on Unix-like filesystems_. +/// +/// It is not enforced on Windows, nor on Unix-like systems if the _project root_ +/// is located on a _filesystem which does not support Unix-like permissions_ +/// (e.g. mounting a removable drive using FAT or using /mnt/c/ on WSL). /// /// ## References /// - [Python documentation: Executable Python Scripts](https://docs.python.org/3/tutorial/appendix.html#executable-python-scripts) /// - [Git documentation: `git update-index --chmod`](https://git-scm.com/docs/git-update-index#Documentation/git-update-index.txt---chmod-x) +/// - [WSL documentation: Working across filesystems](https://learn.microsoft.com/en-us/windows/wsl/filesystems) #[derive(ViolationMetadata)] #[violation_metadata(stable_since = "v0.0.233")] pub(crate) struct ShebangMissingExecutableFile; @@ -47,17 +54,14 @@ impl Violation for ShebangMissingExecutableFile { /// EXE002 #[cfg(target_family = "unix")] pub(crate) fn shebang_missing_executable_file(filepath: &Path, context: &LintContext) { - // WSL supports Windows file systems, which do not have executable bits. - // Instead, everything is executable. Therefore, we skip this rule on WSL. - - if is_wsl::is_wsl() { - return; - } if let Ok(true) = is_executable(filepath) { - context.report_diagnostic_if_enabled( - ShebangMissingExecutableFile, - ruff_text_size::TextRange::default(), - ); + //nested for performance - no need to check filesystem unless this lint fails + if !executable_by_default(context.settings()) { + context.report_diagnostic_if_enabled( + ShebangMissingExecutableFile, + ruff_text_size::TextRange::default(), + ); + } } } diff --git a/crates/ruff_linter/src/rules/flake8_executable/rules/shebang_not_executable.rs b/crates/ruff_linter/src/rules/flake8_executable/rules/shebang_not_executable.rs index c9ea74d86bd94..3944fb7cf2bd8 100644 --- a/crates/ruff_linter/src/rules/flake8_executable/rules/shebang_not_executable.rs +++ b/crates/ruff_linter/src/rules/flake8_executable/rules/shebang_not_executable.rs @@ -29,6 +29,17 @@ use crate::rules::flake8_executable::helpers::is_executable; /// permissions mode intersects with `0o111`). As such, _this rule is only /// available on Unix-like systems_, and is not enforced on Windows or WSL. /// +/// ## Filesystem considerations +/// +/// A file is considered executable if it has the executable bit set (i.e., its +/// permissions mode intersects with `0o111`). As such, _this rule is only +/// available on Unix-like filesystems_. +/// +/// It is not enforced on Windows, and will never trigger on Unix-like systems +/// if the _project root_ is located on a _filesystem which does not support +/// Unix-like permissions_ (e.g. mounting a removable drive using FAT or using +/// /mnt/c/ on WSL). +/// /// ## Example /// ```python /// #!/usr/bin/env python @@ -37,6 +48,7 @@ use crate::rules::flake8_executable::helpers::is_executable; /// ## References /// - [Python documentation: Executable Python Scripts](https://docs.python.org/3/tutorial/appendix.html#executable-python-scripts) /// - [Git documentation: `git update-index --chmod`](https://git-scm.com/docs/git-update-index#Documentation/git-update-index.txt---chmod-x) +/// - [WSL documentation: Working across filesystems](https://learn.microsoft.com/en-us/windows/wsl/filesystems) #[derive(ViolationMetadata)] #[violation_metadata(stable_since = "v0.0.233")] pub(crate) struct ShebangNotExecutable; @@ -51,13 +63,6 @@ impl Violation for ShebangNotExecutable { /// EXE001 #[cfg(target_family = "unix")] pub(crate) fn shebang_not_executable(filepath: &Path, range: TextRange, context: &LintContext) { - // WSL supports Windows file systems, which do not have executable bits. - // Instead, everything is executable. Therefore, we skip this rule on WSL. - - if is_wsl::is_wsl() { - return; - } - if let Ok(false) = is_executable(filepath) { context.report_diagnostic_if_enabled(ShebangNotExecutable, range); } diff --git a/crates/ruff_linter/src/rules/flake8_executable/snapshots/ruff_linter__rules__flake8_executable__tests__EXE001_1_ntfs.py.snap b/crates/ruff_linter/src/rules/flake8_executable/snapshots/ruff_linter__rules__flake8_executable__tests__EXE001_1_ntfs.py.snap new file mode 100644 index 0000000000000..26e075ae3eb6e --- /dev/null +++ b/crates/ruff_linter/src/rules/flake8_executable/snapshots/ruff_linter__rules__flake8_executable__tests__EXE001_1_ntfs.py.snap @@ -0,0 +1,4 @@ +--- +source: crates/ruff_linter/src/rules/flake8_executable/mod.rs +--- + diff --git a/crates/ruff_linter/src/rules/flake8_executable/snapshots/ruff_linter__rules__flake8_executable__tests__EXE002_1_ntfs.py.snap b/crates/ruff_linter/src/rules/flake8_executable/snapshots/ruff_linter__rules__flake8_executable__tests__EXE002_1_ntfs.py.snap new file mode 100644 index 0000000000000..26e075ae3eb6e --- /dev/null +++ b/crates/ruff_linter/src/rules/flake8_executable/snapshots/ruff_linter__rules__flake8_executable__tests__EXE002_1_ntfs.py.snap @@ -0,0 +1,4 @@ +--- +source: crates/ruff_linter/src/rules/flake8_executable/mod.rs +--- +