luci-go: Tools for working with BSD style ar archives.

common/archive/ar/writer.go

+// Copyright 2016 The LUCI Authors. All rights reserved.
+// Use of this source code is governed under the Apache License, Version 2.0
+// that can be found in the LICENSE file.
+
+// Write an ar archive file with BSD style filenames.
+
+package ar
+
+import (
+        "errors"
+        "fmt"
+        "io"
+        "log"
+        "os"
+)
+
+const DEFAULT_MODTIME = 1447140471
+const DEFAULT_USER = 1000
+const DEFAULT_GROUP = 1000
+const DEFAULT_MODE = 0100640 // 100640 -- Octal
+
+type writerStage uint
+
+const (
+        WRITE_HEADER writerStage = iota

[M-A Ruel, 2016/06/10 17:51:11]

Use golang style;
https://github.com/golang/go/wiki/CodeReviewComments#mixed-caps
No need to export

[mithro, 2016/06/14 10:57:52]

Done.

+        WRITE_BODY
+        WRITE_CLOSED
+)
+
+type Writer struct {
+        w     io.Writer
+        stage writerStage

[M-A Ruel, 2016/06/10 17:51:11]

make it a bool, unless you want to delay the "!<arch>\n" write but I don't think
it's a good idea to delay.

[mithro, 2016/06/14 10:57:53]

Done.

+
+        bytesrequired int64
+        needspadding  bool
+}
+
+func NewWriter(w io.Writer) *Writer {
+        io.WriteString(w, "!<arch>\n")

[M-A Ruel, 2016/06/10 17:51:11]

do not ignore the error here
or lazy write the header but I don't think it's worth

[mithro, 2016/06/14 10:57:52]

Done.

+        return &Writer{w: w, stage: WRITE_HEADER}
+}
+
+func (aw *Writer) Close() error {
+        switch aw.stage {

[M-A Ruel, 2016/06/10 17:51:11]

same comment as reader, just flush the file correctly if possible, ignore double
close

[mithro, 2016/06/14 10:57:52]

Most things report an error on a double Close() (as it is normally an indicator
of a more serious problem). tarfile does and so does File() see
https://play.golang.org/p/_OBK2KeAk1

+        case WRITE_HEADER:
+                // Good
+        case WRITE_BODY:
+                return errors.New("usage error, writing a file")
+        case WRITE_CLOSED:
+                return errors.New("usage error, archive already closed")
+        default:
+                log.Fatalf("unknown writer mode: %d", aw.stage)
+        }
+        aw.stage = WRITE_CLOSED
+        aw.w = nil
+        return nil
+}
+
+func (aw *Writer) wroteBytes(numbytes int64) error {
+        if numbytes > aw.bytesrequired {
+                log.Fatalf("to much data written, needed %d, got %d", aw.bytesrequired, numbytes)
+        }
+
+        aw.bytesrequired -= numbytes
+        if aw.bytesrequired != 0 {
+                return nil
+        }
+
+        // Padding to 16bit boundary
+        if aw.needspadding {
+                io.WriteString(aw.w, "\n")
+                aw.needspadding = false
+        }
+        aw.stage = WRITE_HEADER
+        return nil
+}
+
+// Check you can write bytes to the ar at this moment.

[M-A Ruel, 2016/06/10 17:51:11]

// canWriteContent returns nil if the stream is in a position to write a stream
content.

[mithro, 2016/06/14 10:57:53]

Done.

