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

Port to Gtk3 and Python3 #9

Merged
merged 23 commits into from
Aug 13, 2020
Merged

Port to Gtk3 and Python3 #9

merged 23 commits into from
Aug 13, 2020

Conversation

Saumya-Mishra9129
Copy link
Member

@Saumya-Mishra9129 Saumya-Mishra9129 commented Apr 2, 2020

The activity is working as expected.
The Pr includes-
Port to Gtk3
Python3
Fixes #2.
Fixes #3.
Fixes #7.

vnclauncher.py Outdated Show resolved Hide resolved
vnclauncher.py Outdated Show resolved Hide resolved
@chimosky
Copy link
Member

chimosky commented Apr 2, 2020

Reviewed, thanks.

a1c423b contains changes not included in the commit message.

Starting a server throws this error,

/home/ibiam/Activities/VncLauncher/bin/x86-64/x11vnc: error while loading shared
libraries: libssl.so.10: cannot open shared object file: No such file or directory

0493182 could have a better commit message.

You can consider demoting logging as the logs seem to be noisy.

As this is a Port to Gtk3, you should change the use of jarabe.model.network in L35 of vnclauncher.py, you can look at sugar-toolkit-gtk3 for a replacement.

Also consider fixing #7.

Kindly test your changes before making a PR.

vnclauncher.py Outdated Show resolved Hide resolved
@quozl
Copy link
Contributor

quozl commented Apr 2, 2020

Please review and re-use any changes from #4 which was closed after progress stalled. When re-using, use "git commit --author" to credit properly.

@Saumya-Mishra9129
Copy link
Member Author

@quozl Reviewed #4 didn't find anything . The changes are almost same.
@chimosky
/home/ibiam/Activities/VncLauncher/bin/x86-64/x11vnc: error while loading shared libraries: libssl.so.10: cannot open shared object file: No such file or directory
can be faced when your SSL is not installed or updated. If libssl is installed then cause of this error is a different name for the same library present. So you have to create a symbolic link with libssl.so.10 to lib.so.1.0.0(if it is the library present in system). Symbolic links can be created by these commands-

  • cd /lib/x86_64-linux-gnu
  • sudo ln -s libssl.so.1.0.0 libssl.so.10
  • sudo ln -s libcrypto.so.1.0.0 libcrypto.so.10

The steps for installation are-

  1. sudo apt-get update
  2. sudo apt-get install libssl1.0.0 libssl-dev
  3. cd /lib/x86_64-linux-gnu
  4. sudo ln -s libssl.so.1.0.0 libssl.so.10
  5. sudo ln -s libcrypto.so.1.0.0 libcrypto.so.10
    The question is here now that do we need to put these libraries in activity itself because it is problem of ubuntu itself so can't be solved without re-installation libraries or creating symbolic links.

@quozl
Copy link
Contributor

quozl commented Apr 3, 2020

The installation steps are not practical. A child learner or elementary school teacher won't be able to follow those steps, because they are too complex, or because they require administration rights. The x11vnc file is also very very old, so we can't use it. See #7 for how this should be fixed.

@Saumya-Mishra9129
Copy link
Member Author

@quozl What vnc file we can use other than x11vnc file to remove #7 .I am not getting this. Do we need replacement of vnc file?

@chimosky
Copy link
Member

chimosky commented Apr 3, 2020

There's a checklist in #7, you can take a look at it.

To elaborate, use distro specific binaries as the ones used in the activity are old and have vulnerabilities.

@quozl
Copy link
Contributor

quozl commented Apr 4, 2020

@quozl What vnc file we can use other than x11vnc file to remove #7 .I am not getting this. Do we need replacement of vnc file?

Sorry, I did think it was obvious, and surprised I have to explain. 😁 Rephrasing part of #7, /usr/bin/x11vnc is available on Linux as a package, so assume it to be present as a system dependency.

It is much the same reason why we don't include /usr/bin/python in this repository. We could include python, but then we'd be running an old version of python fairly quickly.

In future, strategic plans for an issue are best discussed in the issue; #7, otherwise we fragment the discussion.

@Saumya-Mishra9129
Copy link
Member Author

@quozl If you want to merge , you can merge this pr , I will create a new one for #7 , as it is difficult to find all related dependencies related to x11vnc. Thanks

@chimosky
Copy link
Member

chimosky commented Apr 30, 2020

@quozl If you want to merge , you can merge this pr , I will create a new one for #7 , as it is difficult to find all related dependencies related to x11vnc. Thanks

