[new] mail/alps

2023-08-19 Thread Igor Zornik

Greetings ports@,

Long time listener, but first time caller so I hope I didn't get to many 
things wrong.


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?

With regards,

--

IZ

alps-20230809211724.tgz
Description: application/compressed-tar


Re: [new] mail/alps

2023-09-06 Thread Omar Polo
Hello,

On 2023/08/19 10:04:54 +0200, Igor Zornik  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. 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.



alps.tar.gz
Description: GNU Zip compressed data


Re: [new] mail/alps

2023-09-07 Thread Igor Zornik

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  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. 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:
 
 

Re: [new] mail/alps

2023-09-20 Thread Igor Zornik

Ping

On 9/7/23 22:05, Igor Zornik wrote:

On 9/6/23 18:14, Omar Polo wrote:
 Hello,
 
 On 2023/08/19 10:04:54 +0200, Igor Zornik   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.  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.


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,