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

common/archive/ar/writer.go

Index: common/archive/ar/writer.go
diff --git a/common/archive/ar/writer.go b/common/archive/ar/writer.go
new file mode 100644
index 0000000000000000000000000000000000000000..aac9cc3921af56b17a257df7b158d95f6b1d35ea
--- /dev/null
+++ b/common/archive/ar/writer.go
@@ -0,0 +1,223 @@
+// 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)
+}