From e79f7cbbacf1a40cc72ad181e6ebee53973c827a Mon Sep 17 00:00:00 2001
From: Jay Lim <jay@cockroachlabs.com>
Date: Wed, 7 Oct 2020 19:05:10 -0400
Subject: [PATCH] convert: fix missing assets due to symlinks pointing to the
 same source in different inputs
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Symlinks implementation was added in https://github.com/kevinburke/go-bindata/commit/465fc29db9b716ec2ac11b1125939d383c143845.
However, that did not account for the fact that multiple symlinks can point to
the same source. When that happens, we would still want assets to be generated
accordingly.

This commit fixes missing assets due to symlinks pointing to the same source
in different inputs. Consider the following example:

symlinkFile
└── file1 -> ../symlinkSrc/file1
symlinkFileDup
└── file1 -> ../symlinkSrc/file1
symlinkSrc
└── file1

If both symlinkFile and symlinkFileDup are passed to go-bindata, we would
still want both assets to be generated. The current implementation only
generates the first asset encountered, which is symlinkFile/file1.

That said, we still have a bug in which multiple symlinks in the same *input*
pointing to the same source won't be generated properly. Consider the
following case:

├── file1 -> ../symlinkSrc/file1
└── file2 -> ../symlinkSrc/file1

We would expect both file1 and file2 to be generated, but that is not the
case. Fixing this requires a much more thorough approach as the current
implementation does not support that easily. For now, we could generate those
two files by passing both file1 and file2 as separate *inputs*.

Signed-off-by: Jay Lim <6175557+imjching@users.noreply.github.com>
---
 convert.go                    |  2 +-
 convert_test.go               | 35 +++++++++++++++++++++++++++++++++++
 filewriter.go                 |  5 ++++-
 testdata/symlinkFileDup/file1 |  1 +
 4 files changed, 41 insertions(+), 2 deletions(-)
 create mode 120000 testdata/symlinkFileDup/file1

diff --git a/convert.go b/convert.go
index b991303..5185ed0 100644
--- a/convert.go
+++ b/convert.go
@@ -26,9 +26,9 @@ func Translate(c *Config) error {
 	}
 
 	var knownFuncs = make(map[string]int)
-	var visitedPaths = make(map[string]bool)
 	// Locate all the assets.
 	for _, input := range c.Input {
+		var visitedPaths = make(map[string]bool)
 		if err := findFiles(input.Path, c.Prefix, input.Recursive, &toc, c.Ignore, knownFuncs, visitedPaths); err != nil {
 			return err
 		}
diff --git a/convert_test.go b/convert_test.go
index 6b5c971..c507774 100644
--- a/convert_test.go
+++ b/convert_test.go
@@ -1,6 +1,7 @@
 package bindata
 
 import (
+	"os"
 	"regexp"
 	"strings"
 	"testing"
@@ -134,3 +135,37 @@ func TestNoPrefixExtensionMatch(t *testing.T) {
 		t.Fatal("should have returned err retrieving nonexistent file, got nil")
 	}
 }
+
+func TestTranslate(t *testing.T) {
+	t.Run("multiple symlinks pointing to the same file", func(t *testing.T) {
+		// Build the config.
+		c := NewConfig()
+		c.Input = []InputConfig{
+			{Path: "testdata/symlinkFile", Recursive: true},
+			{Path: "testdata/symlinkFileDup", Recursive: true},
+		}
+		c.Output = "testdata/symlink_test.go" // Dummy value that isn't used
+
+		// Stub out implementation for diffAndWrite.
+		var output string
+		oldDiffAndWrite := diffAndWrite
+		diffAndWrite = func(filename string, data []byte, mode os.FileMode) error {
+			output = string(data)
+			return nil
+		}
+		defer func() { diffAndWrite = oldDiffAndWrite }()
+
+		if err := Translate(c); err != nil {
+			t.Fatal(err)
+		}
+
+		if len(output) == 0 {
+			t.Fatal("should have data in output file")
+		}
+
+		if !strings.Contains(output, "testdata/symlinkFile/file1") ||
+			!strings.Contains(output, "testdata/symlinkFileDup/file1") {
+			t.Fatal("should have data for both symlinkFile and symlinkFileDup")
+		}
+	})
+}
diff --git a/filewriter.go b/filewriter.go
index 2e613c9..957f1e9 100644
--- a/filewriter.go
+++ b/filewriter.go
@@ -7,7 +7,10 @@ import (
 	"os"
 )
 
-func diffAndWrite(filename string, data []byte, mode os.FileMode) error {
+// Note: diffAndWrite is doubled for testing purposes.
+var diffAndWrite = diffAndWriteImpl
+
+func diffAndWriteImpl(filename string, data []byte, mode os.FileMode) error {
 	// If the file has the same contents as data, try to avoid a write.
 	f, err := os.Open(filename)
 	if err != nil {
diff --git a/testdata/symlinkFileDup/file1 b/testdata/symlinkFileDup/file1
new file mode 120000
index 0000000..6e09272
--- /dev/null
+++ b/testdata/symlinkFileDup/file1
@@ -0,0 +1 @@
+../symlinkSrc/file1
\ No newline at end of file