From db10911cdff3ef16769154761e457d94acc423dc Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Thu, 11 Jan 2024 16:41:48 +0100 Subject: [PATCH 1/8] Removed unneeded pointer --- internal/arduino/sketch/sketch.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/arduino/sketch/sketch.go b/internal/arduino/sketch/sketch.go index ff827898c87..c38e74cb560 100644 --- a/internal/arduino/sketch/sketch.go +++ b/internal/arduino/sketch/sketch.go @@ -115,7 +115,7 @@ func New(path *paths.Path) (*Sketch, error) { } // Collect files - for _, p := range *sketchFolderFiles { + for _, p := range sketchFolderFiles { // Skip files that can't be opened f, err := p.Open() if err != nil { @@ -160,7 +160,7 @@ func New(path *paths.Path) (*Sketch, error) { // supportedFiles reads all files recursively contained in Sketch and // filter out unneded or unsupported ones and returns them -func (s *Sketch) supportedFiles() (*paths.PathList, error) { +func (s *Sketch) supportedFiles() (paths.PathList, error) { filterValidExtensions := func(p *paths.Path) bool { return globals.MainFileValidExtensions[p.Ext()] || globals.AdditionalFileValidExtensions[p.Ext()] } @@ -180,7 +180,7 @@ func (s *Sketch) supportedFiles() (*paths.PathList, error) { if err != nil { return nil, err } - return &files, nil + return files, nil } // GetProfile returns the requested profile or an error if not found From 49da311294bebe058b894ceff52a6418f0ac1718 Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Thu, 11 Jan 2024 17:21:23 +0100 Subject: [PATCH 2/8] Improved error message --- internal/arduino/sketch/sketch.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/arduino/sketch/sketch.go b/internal/arduino/sketch/sketch.go index c38e74cb560..b1993a1defc 100644 --- a/internal/arduino/sketch/sketch.go +++ b/internal/arduino/sketch/sketch.go @@ -111,7 +111,7 @@ func New(path *paths.Path) (*Sketch, error) { sketchFolderFiles, err := sketch.supportedFiles() if err != nil { - return nil, err + return nil, fmt.Errorf("%s: %w", tr("reading sketch files"), err) } // Collect files From 134c1e675e3b65061168568471cdf59a36036969 Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Fri, 12 Jan 2024 11:51:00 +0100 Subject: [PATCH 3/8] Update go-paths library --- .licenses/go/github.com/arduino/go-paths-helper.dep.yml | 2 +- go.mod | 2 +- go.sum | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/.licenses/go/github.com/arduino/go-paths-helper.dep.yml b/.licenses/go/github.com/arduino/go-paths-helper.dep.yml index c821a1e1141..05f3ecab7a7 100644 --- a/.licenses/go/github.com/arduino/go-paths-helper.dep.yml +++ b/.licenses/go/github.com/arduino/go-paths-helper.dep.yml @@ -1,6 +1,6 @@ --- name: github.com/arduino/go-paths-helper -version: v1.11.0 +version: v1.12.0 type: go summary: homepage: https://pkg.go.dev/github.com/arduino/go-paths-helper diff --git a/go.mod b/go.mod index c52a74c4774..bd06bc1058a 100644 --- a/go.mod +++ b/go.mod @@ -7,7 +7,7 @@ replace github.com/mailru/easyjson => github.com/cmaglie/easyjson v0.8.1 require ( github.com/ProtonMail/go-crypto v0.0.0-20230828082145-3c4c8a2d2371 - github.com/arduino/go-paths-helper v1.11.0 + github.com/arduino/go-paths-helper v1.12.0 github.com/arduino/go-properties-orderedmap v1.8.0 github.com/arduino/go-timeutils v0.0.0-20171220113728-d1dd9e313b1b github.com/arduino/go-win32-utils v1.0.0 diff --git a/go.sum b/go.sum index bf71ec2ca0a..74dc55a47ad 100644 --- a/go.sum +++ b/go.sum @@ -11,8 +11,8 @@ github.com/acomagu/bufpipe v1.0.3/go.mod h1:mxdxdup/WdsKVreO5GpW4+M/1CE2sMG4jeGJ github.com/anmitsu/go-shlex v0.0.0-20161002113705-648efa622239 h1:kFOfPq6dUM1hTo4JG6LR5AXSUEsOjtdm0kw0FtQtMJA= github.com/anmitsu/go-shlex v0.0.0-20161002113705-648efa622239/go.mod h1:2FmKhYUyUczH0OGQWaF5ceTx0UBShxjsH6f8oGKYe2c= github.com/arduino/go-paths-helper v1.0.1/go.mod h1:HpxtKph+g238EJHq4geEPv9p+gl3v5YYu35Yb+w31Ck= -github.com/arduino/go-paths-helper v1.11.0 h1:hkpGb9AtCTByTj2FKutuHWb3klDf4kAKL10hW+fN+oE= -github.com/arduino/go-paths-helper v1.11.0/go.mod h1:jcpW4wr0u69GlXhTYydsdsqAjLaYK5n7oWHfKqOG6LM= +github.com/arduino/go-paths-helper v1.12.0 h1:xizOQtI9iHdl19qXd1EmWg5i9W//2bOCOYwlNv8F61E= +github.com/arduino/go-paths-helper v1.12.0/go.mod h1:jcpW4wr0u69GlXhTYydsdsqAjLaYK5n7oWHfKqOG6LM= github.com/arduino/go-properties-orderedmap v1.8.0 h1:wEfa6hHdpezrVOh787OmClsf/Kd8qB+zE3P2Xbrn0CQ= github.com/arduino/go-properties-orderedmap v1.8.0/go.mod h1:DKjD2VXY/NZmlingh4lSFMEYCVubfeArCsGPGDwb2yk= github.com/arduino/go-timeutils v0.0.0-20171220113728-d1dd9e313b1b h1:9hDi4F2st6dbLC3y4i02zFT5quS4X6iioWifGlVwfy4= From 58150182bd4c836edab574f9d0637e87ca57d091 Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Thu, 11 Jan 2024 17:22:33 +0100 Subject: [PATCH 4/8] Added integration tests --- .../compile_4/broken_symlink_test.go | 54 +++++++++++++++++++ ...ValidSketchWithBrokenSketchFileSymlink.ino | 2 + .../other_file.ino | 1 + .../ValidSketchWithBrokenSymlink.ino | 2 + .../ValidSketchWithBrokenSymlink/other_file | 1 + ...ketchWithNonInoBrokenSketchFileSymlink.ino | 2 + .../other_file.c | 1 + 7 files changed, 63 insertions(+) create mode 100644 internal/integrationtest/compile_4/broken_symlink_test.go create mode 100644 internal/integrationtest/compile_4/testdata/ValidSketchWithBrokenSketchFileSymlink/ValidSketchWithBrokenSketchFileSymlink.ino create mode 120000 internal/integrationtest/compile_4/testdata/ValidSketchWithBrokenSketchFileSymlink/other_file.ino create mode 100644 internal/integrationtest/compile_4/testdata/ValidSketchWithBrokenSymlink/ValidSketchWithBrokenSymlink.ino create mode 120000 internal/integrationtest/compile_4/testdata/ValidSketchWithBrokenSymlink/other_file create mode 100644 internal/integrationtest/compile_4/testdata/ValidSketchWithNonInoBrokenSketchFileSymlink/ValidSketchWithNonInoBrokenSketchFileSymlink.ino create mode 120000 internal/integrationtest/compile_4/testdata/ValidSketchWithNonInoBrokenSketchFileSymlink/other_file.c diff --git a/internal/integrationtest/compile_4/broken_symlink_test.go b/internal/integrationtest/compile_4/broken_symlink_test.go new file mode 100644 index 00000000000..42d948269c6 --- /dev/null +++ b/internal/integrationtest/compile_4/broken_symlink_test.go @@ -0,0 +1,54 @@ +// This file is part of arduino-cli. +// +// Copyright 2023 ARDUINO SA (http://www.arduino.cc/) +// +// This software is released under the GNU General Public License version 3, +// which covers the main part of arduino-cli. +// The terms of this license can be found at: +// https://www.gnu.org/licenses/gpl-3.0.en.html +// +// You can be released from the requirements of the above licenses by purchasing +// a commercial license. Buying such a license is mandatory if you want to +// modify or otherwise use the software for commercial activities involving the +// Arduino software without disclosing the source code of your own applications. +// To purchase a commercial license, send an email to license@arduino.cc. + +package compile_test + +import ( + "testing" + + "github.com/arduino/arduino-cli/internal/integrationtest" + "github.com/arduino/go-paths-helper" + "github.com/stretchr/testify/require" +) + +func TestCompileWithBrokenSymLinks(t *testing.T) { + env, cli := integrationtest.CreateArduinoCLIWithEnvironment(t) + t.Cleanup(env.CleanUp) + + // Install Arduino AVR Boards + _, _, err := cli.Run("core", "install", "arduino:avr@1.8.6") + require.NoError(t, err) + + t.Run("NonSketchFileBroken", func(t *testing.T) { + sketch, err := paths.New("testdata", "ValidSketchWithBrokenSymlink").Abs() + require.NoError(t, err) + _, _, err = cli.Run("compile", "-b", "arduino:avr:uno", sketch.String()) + require.NoError(t, err) + }) + + t.Run("SketchFileBroken", func(t *testing.T) { + sketch, err := paths.New("testdata", "ValidSketchWithBrokenSketchFileSymlink").Abs() + require.NoError(t, err) + _, _, err = cli.Run("compile", "-b", "arduino:avr:uno", sketch.String()) + require.Error(t, err) + }) + + t.Run("NonInoSketchFileBroken", func(t *testing.T) { + sketch, err := paths.New("testdata", "ValidSketchWithNonInoBrokenSketchFileSymlink").Abs() + require.NoError(t, err) + _, _, err = cli.Run("compile", "-b", "arduino:avr:uno", sketch.String()) + require.Error(t, err) + }) +} diff --git a/internal/integrationtest/compile_4/testdata/ValidSketchWithBrokenSketchFileSymlink/ValidSketchWithBrokenSketchFileSymlink.ino b/internal/integrationtest/compile_4/testdata/ValidSketchWithBrokenSketchFileSymlink/ValidSketchWithBrokenSketchFileSymlink.ino new file mode 100644 index 00000000000..660bdbccfdb --- /dev/null +++ b/internal/integrationtest/compile_4/testdata/ValidSketchWithBrokenSketchFileSymlink/ValidSketchWithBrokenSketchFileSymlink.ino @@ -0,0 +1,2 @@ +void setup() {} +void loop() {} diff --git a/internal/integrationtest/compile_4/testdata/ValidSketchWithBrokenSketchFileSymlink/other_file.ino b/internal/integrationtest/compile_4/testdata/ValidSketchWithBrokenSketchFileSymlink/other_file.ino new file mode 120000 index 00000000000..86a410dd1d3 --- /dev/null +++ b/internal/integrationtest/compile_4/testdata/ValidSketchWithBrokenSketchFileSymlink/other_file.ino @@ -0,0 +1 @@ +broken \ No newline at end of file diff --git a/internal/integrationtest/compile_4/testdata/ValidSketchWithBrokenSymlink/ValidSketchWithBrokenSymlink.ino b/internal/integrationtest/compile_4/testdata/ValidSketchWithBrokenSymlink/ValidSketchWithBrokenSymlink.ino new file mode 100644 index 00000000000..660bdbccfdb --- /dev/null +++ b/internal/integrationtest/compile_4/testdata/ValidSketchWithBrokenSymlink/ValidSketchWithBrokenSymlink.ino @@ -0,0 +1,2 @@ +void setup() {} +void loop() {} diff --git a/internal/integrationtest/compile_4/testdata/ValidSketchWithBrokenSymlink/other_file b/internal/integrationtest/compile_4/testdata/ValidSketchWithBrokenSymlink/other_file new file mode 120000 index 00000000000..86a410dd1d3 --- /dev/null +++ b/internal/integrationtest/compile_4/testdata/ValidSketchWithBrokenSymlink/other_file @@ -0,0 +1 @@ +broken \ No newline at end of file diff --git a/internal/integrationtest/compile_4/testdata/ValidSketchWithNonInoBrokenSketchFileSymlink/ValidSketchWithNonInoBrokenSketchFileSymlink.ino b/internal/integrationtest/compile_4/testdata/ValidSketchWithNonInoBrokenSketchFileSymlink/ValidSketchWithNonInoBrokenSketchFileSymlink.ino new file mode 100644 index 00000000000..660bdbccfdb --- /dev/null +++ b/internal/integrationtest/compile_4/testdata/ValidSketchWithNonInoBrokenSketchFileSymlink/ValidSketchWithNonInoBrokenSketchFileSymlink.ino @@ -0,0 +1,2 @@ +void setup() {} +void loop() {} diff --git a/internal/integrationtest/compile_4/testdata/ValidSketchWithNonInoBrokenSketchFileSymlink/other_file.c b/internal/integrationtest/compile_4/testdata/ValidSketchWithNonInoBrokenSketchFileSymlink/other_file.c new file mode 120000 index 00000000000..86a410dd1d3 --- /dev/null +++ b/internal/integrationtest/compile_4/testdata/ValidSketchWithNonInoBrokenSketchFileSymlink/other_file.c @@ -0,0 +1 @@ +broken \ No newline at end of file From 9c1d4c3c8710adda2f082aa3116848e6628bff86 Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Thu, 11 Jan 2024 18:14:28 +0100 Subject: [PATCH 5/8] Reduced timeout for symlink-loop tests --- internal/arduino/sketch/sketch_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/arduino/sketch/sketch_test.go b/internal/arduino/sketch/sketch_test.go index d6f42f9b805..9f18e45c591 100644 --- a/internal/arduino/sketch/sketch_test.go +++ b/internal/arduino/sketch/sketch_test.go @@ -341,7 +341,7 @@ func TestNewSketchWithSymlinkLoop(t *testing.T) { return false } }, - 20*time.Second, + 5*time.Second, 10*time.Millisecond, "Infinite symlink loop while loading sketch", ) @@ -380,7 +380,7 @@ func TestSketchWithMultipleSymlinkLoops(t *testing.T) { return false } }, - 20*time.Second, + 5*time.Second, 10*time.Millisecond, "Infinite symlink loop while loading sketch", ) From f5bdaa73bbd7f5674fb5220c0163b3c7a6e75ff0 Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Fri, 12 Jan 2024 19:11:11 +0100 Subject: [PATCH 6/8] Fixed unit tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously the unit tests were creating the wrong env to test: internal/arduino/libraries/testdata/TestLib ├── examples │   ├── UpGoer1 -> testdata/TestLib │   └── UpGoer2 -> testdata/TestLib ├── library.properties └── src └── TestLib.h The two UpGoer1 and UpGoer2 are broken links. The correct tree is the following: internal/arduino/libraries/testdata/TestLib ├── examples │   ├── UpGoer1 -> .. │   └── UpGoer2 -> .. ├── library.properties └── src └── TestLib.h that actually triggers the symlink loop we are testing. --- internal/arduino/libraries/libraries_test.go | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/internal/arduino/libraries/libraries_test.go b/internal/arduino/libraries/libraries_test.go index 9896ae577fb..33b0de48a3b 100644 --- a/internal/arduino/libraries/libraries_test.go +++ b/internal/arduino/libraries/libraries_test.go @@ -95,15 +95,16 @@ func TestLibrariesLoader(t *testing.T) { func TestSymlinkLoop(t *testing.T) { // Set up directory structure of test library. testLib := paths.New("testdata", "TestLib") - examplesPath := testLib.Join("examples") + examplesPath, err := testLib.Join("examples").Abs() + require.NoError(t, err) require.NoError(t, examplesPath.Mkdir()) defer examplesPath.RemoveAll() // It's probably most friendly for contributors using Windows to create the symlinks needed for the test on demand. - err := os.Symlink(examplesPath.Join("..").String(), examplesPath.Join("UpGoer1").String()) + err = os.Symlink(examplesPath.String(), examplesPath.Join("UpGoer1").String()) require.NoError(t, err, "This test must be run as administrator on Windows to have symlink creation privilege.") // It's necessary to have multiple symlinks to a parent directory to create the loop. - err = os.Symlink(examplesPath.Join("..").String(), examplesPath.Join("UpGoer2").String()) + err = os.Symlink(examplesPath.String(), examplesPath.Join("UpGoer2").String()) require.NoError(t, err) // The failure condition is Load() never returning, testing for which requires setting up a timeout. @@ -123,15 +124,16 @@ func TestSymlinkLoop(t *testing.T) { func TestLegacySymlinkLoop(t *testing.T) { // Set up directory structure of test library. testLib := paths.New("testdata", "LegacyLib") - examplesPath := testLib.Join("examples") + examplesPath, err := testLib.Join("examples").Abs() + require.NoError(t, err) require.NoError(t, examplesPath.Mkdir()) defer examplesPath.RemoveAll() // It's probably most friendly for contributors using Windows to create the symlinks needed for the test on demand. - err := os.Symlink(examplesPath.Join("..").String(), examplesPath.Join("UpGoer1").String()) + err = os.Symlink(examplesPath.String(), examplesPath.Join("UpGoer1").String()) require.NoError(t, err, "This test must be run as administrator on Windows to have symlink creation privilege.") // It's necessary to have multiple symlinks to a parent directory to create the loop. - err = os.Symlink(examplesPath.Join("..").String(), examplesPath.Join("UpGoer2").String()) + err = os.Symlink(examplesPath.String(), examplesPath.Join("UpGoer2").String()) require.NoError(t, err) // The failure condition is Load() never returning, testing for which requires setting up a timeout. From 754e35d6d3e97da1516079c2da0bfa2f1621598f Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Sat, 13 Jan 2024 12:56:54 +0100 Subject: [PATCH 7/8] Fixed integration test --- .../integrationtest/compile_1/compile_test.go | 26 ++++++++++++++++--- 1 file changed, 23 insertions(+), 3 deletions(-) diff --git a/internal/integrationtest/compile_1/compile_test.go b/internal/integrationtest/compile_1/compile_test.go index 2808d14f5e0..554c7c9d365 100644 --- a/internal/integrationtest/compile_1/compile_test.go +++ b/internal/integrationtest/compile_1/compile_test.go @@ -220,16 +220,36 @@ func compileWithSketchWithSymlinkSelfloop(t *testing.T, env *integrationtest.Env require.NoError(t, err) require.Contains(t, string(stdout), "Sketch created in: "+sketchPath.String()) - // create a symlink that loops on himself + // Create a symlink that loops on himself + // + // /tmp/cli2843369229/Arduino/CompileIntegrationTestSymlinkSelfLoop + // ├── CompileIntegrationTestSymlinkSelfLoop.ino + // └── loop -> /tmp/cli2843369229/Arduino/CompileIntegrationTestSymlinkSelfLoop/loop + // + // in this case the link is "broken", and it will be ignored by the compiler loopFilePath := sketchPath.Join("loop") err = os.Symlink(loopFilePath.String(), loopFilePath.String()) require.NoError(t, err) - // Build sketch for arduino:avr:uno + _, _, err = cli.Run("compile", "-b", fqbn, sketchPath.String()) + require.NoError(t, err) + + // Add a symlink that loops on himself named as a .ino file + // + // /tmp/cli2843369229/Arduino/CompileIntegrationTestSymlinkSelfLoop + // ├── CompileIntegrationTestSymlinkSelfLoop.ino + // ├── loop -> /tmp/cli2843369229/Arduino/CompileIntegrationTestSymlinkSelfLoop/loop + // └── loop.ino -> /tmp/cli2843369229/Arduino/CompileIntegrationTestSymlinkSelfLoop/loop.ino + // + // in this case the new link is "broken" as before, but being part of the sketch will trigger an error. + loopInoFilePath := sketchPath.Join("loop.ino") + err = os.Symlink(loopFilePath.String(), loopInoFilePath.String()) + require.NoError(t, err) + _, stderr, err := cli.Run("compile", "-b", fqbn, sketchPath.String()) // The assertion is a bit relaxed in this case because win behaves differently from macOs and linux // returning a different error detailed message - require.Contains(t, string(stderr), "Can't open sketch:") + require.Contains(t, string(stderr), "Error during build:") require.Error(t, err) } { From e9dc764e287c9db25e5abd831b63bb719950f846 Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Sat, 13 Jan 2024 12:57:02 +0100 Subject: [PATCH 8/8] Removed apparently useless check for "readable" files --- internal/arduino/sketch/sketch.go | 7 ------- 1 file changed, 7 deletions(-) diff --git a/internal/arduino/sketch/sketch.go b/internal/arduino/sketch/sketch.go index b1993a1defc..7833823b424 100644 --- a/internal/arduino/sketch/sketch.go +++ b/internal/arduino/sketch/sketch.go @@ -116,13 +116,6 @@ func New(path *paths.Path) (*Sketch, error) { // Collect files for _, p := range sketchFolderFiles { - // Skip files that can't be opened - f, err := p.Open() - if err != nil { - continue - } - f.Close() - ext := p.Ext() if globals.MainFileValidExtensions[ext] { if p.EqualsTo(mainFile) {