setup-ip: Invoke 'ip' via $PATH #516
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #516 +/- ##
==========================================
- Coverage 70.57% 70.14% -0.43%
==========================================
Files 3 3
Lines 469 469
==========================================
- Hits 331 329 -2
- Misses 138 140 +2 ☔ View full report in Codecov by Sentry. |
|
For tools normally in /bin (or similar), I would agree that calling by just name is okay. But for tools in /sbin (or similar), better not, ironically exactly because of Debian - as (/usr)/sbin is not in default |
|
Ah, good point. While I'm not sure it's the tradeoff [1] I prefer that's a valid reason. Would you like a more neutral commit message? (Didn't just push one, since you already added the openqa-pending label) [1]: For example it makes local overrides with /usr/local or similar harder. |
|
Yes, you can update commit message. I've recently added better reporting which commit was used by openqa, so there is no reason to avoid (especially non-functional) changes when label is already added (at worst, the test will need to be repeated, but if some changes needing re-tested are necessary, that would be the case anyway). |
|
And BTW, on Fedora there is only |
Debian recently [1] removed the symlink in /sbin which breaks our
setup-ip script. Fedora on the other hand has 'ip' only in {/usr,}/sbin.
So invoke ip without specifying a path and rely on the shell's $PATH.
[1]: https://salsa.debian.org/kernel-team/iproute2/-/commit/c4bb148dd4ed0601ca32ee8a458007d0c348d6c3
Ah well, then relying on $PATH is clearly the easiest solution.
Good to know. Wasn't sure if I would abort and/or trigger a retest, which would be undesirable for an edited commit message. Pushed updated commit message. |
HW42
changed the title
OpenQA test summaryComplete test suite and dependencies: https://openqa.qubes-os.org/tests/overview?distri=qubesos&version=4.3&build=2024080713-4.3&flavor=pull-requests Test run included the following:
New failures, excluding unstableCompared to: https://openqa.qubes-os.org/tests/overview?distri=qubesos&version=4.3&build=2024070519-4.3&flavor=update
Failed tests9 failures
Fixed failuresCompared to: https://openqa.qubes-os.org/tests/105374#dependencies 7 fixed
Unstable tests
|
Debian recently 1 removed the symlink in /sbin which breaks our setup-ip script.
Instead of hardcoding the new path, rely on normal $PATH resolution. Saving a few file existence checks isn't worth such breakage, I think (or if that really matters here, shell is the wrong tool in the first place).
Assuming you agree with my reasoning above, one could argue to apply this change to all invocations of programs normally in the $PATH. Given that usage was already inconsistent I for now kept the change minimal instead.
The fix (this version or an update to /bin/ip) should be backported to 4.2.