From 1e1db0a58404c6bd98c7652d086b591553695cb4 Mon Sep 17 00:00:00 2001 From: Matthijs Kooijman Date: Wed, 8 Sep 2021 18:11:05 +0200 Subject: [PATCH 1/4] Add test for working and broken symlinks This tests that a symlink to an external file is compiled correctly, and tests the current behavior of breaking on any broken symlink in the sketch directory. --- test/test_compile.py | 72 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 72 insertions(+) diff --git a/test/test_compile.py b/test/test_compile.py index 5369a0fe185..89785a84885 100644 --- a/test/test_compile.py +++ b/test/test_compile.py @@ -158,6 +158,78 @@ def test_compile_with_sketch_with_symlink_selfloop(run_command, data_dir): assert not result.ok +def test_compile_with_symlink(run_command, data_dir): + # Init the environment explicitly + run_command(["core", "update-index"]) + + # Install Arduino AVR Boards + run_command(["core", "install", "arduino:avr@1.8.3"]) + + sketch_name = "CompileIntegrationTestSymlinkOk" + sketch_path = os.path.join(data_dir, sketch_name) + main_file = os.path.join(sketch_path, "{}.ino".format(sketch_name)) + symlink_file = os.path.join(sketch_path, "link.ino") + fqbn = "arduino:avr:uno" + + # Create a test sketch + result = run_command(["sketch", "new", sketch_path]) + assert result.ok + assert "Sketch created in: {}".format(sketch_path) in result.stdout + + # create a symlink to some external file and move the main .ino + # contents into that to check that the symlink is actually compiled + with tempfile.NamedTemporaryFile(delete=False) as external: + try: + os.symlink(external.name, symlink_file) + + with open(main_file, mode="rb+") as main: + external.write(main.read()) + external.flush() + main.truncate(0) + main.flush() + + # Close to allow re-opening the file on Windows + # See https://bugs.python.org/issue14243 + external.close() + + # Build sketch for arduino:avr:uno + result = run_command(["compile", "-b", fqbn, sketch_path]) + assert result.stderr == "" + assert result.ok + finally: + # Explicit cleanup needed because the file is closed above + os.unlink(external.name) + + def test_broken_symlink(sketch_name, link_name, target_name, expect_error): + sketch_path = os.path.join(data_dir, sketch_name) + symlink_file = os.path.join(sketch_path, link_name) + target_file = os.path.join(sketch_path, target_name) + fqbn = "arduino:avr:uno" + + # Create a test sketch + result = run_command(["sketch", "new", sketch_path]) + assert result.ok + assert "Sketch created in: {}".format(sketch_path) in result.stdout + + os.symlink(target_file, symlink_file) + + # Build sketch for arduino:avr:uno + result = run_command(["compile", "-b", fqbn, sketch_path]) + + if expect_error: + expected_error = "Error during build: Can't open sketch: stat {}: ".format(symlink_file) + assert expected_error in result.stderr + assert not result.ok + else: + assert result.ok + + test_broken_symlink("CompileIntegrationTestSymlinkBrokenIno", "link.ino", "doesnotexist.ino", expect_error=True) + test_broken_symlink("CompileIntegrationTestSymlinkBrokenCpp", "link.cpp", "doesnotexist.cpp", expect_error=True) + test_broken_symlink("CompileIntegrationTestSymlinkBrokenH", "link.h", "doesnotexist.h", expect_error=True) + test_broken_symlink("CompileIntegrationTestSymlinkBrokenJson", "link.json", "doesnotexist.json", expect_error=True) + test_broken_symlink("CompileIntegrationTestSymlinkBrokenXXX", "link.xxx", "doesnotexist.xxx", expect_error=True) + + def test_compile_blacklisted_sketchname(run_command, data_dir): """ Compile should ignore folders named `RCS`, `.git` and the likes, but From 38f72efa861ce6c91029ce2be0946415b5d12406 Mon Sep 17 00:00:00 2001 From: Matthijs Kooijman Date: Wed, 8 Sep 2021 15:25:45 +0200 Subject: [PATCH 2/4] Draft: Upgrade go-paths-helper to patched version This commit should not be merged, instead the indicated commit should be merged into upstream go-paths-helper first and this commit adapted. This makes two relevant changes: - ReadDirRecursive now returns broken symlinks as-is (and other files that cannot be stat'd due to permission errors) rather than failing entirely. This causes broken symlinks to no longer break the build, unless they are actually used (i.e. broken .cpp links will cause gcc to error out later). - FilterOutDirs no longer filters out broken symlinks (and other files that cannot be stat'd due to whatever error). This ensures that broken symlinks are actually returned by Sketch.supportedFiles, so sketch.New can check them (though that still ignores any errors currently). The test suite is updated for these changes. --- go.mod | 2 ++ go.sum | 2 ++ test/test_compile.py | 10 +++++----- 3 files changed, 9 insertions(+), 5 deletions(-) diff --git a/go.mod b/go.mod index 4c883effe00..d198a49d634 100644 --- a/go.mod +++ b/go.mod @@ -52,3 +52,5 @@ require ( gopkg.in/src-d/go-git.v4 v4.13.1 gopkg.in/yaml.v2 v2.4.0 ) + +replace github.com/arduino/go-paths-helper v1.6.1 => github.com/matthijskooijman/go-paths-helper v1.6.2-0.20210908151509-429170ecd10a diff --git a/go.sum b/go.sum index eec8bc49746..e7a72452fa5 100644 --- a/go.sum +++ b/go.sum @@ -255,6 +255,8 @@ github.com/magiconair/properties v1.8.5 h1:b6kJs+EmPFMYGkow9GiUyCyOvIwYetYJ3fSaW github.com/magiconair/properties v1.8.5/go.mod h1:y3VJvCyxH9uVvJTWEGAELF3aiYNyPKd5NZ3oSwXrF60= github.com/marcinbor85/gohex v0.0.0-20210308104911-55fb1c624d84 h1:hyAgCuG5nqTMDeUD8KZs7HSPs6KprPgPP8QmGV8nyvk= github.com/marcinbor85/gohex v0.0.0-20210308104911-55fb1c624d84/go.mod h1:Pb6XcsXyropB9LNHhnqaknG/vEwYztLkQzVCHv8sQ3M= +github.com/matthijskooijman/go-paths-helper v1.6.2-0.20210908151509-429170ecd10a h1:Mp/RCywh1EVgopMuG3CMjZ9S8xwLhvCZ+oDb4hLNSYY= +github.com/matthijskooijman/go-paths-helper v1.6.2-0.20210908151509-429170ecd10a/go.mod h1:V82BWgAAp4IbmlybxQdk9Bpkz8M4Qyx+RAFKaG9NuvU= github.com/mattn/go-colorable v0.0.9/go.mod h1:9vuHe8Xs5qXnSaW/c/ABM9alt+Vo+STaOChaDxuIBZU= github.com/mattn/go-colorable v0.1.8 h1:c1ghPdyEDarC70ftn0y+A/Ee++9zz8ljHG1b13eJ0s8= github.com/mattn/go-colorable v0.1.8/go.mod h1:u6P/XSegPjTcexA+o6vUJrdnUu04hMope9wVRipJSqc= diff --git a/test/test_compile.py b/test/test_compile.py index 89785a84885..7be00f95db7 100644 --- a/test/test_compile.py +++ b/test/test_compile.py @@ -223,11 +223,11 @@ def test_broken_symlink(sketch_name, link_name, target_name, expect_error): else: assert result.ok - test_broken_symlink("CompileIntegrationTestSymlinkBrokenIno", "link.ino", "doesnotexist.ino", expect_error=True) - test_broken_symlink("CompileIntegrationTestSymlinkBrokenCpp", "link.cpp", "doesnotexist.cpp", expect_error=True) - test_broken_symlink("CompileIntegrationTestSymlinkBrokenH", "link.h", "doesnotexist.h", expect_error=True) - test_broken_symlink("CompileIntegrationTestSymlinkBrokenJson", "link.json", "doesnotexist.json", expect_error=True) - test_broken_symlink("CompileIntegrationTestSymlinkBrokenXXX", "link.xxx", "doesnotexist.xxx", expect_error=True) + test_broken_symlink("CompileIntegrationTestSymlinkBrokenIno", "link.ino", "doesnotexist.ino", expect_error=False) + test_broken_symlink("CompileIntegrationTestSymlinkBrokenCpp", "link.cpp", "doesnotexist.cpp", expect_error=False) + test_broken_symlink("CompileIntegrationTestSymlinkBrokenH", "link.h", "doesnotexist.h", expect_error=False) + test_broken_symlink("CompileIntegrationTestSymlinkBrokenJson", "link.json", "doesnotexist.json", expect_error=False) + test_broken_symlink("CompileIntegrationTestSymlinkBrokenXXX", "link.xxx", "doesnotexist.xxx", expect_error=False) def test_compile_blacklisted_sketchname(run_command, data_dir): From 2f0db251ce01bfabf7c0246dab11fd6f647a5f58 Mon Sep 17 00:00:00 2001 From: Matthijs Kooijman Date: Wed, 8 Sep 2021 18:21:17 +0200 Subject: [PATCH 3/4] Fail when sketch files cannot be opened during load Before commit e7d4eaab ([breaking] Refactor codebase to use a single Sketch struct (#1353)), any sketch file that could not be opened would report a fatal error, but the behavior was changed to silently skip such failing files instead. This restores the original behavior and fails to load a sketch of some of the contained files (after filtering on file extension, so this only includes files that are actually intended to be processed), instead of silently excluding the files (which likely produces hard to explain compilation errors later). --- arduino/sketch/sketch.go | 2 +- test/test_compile.py | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/arduino/sketch/sketch.go b/arduino/sketch/sketch.go index c5d8536cc42..88418e22a03 100644 --- a/arduino/sketch/sketch.go +++ b/arduino/sketch/sketch.go @@ -115,7 +115,7 @@ func New(path *paths.Path) (*Sketch, error) { // Skip files that can't be opened f, err := p.Open() if err != nil { - continue + return nil, err } f.Close() diff --git a/test/test_compile.py b/test/test_compile.py index 7be00f95db7..23558cbb040 100644 --- a/test/test_compile.py +++ b/test/test_compile.py @@ -217,16 +217,16 @@ def test_broken_symlink(sketch_name, link_name, target_name, expect_error): result = run_command(["compile", "-b", fqbn, sketch_path]) if expect_error: - expected_error = "Error during build: Can't open sketch: stat {}: ".format(symlink_file) + expected_error = "Error during build: Can't open sketch: open {}: ".format(symlink_file) assert expected_error in result.stderr assert not result.ok else: assert result.ok - test_broken_symlink("CompileIntegrationTestSymlinkBrokenIno", "link.ino", "doesnotexist.ino", expect_error=False) - test_broken_symlink("CompileIntegrationTestSymlinkBrokenCpp", "link.cpp", "doesnotexist.cpp", expect_error=False) - test_broken_symlink("CompileIntegrationTestSymlinkBrokenH", "link.h", "doesnotexist.h", expect_error=False) - test_broken_symlink("CompileIntegrationTestSymlinkBrokenJson", "link.json", "doesnotexist.json", expect_error=False) + test_broken_symlink("CompileIntegrationTestSymlinkBrokenIno", "link.ino", "doesnotexist.ino", expect_error=True) + test_broken_symlink("CompileIntegrationTestSymlinkBrokenCpp", "link.cpp", "doesnotexist.cpp", expect_error=True) + test_broken_symlink("CompileIntegrationTestSymlinkBrokenH", "link.h", "doesnotexist.h", expect_error=True) + test_broken_symlink("CompileIntegrationTestSymlinkBrokenJson", "link.json", "doesnotexist.json", expect_error=True) test_broken_symlink("CompileIntegrationTestSymlinkBrokenXXX", "link.xxx", "doesnotexist.xxx", expect_error=False) From 444c506f57ec715e38aa17013d0dab80d506cbad Mon Sep 17 00:00:00 2001 From: Matthijs Kooijman Date: Wed, 8 Sep 2021 20:09:49 +0200 Subject: [PATCH 4/4] Draft: Exclude compile_commands.json from sketch loading This allows users to create a symlink to the autogenerated file in the sketch directory, without that file being handled or causing failure when the target file does not exist yet (or anymore). Draft: This abuses FilterOutSuffix, it should match the filename, but go-paths-helper does not support this now. Draft: Is this really the right place/approach to exclude this file? --- arduino/sketch/sketch.go | 8 ++++++++ test/test_compile.py | 9 +++++++++ 2 files changed, 17 insertions(+) diff --git a/arduino/sketch/sketch.go b/arduino/sketch/sketch.go index 88418e22a03..e974d8c7bcf 100644 --- a/arduino/sketch/sketch.go +++ b/arduino/sketch/sketch.go @@ -166,6 +166,14 @@ func (s *Sketch) supportedFiles() (*paths.PathList, error) { } files.FilterOutDirs() files.FilterOutHiddenFiles() + // Exclude files named compile_commands.json, to allow the user + // creating a symlink to the autogenerated file in the sketch + // directory, without that being handled or causing failure when + // the target file does not exist yet. + // TODO: This abuses FilterOutSuffix, it should match the + // filename, but go-paths-helper does not support this now. + files.FilterOutSuffix("compile_commands.json") + validExtensions := []string{} for ext := range globals.MainFileValidExtensions { validExtensions = append(validExtensions, ext) diff --git a/test/test_compile.py b/test/test_compile.py index 23558cbb040..93594c386bb 100644 --- a/test/test_compile.py +++ b/test/test_compile.py @@ -228,6 +228,15 @@ def test_broken_symlink(sketch_name, link_name, target_name, expect_error): test_broken_symlink("CompileIntegrationTestSymlinkBrokenH", "link.h", "doesnotexist.h", expect_error=True) test_broken_symlink("CompileIntegrationTestSymlinkBrokenJson", "link.json", "doesnotexist.json", expect_error=True) test_broken_symlink("CompileIntegrationTestSymlinkBrokenXXX", "link.xxx", "doesnotexist.xxx", expect_error=False) + # Support a usecase where a user makes a symlink to + # compile_commands.json in the sketch root, even when it does not + # exist yet/anymore + test_broken_symlink( + "CompileIntegrationTestSymlinkBrokenCompileCommands", + "compile_commands.json", + "doesnotexist.json", + expect_error=False, + ) def test_compile_blacklisted_sketchname(run_command, data_dir):