Ciao!
Thanks for taking your time with the port. I'm just replying to say that
even with the changes it still works on my machine. Also to clarify a
few points.
> I'd add some commentary to the patches.
Nice tip. I didn't know you could include comments like that in a patch.
It does mess up syntax highlighting in some editors though.
> Have you sent the patch upstream?
Patch for FCGI has not been sent to upstream yet because I'm still
waiting for the response on the first patch proposal (the `go.mod` one).
In due time I'll probably just consider it rejected and move on to the
next proposal.
> It doesn't work without altering the flags to specify the upstream
server. This has confused me initially, so add that info to the README.
I've only included things that aren't mentioned in the original
documentation. But if a little coping saves some confusion then it's OK.
I would however like to suggest some changes for the new README for
consistency sake (see attached diff):
Anyhow, looking forward to submitting additional ports.
With regards,
--
IZ
https://mocheryl.org/ | moche...@mocheryl.org | 041/517-106
On 9/6/23 18:14, Omar Polo wrote:
Hello,
On 2023/08/19 10:04:54 +0200, Igor Zornik <moche...@mocheryl.org> wrote:
Greetings ports@,
Long time listener, but first time caller so I hope I didn't get to many
things wrong.
Sorry for the delay. I wanted to review this the day it was sent
but only managed to look at it today.
Attached I have a port of alps, an e-mail web client. It's not as
feature-full as some other web clients, but I like this one because it
has not run-time dependencies which makes it easier to deploy and maintain.
Notes:
- Port initially created with `portgen`. When modified I relied heavily
on gitea port.
- Upstream isn't very responsive to my change proposals so I provided
some fixes and features locally.
- There isn't any special functionality associated with the service
account so I just made it to run as `www`. Should I still run it under a
dedicated service account?
- I had to do some `sed` trick in the `rc` script to get the `rc`
command to pick up the daemon properly. Could anyone confirm if i have
taken the right path here?
- Tested on amd64 using -current and 7.2.
Any comments or suggestions?
a few nits, but otherwise it's a solid submission.
- I'd add some commentary to the patches. Just something brief like
"adding FastCGI support", so that it's clear what it does when
skimming the files.
- I'd drop the patch for go.mod; I'm not sure go.port.mk even looks
at the fetched go.mod file, let alone parsing it once patched.
- there's a small error in the fastcgi patch:
+ addr = strings.TrimPrefix(addr, "fcgi+unix:/")
should be
+ addr = strings.TrimPrefix(addr, "fcgi+unix://")
i.e. remove two slashes. In practice it's not an issue.
- While I really like the fastcgi support (I'm a bit lazy and never
bothered to learn how to use relayd), I don't think we've usually
patched the ports to add features like these. Have you sent the
patch upstream? I can't find it searching for 'fastcgi' on
sourcehut.
To be fair I don't think it's an issue, but still felt like
pointing this out.
- It doesn't work without altering the flags to specify the upstream
server. This has confused me initially, so add that info to the
README.
I've also tweaked the fastcgi example to use `rcctl set alps flags'
and add a dummy example.com at the end to minimize the chance of
getting things wrong.
- (opinable) I've tweaked the http example to listen on port 80.
Otherwise it's not copy-paste-able without further fiddling.
- I'm not sure about ${SYSCONFDIR}/httpd.conf. Even if portcheck
complains, the correct path is /etc/httpd.conf: it is hardcoded in
the httpd(8) binary.
- make update-plist changed @cwd ${LOCALSTATEDIR}/www into /var/www
and I'm not sure how much we care about using the variables instead
of the hardcoded paths. Left as /var for the moment.
- no opinion on the sed machinery to escape '+'s in the pexp. Seems
to work :-)
- changed the version to 0.0.0.<date> so if upstream starts to do
proper releases we don't have to use EPOCH.
I'm reattaching a tarball with these points addressed. It's ok op@ to
import if someone wants to, otherwise i'll be happy to import it
myself (needs an ok to do so).
I don't use web ui for mails if I can, but this is one of the nicer
I've seen.
--- pkg/README Wed Sep 6 17:40:55 2023
+++ pkg/README Thu Sep 7 21:56:34 2023
@@ -11,8 +11,8 @@ configured to run behind HTTP proxy like relayd(8) or
Another option is to let OpenBSD httpd(8) serve HTTP(S) requests
and pass it to alps via FastCGI protocol.
-In every case, it needs to be told the upstream server. Assuming SRV
-DNS records are properly set up:
+In every case, it needs to be told the upstream server. Assuming
+SRV DNS records are properly set up:
# rcctl set alps flags "example.com"
@@ -26,8 +26,8 @@ Example configuration for httpd(8) and alps communicat
Relevant configuration directives in /etc/httpd.conf:
- server "alps.example.org" {
- listen on * port 80
+ server "mail.example.org" {
+ listen on * port http
location "*" {
fastcgi socket "/run/alps/alps.sock"
@@ -45,4 +45,5 @@ then set the flags so that it uses FastCGI:
Customizing alps
================
-For custom public files and templates please use /var/www/alps directory.
+For custom public files and templates please use /var/www/alps
+directory.