I think the fix for #7 needs to be incorporated here so we can know if the activity works as intended and why are you looking for all dependencies related to x11vnc if I may ask?

The idea behind #7 is using a distro specific binary when available, pkgs.org lists the x11vnc package name for various distros.

You'll want to;

  • Install x11vnc in your test environment.
  • Make the necessary changes .ie. remove support for the use of embedded binaries and use distro package if present.
  • add a warning to advise of any missing dependency when the activity is started, if x11vnc isn't installed.
  • add dependency for Fedora and Ubuntu to README.md, (see other activities with detailed dependencies for examples)

The x11vnc homepage might be of help.

@quozl
Copy link
Contributor

quozl commented May 1, 2020

I agree, it should be straightforward, and you don't need to know the dependencies of x11vnc as these will be handled automatically by whatever package management system is used.

It may help to understand why this situation happened. The activity was written for the OLPC XO laptop. Activities could not depend on the ability to install dependencies, because they did not run as root, and there was an amazing security system called Bitfrost. So activities that needed dependencies ended up with embedded binaries, which complicated their maintenance but got the job done.

Now, the activities don't have to focus on the same environment, and we can assume that the user has the capacity to install dependencies if needed.

This does mean that any user who downloads the .xo bundle from a Sugar app store won't necessarily be able to install the dependency, which is why the warning is needed.

@Saumya-Mishra9129
Copy link
Member Author

@chimosky @quozl Thanks for your suggestions. Now I am thinking to include a script related to installation of x11vnc which must work on all distros possible.. and I will add steps for running the script in Readme. Will It be good or not, You both suggest please.

@chimosky
Copy link
Member

chimosky commented May 2, 2020 via email

@quozl
Copy link
Contributor

quozl commented May 4, 2020

@Saumya-Mishra9129, thanks for the offer, but I don't think a script is needed.

Let me put this activity into perspective.

  • it is for teachers to use,
  • it shares the display of a Sugar system on a local network,
  • it does not provide any authentication or password.

It's 2006. When the OLPC XO was produced, classroom displays and projectors were rare and expensive, and usually were not available in the third-world global-south where we wanted to deploy the laptops. So we did not include an external socket for connecting to them. This saved cost, risks of a physical gap in the casing, risks of electrostatic discharge damage, risks of a VGA connector being used to bend the circuit board when someone tripped over a cable, and VGA connectors had a limited lifetime of several hundred insertion removal cycles. We also increased a focus on collaboration via wireless networking. Sugar was part of OLPC XO product and was not easily available on other computers.

It's 2008. The VNC Launcher activity was also created, by volunteers, to provide a way for teachers in first-world scenarios to use a projector with an attached computer.

It's 2020. What has changed since then;

  • Sugar can be run on ordinary computers, and demonstrated using a projector,
  • most laptops now have external video connectors of some sort, (HDMI is reasonably ESD resistant and the connector would more likely break rather than transfer significant force to the laptop),
  • cost of displays and projectors has fallen, by a factor or ten or more,
  • risk of automated network attack has risen, along with a focus on encryption and security protocols,

I don't recommend spending much effort in maintaining this activity. It is such a small corner case, and the people who need it will have the skills to follow instructions. They won't need a script. We also can't afford to maintain a script against changes made to all distros.

Hope that helps!

@Saumya-Mishra9129
Copy link
Member Author

@Saumya-Mishra9129, thanks for the offer, but I don't think a script is needed.
Let me put this activity into perspective.
it is for teachers to use,
it shares the display of a Sugar system on a local network,
it does not provide any authentication or password.
It's 2006. When the OLPC XO was produced, classroom displays and projectors were rare and expensive, and usually were not available in the third-world global-south where we wanted to deploy the laptops. So we did not include an external socket for connecting to them. This saved cost, risks of a physical gap in the casing, risks of electrostatic discharge damage, risks of a VGA connector being used to bend the circuit board when someone tripped over a cable, and VGA connectors had a limited lifetime of several hundred insertion removal cycles. We also increased a focus on collaboration via wireless networking. Sugar was part of OLPC XO product and was not easily available on other computers.
It's 2008. The VNC Launcher activity was also created, by volunteers, to provide a way for teachers in first-world scenarios to use a projector with an attached computer.
It's 2020. What has changed since then;
Sugar can be run on ordinary computers, and demonstrated using a projector,
most laptops now have external video connectors of some sort, (HDMI is reasonably ESD resistant and the connector would more likely break rather than transfer significant force to the laptop),
cost of displays and projectors has fallen, by a factor or ten or more,
risk of automated network attack has risen, along with a focus on encryption and security protocols,
I don't recommend spending much effort in maintaining this activity. It is such a small corner case, and the people who need it will have the skills to follow instructions. They won't need a script. We also can't afford to maintain a script against changes made to all distros.
Hope that helps!