+func (aw *Writer) checkWrite() error {
+        switch aw.stage {

[M-A Ruel, 2016/06/10 17:51:11]

if a.r == nil {
  return errors.New(...
}
if a.inHeader {
  return errors.New(...
}
return nil

[mithro, 2016/06/14 10:57:52]

I still don't like this change, I personally think that having a single variable
which tracks the state is less error prone because you always check the whole
enum.

+        case WRITE_HEADER:
+                return errors.New("usage error, need to write header first")
+                // Good
+        case WRITE_BODY:
+                return nil
+        case WRITE_CLOSED:
+                return errors.New("usage error, archive closed")
+        default:
+                log.Fatalf("unknown writer mode: %d", aw.stage)
+        }
+        return nil
+}
+
+// Check we have finished writing bytes
+func (aw *Writer) checkFinished() {
+        if aw.bytesrequired != 0 {
+                log.Fatalf("didn't write enough bytes %d still needed, archive corrupted", aw.bytesrequired)

[M-A Ruel, 2016/06/10 17:51:11]

return errors.New()

then have the call sites return this call.

[mithro, 2016/06/14 10:57:52]

Kinda done - see my comment previously.

+        }
+}
+
+func (aw *Writer) writePartial(data []byte) error {
+        err := aw.checkWrite()

[M-A Ruel, 2016/06/10 17:51:11]

similar to reader, inline here and at other places where possible so the the
number of LoC is reduced

[mithro, 2016/06/14 10:57:52]

Done.

+        if err != nil {
+                return err
+        }
+
+        datalen := int64(len(data))
+        if datalen > aw.bytesrequired {
+                return fmt.Errorf("to much data, needed %d, got %d", aw.bytesrequired, datalen)
+        }
+
+        aw.w.Write(data)
+        aw.wroteBytes(datalen)
+        return nil
+}
+
+func (aw *Writer) WriteReader(data io.Reader) error {

[M-A Ruel, 2016/06/10 17:51:11]

https://golang.org/pkg/io/#ReaderFrom would probably be more appropriate. 

[mithro, 2016/06/14 10:57:52]

I agree that interface looks like what this should implement. It does kind of
break the naming though :(

+        err := aw.checkWrite()
+        if err != nil {
+                return err
+        }
+
+        count, err := io.Copy(aw.w, data)
+        if err != nil {
+                log.Fatalf("error while copying (%s), archive is probably corrupted", err)
+        }
+        aw.wroteBytes(count)
+        aw.checkFinished()
+
+        return nil
+}
+
+func (aw *Writer) WriteBytes(data []byte) error {

[M-A Ruel, 2016/06/10 17:51:11]

https://golang.org/pkg/io/#ReaderFrom would probably be more appropriate. Then
it's a io.Copy() call.
add doc

[mithro, 2016/06/14 10:57:53]

io.Copy() seems to use this function under the hood if it is available but is
more generic it seems?
Done.

+        err := aw.checkWrite()
+        if err != nil {
+                return err
+        }
+
+        datalen := int64(len(data))
+        if datalen != aw.bytesrequired {
+                return fmt.Errorf("wrong amount of data, needed %d, got %d", aw.bytesrequired, datalen)
+        }
+
+        aw.writePartial(data)

[M-A Ruel, 2016/06/10 17:51:11]

return value is ignored

[mithro, 2016/06/14 10:57:53]

Fixed.

+        aw.checkFinished()
+        return nil
+}
+
+func (aw *Writer) writeHeaderBytes(name string, size int64, modtime uint64, ownerid uint, groupid uint, filemod uint) error {

[M-A Ruel, 2016/06/10 17:51:11]

writeHeaderInt
or something like that?

[mithro, 2016/06/14 10:57:52]

Went with writeHeaderInternal

+        switch aw.stage {
+        case WRITE_HEADER:
+                // Good
+        case WRITE_BODY:
+                return errors.New("usage error, already writing a file.")
+        case WRITE_CLOSED:
+                return errors.New("usage error, archive closed.")
+        default:
+                log.Fatalf("unknown writer mode: %d", aw.stage)
+        }
+
+        // File name length prefixed with '#1/' (BSD variant), 16 bytes
+        fmt.Fprintf(aw.w, "#1/%-13d", len(name))

[M-A Ruel, 2016/06/10 17:51:11]

do not ignore the return value

[mithro, 2016/06/14 10:57:52]

Done.

+
+        // Modtime, 12 bytes
+        fmt.Fprintf(aw.w, "%-12d", modtime)
+
+        // Owner ID, 6 bytes
+        fmt.Fprintf(aw.w, "%-6d", ownerid)
+
+        // Group ID, 6 bytes
+        fmt.Fprintf(aw.w, "%-6d", groupid)
+
+        // File mode, 8 bytes
+        fmt.Fprintf(aw.w, "%-8o", filemod)
+
+        // In BSD variant, file size includes the filename length
+        aw.bytesrequired = int64(len(name)) + size
+
+        // File size, 10 bytes
+        fmt.Fprintf(aw.w, "%-10d", aw.bytesrequired)
+
+        // File magic, 2 bytes
+        io.WriteString(aw.w, "\x60\n")
+
+        aw.stage = WRITE_BODY
+        aw.needspadding = (aw.bytesrequired%2 != 0)
+
+        // Filename - BSD variant
+        return aw.writePartial([]byte(name))
+}
+
+func (aw *Writer) WriteHeaderDefault(name string, size int64) error {
+        return aw.writeHeaderBytes(name, size, 1447140471, DEFAULT_USER, DEFAULT_GROUP, DEFAULT_MODE)
+}
+
+func (aw *Writer) WriteHeader(stat os.FileInfo) error {
+        if stat.IsDir() {
+                return errors.New("only work with files, not directories")
+        }
+
+        mode := stat.Mode()
+        if mode&os.ModeSymlink == os.ModeSymlink {
+                return errors.New("only work with files, not symlinks")
+        }
+
+        /* FIXME: Should we also exclude other "special" files?
+        if (stat.Mode().ModeType != 0) {
+                return &argError{stat, "Only work with plain files."}
+        }
+        */
+
+        // FIXME: Where do we get user/group from - they don't appear to be in Go's Mode() object?

[M-A Ruel, 2016/06/10 17:51:11]

Don't bother, I'd prefer you to not claim to support that

[mithro, 2016/06/14 10:57:53]

Done.

+        return aw.writeHeaderBytes(stat.Name(), stat.Size(), uint64(stat.ModTime().Unix()), DEFAULT_USER, DEFAULT_GROUP, uint(mode&os.ModePerm))
+}
+
+func (aw *Writer) Add(name string, data []byte) error {
+        err := aw.WriteHeaderDefault(name, int64(len(data)))
+        if err != nil {
+                return err
+        }
+
+        return aw.WriteBytes(data)
+}