Merge pull request #218 from ianfixes/2020-11-16_arduino-cli

Use `arduino-cli` backend

Author: ianfixes

Diff for: CHANGELOG.md

@@ -7,14 +7,31 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 
 ## [Unreleased]
 ### Added
+- Special handling of attempts to run the `arduino_ci.rb` CI script against the ruby library instead of an actual Arduino project
+- Explicit checks for attemping to test `arduino_ci` itself as if it were a library, resolving a minor annoyance to this developer.
+- Code coverage tooling
+- Explicit check and warning for library directory names that do not match our guess of what the library should/would be called
+- Symlink tests for `Host`
 
 ### Changed
+- Arduino backend is now `arduino-cli` version `0.13.0`
+- `ArduinoCmd` is now `ArduinoBackend`
+- `CppLibrary` now relies largely on `ArduinoBackend` instead of making its own judgements about libraries (metadata, includes, and examples)
+- `ArduinoBackend` functionality related to `CppLibrary` now lives in `CppLibrary`
+- `CppLibrary` now works in an installation-first manner for exposure to `arduino-cli`'s logic -- without installation, there is no ability to reason about libraries
+- `CppLibrary` forces just-in-time recursive dependency installation in order to work sensibly
+- `ArduinoBackend` maintains the central "best guess" logic on what a library (on disk) might be named
 
 ### Deprecated
+- `arduino_ci_remote.rb` CLI switch `--skip-compilation`
+- Deprecated `arduino_ci_remote.rb` in favor of `arduino_ci.rb`
 
 ### Removed
+- `ARDUINO_CI_SKIP_SPLASH_SCREEN_RSPEC_TESTS` no longer affects any tests because there are no longer splash screens since switching to `arduino-cli`
 
 ### Fixed
+- Mismatches between library names in `library.properties` and the directory names, which can cause cryptic failures
+- `LibraryProperties` skips over parse errors instead of crashing: only lines with non-empty keys and non-nil values are recorded
 
 ### Security
 

Diff for: CONTRIBUTING.md