Yeah!! It will help for sure. Now I think including instructions for some distros in the Readme will be enough for it. Thanks

@Saumya-Mishra9129
Copy link
Member Author

@quozl @chimosky @pro-panda I have done with changes required for x11vnc , kindly review now. Done as suggested by @chimosky here.

@Saumya-Mishra9129
Copy link
Member Author

Saumya-Mishra9129 commented May 16, 2020

Perhaps we can think here for releasing new version as specified in #1 . Also activity doesn't specify a license.

README.md Outdated Show resolved Hide resolved
@quozl
Copy link
Contributor

quozl commented May 21, 2020

I guess so.

Users of this activity are teachers, and the instructions from 2011 do give some indication.

If you think teachers won't be able to use IP addresses, then I guess the next thing to try is registering an mDNS name on the network and having the VNC client use that.

@Saumya-Mishra9129
Copy link
Member Author

Saumya-Mishra9129 commented Jun 2, 2020

@JuiP Can you please look and plan for a new release of activity?

I guess so.

Users of this activity are teachers, and the instructions from 2011 do give some indication.

If you think teachers won't be able to use IP addresses, then I guess the next thing to try is registering an mDNS name on the network and having the VNC client use that.

Can we plan a release now, and leave it as I tried to find out the ways , I got to know about python3-avahi but it is not supported by all linux distributions, I couldn't find it for Ubuntu.
Also can you look for a merge, so that @JuiP can start on new release.

@quozl
Copy link
Contributor

quozl commented Jun 2, 2020 via email

@Saumya-Mishra9129
Copy link
Member Author

PyGObject has an Avahi API. I've not used it.

Yeah I saw about this. But I have no experience with this and how this works. I am planning to work on this. But for now it would be out of scope of this pull request. I will make a new one probably after implementing Avahi.

@quozl
Copy link
Contributor

quozl commented Jun 22, 2020

Please read the manual page for x11vnc, in particular the -avahi option.

Tested.

  • the Vte does not have focus to begin with, so nothing can be done without clicking in the widget first,
  • exiting the Vte with ctrl+d or exit does not exit the activity; this is unlike Terminal; possibly the problem was fixed in Terminal long ago but the sources have diverged,
  • there is no message to inform the user that /usr/bin/x11vnc is not available; the button just inserts the command into the Vte without pressing enter, and when you press enter the shell reports No such file or directory,
  • when in Sugar Ad-hoc network mode, the current IP address is not displayed, instead "Error!! check connection", workaround is to use Sugar Frame F6
  • README.md should advise use of apt not apt-get,
  • in the default operating mode, x11vnc is used insecurely, related to but not mentioned in Cannot be used; critical security vulnerabilities #7,
  • the critical how to use information at http://wiki.laptop.org/go/VNC_Launcher should be in the README.md instead of being externally hosted,
  • no information is present on how to install a vncviewer (e.g. sudo apt install xtightvncviewer), and run it,

That said, it does work.

configparser accepts <class 'str'> as *args.

Boolean and integer args had to be converted to str
Fixes TypeError: Expected a Vte.CursorBlinkMode, but got str
TypeError: Must be number, not str
configparser accepts str as argument. Boolean and integers values need to be converted to str first
 Vte didn't had focus to begin with. Workaround was to click in the widget first
@Saumya-Mishra9129
Copy link
Member Author

Update - Please review till 05995ea. @chimosky @quozl .

@quozl
Copy link
Contributor

quozl commented Aug 8, 2020

  • scenario of two IP addresses (wireless and ethernet) still isn't handled; while we had discussed using Zeroconf/mDNS above, you decided against it, so that leaves the problem of having to handle the scenario,
  • the README.md doesn't show how to use the activity,
  • the README.md explains how to install a VNC viewer, but it isn't the packaged viewer available for the operating system.

Re: my earlier comment #9 (comment) ... I still don't see this winning any users. As far as I can see from the limited information; a teacher would have to install this activity on her device, install x11vnc on her device, then for each child device install a VNC viewer, and teach the child how to type a Terminal command. Where's the corresponding viewer activity? This search is interesting; there's a viewer that isn't on GitHub yet.

