Skip to content

Fix WSL detection in non-Docker containers#22879

Merged
ntBre merged 6 commits intomainfrom
brent/is-wsl
Mar 10, 2026
Merged

Fix WSL detection in non-Docker containers#22879
ntBre merged 6 commits intomainfrom
brent/is-wsl

Conversation

@ntBre
Copy link
Copy Markdown
Contributor

@ntBre ntBre commented Jan 26, 2026

Summary

I was initially just debugging astral-sh/ruff-action#307 with Claude and was quite skeptical when it started rewriting the is-wsl crate, but after taking a closer look at is-wsl and its dependency is-docker, I thought it might actually be worth doing.

This PR fixes astral-sh/ruff-action#307 by essentially inlining both the is-wsl and is-docker crates and expanding the is_docker check to account for additional types of containers. These crates are both one-file dependencies that haven't been updated in a couple of years.

As shown in the debug logs from my comment on the issue, the minimal change to resolve the problem for the ubuntu-slim GitHub runner is to check for /run/.containerenv in addition to /.dockerenv, but Claude added a few additional checks on the cgroup to cover LXC and kubernetes containers too.

Test Plan

I forked the reproduction repo shared in the issue and tested that the Ruff version on this branch produces an EXE001 diagnostic, as expected: brentbot/my-ruff-issue#1

image

Summary
--

I was initially just debugging astral-sh/ruff-action#307 with Claude and was
quite skeptical when it started rewriting the [`is-wsl`] crate, but after taking
a closer look at `is-wsl` and its dependency [`is-docker`], I thought it might
actually be worth doing.

This PR fixes astral-sh/ruff-action#307 by essentially inlining both the
`is-wsl` and `is-docker` crates and expanding the `is_docker` check to account
for additional types of containers. These crates are both one-file dependencies
that haven't been updated in a couple of years.

As shown in the debug logs from my comment on the issue, the minimal change to
resolve the problem for the `ubuntu-slim` GitHub runner is to check for
`/run/.containerenv` in addition to `/.dockerenv`, but Claude added a few
additional checks on the `cgroup` to cover LXC and kubernetes containers too.

Test Plan
--

I forked the reproduction repo shared in the issue and tested that the Ruff
version on this branch produces an `EXE001` diagnostic, as expected:
brentbot/my-ruff-issue#1

[`is-wsl`]: https://github.com/TheLarkInn/is-wsl/blob/main/src/lib.rs
[`is-docker`]: https://github.com/TheLarkInn/is-docker/blob/main/src/lib.rs
@astral-sh-bot
Copy link
Copy Markdown

astral-sh-bot bot commented Jan 26, 2026

Typing conformance results

No changes detected ✅

Current numbers
The percentage of diagnostics emitted that were expected errors held steady at 85.05%. The percentage of expected errors that received a diagnostic held steady at 78.05%. The number of fully passing files held steady at 63/132.

@astral-sh-bot
Copy link
Copy Markdown

astral-sh-bot bot commented Jan 26, 2026

mypy_primer results

No ecosystem changes detected ✅

@astral-sh-bot
Copy link
Copy Markdown

astral-sh-bot bot commented Jan 26, 2026

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

Formatter (stable)

✅ ecosystem check detected no format changes.

Formatter (preview)

✅ ecosystem check detected no format changes.

@ntBre ntBre added the rule Implementing or modifying a lint rule label Jan 27, 2026
@ntBre ntBre marked this pull request as ready for review January 27, 2026 00:31
@ntBre
Copy link
Copy Markdown
Contributor Author

ntBre commented Feb 18, 2026

I'm going to put this back in draft for now. I'll be curious to get @MichaReiser's thoughts when he gets back.

@ntBre ntBre marked this pull request as draft February 18, 2026 23:20
@ntBre ntBre requested a review from MichaReiser February 18, 2026 23:20
@MusicalNinjaDad
Copy link
Copy Markdown

FWIW #17584 also fixes this issue by resolving the underlying problem (I've just run a CI on ubuntu-slim with and without the changes to check)

Copy link
Copy Markdown
Member

@MichaReiser MichaReiser left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you say more about why we have to fork is-wsl? Is it because it depends on is-docker? I'm not sure if it's worth it but we could consider creating an upstream PR to is-docker, they certainly have more background knowledge than I to review the change.

We can then still decide to fork the project if the PR remains unmerged after a week or so.

If we decide to fork the crates, it's important that we attribute the source of the code by including a header like this / Adapted from <crate> by <author>, MIT License

@ntBre
Copy link
Copy Markdown
Contributor Author

