Repository: goreleaser/goreleaser
Pull request #1422 (2020-04-03 20:17:04)

feat: xz compression
If applied, this commit will allow users to use the .xz and .tar.xz file formats.

This will allow for higher compression ratios.

This is a work in progress.  The code is working and creates valid `.tar.xz` files within the `dist` directory.  I need some help with `xz_test.go` and `tarxz_test.go` as these test are failing when running `make test`. I am looking for guidance on how to fix this.  Also, I feel confident that `.tar.xz` files are properly created, but would like someone to also test just `.xz` files.

```
    --- FAIL: TestXzFile (0.01s)
    xz_test.go:23:
                Error Trace:    xz_test.go:23
                Error:          An error is expected but got nil.
                Test:           TestXzFile
    xz_test.go:28: /tmp/774984967/test.xz
    xz_test.go:45:
                Error Trace:    xz_test.go:45
                Error:          Not equal:
                                expected: "sub\n"
                                actual  : "sub\nfoo\n"

                                Diff:
                                --- Expected
                                +++ Actual
                                @@ -1,2 +1,3 @@
                                 sub
                                +foo

                Test:           TestXzFile
    FAIL
    FAIL        github.com/goreleaser/goreleaser/pkg/archive/xz 0.073s
```





caarlos0: I'm not sure about the tests as well, when I have some time will checkout and see
caarlos0: I think the problem with the `.xz` right now is that we can't find a way to tell if it already has a file in it.

on gzip we have this if:

```go
	if a.gw.Header.Name != "" {
		return fmt.Errorf("gzip: failed to add %s, only one file can be archived in gz format", name)
	}
```

so it will error if you try more than one file.

`.Header` is not available on the xz writer, so we need to keep that internally... I made it work like this:

```diff
diff --git a/pkg/archive/xz/xz.go b/pkg/archive/xz/xz.go
index ea9f4ca..9cf6bdb 100644
--- a/pkg/archive/xz/xz.go
+++ b/pkg/archive/xz/xz.go
@@ -3,7 +3,7 @@
 package xz
 
 import (
-	//"fmt"
+	"fmt"
 	"io"
 	"os"
 
@@ -12,28 +12,28 @@ import (
 
 // Archive as xz
 type Archive struct {
-	xzw *xz.Writer
+	xzw  *xz.Writer
+	name string
 }
 
 // Close all closeables
-func (a Archive) Close() error {
+func (a *Archive) Close() error {
 	return a.xzw.Close()
 }
 
 // New xz archive
-func New(target io.Writer) Archive {
+func New(target io.Writer) *Archive {
 	xzw, _ := xz.NewWriter(target)
-	return Archive{
+	return &Archive{
 		xzw: xzw,
 	}
 }
 
 // Add file to the archive
-func (a Archive) Add(name, path string) error {
-	/*
-	if a.xzw.Header.Name != "" {
+func (a *Archive) Add(name, path string) error {
+	if a.name != "" {
 		return fmt.Errorf("xz: failed to add %s, only one file can be archived in xz format", name)
-	} */
+	}
 	file, err := os.Open(path) // #nosec
 	if err != nil {
 		return err
@@ -46,8 +46,7 @@ func (a Archive) Add(name, path string) error {
 	if info.IsDir() {
 		return nil
 	}
-	//a.xzw.Header.Name = name
-	//a.xzw.Header.ModTime = info.ModTime()
 	_, err = io.Copy(a.xzw, file)
+	a.name = name
 	return err
 }
diff --git a/pkg/archive/xz/xz_test.go b/pkg/archive/xz/xz_test.go
index 3c0f512..af630a0 100644
--- a/pkg/archive/xz/xz_test.go
+++ b/pkg/archive/xz/xz_test.go
@@ -28,7 +28,7 @@ func TestXzFile(t *testing.T) {
 	t.Log(f.Name())
 	f, err = os.Open(f.Name())
 	assert.NoError(err)
-	//defer f.Close() // nolint: errcheck
+	defer f.Close() // nolint: errcheck
 
 	info, err := f.Stat()
 	assert.NoError(err)
@@ -36,9 +36,6 @@ func TestXzFile(t *testing.T) {
 
 	xzf, err := xz.NewReader(f)
 	assert.NoError(err)
-	//defer xzf.Close() // nolint: errcheck
-
-	//assert.Equal("sub1/sub2/subfoo.txt", xzf.Name)
 
 	bts, err := ioutil.ReadAll(xzf)
 	assert.NoError(err)
```

which is pretty ugly, but works 🤔 
caarlos0: but still, I'm not sure `.xz` only is useful (as I also don't think anyone uses `.gz` only on goreleaser), as it seems permissions are lost...

maybe keep `tar.xz` only?
jftuga: Hi Carlos,

Thank you for taking the time to investigate this.  I really like the idea of skipping .xz and only having .tar.xz.  I will create this patch and push it.  I noticed that my branch is out of date with the base branch.  What do I do first? Do I *Update Branch* and then push to my *xz_compression* branch or vice-versa?

