Changed AdditionalSketchFilesCopier legacy command into new builder API function (#293)

* add convention to func names

* move sketch copy ops out of legacy into new API

* removed dead code

arduino/builder/sketch.go

@@ -16,6 +16,7 @@
 package builder
 
 import (
+	"bytes"
 	"io/ioutil"
 	"os"
 	"path/filepath"
@@ -39,16 +40,16 @@ func QuoteCppString(str string) string {
 	return "\"" + str + "\""
 }
 
-// SaveSketchItemCpp saves a preprocessed .cpp sketch file on disk
-func SaveSketchItemCpp(item *sketch.Item, buildPath string) error {
+// SketchSaveItemCpp saves a preprocessed .cpp sketch file on disk
+func SketchSaveItemCpp(item *sketch.Item, destPath string) error {
 
 	sketchName := filepath.Base(item.Path)
 
-	if err := os.MkdirAll(buildPath, os.FileMode(0755)); err != nil {
+	if err := os.MkdirAll(destPath, os.FileMode(0755)); err != nil {
 		return errors.Wrap(err, "unable to create a folder to save the sketch")
 	}
 
-	destFile := filepath.Join(buildPath, sketchName+".cpp")
+	destFile := filepath.Join(destPath, sketchName+".cpp")
 
 	if err := ioutil.WriteFile(destFile, item.Source, os.FileMode(0644)); err != nil {
 		return errors.Wrap(err, "unable to save the sketch on disk")
@@ -57,10 +58,10 @@ func SaveSketchItemCpp(item *sketch.Item, buildPath string) error {
 	return nil
 }
 
-// LoadSketch collects all the files composing a sketch.
+// SketchLoad collects all the files composing a sketch.
 // The parameter `sketchPath` holds a path pointing to a single sketch file or a sketch folder,
 // the path must be absolute.
-func LoadSketch(sketchPath, buildPath string) (*sketch.Sketch, error) {
+func SketchLoad(sketchPath, buildPath string) (*sketch.Sketch, error) {
 	stat, err := os.Stat(sketchPath)
 	if err != nil {
 		return nil, errors.Wrap(err, "unable to stat Sketch location")
@@ -133,8 +134,8 @@ func LoadSketch(sketchPath, buildPath string) (*sketch.Sketch, error) {
 	return sketch.New(sketchFolder, mainSketchFile, buildPath, files)
 }
 
-// MergeSketchSources merges all the source files included in a sketch
-func MergeSketchSources(sketch *sketch.Sketch) (int, string) {
+// SketchMergeSources merges all the source files included in a sketch
+func SketchMergeSources(sketch *sketch.Sketch) (int, string) {
 	lineOffset := 0
 	mergedSource := ""
 
@@ -155,3 +156,60 @@ func MergeSketchSources(sketch *sketch.Sketch) (int, string) {
 
 	return lineOffset, mergedSource
 }
+
+// SketchCopyAdditionalFiles copies the additional files for a sketch to the
+// specified destination directory.
+func SketchCopyAdditionalFiles(sketch *sketch.Sketch, destPath string) error {
+	if err := os.MkdirAll(destPath, os.FileMode(0755)); err != nil {
+		return errors.Wrap(err, "unable to create a folder to save the sketch files")
+	}
+
+	for _, item := range sketch.AdditionalFiles {
+		relpath, err := filepath.Rel(sketch.LocationPath, item.Path)
+		if err != nil {
+			return errors.Wrap(err, "unable to compute relative path to the sketch for the item")
+		}
+
+		targetPath := filepath.Join(destPath, relpath)
+		// create the directory containing the target
+		if err = os.MkdirAll(filepath.Dir(targetPath), os.FileMode(0755)); err != nil {
+			return errors.Wrap(err, "unable to create the folder containing the item")
+		}
+
+		err = writeIfDifferent(item.Path, targetPath)
+		if err != nil {
+			return errors.Wrap(err, "unable to write to destination file")
+		}
+	}
+
+	return nil
+}
+
+func writeIfDifferent(sourcePath, destPath string) error {
+	// read the source file
+	newbytes, err := ioutil.ReadFile(sourcePath)
+	if err != nil {
+		return errors.Wrap(err, "unable to read contents of the source item")
+	}
+
+	// check whether the destination file exists
+	_, err = os.Stat(destPath)
+	if os.IsNotExist(err) {
+		// write directly
+		return ioutil.WriteFile(destPath, newbytes, os.FileMode(0644))
+	}
+
+	// read the destination file if it ex
+	existingBytes, err := ioutil.ReadFile(destPath)
+	if err != nil {
+		return errors.Wrap(err, "unable to read contents of the destination item")
+	}
+
+	// overwrite if contents are different
+	if bytes.Compare(existingBytes, newbytes) != 0 {
+		return ioutil.WriteFile(destPath, newbytes, os.FileMode(0644))
+	}
+
+	// source and destination are the same, don't write anything
+	return nil
+}

arduino/builder/sketch_test.go

@@ -16,6 +16,7 @@
 package builder_test
 
 import (
+	"fmt"
 	"io/ioutil"
 	"os"
 	"path/filepath"
@@ -24,7 +25,7 @@ import (
 
 	"github.com/arduino/arduino-cli/arduino/builder"
 	"github.com/arduino/arduino-cli/arduino/sketch"
-	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 func TestSaveSketch(t *testing.T) {
@@ -38,64 +39,64 @@ func TestSaveSketch(t *testing.T) {
 		t.Fatalf("unable to read golden file %s: %v", sketchFile, err)
 	}
 
-	builder.SaveSketchItemCpp(&sketch.Item{Path: sketchName, Source: source}, tmp)
+	builder.SketchSaveItemCpp(&sketch.Item{Path: sketchName, Source: source}, tmp)
 
 	out, err := ioutil.ReadFile(filepath.Join(tmp, outName))
 	if err != nil {
 		t.Fatalf("unable to read output file %s: %v", outName, err)
 	}
 
-	assert.Equal(t, source, out)
+	require.Equal(t, source, out)
 }
 
 func TestLoadSketchFolder(t *testing.T) {
 	// pass the path to the sketch folder
 	sketchPath := filepath.Join("testdata", t.Name())
 	mainFilePath := filepath.Join(sketchPath, t.Name()+".ino")
-	s, err := builder.LoadSketch(sketchPath, "")
-	assert.Nil(t, err)
-	assert.NotNil(t, s)
-	assert.Equal(t, mainFilePath, s.MainFile.Path)
-	assert.Equal(t, sketchPath, s.LocationPath)
-	assert.Len(t, s.OtherSketchFiles, 2)
-	assert.Equal(t, "old.pde", filepath.Base(s.OtherSketchFiles[0].Path))
-	assert.Equal(t, "other.ino", filepath.Base(s.OtherSketchFiles[1].Path))
-	assert.Len(t, s.AdditionalFiles, 3)
-	assert.Equal(t, "header.h", filepath.Base(s.AdditionalFiles[0].Path))
-	assert.Equal(t, "s_file.S", filepath.Base(s.AdditionalFiles[1].Path))
-	assert.Equal(t, "helper.h", filepath.Base(s.AdditionalFiles[2].Path))
+	s, err := builder.SketchLoad(sketchPath, "")
+	require.Nil(t, err)
+	require.NotNil(t, s)
+	require.Equal(t, mainFilePath, s.MainFile.Path)
+	require.Equal(t, sketchPath, s.LocationPath)
+	require.Len(t, s.OtherSketchFiles, 2)
+	require.Equal(t, "old.pde", filepath.Base(s.OtherSketchFiles[0].Path))
+	require.Equal(t, "other.ino", filepath.Base(s.OtherSketchFiles[1].Path))
+	require.Len(t, s.AdditionalFiles, 3)
+	require.Equal(t, "header.h", filepath.Base(s.AdditionalFiles[0].Path))
+	require.Equal(t, "s_file.S", filepath.Base(s.AdditionalFiles[1].Path))
+	require.Equal(t, "helper.h", filepath.Base(s.AdditionalFiles[2].Path))
 
 	// pass the path to the main file
 	sketchPath = mainFilePath
-	s, err = builder.LoadSketch(sketchPath, "")
-	assert.Nil(t, err)
-	assert.NotNil(t, s)
-	assert.Equal(t, mainFilePath, s.MainFile.Path)
-	assert.Len(t, s.OtherSketchFiles, 2)
-	assert.Equal(t, "old.pde", filepath.Base(s.OtherSketchFiles[0].Path))
-	assert.Equal(t, "other.ino", filepath.Base(s.OtherSketchFiles[1].Path))
-	assert.Len(t, s.AdditionalFiles, 3)
-	assert.Equal(t, "header.h", filepath.Base(s.AdditionalFiles[0].Path))
-	assert.Equal(t, "s_file.S", filepath.Base(s.AdditionalFiles[1].Path))
-	assert.Equal(t, "helper.h", filepath.Base(s.AdditionalFiles[2].Path))
+	s, err = builder.SketchLoad(sketchPath, "")
+	require.Nil(t, err)
+	require.NotNil(t, s)
+	require.Equal(t, mainFilePath, s.MainFile.Path)
+	require.Len(t, s.OtherSketchFiles, 2)
+	require.Equal(t, "old.pde", filepath.Base(s.OtherSketchFiles[0].Path))
+	require.Equal(t, "other.ino", filepath.Base(s.OtherSketchFiles[1].Path))
+	require.Len(t, s.AdditionalFiles, 3)
+	require.Equal(t, "header.h", filepath.Base(s.AdditionalFiles[0].Path))
+	require.Equal(t, "s_file.S", filepath.Base(s.AdditionalFiles[1].Path))
+	require.Equal(t, "helper.h", filepath.Base(s.AdditionalFiles[2].Path))
 }
 
 func TestLoadSketchFolderWrongMain(t *testing.T) {
 	sketchPath := filepath.Join("testdata", t.Name())
-	_, err := builder.LoadSketch(sketchPath, "")
-	assert.Error(t, err)
-	assert.Contains(t, err.Error(), "unable to find the main sketch file")
+	_, err := builder.SketchLoad(sketchPath, "")
+	require.Error(t, err)
+	require.Contains(t, err.Error(), "unable to find the main sketch file")
 
-	_, err = builder.LoadSketch("does/not/exist", "")
-	assert.Error(t, err)
-	assert.Contains(t, err.Error(), "no such file or directory")
+	_, err = builder.SketchLoad("does/not/exist", "")
+	require.Error(t, err)
+	require.Contains(t, err.Error(), "no such file or directory")
 }
 
 func TestMergeSketchSources(t *testing.T) {
 	// borrow the sketch from TestLoadSketchFolder to avoid boilerplate
-	s, err := builder.LoadSketch(filepath.Join("testdata", "TestLoadSketchFolder"), "")
-	assert.Nil(t, err)
-	assert.NotNil(t, s)
+	s, err := builder.SketchLoad(filepath.Join("testdata", "TestLoadSketchFolder"), "")
+	require.Nil(t, err)
+	require.NotNil(t, s)
 
 	// load expected result
 	mergedPath := filepath.Join("testdata", t.Name()+".txt")
@@ -104,17 +105,51 @@ func TestMergeSketchSources(t *testing.T) {
 		t.Fatalf("unable to read golden file %s: %v", mergedPath, err)
 	}
 
-	offset, source := builder.MergeSketchSources(s)
-	assert.Equal(t, 2, offset)
-	assert.Equal(t, string(mergedBytes), source)
+	offset, source := builder.SketchMergeSources(s)
+	require.Equal(t, 2, offset)
+	require.Equal(t, string(mergedBytes), source)
 }
 
 func TestMergeSketchSourcesArduinoIncluded(t *testing.T) {
-	s, err := builder.LoadSketch(filepath.Join("testdata", t.Name()), "")
-	assert.Nil(t, err)
-	assert.NotNil(t, s)
+	s, err := builder.SketchLoad(filepath.Join("testdata", t.Name()), "")
+	require.Nil(t, err)
+	require.NotNil(t, s)
 
 	// ensure not to include Arduino.h when it's already there
-	_, source := builder.MergeSketchSources(s)
-	assert.Equal(t, 1, strings.Count(source, "<Arduino.h>"))
+	_, source := builder.SketchMergeSources(s)
+	require.Equal(t, 1, strings.Count(source, "<Arduino.h>"))
+}
+
+func TestCopyAdditionalFiles(t *testing.T) {
+	tmp := tmpDirOrDie()
+	defer os.RemoveAll(tmp)
+
+	// load the golden sketch
+	s1, err := builder.SketchLoad(filepath.Join("testdata", t.Name()), "")
+	require.Nil(t, err)
+	require.Len(t, s1.AdditionalFiles, 1)
+
+	// copy the sketch over, create a fake main file we don't care about it
+	// but we need it for `SketchLoad` to suceed later
+	err = builder.SketchCopyAdditionalFiles(s1, tmp)
+	require.Nil(t, err)
+	fakeIno := filepath.Join(tmp, fmt.Sprintf("%s.ino", filepath.Base(tmp)))
+	require.Nil(t, ioutil.WriteFile(fakeIno, []byte{}, os.FileMode(0644)))
+
+	// compare
+	s2, err := builder.SketchLoad(tmp, "")
+	require.Nil(t, err)
+	require.Len(t, s2.AdditionalFiles, 1)
+
+	// save file info
+	info1, err := os.Stat(s2.AdditionalFiles[0].Path)
+	require.Nil(t, err)
+
+	// copy again
+	err = builder.SketchCopyAdditionalFiles(s1, tmp)
+	require.Nil(t, err)
+
+	// verify file hasn't changed
+	info2, err := os.Stat(s2.AdditionalFiles[0].Path)
+	require.Equal(t, info1.ModTime(), info2.ModTime())
 }

legacy/builder/additional_sketch_files_copier.go

-/*
- * This file is part of Arduino Builder.
- *
- * Arduino Builder is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; either version 2 of the License, or
- * (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, write to the Free Software
- * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
- *
- * As a special exception, you may use this file as part of a free software
- * library without restriction.  Specifically, if other files instantiate
- * templates or use macros or inline functions from this file, or you compile
- * this file and link it with other files to produce an executable, this
- * file does not by itself cause the resulting executable to be covered by
- * the GNU General Public License.  This exception does not however
- * invalidate any other reasons why the executable file might be covered by
- * the GNU General Public License.
- *
- * Copyright 2015 Arduino LLC (http://www.arduino.cc/)
- */
-
-package builder
-
-import (
-	"bytes"
-
-	"github.com/arduino/arduino-cli/legacy/builder/i18n"
-	"github.com/arduino/arduino-cli/legacy/builder/types"
-	"github.com/arduino/go-paths-helper"
-)
-
-type AdditionalSketchFilesCopier struct{}
-
-func (s *AdditionalSketchFilesCopier) Run(ctx *types.Context) error {
-	sketch := ctx.Sketch
-	sketchBuildPath := ctx.SketchBuildPath
-
-	if err := sketchBuildPath.MkdirAll(); err != nil {
-		return i18n.WrapError(err)
-	}
-
-	sketchBasePath := sketch.MainFile.Name.Parent()
-
-	for _, file := range sketch.AdditionalFiles {
-		relativePath, err := sketchBasePath.RelTo(file.Name)
-		if err != nil {
-			return i18n.WrapError(err)
-		}
-
-		targetFilePath := sketchBuildPath.JoinPath(relativePath)
-		if err = targetFilePath.Parent().MkdirAll(); err != nil {
-			return i18n.WrapError(err)
-		}
-
-		bytes, err := file.Name.ReadFile()
-		if err != nil {
-			return i18n.WrapError(err)
-		}
-
-		if targetFileChanged(bytes, targetFilePath) {
-			if err := targetFilePath.WriteFile(bytes); err != nil {
-				return i18n.WrapError(err)
-			}
-		}
-	}
-
-	return nil
-}
-
-func targetFileChanged(currentBytes []byte, targetFilePath *paths.Path) bool {
-	oldBytes, err := targetFilePath.ReadFile()
-	if err != nil {
-		return true
-	}
-
-	return bytes.Compare(currentBytes, oldBytes) != 0
-}

legacy/builder/container_add_prototypes.go

@@ -68,7 +68,7 @@ func (s *ContainerAddPrototypes) Run(ctx *types.Context) error {
 		}
 	}
 
-	if err := bldr.SaveSketchItemCpp(&sketch.Item{ctx.Sketch.MainFile.Name.String(), []byte(ctx.Source)}, ctx.SketchBuildPath.String()); err != nil {
+	if err := bldr.SketchSaveItemCpp(&sketch.Item{ctx.Sketch.MainFile.Name.String(), []byte(ctx.Source)}, ctx.SketchBuildPath.String()); err != nil {
 		return i18n.WrapError(err)
 	}
 

legacy/builder/container_merge_copy_sketch_files.go

@@ -44,15 +44,15 @@ func (s *ContainerMergeCopySketchFiles) Run(ctx *types.Context) error {
 	if sk == nil {
 		return i18n.WrapError(errors.New("unable to convert legacy sketch to the new type"))
 	}
-	offset, source := bldr.MergeSketchSources(sk)
+	offset, source := bldr.SketchMergeSources(sk)
 	ctx.LineOffset = offset
 	ctx.Source = source
 
-	if err := bldr.SaveSketchItemCpp(&sketch.Item{ctx.Sketch.MainFile.Name.String(), []byte(ctx.Source)}, ctx.SketchBuildPath.String()); err != nil {
+	if err := bldr.SketchSaveItemCpp(&sketch.Item{ctx.Sketch.MainFile.Name.String(), []byte(ctx.Source)}, ctx.SketchBuildPath.String()); err != nil {
 		return i18n.WrapError(err)
 	}
 
-	if err := new(AdditionalSketchFilesCopier).Run(ctx); err != nil {
+	if err := bldr.SketchCopyAdditionalFiles(sk, ctx.SketchBuildPath.String()); err != nil {
 		return i18n.WrapError(err)
 	}
 

legacy/builder/container_setup.go

@@ -70,7 +70,7 @@ func (s *ContainerSetupHardwareToolsLibsSketchAndProps) Run(ctx *types.Context)
 	}
 
 	// load sketch
-	sketch, err := bldr.LoadSketch(sketchLocation.String(), ctx.BuildPath.String())
+	sketch, err := bldr.SketchLoad(sketchLocation.String(), ctx.BuildPath.String())
 	if err != nil {
 		return i18n.WrapError(err)
 	}

legacy/builder/preprocess_sketch.go

@@ -82,7 +82,7 @@ func (s *PreprocessSketchArduino) Run(ctx *types.Context) error {
 	if ctx.CodeCompleteAt != "" {
 		err = new(OutputCodeCompletions).Run(ctx)
 	} else {
-		err = bldr.SaveSketchItemCpp(&sketch.Item{ctx.Sketch.MainFile.Name.String(), []byte(ctx.Source)}, ctx.SketchBuildPath.String())
+		err = bldr.SketchSaveItemCpp(&sketch.Item{ctx.Sketch.MainFile.Name.String(), []byte(ctx.Source)}, ctx.SketchBuildPath.String())
 	}
 
 	return err

legacy/builder/test/additional_sketch_files_copier_test.go

-/*
- * This file is part of Arduino Builder.
- *
- * Arduino Builder is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; either version 2 of the License, or
- * (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, write to the Free Software
- * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
- *
- * As a special exception, you may use this file as part of a free software
- * library without restriction.  Specifically, if other files instantiate
- * templates or use macros or inline functions from this file, or you compile
- * this file and link it with other files to produce an executable, this
- * file does not by itself cause the resulting executable to be covered by
- * the GNU General Public License.  This exception does not however
- * invalidate any other reasons why the executable file might be covered by
- * the GNU General Public License.
- *
- * Copyright 2015 Arduino LLC (http://www.arduino.cc/)
- */
-
-package test
-
-import (
-	"os"
-	"sort"
-	"testing"
-	"time"
-
-	"github.com/arduino/arduino-cli/legacy/builder"
-	"github.com/arduino/arduino-cli/legacy/builder/constants"
-	"github.com/arduino/arduino-cli/legacy/builder/gohasissues"
-	"github.com/arduino/arduino-cli/legacy/builder/types"
-	paths "github.com/arduino/go-paths-helper"
-	"github.com/stretchr/testify/require"
-)
-
-type ByFileInfoName []os.FileInfo
-
-func (s ByFileInfoName) Len() int {
-	return len(s)
-}
-func (s ByFileInfoName) Swap(i, j int) {
-	s[i], s[j] = s[j], s[i]
-}
-func (s ByFileInfoName) Less(i, j int) bool {
-	return s[i].Name() < s[j].Name()
-}
-
-func TestCopyOtherFiles(t *testing.T) {
-	ctx := &types.Context{
-		SketchLocation: paths.New("sketch1", "sketch.ino"),
-	}
-
-	buildPath := SetupBuildPath(t, ctx)
-	defer buildPath.RemoveAll()
-
-	commands := []types.Command{
-		&builder.AddAdditionalEntriesToContext{},
-		&builder.SketchLoader{},
-		&builder.AdditionalSketchFilesCopier{},
-	}
-
-	for _, command := range commands {
-		err := command.Run(ctx)
-		NoError(t, err)
-	}
-
-	exist, err1 := buildPath.Join(constants.FOLDER_SKETCH, "header.h").ExistCheck()
-	NoError(t, err1)
-	require.True(t, exist)
-
-	files, err1 := gohasissues.ReadDir(buildPath.Join(constants.FOLDER_SKETCH).String())
-	NoError(t, err1)
-	require.Equal(t, 3, len(files))
-
-	sort.Sort(ByFileInfoName(files))
-	require.Equal(t, "header.h", files[0].Name())
-	require.Equal(t, "s_file.S", files[1].Name())
-	require.Equal(t, "src", files[2].Name())
-
-	files, err1 = gohasissues.ReadDir(buildPath.Join(constants.FOLDER_SKETCH, "src").String())
-	NoError(t, err1)
-	require.Equal(t, 1, len(files))
-	require.Equal(t, "helper.h", files[0].Name())
-}
-
-func TestCopyOtherFilesOnlyIfChanged(t *testing.T) {
-	ctx := &types.Context{
-		SketchLocation: paths.New("sketch1", "sketch.ino"),
-	}
-
-	buildPath := SetupBuildPath(t, ctx)
-	defer buildPath.RemoveAll()
-
-	commands := []types.Command{
-		&builder.AddAdditionalEntriesToContext{},
-		&builder.SketchLoader{},
-		&builder.AdditionalSketchFilesCopier{},
-	}
-
-	for _, command := range commands {
-		err := command.Run(ctx)
-		NoError(t, err)
-	}
-
-	headerStatBefore, err := buildPath.Join(constants.FOLDER_SKETCH, "header.h").Stat()
-	NoError(t, err)
-
-	time.Sleep(2 * time.Second)
-
-	ctx = &types.Context{
-		SketchLocation: paths.New("sketch1", "sketch.ino"),
-		BuildPath:      buildPath,
-	}
-
-	for _, command := range commands {
-		err := command.Run(ctx)
-		NoError(t, err)
-	}
-
-	headerStatAfter, err := buildPath.Join(constants.FOLDER_SKETCH, "header.h").Stat()
-	NoError(t, err)
-
-	require.Equal(t, headerStatBefore.ModTime().Unix(), headerStatAfter.ModTime().Unix())
-}