Port to Python3 and WebKit2, flake8 fixes and license

[Saumya-Mishra9129]

@quozl @chimosky Kindly review.

[quozl]

I've never seen this activity before. Reviewed as at 80aaf03.

• copyright of source code is clear, license of source code is LGPLv3+, file LICENSE is GPLv3 (a conflict), but you've ignored or missed this conflict and set license metadata to GPLv3,
• the activity hasn't been tested during your development; one of the buttons generates an AttributeError,
• the activity consumes a lot of CPU time,
• no test plan, apart from testing buttons without knowing what should happen,

Did you derive any of your work from #2? If so, please set commit author properly.

@cristian99garcia, please confirm your activity should be LGPLv3+, and we'll fix the license file.

[Saumya-Mishra9129]

the activity consumes a lot of CPU time,

@quozl Can you suggest what can be probably done in this case? Unfortunately I have no idea at this point of time.

[quozl]

An activity would normally be idle unless it were animating something on screen. I guess the animation is the most likely cause.

I've looked briefly three hours ago, but did not immediately see where execution time was being spent.

You might use Python Profiling to determine what parts of the program are consuming the most execution time over a one minute operation. Profiling will likely identify the parts that manage the animation.

You might use human static analysis of the source code to look for timers, frame rates, clocks, redraws, and other time or redrawing related program features.

[chimosky]

Tested, thanks.

There's no setup.py file in the activity, do add one.
Also set maximum participants to 1as I believe this activity wasn't setup for collaboration, this is seen in the logs;

/usr/lib/python3/dist-packages/gi/overrides/Gtk.py:1641: Warning: g_value_transform: assertion 'G_IS_VALUE (src_value)' failed
  return _Gtk_main(*args, **kwargs)
/usr/lib/python3/dist-packages/gi/overrides/Gtk.py:1641: Warning: unable to set property 'buddy' of type 'PyObject' from value of type '(null)'
  return _Gtk_main(*args, **kwargs)
/usr/lib/python3/dist-packages/gi/overrides/Gtk.py:1641: Warning: g_value_transform: assertion 'G_IS_VALUE (src_value)' failed
  return _Gtk_main(*args, **kwargs)
/usr/lib/python3/dist-packages/gi/overrides/Gtk.py:1641: Warning: unable to set property 'buddy' of type 'PyObject' from value of type '(null)'
  return _Gtk_main(*args, **kwargs)
1587566883.852577 ERROR root: set_active() failed: org.freedesktop.DBus.Error.NoReply: Message recipient disconnected from message bus without replying

When the activity is shared, the buddy disappears in the joiners neighbourhood and this is shown in the logs.

/usr/lib/python3/dist-packages/gi/overrides/Gtk.py:1641: Warning: g_value_transform: assertion 'G_IS_VALUE (src_value)' failed
  return _Gtk_main(*args, **kwargs)
/usr/lib/python3/dist-packages/gi/overrides/Gtk.py:1641: Warning: unable to set property 'buddy' of type 'PyObject' from value of type '(null)'
  return _Gtk_main(*args, **kwargs)
1587568560.591726 WARNING root: Ignoring shared activity we dont have
1587568560.593066 WARNING root: Ignoring shared activity we dont have

[quozl]

@chimosky wrote:

1587568560.593066 WARNING root: Ignoring shared activity we dont have

That's more likely to be because the activity isn't known to Sugar. Restart Sugar or rename the directory away and back into place. Still, now that we have max_participants of 1 it doesn't matter.

@Saumya-Mishra9129 wrote: 8a626f3

What is the purpose of using /usr/bin/env python3 instead of /usr/bin/python3?

[Saumya-Mishra9129]

What is the purpose of using /usr/bin/env python3 instead of /usr/bin/python3?

@quozl
From what I understood, executing python as we would do in our shell, so it looks in $PATH. The second one is not a fixed path, which has the inconvenient that in a different system the python interpreter might be located in another path.

[quozl]

Thanks. I agree, that seems the only use. I'm not personally willing to support systems that don't have /usr/bin/python3, because that increases the combinations of complexity. We have relied on /usr/bin/python3 much of the time. I see now many instances of using /usr/bin/env instead. It seems an unnecessary extra step. I'm sad. I've got 52 matches for /usr/bin/python3 and 85 matches for /usr/bin/env python3. Late to the party. Oh well.

@tchx84, @chimosky, what are your opinions? Do you want to support people using non-system interpreters at the expense of a second execve?

[chimosky]

@tchx84, @chimosky, what are your opinions? Do you want to support people using non-system interpreters at the expense of a second execve?

I can see the reason why @Saumya-Mishra9129 used /usr/bin/env python3 but I don't plan on supporting anyone using non-system interpreters.

[quozl]

Thanks. I got an opinion from downstream that may be interesting. It looks like this is a wider issue.

[Saumya-Mishra9129]

@cristian99garcia, please confirm your activity should be LGPLv3+, and we'll fix the license file.

As we got no response here so I am fixing the license file.

[chimosky]

Thanks. I got an opinion from downstream that may be interesting. It looks like this is a wider issue.

That was interesting, thanks for sharing.

I'm yet to understand how the __file_monitor_changed_cb races with dpkg as I don't know much about dpkg but I do understand that on update the callback gets triggered but how does the directory getting created before the activity.info file lead to the path being treated as a malformed or missing bundle.

[quozl]

I'm yet to understand how the __file_monitor_changed_cb races with dpkg as I don't know much about dpkg but I do understand that on update the callback gets triggered but how does the directory getting created before the activity.info file lead to the path being treated as a malformed or missing bundle.

Happy to explain. Here's the sequence of the race, in time order;

• sugar starts, and begins a watch on /usr/share/sugar/activities directory,
• dpkg starts,
• dpkg creates the activity directory inside /usr/share/sugar/activities directory,
• file monitor callback begins, sees no info file in the activity directory, reports an error, and proceeds to ignore the event,
• dpkg creates the files in the activity directory,
• dpkg exits.

It is a race condition because the open-for-read of the info file by the callback may happen before or after the info file is created.

[Saumya-Mishra9129]

I've never seen this activity before. Reviewed as at 80aaf03.

• copyright of source code is clear, license of source code is LGPLv3+, file LICENSE is GPLv3 (a conflict), but you've ignored or missed this conflict and set license metadata to GPLv3,
• the activity hasn't been tested during your development; one of the buttons generates an AttributeError,
• the activity consumes a lot of CPU time,
• no test plan, apart from testing buttons without knowing what should happen,

Metadata is updated in bc46b79 and License file is also fixed in ff93d08. Attribute error is also fixed by bb1e495.

[quozl]

Thanks. Tested ff93d08. It looks like the CPU time is spent by redrawing without need. In area.py you can see how the orbit positions are updated at 10 Hz, but that the redraws are continuous in the idle loop.

[Saumya-Mishra9129]

It looks like the CPU time is spent by redrawing without need. In area.py you can see how the orbit positions are updated at 10 Hz, but that the redraws are continuous in the idle loop.

I tried removing redraw, but it does not worked for me 😟

[quozl]

Not enough to just remove redraw, must also call queue_redraw somewhere. Best place is just after the planets move. f3acf48. Please review and test.

[Saumya-Mishra9129]

Not enough to just remove redraw, must also call queue_redraw somewhere. Best place is just after the planets move. f3acf48. Please review and test.

Tested , It works fine for me.

[quozl]

Thanks. Further patches made as part of my review, please review.

[Saumya-Mishra9129]

Reviewed, It is fine for me. The work is almost complete here you can merge it.

[quozl]

Thanks all, and @cemeiq for the Port to WebKit2.