ntBre commented Mar 5, 2026

Can you say more about why we have to fork is-wsl? Is it because it depends on is-docker? I'm not sure if it's worth it but we could consider creating an upstream PR to is-docker, they certainly have more background knowledge than I to review the change.

Yes, is-wsl depends on is-docker. Both are one-file dependencies from the same maintainer. Both were last updated 3 years ago, which is why I didn't try opening a PR upstream, but I certainly could try. Their READMEs both say they're Rust ports of https://github.com/sindresorhus/is-wsl and https://github.com/sindresorhus/is-docker from JS.

If we decide to fork the crates, it's important that we attribute the source of the code by including a header like this / Adapted from <crate> by <author>, MIT License

Ah okay, makes sense.

I'm also okay closing this if we'd rather not bother. I just got curious and found a fix but wasn't confident it was a good one.

@MichaReiser
Copy link
Copy Markdown
Member

I'm also okay closing this if we'd rather not bother. I just got curious and found a fix but wasn't confident it was a good one.

The fix itself seems worthwhile to me.

@ntBre
Copy link
Copy Markdown
Contributor Author

ntBre commented Mar 5, 2026

Sounds good, I'll try a PR upstream.

@MichaReiser
Copy link
Copy Markdown
Member

MichaReiser commented Mar 5, 2026

Yeah, is-docker seems more abandoned than is-wsl. Could you use cargo's dependency override feature to only fork is-docker (I don't think it's worth spending much time on but could be nice)?

@ntBre
Copy link
Copy Markdown
Contributor Author

ntBre commented Mar 5, 2026

I think I might have to create a crate for that? I'm not sure but I'll give it a try!

@MichaReiser
Copy link
Copy Markdown
Member

I think I might have to create a crate for that? I'm not sure but I'll give it a try!

Oh yeah, that might be more work than it's worth

@astral-sh-bot
Copy link
Copy Markdown

astral-sh-bot bot commented Mar 5, 2026

Memory usage report

Memory usage unchanged ✅

@ntBre
Copy link
Copy Markdown
Contributor Author

ntBre commented Mar 5, 2026

I prepared a patch for is-docker and confirmed that the issue can also be resolved by patching our Cargo.toml, but I guess this is likely to be out of scope for is-docker since it adds support for non-Docker containers.

Patches

diff --git a/Cargo.toml b/Cargo.toml
index 1a7e482d66..0ee9faddd8 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -215,6 +215,9 @@ which = { version = "8.0.0" }
 wild = { version = "2" }
 zip = { version = "0.6.6", default-features = false }
 
+[patch.crates-io]
+is-docker = { path = "../other/is-docker" }
+
 [workspace.metadata.cargo-shear]
 ignored = [
     "getrandom",
diff --git a/src/lib.rs b/src/lib.rs
index 724f9f6..6d15416 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -3,13 +3,19 @@ extern crate once_cell;
 use std::fs;
 use once_cell::sync::OnceCell;
 
-fn has_docker_env_file() -> bool {
+fn has_env_file() -> bool {
     fs::metadata("/.dockerenv").is_ok()
+        || fs::metadata("/run/.containerenv").is_ok()
 }
 
 fn has_docker_in_cgroup() -> bool {
     match fs::read_to_string("/proc/self/cgroup") {
-        Ok(file_contents) => file_contents.contains("docker"),
+        Ok(file_contents) => {
+            file_contents.contains("docker")
+                || file_contents.contains("/container")
+                || file_contents.contains("/lxc")
+                || file_contents.contains("kubepods")
+        }
         Err(_error) => false,
     }
 }
@@ -18,6 +24,6 @@ pub fn is_docker() -> bool {
     static CACHED_RESULT: OnceCell<bool> = OnceCell::new();
 
     *CACHED_RESULT.get_or_init(|| {
-        has_docker_env_file() || has_docker_in_cgroup()
+        has_env_file() || has_docker_in_cgroup()
     })   
 }

@ntBre ntBre marked this pull request as ready for review March 10, 2026 15:36
@astral-sh-bot astral-sh-bot bot requested a review from amyreese March 10, 2026 15:37
@ntBre ntBre enabled auto-merge (squash) March 10, 2026 15:41
@ntBre ntBre merged commit 055fb30 into main Mar 10, 2026
50 checks passed
@ntBre ntBre deleted the brent/is-wsl branch March 10, 2026 15:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

rule Implementing or modifying a lint rule

Projects

None yet

Development

Successfully merging this pull request may close these issues.

action on ubuntu-slim runner disables EXE001

4 participants