-
Notifications
You must be signed in to change notification settings - Fork 27
Python 3 Support #8
Comments
Hi, I think I'm going to give this a shot! I plan to fork the repo and implement all of my changes there until I get something workable, and then I'll come back and share. Some questions for @jgoerzen if you see this:
|
I would not make it compatible with py 2 as its now considered defunct. I
too had been thinking about this and would offer to help you with py 3
rework when I can
…On Tue, Nov 17, 2020, 8:26 PM Michael Lazar ***@***.***> wrote:
Hi, I think I'm going to give this a shot! I plan to fork the repo and
implement all of my changes there until I get something workable, and then
I'll come back and share.
Some questions for @jgoerzen <https://github.com/jgoerzen> if you see
this:
- Is this something that you would even be open to merging upstream
(if it meets your standard of quality) or should I plan on maintaining my
own fork long-term.
- How important is keeping backwards compatibility with old python
versions? Ideally I would drop python 2 completely. I could *maybe*
get it working with python 2.7, but it would be difficult without a
compatibility shim like six which seems like a bad idea given this is
a zero-dependency package. Anything below 2.7 I'm not really interested in
working on.
- pygopherd was written in a different era and understandably the code
style is not "pythonic" by modern standards. How would you feel about
running the codebase through a linting tool like black
<https://pypi.org/project/black/> so my IDE doesn't barf all over me.
On one hand you would lose some of the legacy of the software, on the other
hand it could make it easier to spot bugs.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#8 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ACRP6TIWFJ47KFVD6W25N5TSQMPETANCNFSM4SVOY4MQ>
.
|
Hi, This is fantastic! To answer the questions: I would happily merge it. However, perhaps better, if you are able to maintain it long-term would be to just designate your fork as the new official one. I haven't had the time to work on it that I used to. Backwards compatibility with old Python versions is unnecessary. I have no problem with you doing a linting tool or something. The one thing I would mention to watch out for is non-UTF8 filenames. Whether you tackle that problem or pretend it doesn't exist is up to you (valid arguments can be made both directions), but it should be noted. The test cases will definitely bring it to your attention :-) And the standard Python Zipfile may make it a bit challenging. Some of the Zipfile internals are different and the interface in Python 3 doesn't expose some things that were used from Python 2. Mostly this was used for caching. This is probably a pretty obscure feature at this point. I made some progress on it, but kept getting bit by bytes/str. |
As a user of the software, this is nice to see. From my perspective, if the program's functionality is maintained, I don't really care how it works under the hood. Since there's an LTS version of Debian, I think its possible for users to keep on using pygopherd for a few years - so if an alternative is ready within that time span, it would be great. |
Great!
I ran across some of these filenames when I was playing around with your 2007 gopherspace mirror [0]. I was able to push through it with copious amounts of [0] gopher://mozz.us/1/wayback/2007 |
Status UpdateI have a fork here: https://github.com/michael-lazar/pygopherd I ran the code through the black formatter and started fixing a bunch of obvious linting and PEP8 errors. Some of these were style-only issues:
and others uncovered legitimate bugs:
After that, I started going through the code and adding type hints wherever possible. These type hints have been insanely helpful to uncover the places where strings or bytes should be used. I've been trying to follow the unicode sandwich technique: decode immediately after reading from the socket/file, and encode again immediately before writing. I am using The test suite is now passing for me on macOS and Ubuntu 🎉 . I don't think I'm close to being finished yet though. I'm fairly sure the zip implementation will still crash with non-UTF8 filenames. My plan next is to systematically go through all of the handlers and protocols, test them manually, and add test coverage where I can. My initial burst of steam has died out now so work will likely be slower paced moving forward. It's fun to play around with all of the handlers though. I really can't stress enough how helpful the type hints have been. |
Fantastic! Thank you! |
Status UpdateI've finished adding test cases for all of the handlers and protocols now (phew!). I'm pretty confident that everything is working now. Not including weird, non-utf8 files for which I still haven't really tested yet. Everything in the Config filesBecause the pygopherd config file uses evaluated python code, unfortunately everyone's config is going to break on python 3. In particular, this part of the config is broken:
needs to be updated to
There's also the possibility of custom changes in users' personal configs that we won't be able to detect and fix for them automatically. I think the best thing to do here is communicate that config files will need to be migrated by their owners. The default config has been fixed to work with py3 in my fork. PYG filesLike above, the pyg file handler will execute python files, so any server using mbox filesMailboxes are working (both mbox files and directories) and I added tests for both. I did change the directory listings to use the built-in mailbox keys to identify emails instead of looping through the generator and keeping track of indexes. For example, instead of
pointing to the "sixth" email in the directory, where the order of emails is based on whatever the generator decides, the selector now looks like
I try to preserve the old indexing if you think it's a big deal to break these selectors, but it would be a bit hacky with the new python mailbox library. simpletalMiraculously the SimpleTAL library that the tal handler uses has been ported to python 3 and still works. The maintainer tried to upload it to PyPi but they screwed up and the files are missing and you can't actually download it through pip. For this reason, I vendored the dependency directly into my fork in order to get my tests working. For the debian release we can switch to using |
I've been thinking about this some more and I would be willing to take this on if you'd like. I pretty much consider this project "feature complete" at this point so I'm not going to add new features or try to modernize it anymore. But I can keep it tested and fix things as new python versions come out. I can work on publishing a new release and getting it up on PyPI. I'm not really interested in maintaining the debian package myself because I don't use debian, but I can work with someone if they want to handle the packaging stuff downstream. |
This is all fantastic work! Yes, i am fine with the config file format changing and documenting that. The mbox plans also sound fine; good to keep the old selector working for compatibility with old links. I could continue maintaining it for Debian (I'm a DD) and would be happy to do that. When you feel good about the quality of it, archive this repo and point it to yours. Thanks again! |
Just an update: I still intend to wrap this project up, but life got busy and I probably won't get around to it until at least January. |
Thank you for the update! |
Status UpdateI'm back! I finally got around to testing everything with non-UTF8 filenames. This turned out to be a huge pain because I mainly develop on a Macbook, which uses a UTF-8 only APFS filesystem. Even running debian through docker wouldn't allow me to create the files. So I got to dust off my 2010 dell laptop and re-install Ubuntu for old time's sake. Everything filename related is working to the best of my knowledge now. I tested sending ISO 8859-1 filenames through gopher and http. The zip stuff was pretty interesting. I found your bug report on the python issue tracker. I think I came up with a decent workaround but you might want to try testing that one on your own files. The python3 I changed the I spruced up the README a bit and started putting together some Upgrade Notes and a new Changelog. Tests are passing for python 3.7 through 3.9. I haven't touched anything in the I'm bumping the PyGopherd major version to v3.0.0 which seems like the obvious thing to do. I uploaded the package to PyPI. Honestly I'm not sure how useful this will be, but I figured it couldn't hurt to have another "official" package source for releases besides the git repo and I'm comfortable maintaining it. https://pypi.org/project/pygopherd/
Another option would be to transfer ownership of this repo to me through Github. This (I think) would preserve the existing issues/stars/forks and automatically redirect any existing links. Then I could rebase my branch on top of it. This is totally up to you and I understand the value of keeping a clean separation when a project changes hands. But it's kind of fuzzy now since I'm going to make a new release under the same What I need next is for some folks who are already running PyGopherd to try out my branch michael-lazar/pygopherd on their servers. If we can get it working on quux.org then it's probably stable enough to cut the release and work on getting it back into debian. |
If I wanted to test this on my server (Raspbian 10/buster), would I just install via pip and then make the pygopherd.conf change? |
@Shuswap You got it! I just pushed a new beta release to PyPI that's up to date with all of the changes on my branch.
This should install |
It's up and running at gopher://gopher.visiblink.ca Thanks so much for doing this! I know it was a fair bit of work. |
First, I didn't mean to close the issue. I just wanted to close a comment and try a few other things before asking a question. Second, if anyone else is interested in testing and you want to daemonize the process -- don't make the same mistake that I did. Don't set up your systemd unit file to run the program as the user gopher. It won't work. Omit the user. As I learned from actually reading the config file (after a lot of frustration!), Pygopherd will set the user and group itself. |
Here's a systemd unit file that worked for me, for reference
At first I tried running in |
Well that's different than mine. This one's working (so far) on Raspbian 10:
I don't claim to understand it all though. |
It seems really stable. Two weeks and no problems. Knock on wood ;) |
This is fantastic news! I hope to test it on quux.org and update the Debian packaging in the next few days. |
So I know I said earlier that I wouldn't add any new features.... but I couldn't help myself 😄 I've been working on:
|
Oh cool! I'll be curious to see how TLS works on gopher and https (self-signed with trust on first use like gemini or certificate authority). Thanks for all the work you've done on this Michael. It just gets better and better! |
Hello everyone! At long last, I have:
I found only one minor change from the previous, and I'm honestly not quite sure which was correct off the top of my head. If, in a It was readily enough addressed with:
My Debian packaging has added a systemd unit file which is different from both of the examples here. By using I will follow up with a PR to your repo that makes those minor tweaks to the conf file, includes an example systemd unit file for non-Debian users, and updates the documentation in doc/. Question: I assume that going forward, you would like to use your repo as the canonical location for pygopherd? That is fine by me; in that case, I will update the README here to point to yours and close other things out in this one. |
PR now up at: michael-lazar/pygopherd#4 |
Cool! I merged your PR and bumped the official version number to v3.0.0 🎉. I'm glad to see this is making its way back into Debian.
Yea that's good with me, I have no plans of ever taking it down and will do my best to keep it up to date with new python versions, etc. I also invited you as a project collaborator if you would like to have maintainer access. |
Hi John,
I use pygopherd on my server and I really love its ability to serve to both gopher and the web. Such a great idea.
I saw somewhere that you were attempting at one point to update it to python 3. Is that likely to happen? I've also noticed that the package is not in the Debian testing and unstable repositories (at least if I'm reading Debian's package pages correctly). So again, is Python 3 support likely?
Thanks!
The text was updated successfully, but these errors were encountered: