diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a7411d8..87f5240 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1,5 +1,7 @@ name: go-exiftool-ci -on: push +on: + - push + - pull_request jobs: build: runs-on: ubuntu-latest @@ -19,4 +21,4 @@ jobs: - name: Tests run: go test -coverprofile=coverage.txt -covermode=atomic ./... - name: Coverage publication - run: bash <(curl -s https://codecov.io/bash) \ No newline at end of file + run: bash <(curl -s https://codecov.io/bash) diff --git a/exiftool.go b/exiftool.go index 6112591..b0451dc 100644 --- a/exiftool.go +++ b/exiftool.go @@ -4,21 +4,24 @@ import ( "bufio" "bytes" "encoding/json" + "errors" "fmt" "io" "os" "os/exec" "sync" - - "errors" + "time" ) var executeArg = "-execute" -var initArgs = []string{"-stay_open", "True", "-@", "-", "-common_args"} +var initArgs = []string{"-stay_open", "True", "-@", "-"} var extractArgs = []string{"-j"} var closeArgs = []string{"-stay_open", "False", executeArg} var readyTokenLen = len(readyToken) +// WaitTimeout specifies the duration to wait for exiftool to exit when closing before timing out +var WaitTimeout = time.Second + // ErrNotExist is a sentinel error for non existing file var ErrNotExist = errors.New("file does not exist") @@ -33,6 +36,7 @@ type Exiftool struct { bufferMaxSize int extraInitArgs []string exiftoolBinPath string + cmd *exec.Cmd } // NewExiftool instanciates a new Exiftool with configuration functions. If anything went @@ -48,16 +52,21 @@ func NewExiftool(opts ...func(*Exiftool) error) (*Exiftool, error) { } } - args := append(initArgs, e.extraInitArgs...) - cmd := exec.Command(e.exiftoolBinPath, args...) + args := append([]string(nil), initArgs...) + if len(e.extraInitArgs) > 0 { + args = append(args, "-common_args") + args = append(args, e.extraInitArgs...) + } + + e.cmd = exec.Command(e.exiftoolBinPath, args...) r, w := io.Pipe() e.stdMergedOut = r - cmd.Stdout = w - cmd.Stderr = w + e.cmd.Stdout = w + e.cmd.Stderr = w var err error - if e.stdin, err = cmd.StdinPipe(); err != nil { + if e.stdin, err = e.cmd.StdinPipe(); err != nil { return nil, fmt.Errorf("error when piping stdin: %w", err) } @@ -67,8 +76,8 @@ func NewExiftool(opts ...func(*Exiftool) error) (*Exiftool, error) { } e.scanMergedOut.Split(splitReadyToken) - if err = cmd.Start(); err != nil { - return nil, fmt.Errorf("error when executing commande: %w", err) + if err = e.cmd.Start(); err != nil { + return nil, fmt.Errorf("error when executing command: %w", err) } return &e, nil @@ -95,6 +104,24 @@ func (e *Exiftool) Close() error { errs = append(errs, fmt.Errorf("error while closing stdin: %w", err)) } + ch := make(chan struct{}) + go func() { + if e.cmd != nil { + if err := e.cmd.Wait(); err != nil { + errs = append(errs, fmt.Errorf("error while waiting for exiftool to exit: %w", err)) + } + } + ch <- struct{}{} + close(ch) + }() + + // Wait for wait to finish or timeout + select { + case <- ch: + case <- time.After(WaitTimeout): + errs = append(errs, errors.New("Timed out waiting for exiftool to exit")) + } + if len(errs) > 0 { return fmt.Errorf("error while closing exiftool: %v", errs) } @@ -229,4 +256,4 @@ func SetExiftoolBinaryPath(p string) func(*Exiftool) error { e.exiftoolBinPath = p return nil } -} \ No newline at end of file +} diff --git a/exiftool_test.go b/exiftool_test.go index c049db7..7b99221 100644 --- a/exiftool_test.go +++ b/exiftool_test.go @@ -11,6 +11,8 @@ import ( ) func TestNewExiftoolEmpty(t *testing.T) { + t.Parallel() + e, err := NewExiftool() assert.Nil(t, err) @@ -18,6 +20,8 @@ func TestNewExiftoolEmpty(t *testing.T) { } func TestNewExifToolOptOk(t *testing.T) { + t.Parallel() + var exec1, exec2 bool f1 := func(*Exiftool) error { @@ -40,6 +44,8 @@ func TestNewExifToolOptOk(t *testing.T) { } func TestNewExifToolOptKo(t *testing.T) { + t.Parallel() + f := func(*Exiftool) error { return fmt.Errorf("err") } @@ -61,6 +67,8 @@ func TestSingleExtract(t *testing.T) { for _, tc := range tcs { tc := tc // Pin variable t.Run(tc.tcID, func(t *testing.T) { + t.Parallel() + e, err := NewExiftool() assert.Nilf(t, err, "error not nil: %v", err) defer e.Close() @@ -75,6 +83,8 @@ func TestSingleExtract(t *testing.T) { } func TestMultiExtract(t *testing.T) { + t.Parallel() + e, err := NewExiftool() assert.Nilf(t, err, "error not nil: %v", err) @@ -168,6 +178,16 @@ func TestCloseErrorOnStdout(t *testing.T) { assert.True(t, wClosed) } +func TestCloseExifToolNominal(t *testing.T) { + t.Parallel() + + e, err := NewExiftool() + + assert.Nil(t, err) + assert.Nil(t, e.Close()) +} + + type readWriteCloserMock struct { writeInt int writeErr error @@ -191,6 +211,8 @@ func (e readWriteCloserMock) Close() error { } func TestBuffer(t *testing.T) { + t.Parallel() + e, err := NewExiftool() assert.Nil(t, err) defer e.Close() @@ -204,6 +226,8 @@ func TestBuffer(t *testing.T) { } func TestNewExifTool_WithBuffer(t *testing.T) { + t.Parallel() + buf := make([]byte, 128*1000) e, err := NewExiftool(Buffer(buf, 64*1000)) assert.Nil(t, err) @@ -215,6 +239,8 @@ func TestNewExifTool_WithBuffer(t *testing.T) { } func TestCharset(t *testing.T) { + t.Parallel() + e, err := NewExiftool() assert.Nil(t, err) defer e.Close() @@ -227,6 +253,8 @@ func TestCharset(t *testing.T) { } func TestNewExifTool_WithCharset(t *testing.T) { + t.Parallel() + e, err := NewExiftool(Charset("filename=utf8")) assert.Nil(t, err) defer e.Close() @@ -237,6 +265,8 @@ func TestNewExifTool_WithCharset(t *testing.T) { } func TestNoPrintConversion(t *testing.T) { + t.Parallel() + e, err := NewExiftool(NoPrintConversion()) assert.Nil(t, err) defer e.Close() @@ -256,6 +286,8 @@ func TestNoPrintConversion(t *testing.T) { } func TestExtractEmbedded(t *testing.T) { + t.Parallel() + eWithout, err := NewExiftool() assert.Nil(t, err) defer eWithout.Close() @@ -277,6 +309,8 @@ func TestExtractEmbedded(t *testing.T) { } func TestExtractAllBinaryMetadata(t *testing.T) { + t.Parallel() + eWithout, err := NewExiftool() assert.Nil(t, err) defer eWithout.Close() @@ -299,6 +333,8 @@ func TestExtractAllBinaryMetadata(t *testing.T) { } func TestSetExiftoolBinaryPath(t *testing.T) { + t.Parallel() + // default eDefault, err := NewExiftool() assert.Nil(t, err) @@ -322,4 +358,4 @@ func TestSetExiftoolBinaryPath(t *testing.T) { _, err = NewExiftool(SetExiftoolBinaryPath("/non/existing/path")) assert.NotNil(t, err) -} \ No newline at end of file +}