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

FreeBSD support #1836

Merged
merged 13 commits into from
Apr 2, 2023
Merged

FreeBSD support #1836

merged 13 commits into from
Apr 2, 2023

Conversation

MuhammadMuradG
Copy link
Contributor

Describe your changes in detail

This PR add support to Freebsd13. As the Freebsd is an unix-like OS and could use gtk, it can be used with toga with just few edits to the toga code base.

For consistency, this PR changed the linux backend name to Unix-like, which make sense to any platform that is not linux but using gtk3.

What problem does this change solve?

This PR make Toga possible on Freebsd13 😄.

PR Checklist:

  • All new features have been tested
  • All new features have been documented
  • I have read the CONTRIBUTING.md file
  • I will abide by the code of conduct

Copy link
Member

@freakboy3742 freakboy3742 left a comment

Choose a reason for hiding this comment

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

I don't have any objection adding the internal tooling to support use of FreeBSD; however, I'm not going to degrade the user experience for the larger Linux user base to accomodate it. Comments inline.

@@ -0,0 +1 @@
Add support to Freebsd13 OS.
Copy link
Member

Choose a reason for hiding this comment

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

We try really hard to follow style guides when referring to other operating systems in user-facing text - I'm pretty sure this should be FreeBSD. Also - given FreeBSD12 isn't EOL, and FreeBSD14 is on the horizon - is there any reason to narrow this to v13 specifically?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, you are right, this should be FreeBSD.

The main reason is that I'm tested it on v13, and I don't want to add version that I didn't test it. Also, I don't want to see, in the future, a list of 'How to install Toga in different FreeBSD versions', so I preferred to maintaining only last FreeBSD release, especially since FreeBSD is a rolling release OS. However, I suggest to remove the version returned by the sys.platform to be just freebsd.

core/setup.cfg Outdated
@@ -45,6 +45,8 @@ keywords =
android
windows
winforms
unix-like
freebsd
Copy link
Member

Choose a reason for hiding this comment

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

Indentation is inconsistent here. Also "Unix-like" is a near meaningless term. Android and macOS are both "unix like".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The inconsistent here because I used tab instead of spaces. The fix will be committed.

Because, Toga now support big number of unix-like OS I add it to keywords in metadata. Also, since keywords are used to assist searching for the distribution in a larger catalog, I expected this will help. Is it?

@@ -20,7 +20,8 @@
"android": "android",
"darwin": "macOS",
"ios": "iOS",
"linux": "linux",
"linux": "UnixLike",
"freebsd13": "UnixLike",
Copy link
Member

Choose a reason for hiding this comment

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

This breaks backwards compatibility for existing code that is expecting "linux" as the toga platform.

The right approach here is to add a mapping for freebsd13 to freebsd (or, to freebsd13 if you think there's a need to differentiate versions in a Toga context; I can't say I know FreeBSD well enough to comment on whether this is the case). Adding mappings for 12 and 14 would also make sense.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The changes addressed into the following commits, with removing the freebsd version.

@@ -31,7 +31,7 @@ First thing is to ensure that you have Python 3 and pip installed. To do this ru
$ python3 --version
$ pip3 --version

.. group-tab:: Linux
.. group-tab:: Unix-like
Copy link
Member

Choose a reason for hiding this comment

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

Unix-like is a near meaningless term, and one that only serves to confuse inexperienced Linux users. FreeBSD use is sufficiently niche that I'm comfortable expecting FreeBSD users to work out that their commands are close enough to Linux that they can adapt.

Copy link
Contributor Author

@MuhammadMuradG MuhammadMuradG Mar 31, 2023

Choose a reason for hiding this comment

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

I'm just conservative about referring to FreeBSD into Linux instructions. Is it possible to add new tab with name *BSD which may be add the possibility in the future to support other *BSD platforms like OpenBSD, GhostBSD and etc?

Copy link
Member

Choose a reason for hiding this comment

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

...and I'm telling you that I've been working on this project for almost 8 years, and in that entire time, you're the first person who has asked for FreeBSD support. I haven't had any requests for other BSDs (or any other Unix, for that matter).

I hate to break it to you, but non-Linux Unix platforms are niche. FWIW, Linux usage is pretty niche as well - but it's a lot more common than non-Linux Unix, and I'm not convinced that all Linux users actually know that they're running a "Unix-like" operating system. Many do - but you'd be surprised the sorts of questions we get from users.

