path: Force-remove files in unlink()

[kvmajo]

Fixes #128.

Path.unlink() removes a file with a plain rm:

self.host.exec0("rm", self)

If the file is write-protected (e.g. -r--r--r--) but the containing
directory still permits deletion, GNU rm asks an interactive
confirmation question before removing it:

rm: remove write-protected regular file '...'?

Since tbot's channel has no way to answer that question, exec0()
never sees a shell prompt again and hangs indefinitely instead of
raising an error.

unlink() already resolves existence/missing_ok itself before
running rm, so nothing is lost by making the actual removal
non-interactive as well, via -f.

Testing

• pre-commit's pinned black, flake8, and mypy all pass on the
  changed file.
• selftest/tests/test_path.py has the same pass/fail counts before
  and after this change (two pre-existing, unrelated failures in
  test_rglob_error reproduce identically on unmodified master).
• Reproduced the actual bug: unlinking a chmod 400 file hangs
  (confirmed via timeout) on unmodified master, and completes cleanly
  on this branch.

[Rahix]

Hi, thanks for the contribution!

I just confirmed, Python pathlib's .unlink() also removes a 0400 file without complaining so we should do the same. I think your change is fine, but please add a new testcase that creates such an 0400 file and then unlinks it.

The -f makes me a bit anxious, I think we should add some more safeguards here. This would have been a good idea regardless, I wonder why I didn't do it back then already.

I just checked, busybox rm also supports -- to signal end of flags so maybe we should just add a -- before the self argument to ensure you can't pass filenames that will be interpreted as flags? Alternatively we could add a ./ to any non-absolute paths which may be a bit more portable.

[Rahix]

two pre-existing, unrelated failures in test_rglob_error reproduce identically on unmodified master

What were those about? I cannot reproduce them here and CI also seems happy... Can you open a new issue for this topic, please?

[kvmajo]

Good points, thanks! I've pushed two commits addressing both:

• path: Guard unlink() against flag-like filenames — adds -- before the path argument, so rm -f -- self.
• selftest: Add testcase for unlinking write-protected files — adds test_unlink_write_protected, which creates a 0400 file and confirms unlink() still removes it as long as the containing directory allows it (matching pathlib's behavior).

Ran the selftest suite locally (LocalhostBash/LocalhostAsh backends) and the new test passes alongside the existing path/unlink tests, no regressions.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 62.47%. Comparing base (1853255) to head (1371013).
⚠️ Report is 8 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #130      +/-   ##
==========================================
+ Coverage   62.44%   62.47%   +0.02%     
==========================================
  Files          53       53              
  Lines        3762     3765       +3     
==========================================
+ Hits         2349     2352       +3     
  Misses       1413     1413              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[kvmajo]

It's an assertion in test_rglob_error:

result = list(path.rglob("cpu"))
assert len(result) > 3

On my machine, /sys only has 3 entries literally named cpu: /sys/bus/cpu, /sys/kernel/reboot/cpu, /sys/devices/system/cpu. My best guess is the test's > 3 assumes a 4th one at /sys/fs/cgroup/cpu, which only exists under legacy cgroup v1/hybrid mounts. My machine runs pure cgroup v2 (unified hierarchy — cgroup2 on /sys/fs/cgroup), where CPU accounting is exposed as files, not a cpu-named directory, so there's nothing to make up that 4th match.

I haven't been able to confirm this against the actual CI runner's cgroup mode directly, so take it as a plausible explanation rather than a confirmed one — but it would fit why CI and your own machine are happy while mine isn't.

Given that, do you still want me to open a new issue for this or do you prefer to do it yourself?

[Rahix]

Given that, do you still want me to open a new issue for this or do you prefer to do it yourself?

Damn, I see, made too many assumptions about Linux systems again. Yes, please do open a new issue anyway to track it. Or alternatively, if you want, you can also send a PR to fix it: it's fine if we just reduce the >3 to >2 or more ideally, create a synthetic structure inside the testdir and rglob() against that.

[Rahix]

Looks all good, thanks a lot!

[kvmajo]

Issue #132 opened.