@@ -31,7 +31,6 @@ See `SampleProjects/TestSomething/test/*.cpp` for the existing tests (run by rsp
 
 To speed up testing by targeting only the files you're working on, you can set several environment variables that `bundle exec rspec` will respond to:
 
-* `ARDUINO_CI_SKIP_SPLASH_SCREEN_RSPEC_TESTS`: if set, this will avoid any rspec test that calls the arduino executable (and as such, causes the splash screen to pop up).
 * `ARDUINO_CI_SKIP_RUBY_RSPEC_TESTS`: if set, this will skip all tests against ruby code (useful if you are not changing Ruby code).
 * `ARDUINO_CI_SKIP_CPP_RSPEC_TESTS`: if set, this will skip all tests against the `TestSomething` sample project (useful if you are not changing C++ code).
 * `ARDUINO_CI_SELECT_CPP_TESTS=<glob>`: if set, this will skip all C++ unit tests whose filenames don't match the provided glob (executed in the tests directory)

Diff for: Gemfile

@@ -4,3 +4,10 @@ git_source(:github) { |repo_name| "https://github.com/#{repo_name}" }
 
 # Specify your gem's dependencies in arduino_ci.gemspec
 gemspec
+
+gem "bundler", "> 1.15", require: false, group: :test
+gem "keepachangelog_manager", "~> 0.0.2", require: false, group: :test
+gem "rspec", "~> 3.0", require: false, group: :test
+gem 'rubocop', '~>0.59.0', require: false, group: :test
+gem 'simplecov', require: false, group: :test
+gem 'yard', '~>0.9.11', require: false, group: :test

Diff for: README.md

@@ -1,6 +1,6 @@
 
-# ArduinoCI Ruby gem (`arduino_ci`) 
-[![Gem Version](https://badge.fury.io/rb/arduino_ci.svg)](https://rubygems.org/gems/arduino_ci) 
+# ArduinoCI Ruby gem (`arduino_ci`)
+[![Gem Version](https://badge.fury.io/rb/arduino_ci.svg)](https://rubygems.org/gems/arduino_ci)
 [![Documentation](http://img.shields.io/badge/docs-rdoc.info-blue.svg)](http://www.rubydoc.info/gems/arduino_ci/0.4.0)
 [![Gitter](https://badges.gitter.im/Arduino-CI/arduino_ci.svg)](https://gitter.im/Arduino-CI/arduino_ci?utm_source=badge&utm_medium=badge&utm_campaign=pr-badge)
 
@@ -36,6 +36,9 @@ For a bare-bones example that you can copy from, see [SampleProjects/DoSomething
 
 The complete set of C++ unit tests for the `arduino_ci` library itself are in the [SampleProjects/TestSomething](SampleProjects/TestSomething) project.  The [test files](SampleProjects/TestSomething/test/) are named after the type of feature being tested.
 
+> Arduino expects all libraries to be in a specific `Arduino/libraries` directory on your system.  If your library is elsewhere, `arduino_ci` will _automatically_ create a symbolic link in the `libraries` directory that points to the directory of the project being tested.  This simplifieds working with project dependencies, but **it can have unintended consequences on Windows systems** because [in some cases deleting a folder that contains a symbolic link to another folder can cause the _entire linked folder_ to be removed instead of just the link itself](https://superuser.com/a/306618).
+>
+> If you use a Windows system **it is recommended that you only run `arduino_ci` from project directories that are already inside the `libraries` directory**
 
 ### You Need Ruby and Bundler
 

Diff for: SampleProjects/ExcludeSomething/library.properties

@@ -1,4 +1,4 @@
-name=TestSomething
+name=ExcludeSomething
 version=0.1.0
 author=Ian Katz <[email protected]>
 maintainer=Ian Katz <[email protected]>

Diff for: SampleProjects/NetworkLib/library.properties

@@ -1,4 +1,4 @@
-name=Ethernet
+name=NetworkLib
 version=0.1.0
 author=James Foster <[email protected]>
 maintainer=James Foster <[email protected]>

Diff for: arduino_ci.gemspec

@@ -27,10 +27,4 @@ Gem::Specification.new do |spec|
 
   spec.add_dependency "os", "~> 1.0"
   spec.add_dependency "rubyzip", "~> 1.2"
-
-  spec.add_development_dependency "bundler", "> 1.15"
-  spec.add_development_dependency "keepachangelog_manager", "~> 0.0.2"
-  spec.add_development_dependency "rspec", "~> 3.0"
-  spec.add_development_dependency 'rubocop', '~>0.59.0'
-  spec.add_development_dependency 'yard', '~>0.9.11'
 end

Diff for: exe/arduino_ci.rb

@@ -9,6 +9,7 @@
 
 @failure_count = 0
 @passfail = proc { |result| result ? "✓" : "✗" }
+@backend = nil
 
 # Use some basic parsing to allow command-line overrides of config
 class Parser
@@ -29,11 +30,6 @@ def self.parse(options)
         output_options[:skip_unittests] = p
       end
 
-      opts.on("--skip-compilation", "Don't compile example sketches (deprecated)") do |p|
-        puts "The option --skip-compilation has been deprecated in favor of --skip-examples-compilation"
-        output_options[:skip_compilation] = p
-      end
-
       opts.on("--skip-examples-compilation", "Don't compile example sketches") do |p|
         output_options[:skip_compilation] = p
       end
@@ -68,11 +64,11 @@ def self.parse(options)
 def terminate(final = nil)
   puts "Failures: #{@failure_count}"
   unless @failure_count.zero? || final
-    puts "Last message: #{@arduino_cmd.last_msg}"
+    puts "Last message: #{@backend.last_msg}"
     puts "========== Stdout:"
-    puts @arduino_cmd.last_out
+    puts @backend.last_out
     puts "========== Stderr:"
-    puts @arduino_cmd.last_err
+    puts @backend.last_err
   end
   retcode = @failure_count.zero? ? 0 : 1
   exit(retcode)
@@ -161,7 +157,7 @@ def file_is_hidden_somewhere?(path)
 # print out some files
 def display_files(pathname)
   # `find` doesn't follow symlinks, so we should instead
-  realpath = pathname.symlink? ? pathname.readlink : pathname
+  realpath = Host.symlink?(pathname) ? Host.readlink(pathname) : pathname
 
   # suppress directories and dotfile-based things
   all_files = realpath.find.select(&:file?)
@@ -172,25 +168,33 @@ def display_files(pathname)
   non_hidden.each { |p| puts "#{margin}#{p}" }
 end
 
-def install_arduino_library_dependencies(aux_libraries)
-  aux_libraries.each do |l|
-    if @arduino_cmd.library_present?(l)
-      inform("Using pre-existing library") { l.to_s }
+# @return [Array<String>] The list of installed libraries
+def install_arduino_library_dependencies(library_names, on_behalf_of, already_installed = [])
+  installed = already_installed.clone
+  library_names.map { |n| @backend.library_of_name(n) }.each do |l|
+    if installed.include?(l)
+      # do nothing
+    elsif l.installed?
+      inform("Using pre-existing dependency of #{on_behalf_of}") { l.name }
     else
-      assure("Installing aux library '#{l}'") { @arduino_cmd.install_library(l) }
+      assure("Installing dependency of #{on_behalf_of}: '#{l.name}'") do
+        next nil unless l.install
+
+        l.name
+      end
     end
+    installed << l.name
+    installed += install_arduino_library_dependencies(l.arduino_library_dependencies, l.name, installed)
   end
+  installed
 end
 
-def perform_unit_tests(file_config)
+def perform_unit_tests(cpp_library, file_config)
   if @cli_options[:skip_unittests]
     inform("Skipping unit tests") { "as requested via command line" }
     return
   end
   config = file_config.with_override_config(@cli_options[:ci_config])
-  cpp_library = ArduinoCI::CppLibrary.new(Pathname.new("."),
-                                          @arduino_cmd.lib_dir,
-                                          config.exclude_dirs.map(&Pathname.method(:new)))
 
   # check GCC
   compilers = config.compilers_to_use
@@ -214,10 +218,25 @@ def perform_unit_tests(file_config)
 
   # iterate boards / tests
   if !cpp_library.tests_dir.exist?
-    inform_multiline("Skipping unit tests; no tests dir at #{cpp_library.tests_dir}") do
-      puts "  In case that's an error, this is what was found in the library:"
-      display_files(cpp_library.tests_dir.parent)
-      true
+    # alert future me about running the script from the wrong directory, instead of doing the huge file dump
+    # otherwise, assume that the user might be running the script on a library with no actual unit tests
+    if Pathname.new(__dir__).parent == Pathname.new(Dir.pwd)
+      inform_multiline("arduino_ci seems to be trying to test itself") do
+        [
+          "arduino_ci (the ruby gem) isn't an arduino project itself, so running the CI test script against",
+          "the core library isn't really a valid thing to do... but it's easy for a developer (including the",
+          "owner) to mistakenly do just that.  Hello future me, you probably meant to run this against one of",
+          "the sample projects in SampleProjects/ ... if not, please submit a bug report; what a wild case!"
+        ].each { |l| puts "  #{l}" }
+        false
+      end
+      exit(1)
+    else
+      inform_multiline("Skipping unit tests; no tests dir at #{cpp_library.tests_dir}") do
+        puts "  In case that's an error, this is what was found in the library:"
+        display_files(cpp_library.tests_dir.parent)
+        true
+      end
     end
   elsif cpp_library.test_files.empty?
     inform_multiline("Skipping unit tests; no test files were found in #{cpp_library.tests_dir}") do
@@ -228,7 +247,7 @@ def perform_unit_tests(file_config)
   elsif config.platforms_to_unittest.empty?
     inform("Skipping unit tests") { "no platforms were requested" }
   else
-    install_arduino_library_dependencies(config.aux_libraries_for_unittest)
+    install_arduino_library_dependencies(config.aux_libraries_for_unittest, "<unittest/libraries>")
 
     config.platforms_to_unittest.each do |p|
       config.allowable_unittest_files(cpp_library.test_files).each do |unittest_path|
@@ -256,39 +275,12 @@ def perform_unit_tests(file_config)
   end
 end
 
-def perform_compilation_tests(config)
+def perform_example_compilation_tests(cpp_library, config)
   if @cli_options[:skip_compilation]
     inform("Skipping compilation of examples") { "as requested via command line" }
     return
   end
 
-  # index the existing libraries
-  attempt("Indexing libraries") { @arduino_cmd.index_libraries } unless @arduino_cmd.libraries_indexed
-
-  # initialize library under test
-  installed_library_path = attempt("Installing library under test") do
-    @arduino_cmd.install_local_library(Pathname.new("."))
-  end
-
-  if !installed_library_path.nil? && installed_library_path.exist?
-    inform("Library installed at") { installed_library_path.to_s }
-  else
-    assure_multiline("Library installed successfully") do
-      if installed_library_path.nil?
-        puts @arduino_cmd.last_msg
-      else
-        # print out the contents of the deepest directory we actually find
-        @arduino_cmd.lib_dir.ascend do |path_part|
-          next unless path_part.exist?
-
-          break display_files(path_part)
-        end
-        false
-      end
-    end
-  end
-  library_examples = @arduino_cmd.library_examples(installed_library_path)
-
   # gather up all required boards for compilation so we can install them up front.
   # start with the "platforms to unittest" and add the examples
   # while we're doing that, get the aux libraries as well
@@ -297,6 +289,7 @@ def perform_compilation_tests(config)
   aux_libraries = Set.new(config.aux_libraries_for_build)
   # while collecting the platforms, ensure they're defined
 
+  library_examples = cpp_library.example_sketches
   library_examples.each do |path|
     ovr_config = config.from_example(path)
     ovr_config.platforms_to_build.each do |platform|
@@ -317,35 +310,36 @@ def perform_compilation_tests(config)
   # do that, set the URLs, and download the packages
   all_packages = example_platform_info.values.map { |v| v[:package] }.uniq.reject(&:nil?)
 
+  builtin_packages, external_packages = all_packages.partition { |p| config.package_builtin?(p) }
+
   # inform about builtin packages
-  all_packages.select { |p| config.package_builtin?(p) }.each do |p|
+  builtin_packages.each do |p|
     inform("Using built-in board package") { p }
   end
 
   # make sure any non-builtin package has a URL defined
-  all_packages.reject { |p| config.package_builtin?(p) }.each do |p|
+  external_packages.each do |p|
     assure("Board package #{p} has a defined URL") { board_package_url[p] }
   end
 
   # set up all the board manager URLs.
   # we can safely reject nils now, they would be for the builtins
-  all_urls = all_packages.map { |p| board_package_url[p] }.uniq.reject(&:nil?)
+  all_urls = external_packages.map { |p| board_package_url[p] }.uniq.reject(&:nil?)
 
   unless all_urls.empty?
     assure("Setting board manager URLs") do
-      @arduino_cmd.board_manager_urls = all_urls
+      @backend.board_manager_urls = all_urls
     end
   end
 
-  all_packages.each do |p|
+  external_packages.each do |p|
     assure("Installing board package #{p}") do
-      @arduino_cmd.install_boards(p)
+      @backend.install_boards(p)
     end
   end
 
-  install_arduino_library_dependencies(aux_libraries)
+  install_arduino_library_dependencies(aux_libraries, "<compile/libraries>")
 
-  last_board = nil
   if config.platforms_to_build.empty?
     inform("Skipping builds") { "no platforms were requested" }
     return
@@ -356,46 +350,58 @@ def perform_compilation_tests(config)
     return
   end
 
-  attempt("Setting compiler warning level")  { @arduino_cmd.set_pref("compiler.warning_level", "all") }
-
-  # switching boards takes time, so iterate board first
-  # _then_ whichever examples match it
-  examples_by_platform = library_examples.each_with_object({}) do |example_path, acc|
+  library_examples.each do |example_path|
     ovr_config = config.from_example(example_path)
     ovr_config.platforms_to_build.each do |p|
-      acc[p] = [] unless acc.key?(p)
-      acc[p] << example_path
-    end
-  end
-
-  examples_by_platform.each do |platform, example_paths|
-    board = example_platform_info[platform][:board]
-    assure("Switching to board for #{platform} (#{board})") { @arduino_cmd.use_board(board) } unless last_board == board
-    last_board = board
-
-    example_paths.each do |example_path|
+      board = example_platform_info[p][:board]
       example_name = File.basename(example_path)
-      attempt("Verifying #{example_name}") do
-        ret = @arduino_cmd.verify_sketch(example_path)
+      attempt("Compiling #{example_name} for #{board}") do
+        ret = @backend.compile_sketch(example_path, board)
         unless ret
           puts
-          puts "Last command: #{@arduino_cmd.last_msg}"
-          puts @arduino_cmd.last_err
+          puts "Last command: #{@backend.last_msg}"
+          puts @backend.last_err
         end
         ret
       end
     end
   end
-
 end
 
 # initialize command and config
 config = ArduinoCI::CIConfig.default.from_project_library
 
-@arduino_cmd = ArduinoCI::ArduinoInstallation.autolocate!
-inform("Located Arduino binary") { @arduino_cmd.binary_path.to_s }
+@backend = ArduinoCI::ArduinoInstallation.autolocate!
+inform("Located arduino-cli binary") { @backend.binary_path.to_s }
+
+# initialize library under test
+cpp_library_path = Pathname.new(".")
+cpp_library = assure("Installing library under test") do
+  @backend.install_local_library(cpp_library_path)
+end
+
+assumed_name = @backend.name_of_library(cpp_library_path)
+ondisk_name = cpp_library_path.realpath.basename
+if assumed_name != ondisk_name
+  inform("WARNING") { "Installed library named '#{assumed_name}' has directory name '#{ondisk_name}'" }
+end
+
+if !cpp_library.nil?
+  inform("Library installed at") { cpp_library.path.to_s }
+else
+  # this is a longwinded way of failing, we aren't really "assuring" anything at this point
+  assure_multiline("Library installed successfully") do
+    puts @backend.last_msg
+    false
+  end
+end
+
+install_arduino_library_dependencies(
+  cpp_library.arduino_library_dependencies,
+  "<#{ArduinoCI::CppLibrary::LIBRARY_PROPERTIES_FILE}>"
+)
 
-perform_unit_tests(config)
-perform_compilation_tests(config)
+perform_unit_tests(cpp_library, config)
+perform_example_compilation_tests(cpp_library, config)
 
 terminate(true)