I'm also not wild about adding a second tab, since the instructions will be exactly the same, so you're adding a maintenance burden to the core team and all other contributors for the benefit of, so far, exactly 1 user.

If FreeBSD users are going to start complaining that the best match for their usage instructions are listed under Linux, not "Unix", then I'm more than willing to offer them a 100% refund of their purchase price.


.. code-block:: bash

# Freebsd13
(venv) $ sudo pkg update
(venv) $ sudo pkg install gtk3 gobject-introspection cairo webkit2-gtk3
Copy link
Member

Choose a reason for hiding this comment

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

  1. FreeBSD, not Freebsd
  2. Is there any reason to believe this set of instructions won't also be true for FreeBSD12 and 14? Could this just be FreeBSD, not FreeBSD 13
  3. From a UX perspective, FreeBSD should be last in this list of instructions, not first.
  4. This list should also include anything needed to install a version of Python that can be used for development purposes.

Copy link
Contributor Author

@MuhammadMuradG MuhammadMuradG Mar 31, 2023

Choose a reason for hiding this comment

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

Regarding to the points 1 and 4 I will address it in the next commits.

For 2 and 3, see the previous inline comments.

Copy link
Member

Choose a reason for hiding this comment

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

I'm not I see sure you your previous comments address 2 and 3. Your other commits introduce compatibility with FreeBSD 12/14, but these docs still explicitly say 13 (point 2); and they're still at the top of the list of Unix options (point 3).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, I dropped the comments for 2 and 3, however the next commit will address them.

@@ -71,6 +71,10 @@ Next, install Toga into your virtual environment:

.. code-block:: bash

# Freebsd13
(venv) $ sudo pkg update
(venv) $ sudo pkg install gtk3 gobject-introspection cairo webkit2-gtk3
Copy link
Member

Choose a reason for hiding this comment

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

See notes above about these install instructions.

gtk/setup.cfg Outdated
@@ -38,6 +38,8 @@ keywords =
cross-platform
toga
desktop
unix-like
freebsd
Copy link
Member

Choose a reason for hiding this comment

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

See previous notes about indentation and unix-like

gtk/setup.cfg Outdated
@@ -50,7 +52,7 @@ zip_safe = False

[options.entry_points]
toga.backends =
linux = toga_gtk
UnixLike = toga_gtk
Copy link
Member

Choose a reason for hiding this comment

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

Registering a backend costs nothing; this should 2 independent linux and freebsd registrations.

toga/setup.cfg Outdated
@@ -45,6 +45,8 @@ keywords =
android
windows
winforms
Unix-like
freebsd
Copy link
Member

Choose a reason for hiding this comment

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

See previous comments.

