Use non-fully-qualified path for ifconfig on linux

[mk-fg]

That way, actual path should be resolved through $PATH.

It's /bin/ifconfig on exherbo and should be /usr/sbin/ifconfig on fedora after "/usr migration", and quick internet search reveals /usr/bin/ifconfig in ubuntu forums.

Filed ~1y ago as #1536 in trac.

[warner]

hm, that should make it follow $PATH instead of only looking in /sbin/ifconfig. But won't that fail if the user doesn't have /sbin in their $PATH?

Maybe we need to modify this "_tool_map" thing to include a list of potential paths, instead of just a single string?

[mk-fg]

Indeed it will fail.

$PATH should give user (and distro) a lot more flexibility than hard-coded paths.
Also, it is usually expected behavior, consistent with other tools.
I certainly expect to be able to write some "ifconfig" wrapper around iproute2 ("ip addr") and put it to ~/bin (which is in $PATH).

But I also think that failing when being ran with "env -i tahoe" (no $PATH set) is bad, too.
Plus, calling sutff from $PATH might somewhat increase attack surface from security perspective.
Short list of paths should work in vast majority of cases.

So, as a compromise between these, I propose looking at a list of hard-coded paths, then falling back to $PATH for ifconfig. Lacking any objections, guess I'll update the branch to do that shortly.

The most annoying thing about this error though is quite non-descriptive error message (as presented in #1536), so if paths are hard-coded, imho that's what should be fixed, at least.

[mk-fg]

Upon more consideration, I came to a conclusion that it should be either "only $PATH" or "don't use $PATH at all", because the main benefit from it imho is expected behavior, and it's lost if $PATH isn't searched first.

So I guess I'd rather extend hard-coded list of paths and provide a better error message if none of these suffice.

[mk-fg]

Please see the last commit.

List of paths is generated by map('/'.join, itertools.product(['/sbin', '/bin', '/usr/sbin', '/usr/bin'], ['ifconfig'])).

I've also noticed that some fallback seem to be in place (in foolscrap?) in case get_local_addresses_async() returns an empty list (which seem to do the right thing on local machine), so unified code-path to never raise an exception in that function, just printing failure to log instead.

[kensington]

Ping.

[mk-fg]

@kensington: did it bit you as well?

Fwiw, I've seen following lines (apologies if a bit out of context) in the #tahoe-lafs@freenode (~2012-12-16):

<marcusw> warner: do you remember what we decided regarding netifaces dep
  for foolscap? I remember discussing with DS, but don't remember what the
  conclusion was :/
<warner> marcusw: I think we decided to make foolscap depend on that netifaces
  package, although now I want to find some notes and see if I really felt
  comfortable with that
...
<davidsarah`> (20:32:47) zooko`: Sigh. Re: netifaces: I don't even really value
  autodiscovery of your own IP address very much.
...
<davidsarah`> (20:33:50) zooko: Okay, and if for ipv6 or some other reason
  we're determined to go ahead and keep doing IP addr autodiscovery, then the
  best way to do it is by invoking "ifconfig" or "route.exe" in a subprocess.

So I'm a bit confused about whether the PR is still relevant - if something like netifaces is already a dependency, then it might make sense to implement detection using that module, but I'm quite unsure about whether that dep is agreed on (as @davidsarah quoted different opinion from @warner's).

[kensington]

@mk-fg: Indeed it did, with your diff correcting the issue for me.

[daira]

The netifaces dep is not agreed on yet. We tend to be fairly conservative about adding new deps; it's very hard to get rid of them once they've been added.

[zooko]

Hi, I didn't see this thread until just now. Sorry about that. I need to get a more reliable way to monitor https://tahoe-lafs trac + github.

Anyway, let's open a separate ticket to consider adding a dep on netifaces. I would, at this point, be totally +1 on adding that dep (contrary to what I wrote on IRC a few weeks ago, that got pasted into this thread), except that netifaces is a C module exported through the CPython API. So I'm -1 on adding that dep.

In any case, let's open a trac ticket to record our decision on that.

[zooko]

See https://tahoe-lafs.org/trac/tahoe-lafs/ticket/1988

[daira]

I think netifaces is likely to cause as many portability problems as it solves, TBH, so I'm definitely -1 on that approach now.

[daira]

Note that /sbin is normally not on the PATH (nor should it be) for users without admin privileges. The hardcoded ['/sbin', '/bin', '/usr/sbin', '/usr/bin'] is fine by me though.

[daira]

Obsolete pull request.