From 70fb294a9b7476fd36fa21ea08de04854c0549cd Mon Sep 17 00:00:00 2001 From: Vincent Batts Date: Sat, 25 Mar 2023 14:41:27 -0400 Subject: [PATCH 1/5] tar/asm: go vet fixes on go1.19.7 Signed-off-by: Vincent Batts --- tar/asm/disassemble_test.go | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/tar/asm/disassemble_test.go b/tar/asm/disassemble_test.go index 4e45714..5e03597 100644 --- a/tar/asm/disassemble_test.go +++ b/tar/asm/disassemble_test.go @@ -18,12 +18,13 @@ func TestLargeJunkPadding(t *testing.T) { // Write a normal tar file into the pipe and then load it full of junk // bytes as padding. We have to do this in a goroutine because we can't // store 20GB of junk in-memory. - go func() { + var err error + go func(e error) { // Empty archive. tw := tar.NewWriter(pW) if err := tw.Close(); err != nil { pW.CloseWithError(err) - t.Fatal(err) + e = err return } @@ -35,7 +36,7 @@ func TestLargeJunkPadding(t *testing.T) { devZero, err := os.Open("/dev/zero") if err != nil { pW.CloseWithError(err) - t.Fatal(err) + e = err return } defer devZero.Close() @@ -45,14 +46,17 @@ func TestLargeJunkPadding(t *testing.T) { } if _, err := io.CopyN(pW, devZero, junkChunkSize); err != nil { pW.CloseWithError(err) - t.Fatal(err) + e = err return } } fmt.Fprintln(os.Stderr, "[TestLargeJunkPadding] junk chunk finished") pW.Close() - }() + }(err) + if err != nil { + t.Fatal(err) + } // Disassemble our junk file. nilPacker := storage.NewJSONPacker(ioutil.Discard) From 3c599ed534183c34d9478bd7db97d6bef3da89c0 Mon Sep 17 00:00:00 2001 From: Vincent Batts Date: Sat, 25 Mar 2023 18:19:53 -0400 Subject: [PATCH 2/5] travis: be gone with you! Signed-off-by: Vincent Batts --- .travis.yml | 22 ---------------------- 1 file changed, 22 deletions(-) delete mode 100644 .travis.yml diff --git a/.travis.yml b/.travis.yml deleted file mode 100644 index d2139f6..0000000 --- a/.travis.yml +++ /dev/null @@ -1,22 +0,0 @@ -language: go -arch: - - amd64 - - ppc64le -go: - - tip - - 1.15.x - - 1.14.x - - 1.13.x - - 1.12.x - - 1.11.x - - 1.10.x - -# let us have pretty, fast Docker-based Travis workers! -sudo: false - -install: - - go get -d ./... - -script: - - go test -v ./... - - go vet ./... From 19fa6f3d1e688e8531b2b6c32228774525aff868 Mon Sep 17 00:00:00 2001 From: Vincent Batts Date: Sat, 25 Mar 2023 14:46:31 -0400 Subject: [PATCH 3/5] github/workflow: first pass May add magefile/mage next, but it seems to require go1.17? So, I'm holding off for a minute. Signed-off-by: Vincent Batts --- .github/workflows/go.yml | 35 +++++++++++++++++++++++++++++++++++ .github/workflows/lint.yml | 35 +++++++++++++++++++++++++++++++++++ 2 files changed, 70 insertions(+) create mode 100644 .github/workflows/go.yml create mode 100644 .github/workflows/lint.yml diff --git a/.github/workflows/go.yml b/.github/workflows/go.yml new file mode 100644 index 0000000..8a3c2ca --- /dev/null +++ b/.github/workflows/go.yml @@ -0,0 +1,35 @@ +name: build and vet + +on: + pull_request: + branches_ignore: [] + +jobs: + build: + runs-on: ubuntu-latest + strategy: + matrix: + go: ['1.15', '1.16', '1.17', '1.18', '1.19', '1.20'] + + name: build and vet + steps: + + - uses: actions/checkout@v2 + with: + path: go/src/github.com/vbatts/tar-split + + - uses: actions/setup-go@v4 + with: + go-version: ${{ matrix.go }} + + - name: vet and build + env: + GOPATH: /home/runner/work/tar-split/tar-split/go + run: | + set -x + export PATH=$GOPATH/bin:$PATH + cd go/src/github.com/vbatts/tar-split + go test -v ./... + go vet -v ./... + go build -v ./... + #go run mage.go -v vet build test diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml new file mode 100644 index 0000000..cf62b63 --- /dev/null +++ b/.github/workflows/lint.yml @@ -0,0 +1,35 @@ +name: lint + +on: + pull_request: + branches_ignore: [] + +jobs: + lint: + runs-on: ubuntu-latest + strategy: + matrix: + go: ['1.20'] + + name: Linting + steps: + + - uses: actions/checkout@v2 + with: + path: go/src/github.com/vbatts/tar-split + + - uses: actions/setup-go@v4 + with: + go-version: ${{ matrix.go }} + + - name: lint + env: + GOPATH: /home/runner/work/tar-split/tar-split/go + run: | + set -x + #curl -sSL https://github.com/magefile/mage/releases/download/v1.14.0/mage_1.14.0_Linux-64bit.tar.gz | tar -xzv mage && mv mage $GOPATH/bin/ + go install github.com/golangci/golangci-lint/cmd/golangci-lint@v1.51.2 + export PATH=$GOPATH/bin:$PATH + cd go/src/github.com/vbatts/tar-split + golangci-lint run + #go run mage.go -v lint From 516158dbfbc0636590ac142038d778694104046e Mon Sep 17 00:00:00 2001 From: Vincent Batts Date: Sat, 25 Mar 2023 19:20:31 -0400 Subject: [PATCH 4/5] *.go: linting project specific code the pointer to the pool may be useful, but holding on that until I get benchmarks of memory use to show the benefit. Signed-off-by: Vincent Batts --- cmd/tar-split/tar_benchmark_test.go | 27 +++++++++++++++++++++------ tar/asm/assemble.go | 2 ++ tar/asm/disassemble_test.go | 11 ++--------- tar/storage/packer_test.go | 4 +++- 4 files changed, 28 insertions(+), 16 deletions(-) diff --git a/cmd/tar-split/tar_benchmark_test.go b/cmd/tar-split/tar_benchmark_test.go index f318645..950eccc 100644 --- a/cmd/tar-split/tar_benchmark_test.go +++ b/cmd/tar-split/tar_benchmark_test.go @@ -29,9 +29,14 @@ func BenchmarkUpstreamTar(b *testing.B) { fh.Close() b.Fatal(err) } - io.Copy(ioutil.Discard, tr) + _, err = io.Copy(ioutil.Discard, tr) + if err != nil { + b.Fatal(err) + } + } + if err := fh.Close(); err != nil { + b.Fatal(err) } - fh.Close() } } @@ -52,9 +57,14 @@ func BenchmarkOurTarNoAccounting(b *testing.B) { fh.Close() b.Fatal(err) } - io.Copy(ioutil.Discard, tr) + _, err = io.Copy(ioutil.Discard, tr) + if err != nil { + b.Fatal(err) + } + } + if err := fh.Close(); err != nil { + b.Fatal(err) } - fh.Close() } } func BenchmarkOurTarYesAccounting(b *testing.B) { @@ -76,9 +86,14 @@ func BenchmarkOurTarYesAccounting(b *testing.B) { fh.Close() b.Fatal(err) } - io.Copy(ioutil.Discard, tr) + _, err = io.Copy(ioutil.Discard, tr) + if err != nil { + b.Fatal(err) + } _ = tr.RawBytes() } - fh.Close() + if err := fh.Close(); err != nil { + b.Fatal(err) + } } } diff --git a/tar/asm/assemble.go b/tar/asm/assemble.go index d624450..3eb32ab 100644 --- a/tar/asm/assemble.go +++ b/tar/asm/assemble.go @@ -71,6 +71,8 @@ func WriteOutputTarStream(fg storage.FileGetter, up storage.Unpacker, w io.Write crcSum = make([]byte, 8) multiWriter = io.MultiWriter(w, crcHash) copyBuffer = byteBufferPool.Get().([]byte) + // TODO once we have some benchmark or memory profile then we can experiment with using *bytes.Buffer + //nolint:staticcheck // SA6002 not going to do a pointer here defer byteBufferPool.Put(copyBuffer) } else { crcHash.Reset() diff --git a/tar/asm/disassemble_test.go b/tar/asm/disassemble_test.go index 5e03597..4141aa0 100644 --- a/tar/asm/disassemble_test.go +++ b/tar/asm/disassemble_test.go @@ -18,13 +18,11 @@ func TestLargeJunkPadding(t *testing.T) { // Write a normal tar file into the pipe and then load it full of junk // bytes as padding. We have to do this in a goroutine because we can't // store 20GB of junk in-memory. - var err error - go func(e error) { + go func() { // Empty archive. tw := tar.NewWriter(pW) if err := tw.Close(); err != nil { pW.CloseWithError(err) - e = err return } @@ -36,7 +34,6 @@ func TestLargeJunkPadding(t *testing.T) { devZero, err := os.Open("/dev/zero") if err != nil { pW.CloseWithError(err) - e = err return } defer devZero.Close() @@ -46,17 +43,13 @@ func TestLargeJunkPadding(t *testing.T) { } if _, err := io.CopyN(pW, devZero, junkChunkSize); err != nil { pW.CloseWithError(err) - e = err return } } fmt.Fprintln(os.Stderr, "[TestLargeJunkPadding] junk chunk finished") pW.Close() - }(err) - if err != nil { - t.Fatal(err) - } + }() // Disassemble our junk file. nilPacker := storage.NewJSONPacker(ioutil.Discard) diff --git a/tar/storage/packer_test.go b/tar/storage/packer_test.go index 7d93371..2d8f2b6 100644 --- a/tar/storage/packer_test.go +++ b/tar/storage/packer_test.go @@ -199,7 +199,9 @@ func BenchmarkGetPut(b *testing.B) { b.Fatal(err) } } - fh.Sync() + if err := fh.Sync(); err != nil { + b.Fatal(err) + } up := NewJSONUnpacker(fh) for { From bc1624cbfc717d1e44865c48160dfa1dea0fe49e Mon Sep 17 00:00:00 2001 From: Vincent Batts Date: Sat, 25 Mar 2023 20:20:49 -0400 Subject: [PATCH 5/5] archive/tar: linting errors I intend to not make changes to this `archive/tar` that aren't from upstream, or are not directly related to the usage by this project... Signed-off-by: Vincent Batts --- archive/tar/reader.go | 12 +++++++----- archive/tar/tar_test.go | 4 ++-- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/archive/tar/reader.go b/archive/tar/reader.go index ea64a38..fcf3215 100644 --- a/archive/tar/reader.go +++ b/archive/tar/reader.go @@ -41,7 +41,7 @@ type fileReader interface { // RawBytes accesses the raw bytes of the archive, apart from the file payload itself. // This includes the header and padding. // -// This call resets the current rawbytes buffer +// # This call resets the current rawbytes buffer // // Only when RawAccounting is enabled, otherwise this returns nil func (tr *Reader) RawBytes() []byte { @@ -126,7 +126,9 @@ func (tr *Reader) next() (*Header, error) { return nil, err } if hdr.Typeflag == TypeXGlobalHeader { - mergePAX(hdr, paxHdrs) + if err = mergePAX(hdr, paxHdrs); err != nil { + return nil, err + } return &Header{ Name: hdr.Name, Typeflag: hdr.Typeflag, @@ -381,9 +383,9 @@ func parsePAX(r io.Reader) (map[string]string, error) { // header in case further processing is required. // // The err will be set to io.EOF only when one of the following occurs: -// * Exactly 0 bytes are read and EOF is hit. -// * Exactly 1 block of zeros is read and EOF is hit. -// * At least 2 blocks of zeros are read. +// - Exactly 0 bytes are read and EOF is hit. +// - Exactly 1 block of zeros is read and EOF is hit. +// - At least 2 blocks of zeros are read. func (tr *Reader) readHeader() (*Header, *block, error) { // Two blocks of zero bytes marks the end of the archive. n, err := io.ReadFull(tr.r, tr.blk[:]) diff --git a/archive/tar/tar_test.go b/archive/tar/tar_test.go index 6227e24..f1ce7fb 100644 --- a/archive/tar/tar_test.go +++ b/archive/tar/tar_test.go @@ -833,8 +833,8 @@ func Benchmark(b *testing.B) { // Write the archive to a byte buffer. tw := NewWriter(&buf) for _, file := range v.files { - tw.WriteHeader(file.hdr) - tw.Write(file.body) + _ = tw.WriteHeader(file.hdr) + _, _ = tw.Write(file.body) } tw.Close() b.Run(v.label, func(b *testing.B) {