Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fish_config: No module named 'distutils.spawn' #7514

Closed
Natsurii opened this issue Nov 28, 2020 · 11 comments
Closed

fish_config: No module named 'distutils.spawn' #7514

Natsurii opened this issue Nov 28, 2020 · 11 comments

Comments

@Natsurii
Copy link

Natsurii commented Nov 28, 2020

I was trying to setup fish on fresh install of Raspbian OS 32bit on RPi4.
Running fish_config threw a Python web error:
Traceback (most recent call last): File "/usr/share/fish/tools/web_config/webconfig.py", line 11, in <module> from distutils.spawn import find_executable ModuleNotFoundError: No module named 'distutils.spawn'

This behaviour also shows on sh -c 'env HOME=$(mktemp -d) fish'

Here is some technical details.
$ fish --version
fish, version 3.1.2

$ uname -a
Linux raspberrypi 5.4.72-v7l+ #1356 SMP Thu Oct 22 13:57:51 BST 2020 armv7l GNU/Linux

$ echo $TERM
xterm-256color

$python3 --version
Python 3.7.3

@mqudsi
Copy link
Contributor

mqudsi commented Nov 28, 2020

What method did you use to install fish?

@zanchey
Copy link
Member

zanchey commented Nov 28, 2020

Debian packages don't install distutils by default, so you need to apt install python3-distutils.

Either we should add a dependency to the Debian packages or drop the use of distutils.

@zanchey zanchey added this to the fish 3.2.0 milestone Nov 28, 2020
@faho
Copy link
Member

faho commented Nov 28, 2020

We use distutils for two really rather small things:

  • Checking if termux-open-url exists (via distutils.spawn.find_executable, which we explain elsewhere is quite bad and fails to check that the path is executable!)

  • Checking if the macOS version is >= 10.12.5 via distutils.LooseVersion

I'm just going to remove the former - we already check if $PATH includes a path with "com.termux" in it. If it turns out it's needed we can reuse the path checking we do elsewhere already.

The latter I'd replace with:

import platform

def isMacOS10_12_5_OrLater():
    """ Return whether this system is macOS 10.12.5 or a later version. """
    try:
        version = [int(x) for x in platform.mac_ver()[0].split(".")]
        if version[0] < 10: return False
        if version[0] > 10: return True
        if version[1] < 12: return False
        if version[1] > 12: return True
        if version[2] < 5: return False
        return True
    except:
        return False

If anyone on macOS could confirm this is working that'd be grand.

Alternatively we could remove that check altogether - macOS 10.12 has been EOL for over a year, and the last release was 10.12.6.

faho added a commit to faho/fish-shell that referenced this issue Nov 28, 2020
This writes super cheesy version checking, but allows us to remove
distutils.

Fixes fish-shell#7514.
@zanchey
Copy link
Member

zanchey commented Nov 28, 2020

I think you can just compare the version elements as a list directly:

def isMacOS10_12_5_OrLater():
    return [int(x) for x in platform.mac_ver()[0].split(".")] >= [10, 12, 5]

@faho
Copy link
Member

faho commented Nov 28, 2020

Oooh, that's cool. I'd have thought that does a simple pair-wise comparison, which would fail e.g. on 10.13.4, but it turns out it does a "compare as long as they're equal" thing.

It still needs a try/catch for when there's no mac_ver() to give because it's not a mac, but otherwise, yeah, that should work. As long as apple doesn't start giving version numbers in song instead, that is.

faho added a commit that referenced this issue Nov 28, 2020
Just copy that "find an executable" code we already have,
the one that was commented with "oh, btw, distutils.spawn.find_executable is bad",
and use it here as well.

Work towards #7514.
@faho faho closed this as completed in 9de809e Nov 28, 2020
@faho
Copy link
Member

faho commented Nov 28, 2020

Alright, let's just do it.

@zanchey I'm assuming we can drop the python3-distutils recommends from debian/control now?

@zanchey
Copy link
Member

zanchey commented Nov 29, 2020

@zanchey I'm assuming we can drop the python3-distutils recommends from debian/control now?

done in f396a43

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 28, 2021
@fish-shell fish-shell unlocked this conversation Dec 21, 2021
@floam
Copy link
Member

floam commented Dec 21, 2021

It still needs a try/catch for when there's no mac_ver() to give because it's not a mac, but otherwise, yeah, that should work. As long as apple doesn't start giving version numbers in song instead, that is.

I think mac_ver() exists everywhere.

@faho
Copy link
Member

faho commented Dec 21, 2021

I think mac_ver() exists everywhere.

  1. It exists but returns a list of empty strings, so we have to fiddle around it
  2. This was already fixed in 9de809e by just catching the exception and returning False

@floam
Copy link
Member

floam commented Dec 21, 2021

Comment just left because I was going over that code: I believe the right exception to catch is ValueError, basically everywhere, right?

@faho
Copy link
Member

faho commented Dec 21, 2021

The error you get on a non-mac system is:

ValueError: invalid literal for int() with base 10: ''

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 26, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants