Python 3 Port[In progress]

[Saumya-Mishra9129]

Trying to port from python 2 to 3 , as Sugar-toolkit is also ported , I am getting this error, get stuck at this.Kindly look at error.

and do review also.Thanks

[chimosky]

The activity hasn't been ported to Gtk3, you should do that first. The error you got is as a result of that.
Gtk3 activities use toolbarbox and not toolbox.

[quozl]

@Saumya-Mishra9129, next time please copy and paste errors into GItHub markdown text blocks, using ``` to delimit. Images are expensive, and can't be searched.

[Saumya-Mishra9129]

@quozl Okay sure.
@chimosky thanks. Port to Gtk3 is still in progress. I am working on it.

[chimosky]

Port to Gtk3 is in progress. I am working on it.

Great, we'll wait for it.

[Saumya-Mishra9129]

@pro-panda @quozl @chimosky I have tried to fix toolbar and Vte errors . Please Review. Also some of the methods are removed from Vte.Terminal class in Vte 2.91 as fork_command(), set_emulation() and set_visible_bell(). I couldn't figure out what to do in such cases. Can you suggest here.??

[chimosky]

I think spawn_async replaced fork_command, the changelog confirms that, one thing you can do is check what those functions do in python2 by importing vte in a python2 REPL and find out how to do those things with Vte.Terminal.

[Saumya-Mishra9129]

@srevinsaju I looked at Terminal activity which also contains Vte. You have worked there , Can you have a look at this activity?

[srevinsaju]

@Saumya-Mishra9129 i will check

[quozl]

@Saumya-Mishra9129, the Vte library was changed a few times, may I suggest;

• search for the activities that instantiate Vte.Terminal,
• look through the commits in those activities for changes in support of the various versions of Vte,
• search older versions of those activities for those removed methods, then search for commits that ported to later Vte.

[srevinsaju]

@Saumya-Mishra9129 Please review Saumya-Mishra9129#1

Currently the frotz binary doesn't work;
This is possibly because some of the needed binaries doesn't work
Please recompile frotz and include all the necessary libs within lib to make this work

To include LD_LIBRARY_PATH, you can mention it in envv args in spawn_sync. I have not used spawn_async, instead used spawn_sync. I hope it would be equally good

[Saumya-Mishra9129]

I have not used spawn_async, instead used spawn_sync. I hope it would be equally good

@srevinsaju I just looked at VTE documentation and found that spawn_sync is deprecated since version 0.48. For your reference -https://lazka.github.io/pgi-docs/Vte-2.91/classes/Terminal.html#Vte.Terminal.spawn_sync

[srevinsaju]

@Saumya-Mishra9129
See Saumya-Mishra9129#2

---

Although deprecated; its still usable; Anyways, I have used spawn_async on PR 2

[chimosky]

Reviewed f38db26.

An issue wasn't opened for frotz binaries in this activity but I think you should remove the binaries, the binaries exists for users who would run the activity on an XO laptop; porting to python3 removes the need for that.

• Change to using system specific package for frotz and specify dependency in README.

[quozl]

• Change to using system specific package for frotz and specify dependency in README.

Agreed. This should also result in a fix for #2.

Reminder of standing advice for future use: make sure the activity works properly before porting, close any solved issues, merge any pull requests or branches and release the last Python 2 version.

[quozl]

Re: d57cfd5, my preference is to use Vte.Terminal.spawn_sync until it is unavailable. Vte.Terminal.spawn_async is new, and not available, for example, in Ubuntu 18.04. Moving quickly just because someone shouts deprecated at us breaks compatibility with older operating systems without a good reason.

Or, you can use hasattr and support both methods.

[Saumya-Mishra9129]

Re: d57cfd5, my preference is to use Vte.Terminal.spawn_sync until it is unavailable. Vte.Terminal.spawn_async is new, and not available, for example, in Ubuntu 18.04. Moving quickly just because someone shouts deprecated at us breaks compatibility with older operating systems without a good reason.

Thanks , I tested spawn_async is not working with Ubuntu 18.04.

Or, you can use hasattr and support both methods

Yeah It can be best.

[Saumya-Mishra9129]

the start button is misleading; it does nothing except in the scenario where frotz is not installed; rather than use feed child to send install commands to the terminal, use a subprocess, or packagekit.

Thanks @quozl It is fixed in 52f914c.

[Saumya-Mishra9129]

clicking on "Get more games" just leads me to a Journal object for a link; it should launch Browse, (see Terminal for how),

Only this needs a fix.

[Saumya-Mishra9129]

a164e54 It does not work this error in log:

  File "/usr/share/sugar/activities/frotz/frotz.py", line 247, in _get_games_cb
    subprocess.Popen(cmd, shell=False)
  File "/usr/lib/python3.7/subprocess.py", line 775, in __init__
    restore_signals, start_new_session)
  File "/usr/lib/python3.7/subprocess.py", line 1522, in _execute_child
    raise child_exception_type(errno_num, err_msg, err_filename)
FileNotFoundError: [Errno 2] No such file or directory: 'xdg-open http://wiki.laptop.org/go/Frotz/Games': 'xdg-open http://wiki.laptop.org/go/Frotz/Games'

[quozl]

I suggest using launch_bundle as used by sugarterm.py in Terminal activity.

[Saumya-Mishra9129]

I suggest using launch_bundle as used by sugarterm.py in Terminal activity.

Thanks @quozl Done in ca59b68.:smile:

[srevinsaju]

This still may not work. If I build frotz manually, it always installs to /usr/local/bin or /usr/local/games (on apt based distros). A suggested fix is to do this, as @quozl suggested is to:

https://github.com/sugarlabs-appstore/aslo-v4/blob/41383de6fa580dff1a6863c5d1505d01d5d25050/aslo4/platform/__init__.py#L29-L48

def get_executable_path(executable, raise_error=True):
    """
    Returns the absolute path of the executable
    if it is found on the PATH,
    if it is not found raises a FileNotFoundError
    :param executable:
    :return:
    """

    for i in PATH:
        if os.path.exists(os.path.join(i, executable)):
            return os.path.join(i, executable)
    if raise_error:
        raise FileNotFoundError(
            "Could not find {p} on PATH. "
            "Make sure {p} is added to path and try again".format(
                p=executable))
    else:
        return False

Link to original source provided above the code snippet.
You can do a try...catch on FileNotFoundError or pass raise_error=False and check if the returned value is false to check if Frotz exists on path.

[Saumya-Mishra9129]

Thanks , Tried it in 65c66e6. Tested on debian and Ubuntu works fine. 😃

[srevinsaju]

Perfect! 🎉

[quozl]

Thanks. Reviewed. Pushed review changes. Fixes #4. Fixes #3. Fixes #2.

[Saumya-Mishra9129]

Thanks Reviewed till 3a9baf7. Please merge these changes.