Implement symlink logic for windows hosts

Arduino-CI · ianfixes · Nov 27, 2020 · Nov 16, 2020 · Nov 22, 2020 · Nov 22, 2020
commit 1f8ba56696013f5f794df8296d5ded67df0ac561
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 - 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`

diff --git a/README.md b/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 --git a/exe/arduino_ci.rb b/exe/arduino_ci.rb
@@ -157,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?)

diff --git a/lib/arduino_ci/arduino_backend.rb b/lib/arduino_ci/arduino_backend.rb
@@ -196,10 +196,11 @@ def install_local_library(path)
 
         uhoh = "There is already a library '#{library_name}' in the library directory (#{destination_path})"
         # maybe it's a symlink? that would be OK
-        if destination_path.symlink?
-          return cpp_library if destination_path.readlink == src_path
+        if Host.symlink?(destination_path)
+          current_destination_target = Host.readlink(destination_path)
+          return cpp_library if current_destination_target == src_path
 
-          @last_msg = "#{uhoh} and it's not symlinked to #{src_path}"
+          @last_msg = "#{uhoh} and it's symlinked to #{current_destination_target} (expected #{src_path})"
           return nil
         end
 

diff --git a/lib/arduino_ci/host.rb b/lib/arduino_ci/host.rb
@@ -6,6 +6,13 @@ module ArduinoCI
 
   # Tools for interacting with the host machine
   class Host
+    # TODO: this came from https://stackoverflow.com/a/22716582/2063546
+    #       and I'm not sure if it can be replaced by self.os == :windows
+    WINDOWS_VARIANT_REGEX = /mswin32|cygwin|mingw|bccwin/
+
+    # e.g. 11/27/2020  01:02 AM    <SYMLINKD>     ExcludeSomething [C:\projects\arduino-ci\SampleProjects\ExcludeSomething]
+    DIR_SYMLINK_REGEX = %r{\d+/\d+/\d+\s+[^<]+<SYMLINKD?>\s+(.*) \[([^\]]+)\]}
+
     # Cross-platform way of finding an executable in the $PATH.
     # via https://stackoverflow.com/a/5471032/2063546
     #   which('ruby') #=> /usr/bin/ruby
@@ -38,21 +45,69 @@ def self.os
       return :windows if OS.windows?
     end
 
+    # Cross-platform symlinking
     # if on windows, call mklink, else self.symlink
     # @param [Pathname] old_path
     # @param [Pathname] new_path
     def self.symlink(old_path, new_path)
-      return FileUtils.ln_s(old_path.to_s, new_path.to_s) unless RUBY_PLATFORM =~ /mswin32|cygwin|mingw|bccwin/
+      # we would prefer `new_path.make_symlink(old_path)` but "symlink function is unimplemented on this machine" with windows
+      return new_path.make_symlink(old_path) unless needs_symlink_hack?
 
-      # https://stackoverflow.com/a/22716582/2063546
+      # via https://stackoverflow.com/a/22716582/2063546
       # windows mklink syntax is reverse of unix ln -s
       # windows mklink is built into cmd.exe
       # vulnerable to command injection, but okay because this is a hack to make a cli tool work.
-      orp = old_path.realpath.to_s.tr("/", "\\") # HACK DUE TO REALPATH BUG where it
-      np = new_path.to_s.tr("/", "\\")           # still joins windows paths with '/'
+      orp = pathname_to_windows(old_path.realpath)
+      np  = pathname_to_windows(new_path)
 
       _stdout, _stderr, exitstatus = Open3.capture3('cmd.exe', "/C mklink /D #{np} #{orp}")
       exitstatus.success?
     end
+
+    # Hack for "realpath" which on windows joins paths with slashes instead of backslashes
+    # @param path [Pathname] the path to render
+    # @return [String] A path that will work on windows
+    def self.pathname_to_windows(path)
+      path.to_s.tr("/", "\\")
+    end
+
+    # Hack for "realpath" which on windows joins paths with slashes instead of backslashes
+    # @param str [String] the windows path
+    # @return [Pathname] A path that will be recognized by pathname
+    def self.windows_to_pathname(str)
+      Pathname.new(str.tr("\\", "/"))
+    end
+
+    # Whether this OS requires a hack for symlinks
+    # @return [bool]
+    def self.needs_symlink_hack?
+      RUBY_PLATFORM =~ WINDOWS_VARIANT_REGEX
+    end
+
+    # Cross-platform is-this-a-symlink function
+    # @param [Pathname] path
+    # @return [bool] Whether the file is a symlink
+    def self.symlink?(path)
+      return path.symlink? unless needs_symlink_hack?
+
+      !readlink(path).nil?
+    end
+
+    # Cross-platform "read link" function
+    # @param [Pathname] path
+    # @return [Pathname] the link target
+    def self.readlink(path)
+      return path.readlink unless needs_symlink_hack?
+
+      the_dir  = pathname_to_windows(path.parent)
+      the_file = path.basename.to_s
+
+      stdout, _stderr, _exitstatus = Open3.capture3('cmd.exe', "/c dir /al #{the_dir}")
+      symlinks = stdout.lines.map { |l| DIR_SYMLINK_REGEX.match(l) }.compact
+      our_link = symlinks.find { |m| m[1] == the_file }
+      return nil if our_link.nil?
+
+      windows_to_pathname(our_link[2])
+    end
   end
 end
diff --git a/spec/ci_config_spec.rb b/spec/ci_config_spec.rb
@@ -171,17 +171,27 @@
       expect(cpp_lib_path.exist?).to be(true)
       expect(@cpp_library).to_not be(nil)
       expect(@cpp_library.path.exist?).to be(true)
-      expect(@cpp_library.test_files.map { |f| File.basename(f) }).to match_array([
+      expect(@cpp_library.test_files.map(&:basename).map(&:to_s)).to match_array([
         "sam-squamsh.cpp",
         "yes-good.cpp",
         "mars.cpp"
       ])
     end
 
     it "filters that set of files" do
+      expect(cpp_lib_path.exist?).to be(true)
+      expect(@cpp_library).to_not be(nil)
+      expect(@cpp_library.test_files.map(&:basename).map(&:to_s)).to match_array([
+        "sam-squamsh.cpp",
+        "yes-good.cpp",
+        "mars.cpp"
+      ])
+
       override_file = File.join(File.dirname(__FILE__), "yaml", "o1.yaml")
       combined_config = ArduinoCI::CIConfig.default.with_override(override_file)
-      expect(combined_config.allowable_unittest_files(@cpp_library.test_files).map { |f| File.basename(f) }).to match_array([
+      expect(combined_config.unittest_info[:testfiles][:select]).to match_array(["*-*.*"])
+      expect(combined_config.unittest_info[:testfiles][:reject]).to match_array(["sam-squamsh.*"])
+      expect(combined_config.allowable_unittest_files(@cpp_library.test_files).map(&:basename).map(&:to_s)).to match_array([
         "yes-good.cpp",
       ])
     end

diff --git a/spec/fake_lib_dir.rb b/spec/fake_lib_dir.rb
@@ -18,7 +18,8 @@ def initialize
 
   # designed to be called by rspec's "around" function
   def in_pristine_fake_libraries_dir(example)
-    Dir.mktmpdir do |d|
+    d = Dir.mktmpdir
+    begin
       # write a yaml file containing the current directory
       dummy_config = { "directories" => { "user" => d.to_s } }
       @arduino_dir = Pathname.new(d)
@@ -37,6 +38,17 @@ def in_pristine_fake_libraries_dir(example)
           # cool, already done
         end
       end
+    ensure
+      if ArduinoCI::Host.needs_symlink_hack?
+        stdout, stderr, exitstatus = Open3.capture3('cmd.exe', "/c rmdir /s /q #{ArduinoCI::Host.pathname_to_windows(d)}")
+        unless exitstatus.success?
+          puts "====== rmdir of #{d} failed"
+          puts stdout
+          puts stderr
+        end
+      else
+        FileUtils.remove_entry(d)
+      end
     end
   end
 

diff --git a/spec/host_spec.rb b/spec/host_spec.rb
@@ -0,0 +1,53 @@
+require "spec_helper"
+require 'tmpdir'
+
+
+def idempotent_delete(path)
+  path.delete
+rescue Errno::ENOENT
+end
+
+# creates a dir at <path> then deletes it after block executes
+# this will DESTROY any existing entry at that location in the filesystem
+def with_tmpdir(path)
+  begin
+    idempotent_delete(path)
+    path.mkpath
+    yield
+  ensure
+    idempotent_delete(path)
+  end
+end
+
+
+RSpec.describe ArduinoCI::Host do
+  next if skip_ruby_tests
+
+  context "symlinks" do
+    it "creates symlinks that we agree are symlinks" do
+      our_dir = Pathname.new(__dir__)
+      foo_dir = our_dir + "foo_dir"
+      bar_dir = our_dir + "bar_dir"
+
+      with_tmpdir(foo_dir) do
+        foo_dir.unlink # we just want to place something at this location
+        expect(foo_dir.exist?).to be_falsey
+
+        with_tmpdir(bar_dir) do
+          expect(bar_dir.exist?).to be_truthy
+          expect(bar_dir.symlink?).to be_falsey
+
+          ArduinoCI::Host.symlink(bar_dir, foo_dir)
+          expect(ArduinoCI::Host.symlink?(bar_dir)).to be_falsey
+          expect(ArduinoCI::Host.symlink?(foo_dir)).to be_truthy
+          expect(ArduinoCI::Host.readlink(foo_dir).realpath).to eq(bar_dir.realpath)
+        end
+      end
+
+      expect(foo_dir.exist?).to be_falsey
+      expect(bar_dir.exist?).to be_falsey
+
+    end
+  end
+
+end