This fixes piped full message (:pipe -m) being empty.

Fixes: 904ffacb0e52 ("maildir,notmuch: avoid leaking open files")
Signed-off-by: Nguyễn Gia Phong <mcsinyx@disroot.org>
---
BTW thank you Robin Jarry for the pointer to the recommendation
for Linux change log, TIL.
 worker/maildir/worker.go | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/worker/maildir/worker.go b/worker/maildir/worker.go
index a671d730fc25..2f75f12ae5e0 100644
--- a/worker/maildir/worker.go
+++ b/worker/maildir/worker.go
@@ -1,9 +1,11 @@
 package maildir
 
 import (
+	"bytes"
 	"errors"
 	"fmt"
 	"io"
+	"io/ioutil"
 	"net/url"
 	"os"
 	"path/filepath"
@@ -433,11 +435,15 @@ func (w *Worker) handleFetchFullMessages(msg *types.FetchFullMessages) error {
 			return err
 		}
 		defer r.Close()
+		b, err := ioutil.ReadAll(r)
+		if err != nil {
+			return err
+		}
 		w.worker.PostMessage(&types.FullMessage{
 			Message: types.RespondTo(msg),
 			Content: &models.FullMessage{
 				Uid:    uid,
-				Reader: r,
+				Reader: bytes.NewReader(b),
 			},
 		}, nil)
 	}
-- 
2.34.1

aerc/patches/.build.yml: SUCCESS in 1m14s

[maildir: pass in-memory message to callback][0] v2 from [Nguyễn Gia Phong][1]

[0]: https://lists.sr.ht/~rjarry/aerc-devel/patches/28543
[1]: mcsinyx@disroot.org

✓ #676513 SUCCESS aerc/patches/.build.yml https://builds.sr.ht/~rjarry/job/676513

Nguyễn Gia Phong, Jan 20, 2022 at 14:36:
> This fixes piped full message (:pipe -m) being empty.
>
> Fixes: 904ffacb0e52 ("maildir,notmuch: avoid leaking open files")
> Signed-off-by: Nguyễn Gia Phong <mcsinyx@disroot.org>
> ---
> BTW thank you Robin Jarry for the pointer to the recommendation
> for Linux change log, TIL.

You're welcome :)

>  worker/maildir/worker.go | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/worker/maildir/worker.go b/worker/maildir/worker.go
> index a671d730fc25..2f75f12ae5e0 100644
> --- a/worker/maildir/worker.go
> +++ b/worker/maildir/worker.go
> @@ -1,9 +1,11 @@
>  package maildir
>  
>  import (
> +	"bytes"
>  	"errors"
>  	"fmt"
>  	"io"
> +	"io/ioutil"
>  	"net/url"
>  	"os"
>  	"path/filepath"
> @@ -433,11 +435,15 @@ func (w *Worker) handleFetchFullMessages(msg *types.FetchFullMessages) error {
>  			return err
>  		}
>  		defer r.Close()
> +		b, err := ioutil.ReadAll(r)
> +		if err != nil {
> +			return err
> +		}
>  		w.worker.PostMessage(&types.FullMessage{
>  			Message: types.RespondTo(msg),
>  			Content: &models.FullMessage{
>  				Uid:    uid,
> -				Reader: r,
> +				Reader: bytes.NewReader(b),
>  			},
>  		}, nil)
>  	}

You forgot to address the same problem in notmuch.

Cheers,

On Thu Jan 20, 2022 at 14:36 CET, Nguyễn Gia Phong wrote:
> This fixes piped full message (:pipe -m) being empty.
>
> Fixes: 904ffacb0e52 ("maildir,notmuch: avoid leaking open files")
> Signed-off-by: Nguyễn Gia Phong <mcsinyx@disroot.org>
> ---
> BTW thank you Robin Jarry for the pointer to the recommendation
> for Linux change log, TIL.
> worker/maildir/worker.go | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/worker/maildir/worker.go b/worker/maildir/worker.go
> index a671d730fc25..2f75f12ae5e0 100644
> --- a/worker/maildir/worker.go
> +++ b/worker/maildir/worker.go
> @@ -1,9 +1,11 @@
> package maildir
>  
> import (
> + "bytes"
> "errors"
> "fmt"
> "io"
> + "io/ioutil"
> "net/url"
> "os"
> "path/filepath"
> @@ -433,11 +435,15 @@ func (w *Worker) handleFetchFullMessages(msg
> *types.FetchFullMessages) error {
> return err
> }
> defer r.Close()
> + b, err := ioutil.ReadAll(r)
> + if err != nil {
> + return err
> + }

just a drive-by comment: I don't know what's the policy w/ rjarry/aerc
with regard to supported Go versions (so please take the following with
a grain of salt), but shouldn't we limit our use of deprecated packages?
(io/ioutil is deprecated since ~1.15, IIRC)

ie: shouldn't this read:
b, err := io.ReadAll(r)

instead?

-s

Sebastien Binet, Jan 20, 2022 at 14:56:
> just a drive-by comment: I don't know what's the policy w/ rjarry/aerc
> with regard to supported Go versions (so please take the following
> with a grain of salt), but shouldn't we limit our use of deprecated
> packages? (io/ioutil is deprecated since ~1.15, IIRC)
>
> ie: shouldn't this read:
> b, err := io.ReadAll(r)
>
> instead?

Good point. However, io.ReadAll seems only available since Go 1.16 which
is not even one year old.

https://pkg.go.dev/io#ReadAll

I am all for avoiding deprecated API. But let's give some more time
before breaking support for older go toolchains.

On Thu Jan 20, 2022 at 15:19 CET, Robin Jarry wrote:
> Sebastien Binet, Jan 20, 2022 at 14:56:
> > just a drive-by comment: I don't know what's the policy w/ rjarry/aerc
> > with regard to supported Go versions (so please take the following
> > with a grain of salt), but shouldn't we limit our use of deprecated
> > packages? (io/ioutil is deprecated since ~1.15, IIRC)
> >
> > ie: shouldn't this read:
> > b, err := io.ReadAll(r)
> >
> > instead?
>
> Good point. However, io.ReadAll seems only available since Go 1.16 which
> is not even one year old.
>
> https://pkg.go.dev/io#ReadAll
>
> I am all for avoiding deprecated API. But let's give some more time
> before breaking support for older go toolchains.

IIRC, the Go project only supports the last 2 released versions.
Most of the Go projects I work (eg: Gonum) follow this lead.

I think we'd probably need to clarify this and clearly define somewhere
the policy aerc wants to follow in that regard.
right now, it seems to be "whatever Go version comes with Alpine/edge"
(if .build.yml is any witness).

Either way, I am fine with the decision.

-s

Sebastien Binet, Jan 20, 2022 at 16:15:
> I think we'd probably need to clarify this and clearly define somewhere
> the policy aerc wants to follow in that regard.
> right now, it seems to be "whatever Go version comes with Alpine/edge"
> (if .build.yml is any witness).

I agree that this is rather blurry.

I have been thinking about running builds on other distros as well.
Maybe with different go versions as well. I'll push a patch soon(tm).