toga/setup.py Outdated
@@ -8,6 +8,7 @@
extras_require={
# Automatically installed platform backends
':sys_platform=="win32"': ["toga-winforms==%s" % version],
':sys_platform=="freebsd13"': ["toga-gtk==%s" % version],
Copy link
Member

Choose a reason for hiding this comment

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

Compatibility with 12 and 14 again.

README.rst Outdated
Comment on lines 44 to 46
* If you're on Unix-like, you need to have GTK+ 3.10 or newer. This is the
version that ships starting with Ubuntu 14.04 and Fedora 20. You also need to
install the system packages listed in `Tutorial 0 <docs/tutorial/tutorial-0.rst>`__.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
* If you're on Unix-like, you need to have GTK+ 3.10 or newer. This is the
version that ships starting with Ubuntu 14.04 and Fedora 20. You also need to
install the system packages listed in `Tutorial 0 <docs/tutorial/tutorial-0.rst>`__.
* If you're on Linux (or another Unix-based operating system), you need to have
GTK+ 3.10 or newer. This is the version that ships starting with Ubuntu 14.04
and Fedora 20. You also need to install the system packages listed in
`Tutorial 0 <docs/tutorial/tutorial-0.rst>`__.```

@@ -0,0 +1 @@
Add support to FreeBSD OS.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Add support to FreeBSD OS.
Support for FreeBSD was added.

Comment on lines 41 to 44
if sys.platform.startswith("freebsd"):
current_platform = "freeBSD"
else:
current_platform = _TOGA_PLATFORMS.get(sys.platform)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if sys.platform.startswith("freebsd"):
current_platform = "freeBSD"
else:
current_platform = _TOGA_PLATFORMS.get(sys.platform)
elif sys.platform.startswith("freebsd"):
current_platform = "freeBSD"
else:
current_platform = _TOGA_PLATFORMS.get(sys.platform)

@@ -21,6 +21,7 @@
"darwin": "macOS",
"ios": "iOS",
"linux": "linux",
"freebsd": "freeBSD",
Copy link
Member

Choose a reason for hiding this comment

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

This isn't needed if the startswith("freebsd") change is in place. The android version is technically redundant right now, but there's a possibility that we're going to get sys.platform to return android in future, so the line is there for that future possibility.

@@ -31,7 +31,7 @@ First thing is to ensure that you have Python 3 and pip installed. To do this ru
$ python3 --version
$ pip3 --version

.. group-tab:: Linux
.. group-tab:: Unix-like
Copy link
Member

Choose a reason for hiding this comment

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

...and I'm telling you that I've been working on this project for almost 8 years, and in that entire time, you're the first person who has asked for FreeBSD support. I haven't had any requests for other BSDs (or any other Unix, for that matter).

I hate to break it to you, but non-Linux Unix platforms are niche. FWIW, Linux usage is pretty niche as well - but it's a lot more common than non-Linux Unix, and I'm not convinced that all Linux users actually know that they're running a "Unix-like" operating system. Many do - but you'd be surprised the sorts of questions we get from users.

I'm also not wild about adding a second tab, since the instructions will be exactly the same, so you're adding a maintenance burden to the core team and all other contributors for the benefit of, so far, exactly 1 user.

If FreeBSD users are going to start complaining that the best match for their usage instructions are listed under Linux, not "Unix", then I'm more than willing to offer them a 100% refund of their purchase price.

@@ -9,6 +9,7 @@
# Automatically installed platform backends
':sys_platform=="win32"': ["toga-winforms==%s" % version],
':sys_platform=="linux"': ["toga-gtk==%s" % version],
':"freebsd" in sys_platform': ["toga-gtk==%s" % version],
Copy link
Member

Choose a reason for hiding this comment

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

Does this actually work? Like - have you tested it? I didn't think the sys_platform syntax was "freeform Python". Can you provide a reference to docs that says this works, and/or another package that does this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it works. See this for example.

@@ -63,6 +63,14 @@ flatpak_runtime = "org.gnome.Platform"
flatpak_runtime_version = "42"
flatpak_sdk = "org.gnome.Sdk"

[tool.briefcase.app.testbed.freeBSD]
Copy link
Member

Choose a reason for hiding this comment

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

This won't work until there's a freeBSD backend for Briefcase; and it wouldn't be needed anyway, because it exists to test GTK, not Linux.


.. code-block:: bash

# Freebsd13
(venv) $ sudo pkg update
(venv) $ sudo pkg install gtk3 gobject-introspection cairo webkit2-gtk3
Copy link
Member

Choose a reason for hiding this comment

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

I'm not I see sure you your previous comments address 2 and 3. Your other commits introduce compatibility with FreeBSD 12/14, but these docs still explicitly say 13 (point 2); and they're still at the top of the list of Unix options (point 3).

core/setup.cfg Outdated
@@ -45,7 +45,9 @@ keywords =
android
windows
winforms
unix-like
Copy link
Member

Choose a reason for hiding this comment

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

In case it wasn't clear - these "unix-like" keywords aren't needed. It's not a search term anyone will be looking for.

@MuhammadMuradG
Copy link
Contributor Author

MuhammadMuradG commented Apr 1, 2023

As a side note to the point 4 that you mentioned in your comment, I tested Toga on a clean installation of FreeBSD. FreeBSD comes with all required packages except that I mentioned in the FreeBSD section in the docs or will be installed when these packages are installed.

Copy link
Member

@freakboy3742 freakboy3742 left a comment

Choose a reason for hiding this comment

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

I've taken the opportunity to do a couple of unrelated tweaks to the installation instructions; but this all looks good. Thanks for the PR.

@MuhammadMuradG MuhammadMuradG changed the title Freebsd support FreeBSD support Apr 2, 2023
@freakboy3742 freakboy3742 merged commit 6f4c1e7 into beeware:main Apr 2, 2023
@MuhammadMuradG MuhammadMuradG deleted the freebsd_support branch April 2, 2023 01:08
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.

2 participants