Skip to content
This repository has been archived by the owner on Sep 3, 2021. It is now read-only.

Look for winetricks in /usr/local/bin/winetricks #28

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

aljelly
Copy link

@aljelly aljelly commented Oct 21, 2018

If you use the manual installation script to install winetricks it gets installed to /usr/local/bin/winetricks, so this PR makes the script look in there too.

@NoXPhasma
Copy link

Why do you even use hardcoded paths? Just look in $PATH for winetricks. Something like this:

def find_executable(executable, path=None):
    """Find if 'executable' can be run. Looks for it in 'path'
    (string that lists directories separated by 'os.pathsep';
    defaults to os.environ['PATH']). Checks for all executable
    extensions. Returns full path or None if no command is found.
    """
    if path is None:
        path = os.environ['PATH']
    paths = path.split(os.pathsep)
    extlist = ['']
    for ext in extlist:
        execname = executable + ext
        if os.path.isfile(execname):
            return execname
        else:
            for p in paths:
                f = os.path.join(p, execname)
                if os.path.isfile(f):
                    return f
    else:
        return None

Based on https://gist.github.com/techtonik/4368898

@Sirmentio Sirmentio added the enhancement New feature or request label Nov 7, 2018
@Sirmentio
Copy link
Owner

Sirmentio commented Nov 7, 2018

I am wondering, why couldn't you just use $PATH instead of hardcoding it? I think paths shouldn't necessarily be hardcoded but rather picked by environment variables only if necessary.

@Sirmentio Sirmentio added the question Further information is requested label Nov 7, 2018
@Matoking
Copy link
Contributor

Matoking commented Nov 7, 2018

Why do you even use hardcoded paths? Just look in $PATH for winetricks. Something like this:

def find_executable(executable, path=None):
    """Find if 'executable' can be run. Looks for it in 'path'
    (string that lists directories separated by 'os.pathsep';
    defaults to os.environ['PATH']). Checks for all executable
    extensions. Returns full path or None if no command is found.
    """
    if path is None:
        path = os.environ['PATH']
    paths = path.split(os.pathsep)
    extlist = ['']
    for ext in extlist:
        execname = executable + ext
        if os.path.isfile(execname):
            return execname
        else:
            for p in paths:
                f = os.path.join(p, execname)
                if os.path.isfile(f):
                    return f
    else:
        return None

Based on https://gist.github.com/techtonik/4368898

Python 3.3+ already seems to have this as a built-in:

https://docs.python.org/3.7/library/shutil.html#shutil.which

or in other words

>>> import shutil
>>> shutil.which("winetricks")
'/bin/winetricks'

Seems like a no-brainer to me since we're on Python 3.

@NoXPhasma
Copy link

Yes I know. But by using the code above, it would also work on Python < 3.3. However, if backwards compatibility is not wanted/needed, then I would go with shutil.which as well.

@Matoking
Copy link
Contributor

Matoking commented Nov 7, 2018

Python 3.2 reached end-of-life on 2014-10-12 and I'm pretty sure most Proton users are running fairly modern software, so I don't think this will be an issue.

I've created a pull request for this under #34 which uses shutil.which.

@Sirmentio
Copy link
Owner

My stance with this is that I don't think the effort for supporting end of life python versions should particularly be needed, since they're end of life and all. Though that would probably give some problems to those who use really old LTS versions of certain Linux distros at some point. I suppose it's a balancing act of being in with supporting the latest technology but not keeping those who prefer stable distribution versions in the dust.

@aljelly
Copy link
Author

aljelly commented Nov 13, 2018

Why do you even use hardcoded paths?

It was because it was a quick edit to the original behavior so that it could find where winetricks was on my system, which is where winetricks would likely be for other people too who also did a manual install of winetricks (that's why I made this PR). Yes, #34 is a preferable solution and would also work in, say ~/.local/bin if winetricks was there.

I'd imagine you could probably just fallback to what is already done + what this PR does if you wanted to support old Python? I'm not really familiar with Python, is that something you can do in it?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request question Further information is requested
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants