On 2023/06/05 10:39:49 +0200, "Thim Cederlund" <t...@cederlund.de> wrote:
> 
> > I guess this is fine since it is an executable and not a module.  I'm
> > usure if this warrants the addition of the python module to the port.
> > It should be needed (so that python is added as RDEP), but on the
> > other hand this filter not enable by default.
> 
> I haven't looked in to the manpages in a while but I'm under the
> impression that we cannot mix and match MODGO variables with MODPY
> variables. Perhaps this isn't the case anymore? Should it ever become an
> problem, one solution would be to split the port.

Although I haven't tried in this specific case, there should be no
issue in using multiple modules.  Could cause some headaches, but
should work.  Adding the python module with MODPY_BUILDDEP=No and
MODPY_TESTDEP=No should do it.

In this specific case however I'd avoid adding the dependency on
python since this is a filter not enabled by default.  I haven't added
security/dante as RDEP either for a similar reason (the html filter
uses socksify.)

> > > I can't recall if this was the case previously, but I would assume
> > > so. After all, this is a golang port so those variables are never set. I 
> > > am
> > > sending this mail from aerc which has been running 0.15.2 for a week and I
> > > haven't really encountered any issues.
> > > 
> > > 
> > > Thoughts? @op is the maintainer so he will will have the final say.
> >
> > It's missing a bdep on converters/base64, but does it really build it
> > for you?  I get an error from the 'link' go internal program about a
> > wrong usage; doesn't tell me why though.  That's why after a bit I
> > stopped trying.
> >
> > I looked again today and after a lot of headscratching, with the help
> > of ktrace, I see why it fails to build.  Upstream' makefile has this
> > gem:
> >
> > GO_LDFLAGS+=-X main.Flags=$$(echo -- $(GOFLAGS) | base64 | tr -d '\n')
> >
> > Base64 splits every 72 characters using \r\n, tr deletes \n and the
> > shell seems to do word splitting on the carriage return.
> 
> Strange, it did build for me and I recompiled it a couple of times just to be
> sure before I mailed the diff. But it complained about base64 along with the
> usage of deprecated notmuch functions, but from what I remember these has been
> here all along. The base64 warning is something that could be fixed but
> the deprecated notmuch functions sounds like it is suited to upstream.

If you don't have converters/base64 installed that pipeline fails and
it evaluates to "" which doesn't cause further issues.  But can still
fail in a bulk since packages are installed/removed all the time.

The notmuch warnings are not new.

> [...]
> > Can you try with this patch instead?  I only use aerc to get an
> > immediate notification when I receive an email (the window goes
> > urgent) and otherwise only use mblaze, so my testing is minimal.
> 
> I see. Your patch fails at the linking stage when converts/base64 is
> installed. if I pkg_delete that and remove it from the dependencies list
> it compiles fine.

This is strange.  You get the error I got prior to patching the
Makefile.  Can you make sure the patch added patches/patch-Makefile?

I've upstreamed it: https://lists.sr.ht/~rjarry/aerc-devel/patches/41653

> > By the way, since you seem to actively use it, would you mind to take
> > maintainership?  I'll still be around for reviewing diffs and help,
> > but would be better to have a maintainer that actually uses it.
> 
> Yes why not, I use aerc as my mail client so that works for me.

Thank you!

Since it builds and works fine for me I've gone ahead and committed
the update and the change of maintainer.

Reply via email to