(Also, for history, one of the reasons the activity existed was because the OLPC XO had no external monitor socket, and teachers with classroom projectors needed a way to show the Sugar desktop from an XO using their Windows classroom system. They would install this activity on the XO, a VNC viewer on their Windows system, and demo from there. The method we eventually settled on at OLPC was to sell USB to VGA adapters. Now, the latest OLPC hardware has external monitor sockets.)

@Saumya-Mishra9129
Copy link
Member Author

I still don't see this winning any users. As far as I can see from the limited information; a teacher would have to install this activity on her device, install x11vnc on her device, then for each child device install a VNC viewer, and teach the child how to type a Terminal command.

In 51344d9, I tried to give a feature by which when a user installs activity for the first time, he/she will be automatically able to install x11vnc without a need to type terminal commands. I tested it with Ubuntu and Debian, If @chimosky can test this with fedora then it will be good to go.

Where's the corresponding viewer activity? This search is interesting; there's a viewer that isn't on GitHub yet.

Thanks I saw this , but no information where source code can be found. Should we try to contact to Author of the Xo VNC Viewer. ?

scenario of two IP addresses (wireless and ethernet) still isn't handled; while we had discussed using Zeroconf/mDNS above, you decided against it, so that leaves the problem of having to handle the scenario,

I couldn't find a way to use x11vnc 's avahi option. I am trying to find out a way.

the README.md explains how to install a VNC viewer, but it isn't the packaged viewer available for the operating system.

Actually I think it is a easy way to install RealVNC this way. Setting up TightVNC with terminal is so complex , It requires a large setup. Downloading this way can work on Windows and Linux both as you mentioned about windows also above.

vnclauncher.py Outdated Show resolved Hide resolved
vnclauncher.py Outdated
Comment on lines 148 to 150
if platform.version().find("Ubuntu") > -1 or platform.version().find("Debian"):
cmd = "apt install x11vnc"
if platform.version().find("Fedora") > -1:
Copy link
Member

@chimosky chimosky Aug 8, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be better to use platform.linux_distribution()[0] because on fedora systems, platform.version().find("Fedora") will always return -1 as Fedora isn't part of the string returned by platform.version(). Although it's deprecated in python>=3.5.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Find a way to check for Fedora distros.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

platform.platform() contains the necessary information.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Done in c733b9b.

@chimosky
Copy link
Member

chimosky commented Aug 8, 2020

@quozl said

Re: my earlier comment #9 (comment) ... I still don't see this winning any users. As far as I can see from the limited information; a teacher would have to install this activity on her device, install x11vnc on her device, then for each child device install a VNC viewer, and teach the child how to type a Terminal command.

I agree.

@quozl
Copy link
Contributor

quozl commented Aug 10, 2020

@Saumya-Mishra9129 wrote:

Should we try to contact to Author of the Xo VNC Viewer. ?

What for? We have the activity bundle, isn't that enough to begin with?

An ideal VNC activity would offer a "share my desktop" button, support Sugar Toolkit collaboration, and joiners would automatically start a VNC viewer, perhaps in read-only mode. Obtaining the IP address through the collaboration stack is fairly easy, see the Level activity for details.

- add x11vnc dependency for Fedora , Ubuntu and Debian.
- add info required to install VNC Viewer.
- Modify how to use section
@Saumya-Mishra9129
Copy link
Member Author

What for? We have the activity bundle, isn't that enough to begin with?.

Yeah It is. Thanks. I will try to look at the bundle and will what maintenance required there.

@quozl

the README.md doesn't show how to use the activity,

This I have tried to do in 64df965. You can review.

@quozl
Copy link
Contributor

quozl commented Aug 11, 2020

Thanks. Updated checkboxes.

 The activity was written for an OLPC XO years ago, which had one network device wlan0, which would have one IP address. Nowadays, We use sugar on laptops and computers with both wireless and wired network connections.
 The patch displays all the IP(IPv4 and IPV6) addresses assigned to your system (Wired and Wireless)
@Saumya-Mishra9129
Copy link
Member Author

fix multiple IP addresses issue 477d141,

@quozl Please review and if possible merge the changes. I have tested on Ubuntu and Debian works fine for both systems.
It looks like this -

Screenshot from 2020-08-13 16-35-53

@quozl quozl merged commit ca1d3bc into sugarlabs:master Aug 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cannot be used; critical security vulnerabilities Port to GTK+ 3 and Gio.Settings vte failure
4 participants