caarlos0: hey!

> Do I _Update Branch_ and then push to my _xz_compression_ branch or vice-versa?

either way should be fine :) 

jftuga: I have pushed `remove .xz feature` and have updated my branch.  It also passed `make test`.

I also used this version along with `.tar.xz` in my `.goreleaser.yml` file to build one of my own projects.  This worked without any problems.

jftuga: I also just fixed a code formatting issue by using `gofmt`.

jftuga: I was also able to improve the compression by increasing the xz dictionary size.

See: https://github.com/ulikunitz/xz/issues/32


caarlos0: awesome, some file is not gofmt'ed again it seems but otherwise lgtm
codecov-io: # [Codecov](https://codecov.io/gh/goreleaser/goreleaser/pull/1422?src=pr&el=h1) Report
> Merging [#1422](https://codecov.io/gh/goreleaser/goreleaser/pull/1422?src=pr&el=desc) into [master](https://codecov.io/gh/goreleaser/goreleaser/commit/a5f0343368beb3e0aa26b09eee6ca1e0a476a850&el=desc) will **decrease** coverage by `0.09%`.
> The diff coverage is `73.33%`.

[![Impacted file tree graph](https://codecov.io/gh/goreleaser/goreleaser/pull/1422/graphs/tree.svg?width=650&height=150&src=pr&token=3bJvv6b7hN)](https://codecov.io/gh/goreleaser/goreleaser/pull/1422?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master    #1422      +/-   ##
==========================================
- Coverage   83.91%   83.82%   -0.10%     
==========================================
  Files          58       59       +1     
  Lines        3494     3524      +30     
==========================================
+ Hits         2932     2954      +22     
- Misses        478      482       +4     
- Partials       84       88       +4     
```


| [Impacted Files](https://codecov.io/gh/goreleaser/goreleaser/pull/1422?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pkg/archive/tarxz/tarxz.go](https://codecov.io/gh/goreleaser/goreleaser/pull/1422/diff?src=pr&el=tree#diff-cGtnL2FyY2hpdmUvdGFyeHovdGFyeHouZ28=) | `71.42% <71.42%> (ø)` | |
| [pkg/archive/archive.go](https://codecov.io/gh/goreleaser/goreleaser/pull/1422/diff?src=pr&el=tree#diff-cGtnL2FyY2hpdmUvYXJjaGl2ZS5nbw==) | `100.00% <100.00%> (ø)` | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/goreleaser/goreleaser/pull/1422?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/goreleaser/goreleaser/pull/1422?src=pr&el=footer). Last update [a5f0343...9060ce3](https://codecov.io/gh/goreleaser/goreleaser/pull/1422?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/go.mod b/go.mod
index 67d88aa1..0de879d7 100644
--- a/go.mod
+++ b/go.mod
@@ -19,6 +19,7 @@ require (
 	github.com/mitchellh/go-homedir v1.1.0
 	github.com/pkg/errors v0.9.1
 	github.com/stretchr/testify v1.5.1
+	github.com/ulikunitz/xz v0.5.6
 	github.com/xanzy/go-gitlab v0.29.0
 	gocloud.dev v0.19.0
 	golang.org/x/net v0.0.0-20200114155413-6afb5195e5aa // indirect
diff --git a/pkg/archive/archive.go b/pkg/archive/archive.go
index 2531a472..978e1ca4 100644
--- a/pkg/archive/archive.go
+++ b/pkg/archive/archive.go
@@ -7,6 +7,7 @@ import (
 
 	"github.com/goreleaser/goreleaser/pkg/archive/gzip"
 	"github.com/goreleaser/goreleaser/pkg/archive/targz"
+	"github.com/goreleaser/goreleaser/pkg/archive/tarxz"
 	"github.com/goreleaser/goreleaser/pkg/archive/zip"
 )
 
@@ -24,6 +25,9 @@ func New(file *os.File) Archive {
 	if strings.HasSuffix(file.Name(), ".gz") {
 		return gzip.New(file)
 	}
+	if strings.HasSuffix(file.Name(), ".tar.xz") {
+		return tarxz.New(file)
+	}
 	if strings.HasSuffix(file.Name(), ".zip") {
 		return zip.New(file)
 	}
diff --git a/pkg/archive/tarxz/tarxz.go b/pkg/archive/tarxz/tarxz.go
new file mode 100644
index 00000000..21fc98a1
--- /dev/null
+++ b/pkg/archive/tarxz/tarxz.go
@@ -0,0 +1,61 @@
+// Package tarxz implements the Archive interface providing tar.xz archiving
+// and compression.
+package tarxz
+
+import (
+	"archive/tar"
+	"io"
+	"os"
+
+	"github.com/ulikunitz/xz"
+)
+
+// Archive as tar.xz
+type Archive struct {
+	xzw *xz.Writer
+	tw  *tar.Writer
+}
+
+// Close all closeables
+func (a Archive) Close() error {
+	if err := a.tw.Close(); err != nil {
+		return err
+	}
+	return a.xzw.Close()
+}
+
+// New tar.xz archive
+func New(target io.Writer) Archive {
+	xzw, _ := xz.WriterConfig{DictCap: 16 * 1024 * 1024}.NewWriter(target)
+	tw := tar.NewWriter(xzw)
+	return Archive{
+		xzw: xzw,
+		tw:  tw,
+	}
+}
+
+// Add file to the archive
+func (a Archive) Add(name, path string) error {
+	file, err := os.Open(path) // #nosec
+	if err != nil {
+		return err
+	}
+	defer file.Close() // nolint: errcheck
+	info, err := file.Stat()
+	if err != nil {
+		return err
+	}
+	header, err := tar.FileInfoHeader(info, name)
+	if err != nil {
+		return err
+	}
+	header.Name = name
+	if err = a.tw.WriteHeader(header); err != nil {
+		return err
+	}
+	if info.IsDir() {
+		return nil
+	}
+	_, err = io.Copy(a.tw, file)
+	return err
+}


Test patch:
diff --git a/pkg/archive/archive_test.go b/pkg/archive/archive_test.go
index 342c9f7c..6bdc67d9 100644
--- a/pkg/archive/archive_test.go
+++ b/pkg/archive/archive_test.go
@@ -16,7 +16,7 @@ func TestArchive(t *testing.T) {
 	assert.NoError(err)
 	assert.NoError(os.Mkdir(folder+"/folder-inside", 0755))
 
-	for _, format := range []string{"tar.gz", "zip", "gz", "willbeatargzanyway"} {
+	for _, format := range []string{"tar.gz", "zip", "gz", "tar.xz", "willbeatargzanyway"} {
 		format := format
 		t.Run(format, func(t *testing.T) {
 			var archive = newArchive(folder, format, t)
diff --git a/pkg/archive/tarxz/tarxz_test.go b/pkg/archive/tarxz/tarxz_test.go
new file mode 100644
index 00000000..9b8400fd
--- /dev/null
+++ b/pkg/archive/tarxz/tarxz_test.go
@@ -0,0 +1,72 @@
+package tarxz
+
+import (
+	"archive/tar"
+	"io"
+	"io/ioutil"
+	"os"
+	"path/filepath"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/ulikunitz/xz"
+)
+
+func TestTarXzFile(t *testing.T) {
+	var assert = assert.New(t)
+	tmp, err := ioutil.TempDir("", "")
+	assert.NoError(err)
+	f, err := os.Create(filepath.Join(tmp, "test.tar.xz"))
+	assert.NoError(err)
+	defer f.Close() // nolint: errcheck
+	archive := New(f)
+
+	assert.Error(archive.Add("nope.txt", "../testdata/nope.txt"))
+	assert.NoError(archive.Add("foo.txt", "../testdata/foo.txt"))
+	assert.NoError(archive.Add("sub1", "../testdata/sub1"))
+	assert.NoError(archive.Add("sub1/bar.txt", "../testdata/sub1/bar.txt"))
+	assert.NoError(archive.Add("sub1/executable", "../testdata/sub1/executable"))
+	assert.NoError(archive.Add("sub1/sub2", "../testdata/sub1/sub2"))
+	assert.NoError(archive.Add("sub1/sub2/subfoo.txt", "../testdata/sub1/sub2/subfoo.txt"))
+
+	assert.NoError(archive.Close())
+	assert.Error(archive.Add("tar.go", "tar.go"))
+	assert.NoError(f.Close())
+
+	t.Log(f.Name())
+	f, err = os.Open(f.Name())
+	assert.NoError(err)
+	defer f.Close() // nolint: errcheck
+
+	info, err := f.Stat()
+	assert.NoError(err)
+	assert.Truef(info.Size() < 500, "archived file should be smaller than %d", info.Size())
+
+	xzf, err := xz.NewReader(f)
+	assert.NoError(err)
+	//defer xzf.Close() // nolint: errcheck
+
+	var paths []string
+	r := tar.NewReader(xzf)
+	for {
+		next, err := r.Next()
+		if err == io.EOF {
+			break
+		}
+		assert.NoError(err)
+		paths = append(paths, next.Name)
+		t.Logf("%s: %v", next.Name, next.FileInfo().Mode())
+		if next.Name == "sub1/executable" {
+			var ex = next.FileInfo().Mode() | 0111
+			assert.Equal(next.FileInfo().Mode().String(), ex.String())
+		}
+	}
+	assert.Equal([]string{
+		"foo.txt",
+		"sub1",
+		"sub1/bar.txt",
+		"sub1/executable",
+		"sub1/sub2",
+		"sub1/sub2/subfoo.txt",
+	}, paths)
+}