Properly separate ConvenienceFunctions out of PackageUtils and move additional functions into MiscFunctions, and document the various roles of each class (#10343)

bin/crew

@@ -55,9 +55,9 @@ require_relative '../lib/deb_utils'
 require_relative '../lib/docopt'
 require_relative '../lib/downloader'
 require_relative '../lib/gnome'
+require_relative '../lib/misc_functions'
 require_relative '../lib/package'
 require_relative '../lib/package_utils'
-require_relative '../lib/util'
 
 # Add lib to LOAD_PATH
 $LOAD_PATH << File.join(CREW_LIB_PATH, 'lib')
@@ -83,7 +83,7 @@ rescue Docopt::Exit => e
         puts "Could not understand \"crew #{ARGV.join(' ')}\".".lightred
         # Looking for similar commands
         unless CREW_COMMANDS.include?(ARGV[0])
-          similar = CREW_COMMANDS.split.select { |word| edit_distance(ARGV[0], word) < 4 }
+          similar = CREW_COMMANDS.split.select { |word| MiscFunctions.edit_distance(ARGV[0], word) < 4 }
           unless similar.empty?
             abort <<~EOT
               Did you mean?
@@ -203,7 +203,7 @@ def generate_compatible
   @device[:essential_deps] = []
   @device[:essential_deps].concat(CREW_ESSENTIAL_PACKAGES.flat_map { |i| Package.load_package("#{i}.rb").get_deps_list }.push(*CREW_ESSENTIAL_PACKAGES).uniq.sort)
   crewlog "Essential packages: #{@device[:essential_deps]}"
-  PackageUtils.save_json(@device)
+  ConvenienceFunctions.save_json(@device)
   puts 'Determined compatibility & which packages are essential.'.orange if CREW_VERBOSE
 end
 
@@ -695,7 +695,7 @@ def build_and_preconfigure(target_dir)
 
     build_end_time = Time.now.to_i
 
-    crewlog "Build for #{@pkg.name} took #{time_difference(build_start_time, build_end_time)}."
+    crewlog "Build for #{@pkg.name} took #{MiscFunctions.time_difference(build_start_time, build_end_time)}."
   end
 end
 
@@ -711,7 +711,7 @@ def pre_install(dest_dir)
     @pkg.preinstall
     # Reload device.json in case preinstall modified it via
     # running 'crew remove packages...'
-    @device = PackageUtils.load_json
+    @device = ConvenienceFunctions.load_symbolized_json
   end
 end
 
@@ -1031,27 +1031,6 @@ def shrink_dir(dir)
   end
 end
 
-def time_difference(start_time = nil, end_time = nil)
-  return 'ERROR' if start_time.nil? || end_time.nil?
-
-  time_elapsed = end_time.to_i - start_time.to_i
-  time_hours = time_elapsed / 3600
-  time_minutes = time_elapsed / 60 % 60
-  time_seconds = time_elapsed % 60
-  time_hour_string = if time_hours.zero?
-                       ''
-                     else
-                       "#{time_hours} hr#{time_hours > 1 ? 's, ' : ', '}"
-                     end
-  time_minutes_string = if time_minutes.zero?
-                          time_hour_string.empty? ? '' : "#{time_minutes} min, "
-                        else
-                          "#{time_minutes} min, "
-                        end
-  time_seconds_string = "#{time_seconds} second#{time_seconds == 1 ? '' : 's'}"
-  return time_hour_string + time_minutes_string + time_seconds_string
-end
-
 def install_files(src, dst = File.join(CREW_PREFIX, src.delete_prefix('./usr/local')))
   if Dir.exist?(src)
     if File.executable?("#{CREW_PREFIX}/bin/crew-mvdir") && !CREW_DISABLE_MVDIR
@@ -1317,14 +1296,14 @@ def install
 
   install_end_time = Time.now.to_i
 
-  install_time_elapsed_string = time_difference(install_start_time, install_end_time)
+  install_time_elapsed_string = MiscFunctions.time_difference(install_start_time, install_end_time)
   crewlog "Build & install for #{@pkg.name} took #{install_time_elapsed_string}."
   puts "Build & install for #{@pkg.name} took #{install_time_elapsed_string}. Please ask for #{ARCH} binaries to be generated for #{@pkg.name}.".lightpurple if (install_start_time - install_end_time) > 60
 
   # Add to installed packages list in devices.json, but remove first if it is already there.
   crewlog "Adding package #{@pkg.name} to device.json."
   @device[:installed_packages].delete_if { |entry| entry[:name] == @pkg.name } and @device[:installed_packages].push(name: @pkg.name, version: @pkg.version, sha256: PackageUtils.get_sha256(@pkg, build_from_source: @opt_source))
-  PackageUtils.save_json(@device)
+  ConvenienceFunctions.save_json(@device)
   crewlog "#{@pkg.name} in device.json after install: #{`jq --arg key '#{@pkg.name}' -e '.installed_packages[] | select(.name == $key )' #{File.join(CREW_CONFIG_PATH, 'device.json')}`}" if Kernel.system 'which jq', %i[out err] => File::NULL
 end
 
@@ -1837,10 +1816,10 @@ Signal.trap('INT') do
   exit 1
 end
 
-@device = PackageUtils.load_json
+@device = ConvenienceFunctions.load_symbolized_json
 
 @last_update_check = Dir["#{CREW_LIB_PATH}/{.git/FETCH_HEAD,lib/const.rb}"].compact.map { |i| File.mtime(i).utc.to_i }.max
-crewlog("The last update was #{time_difference(@last_update_check, Time.now.to_i)} ago.")
+crewlog("The last update was #{MiscFunctions.time_difference(@last_update_check, Time.now.to_i)} ago.")
 puts "It has been more than #{CREW_UPDATE_CHECK_INTERVAL} day#{CREW_UPDATE_CHECK_INTERVAL < 2 ? '' : 's'} since crew was last updated. Please run 'crew update'".lightpurple if Time.now.to_i - @last_update_check > (CREW_UPDATE_CHECK_INTERVAL * 3600 * 24)
 command_name = args.select { |k, v| v && command?(k) }.keys[0]
 send("#{command_name}_command", args)

commands/files.rb

@@ -1,5 +1,5 @@
 require_relative '../lib/const'
-require_relative '../lib/convert_size'
+require_relative '../lib/misc_functions'
 require_relative '../lib/package_utils'
 
 class Command
@@ -41,6 +41,6 @@ class Command
     # Print the filelist, the total number of files, and the total size of those files.
     puts filelist
     puts "\nTotal found: #{filelist.count}".lightgreen
-    puts "Disk usage: #{human_size(size)}".lightgreen
+    puts "Disk usage: #{MiscFunctions.human_size(size)}".lightgreen
   end
 end

commands/remove.rb

@@ -1,11 +1,12 @@
 require 'fileutils'
 require_relative '../lib/const'
+require_relative '../lib/convenience_functions'
 require_relative '../lib/package'
 require_relative '../lib/package_utils'
 
 class Command
   def self.remove(pkg, verbose)
-    device_json = PackageUtils.load_json
+    device_json = ConvenienceFunctions.load_symbolized_json
 
     # Make sure the package is actually installed before we attempt to remove it.
     unless PackageUtils.installed?(pkg.name)
@@ -98,7 +99,7 @@ class Command
     device_json[:installed_packages].delete_if { |entry| entry[:name] == pkg.name }
 
     # Update device.json with our changes.
-    PackageUtils.save_json(device_json)
+    ConvenienceFunctions.save_json(device_json)
 
     # Perform any operations required after package removal.
     pkg.postremove

lib/const.rb

@@ -2,7 +2,7 @@
 # Defines common constants used in different parts of crew
 require 'etc'
 
-CREW_VERSION = '1.51.1'
+CREW_VERSION = '1.51.2'
 
 # Kernel architecture.
 KERN_ARCH = Etc.uname[:machine]

lib/convenience_functions.rb

@@ -0,0 +1,24 @@
+# lib/convenience_functions.rb
+# Extracted bits of crew-specific code that we use frequently enough that it makes sense to split them out to here.
+require 'json'
+require_relative 'const'
+require_relative 'crewlog'
+
+class ConvenienceFunctions
+  def self.load_symbolized_json
+    return JSON.load_file(File.join(CREW_CONFIG_PATH, 'device.json'), symbolize_names: true).transform_values! { |val| val.is_a?(String) ? val.to_sym : val }
+  end
+
+  def self.save_json(json_object)
+    crewlog 'Saving device.json...'
+    begin
+      File.write File.join(CREW_CONFIG_PATH, 'device.json.tmp'), JSON.pretty_generate(JSON.parse(json_object.to_json))
+    rescue StandardError
+      puts 'Error writing updated packages json file!'.lightred
+      abort
+    end
+
+    # Copy over original if the write to the tmp file succeeds.
+    FileUtils.cp("#{CREW_CONFIG_PATH}/device.json.tmp", File.join(CREW_CONFIG_PATH, 'device.json')) && FileUtils.rm("#{CREW_CONFIG_PATH}/device.json.tmp")
+  end
+end

lib/convert_size.rb

@@ -1,22 +0,0 @@
-def human_size(bytes)
-  kilobyte = 1024.0
-  megabyte = kilobyte * kilobyte
-  gigabyte = megabyte * kilobyte
-  if bytes < kilobyte
-    units = 'B'
-    size = bytes
-  end
-  if (bytes >= kilobyte) && (bytes < megabyte)
-    units = 'KB'
-    size = bytes / kilobyte
-  end
-  if (bytes >= megabyte) && (bytes < gigabyte)
-    units = 'MB'
-    size = bytes / megabyte
-  end
-  if bytes >= gigabyte
-    units = 'GB'
-    size = bytes / gigabyte
-  end
-  return format('%.2f %s', size, units)
-end

lib/fixup.rb

@@ -4,7 +4,7 @@ require 'etc'
 require 'json'
 require_relative 'color'
 require_relative 'package'
-require_relative 'package_utils'
+require_relative 'convenience_functions'
 
 # All needed constants & variables should be defined here in case they
 # have not yet been loaded or fixup is being run standalone.
@@ -26,7 +26,7 @@ unless defined?(ARCH)
 end
 LIBC_VERSION = Etc.confstr(Etc::CS_GNU_LIBC_VERSION).split.last unless defined?(LIBC_VERSION)
 CREW_PACKAGES_PATH = File.join(CREW_LIB_PATH, 'packages') unless defined?(CREW_PACKAGES_PATH)
-@device = PackageUtils.load_json unless defined? @device
+@device = ConvenienceFunctions.load_symbolized_json unless defined? @device
 
 # remove deprecated directory
 FileUtils.rm_rf "#{HOME}/.cache/crewcache/manifest"

lib/misc_functions.rb

@@ -0,0 +1,75 @@
+# lib/misc_functions.rb
+# Generic implementations of various functions/algorithms that are not crew-specific.
+require 'matrix'
+
+class MiscFunctions
+  def self.human_size(bytes)
+    kilobyte = 1024.0
+    megabyte = kilobyte * kilobyte
+    gigabyte = megabyte * kilobyte
+    if bytes < kilobyte
+      units = 'B'
+      size = bytes
+    end
+    if (bytes >= kilobyte) && (bytes < megabyte)
+      units = 'KB'
+      size = bytes / kilobyte
+    end
+    if (bytes >= megabyte) && (bytes < gigabyte)
+      units = 'MB'
+      size = bytes / megabyte
+    end
+    if bytes >= gigabyte
+      units = 'GB'
+      size = bytes / gigabyte
+    end
+    return format('%.2f %s', size, units)
+  end
+
+  # Returns the edit distance between strings string1 and string12
+  # https://en.wikipedia.org/wiki/Edit_distance
+  def self.edit_distance(string1, string2)
+    # memo is the matrix for dynamic programming
+    # memo[i, j] = the edit distance between the
+    # prefixes of string1 and string2 of size i and j.
+    memo = Matrix.zero(string1.size + 1, string2.size + 1)
+    string1.size.times { |i| memo[i + 1, 0] = i + 1 }
+    string2.size.times { |j| memo[0, j + 1] = j + 1 }
+    string1.size.times do |i|
+      string2.size.times do |j|
+        memo[i + 1, j + 1] = if string1[i] == string2[j]
+                               memo[i, j]
+                             else
+                               [
+                                 memo[i + 1, j],
+                                 memo[i, j + 1],
+                                 memo[i, j]
+                               ].min + 1
+                             end
+      end
+    end
+
+    return memo[string1.size, string2.size]
+  end
+
+  def self.time_difference(start_time = nil, end_time = nil)
+    return 'ERROR' if start_time.nil? || end_time.nil?
+
+    time_elapsed = end_time.to_i - start_time.to_i
+    time_hours = time_elapsed / 3600
+    time_minutes = time_elapsed / 60 % 60
+    time_seconds = time_elapsed % 60
+    time_hour_string = if time_hours.zero?
+                         ''
+                       else
+                         "#{time_hours} hr#{time_hours > 1 ? 's, ' : ', '}"
+                       end
+    time_minutes_string = if time_minutes.zero?
+                            time_hour_string.empty? ? '' : "#{time_minutes} min, "
+                          else
+                            "#{time_minutes} min, "
+                          end
+    time_seconds_string = "#{time_seconds} second#{time_seconds == 1 ? '' : 's'}"
+    return time_hour_string + time_minutes_string + time_seconds_string
+  end
+end

lib/package_utils.rb

@@ -1,27 +1,12 @@
+# lib/package_utils.rb
+# Utility functions that take either a package object or a component of a package object as primary input.
 require 'json'
 require_relative 'const'
-require_relative 'crewlog'
 
 class PackageUtils
-  def self.load_json
-    return JSON.load_file(File.join(CREW_CONFIG_PATH, 'device.json'), symbolize_names: true).transform_values! { |val| val.is_a?(String) ? val.to_sym : val }
-  end
-
-  def self.save_json(json_object)
-    crewlog 'Saving device.json...'
-    begin
-      File.write File.join(CREW_CONFIG_PATH, 'device.json.tmp'), JSON.pretty_generate(JSON.parse(json_object.to_json))
-    rescue StandardError
-      puts 'Error writing updated packages json file!'.lightred
-      abort
-    end
-
-    # Copy over original if the write to the tmp file succeeds.
-    FileUtils.cp("#{CREW_CONFIG_PATH}/device.json.tmp", File.join(CREW_CONFIG_PATH, 'device.json')) && FileUtils.rm("#{CREW_CONFIG_PATH}/device.json.tmp")
-  end
-
   def self.installed?(pkg_name)
-    return load_json[:installed_packages].any? { |elem| elem[:name] == pkg_name }
+    device_json = JSON.load_file(File.join(CREW_CONFIG_PATH, 'device.json'))
+    return device_json['installed_packages'].any? { |elem| elem['name'] == pkg_name }
   end
 
   def self.compatible?(pkg)

lib/progress_bar.rb

@@ -1,6 +1,6 @@
 require 'io/console'
 require_relative 'color'
-require_relative 'convert_size'
+require_relative 'misc_functions'
 
 class ProgressBar
   class InvalidSizeError < StandardError; end
@@ -23,7 +23,7 @@ class ProgressBar
     @percentage = @downloaded = 0
     @total_size = total_size.to_f
 
-    @total_size_in_str = human_size(@total_size)
+    @total_size_in_str = MiscFunctions.human_size(@total_size)
 
     trap('WINCH') do
       # reset width settings after terminal resized
@@ -48,7 +48,7 @@ class ProgressBar
       @percentage_in_str = '---'
 
       @total_size_in_str = ''
-      @downloaded_size_in_str = human_size(downloaded_size)
+      @downloaded_size_in_str = MiscFunctions.human_size(downloaded_size)
 
       # raise error unless #{invalid_size_error} is set to false
       if invalid_size_error
@@ -73,7 +73,7 @@ class ProgressBar
     @percentage_in_str = "#{@percentage.to_i}%"
 
     # {downloaded size}/{total size}
-    @downloaded_size_in_str = "#{human_size(downloaded_size)}/#{@total_size_in_str}"
+    @downloaded_size_in_str = "#{MiscFunctions.human_size(downloaded_size)}/#{@total_size_in_str}"
   end
 
   def show

lib/util.rb

@@ -1,31 +0,0 @@
-require 'matrix'
-
-class MutableMatrix < Matrix
-  public :'[]='
-end
-
-# Returns the edit distance between strings string1 and string12
-# https://en.wikipedia.org/wiki/Edit_distance
-def edit_distance(string1, string2)
-  # memo is the matrix for dynamic programming
-  # memo[i, j] = the edit distance between the
-  # prefixes of string1 and string2 of size i and j.
-  memo = MutableMatrix.zero(string1.size + 1, string2.size + 1)
-  string1.size.times { |i| memo[i + 1, 0] = i + 1 }
-  string2.size.times { |j| memo[0, j + 1] = j + 1 }
-  string1.size.times do |i|
-    string2.size.times do |j|
-      memo[i + 1, j + 1] = if string1[i] == string2[j]
-                             memo[i, j]
-                           else
-                             [
-                               memo[i + 1, j],
-                               memo[i, j + 1],
-                               memo[i, j]
-                             ].min + 1
-                           end
-    end
-  end
-
-  return memo[string1.size, string2.size]
-end

tests/commands/list.rb

@@ -1,6 +1,6 @@
 require 'minitest/autorun'
 require_relative '../../commands/list'
-require_relative '../../lib/package_utils'
+require_relative '../../lib/convenience_functions'
 
 # Add lib to LOAD_PATH
 $LOAD_PATH << File.join(CREW_LIB_PATH, 'lib')
@@ -9,7 +9,7 @@ String.use_color = false
 
 class ListCommandTest < Minitest::Test
   def setup
-    @essential_deps = PackageUtils.load_json[:essential_deps].join("\n") + "\n".to_s
+    @essential_deps = ConvenienceFunctions.load_symbolized_json[:essential_deps].join("\n") + "\n".to_s
   end
 
   def test_list_essential_deps

tests/commands/remove.rb

@@ -1,6 +1,7 @@
 require 'minitest/autorun'
 require_relative '../../commands/remove'
 require_relative '../../lib/const'
+require_relative '../../lib/convenience_functions'
 require_relative '../../lib/package_utils'
 
 # Add lib to LOAD_PATH
@@ -11,7 +12,7 @@ String.use_color = false
 
 class RemoveCommandTest < Minitest::Test
   def setup
-    essential_deps = PackageUtils.load_json[:essential_deps]
+    essential_deps = ConvenienceFunctions.load_symbolized_json[:essential_deps]
     @random_essential_package_name = essential_deps[rand(0...(essential_deps.length - 1))]
     puts <<~ESSENTIAL_PACKAGE_REMOVAL_TEST_EOF