tabreturn / thonny-py5mode

A py5 plug-in for Thonny
Do What The F*ck You Want To Public License
23 stars 8 forks source link

Portable support enhancement ideas #40

Closed villares closed 4 months ago

villares commented 2 years ago

I'm looking at this situation:

Then, when I move the portable folder arround, it breaks: The JDK is there in the right place, but the JAVA_HOME information in Thonny's configs/options got outdated.

It can be hard for beginners to understand what went wrong. If you uncheck and check the "imported mode for py5" menu a new updated JAVA_HOME line is added to the Thonny options, but the old one is not removed, and as far as I have tested this still won't work (you have to remove the outaded line, I think).

How could we improve this?

I had a proof of concept portable with everything already installed, but it counted on a laucher that would set JAVA-HOME every time, and also I had to modify thonny-py5mode so it would not add JAVA_HOME to Thonny options... I think this was far from ideal. So I'd like to enable people to build their own portables, and move them arround: that would be a good thing.

Is it too dangerous to check during plug-in loading if Thonny is portable and if so if JDK-17 is already installed and somehow act on it?

I hate to add too much magic and to override something the user could change in the options panel manually but maybe for this kind of beginner friendly tool it's reasonable. I'm really not sure at all how we should proceed.

villares commented 2 years ago

Some ideas:

# proposed change
def set_java_home(jdk_path: pathlib.Path) -> None:
    '''
    Add jdk path to current envirnonment and to Thonny's config file
    as visible in (tools > options > general > env vars)
    '''
    os.environ['JAVA_HOME'] = str(pathlib.Path(THONNY_USER_DIR) / jdk_path)
    jdk_path = 'JAVA_HOME=' + str(jdk_path)
    env_vars = get_workbench().get_option('general.environment')
    # showinfo('debug', env_vars)
    for i, var in enumerate(env_vars):
        if 'JAVA_HOME' in var:
           env_vars[i] = jdk_path
           break
    else:
        env_vars.append(jdk_path)
    # showinfo('debug', env_vars)
    get_workbench().set_option('general.environment', env_vars)

UPDATE 1: looks like env_vars is a list.. I thought it would be a multiline string... UPDATE 2: I tried calling jdk_install_exists() on plug-in load (afterr the modification above to set_java_home()) but it is not soon enough to make py5 work. It only sees the env_vars modification in the next launch. UPDATE 3: Now set_java_home() also sets the current envirnoment with os.environ['JAVA_HOME'] = str(pathlib.Path(THONNY_USER_DIR) / jdk_path), as it was doing after the download... Is this dangerous?

So... this seems to work. Should I check if this is a portable before I overwrite the JAVA_HOME from the config? Should I ask the user for permission? Will set_java_home() even be called if a valid JAVA_HOME was set for starts? (maybe it won't then we are safe).

UPDATE 4: I can't wrap my head arround this right now, but I'm sure that with a little tought we could change the following function to only search for the jdk folder, and actually call set_java_home, when a valid JAVA_HOME is not already set. And that would be a bit safer, not overriding some user's manual (and valid!) config.

UPDATE 5: Well, I couldn't resist working on it a bit more. See if this makes any sense, pelase:

# proposed change
def jdk_install_exists() -> bool:
    '''check system for existing jdk that meets the py5 version requirements'''
    jdk_dir = 'jdk-' + str(_REQUIRE_JDK)
    system_jdk = 'TBD'
    # check system for existing jdk install (and the version number)
    if os.environ.get('JAVA_HOME') is not None:
        jdk_ver = os.environ['JAVA_HOME'].split('jdk-')[-1].split('.')[0]
        system_jdk = jdk_ver
    # if no JAVA_HOME was set, or if it is not the required version
    if not system_jdk.isdigit() or int(system_jdk) < _REQUIRE_JDK:
        # search for jdk in Thonny's user config directory
        for name in os.listdir(THONNY_USER_DIR):
            if name.startswith(jdk_dir):
                # set JAVA_HOME in Thonny's config with jdk_dir path 
                set_java_home(pathlib.Path(THONNY_USER_DIR) / jdk_dir)
                return True
        # no jdk directory was found
        return False
    else:
        return True
villares commented 2 years ago

Testing this... it still is some trouble, a useless JAVA_HOME setting remained in Thonny options after moving the folder around... I have to think harder.

UPDATE: I'll try this...

def jdk_install_exists() -> bool:
    '''
    Look for a jdk in Thonny's user directory first, and if one is present,
    set up JAVA_HOME, add it to Thonny's options, and return True. Otherwise,
    look for existing JAVA_HOME pointing to a jdk that meets the py5 version
    requirements and return True/False accordingly.
    '''
    # search for jdk in Thonny's user config directory
    jdk_dir = 'jdk-' + str(_REQUIRE_JDK)
    for name in os.listdir(THONNY_USER_DIR):
        if name.startswith(jdk_dir):
            # set JAVA_HOME in Thonny's config (and in current env) with jdk_dir path 
            set_java_home(pathlib.Path(THONNY_USER_DIR) / jdk_dir)
            return True    
    # check system for existing jdk install (and the version number)
    system_jdk = 'TBD'
    if os.environ.get('JAVA_HOME') is not None:
        jdk_ver = os.environ['JAVA_HOME'].split('jdk-')[-1].split('.')[0]
        system_jdk = jdk_ver
    # False if no JAVA_HOME was set, or if it is not the required version
    return system_jdk.isdigit() and int(system_jdk) >= _REQUIRE_JDK  
villares commented 1 year ago

@tabreturn I think I have implemented the ideas on this thread in my latest portable experiment, but it was some weeks ago I'm not sure anymore. It would be great if you could test it when you have the time (and if you could validate my assumptions and choices):

https://www.dropbox.com/s/3ue4cx3yf372teg/thonny-4-with-py5-windows-portable.zip?dl=0

tabreturn commented 1 year ago

Sorry @villares -- I've been meaning to get back to this. Work has been manic, but I'll try look at this later in the week.

villares commented 1 year ago

No hurry! Thanks! (I should be writing for the PhD...)

villares commented 1 year ago

@tabreturn I think I have implemented the ideas on this thread in my latest portable experiment, but it was some weeks ago I'm not sure anymore. It would be great if you could test it when you have the time (and if you could validate my assumptions and choices):

https://www.dropbox.com/s/3ue4cx3yf372teg/thonny-4-with-py5-windows-portable.zip?dl=0

Cheers @tabreturn!

Are you planning to do a new release of thonny-py5mode? I think (I might be wrong) that there are stuff you merged and improved here that are not yet in the PyPI release...

If you could validate the hack I made in the portable version for setting JAVA_HOME even when the portable folder is moved arround, I think this would be something good to incorporate before the next release...

tabreturn commented 1 year ago

Just got back to working on the plug-in, @villares. I've started with upgrading install_jdk, but I'm facing a few issues with the new version. I'll see how that goes, maybe roll back if I'm not getting anywhere and focus on this issue.