Skip to content

Commit

Permalink
Merge pull request #381 from yast/software_proposal_validation
Browse files Browse the repository at this point in the history
Initial software validation
  • Loading branch information
joseivanlopez committed Jan 17, 2023
2 parents 9fc1b69 + f41a37d commit 5d045aa
Show file tree
Hide file tree
Showing 18 changed files with 283 additions and 18 deletions.
1 change: 1 addition & 0 deletions service/.rubocop.yml
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ AllCops:
- vendor/**/*
- lib/dinstaller/dbus/y2dir/**/*
- d-installer.gemspec
- package/*.spec

# a D-Bus method definition may take up more line lenght than usual
Layout/LineLength:
Expand Down
2 changes: 2 additions & 0 deletions service/lib/dinstaller/dbus/clients/software.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
require "dinstaller/dbus/clients/base"
require "dinstaller/dbus/clients/with_service_status"
require "dinstaller/dbus/clients/with_progress"
require "dinstaller/dbus/clients/with_validation"

module DInstaller
module DBus
Expand All @@ -30,6 +31,7 @@ module Clients
class Software < Base
include WithServiceStatus
include WithProgress
include WithValidation

TYPES = [:package, :pattern].freeze
private_constant :TYPES
Expand Down
10 changes: 10 additions & 0 deletions service/lib/dinstaller/dbus/software/manager.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
require "dinstaller/dbus/clients/language"
require "dinstaller/dbus/interfaces/progress"
require "dinstaller/dbus/interfaces/service_status"
require "dinstaller/dbus/interfaces/validation"

module DInstaller
module DBus
Expand All @@ -34,6 +35,7 @@ class Manager < BaseObject
include WithServiceStatus
include Interfaces::Progress
include Interfaces::ServiceStatus
include Interfaces::Validation

PATH = "/org/opensuse/DInstaller/Software1"
private_constant :PATH
Expand Down Expand Up @@ -63,6 +65,7 @@ def initialize(backend, logger)

select_product(product_id)
dbus_properties_changed(SOFTWARE_INTERFACE, { "SelectedBaseProduct" => product_id }, [])
update_validation # as different product means different software selection
end

# TODO: just for performance comparison (see `perf.rb`)
Expand All @@ -78,6 +81,8 @@ def initialize(backend, logger)
backend.package_installed?(name)
end

dbus_method(:UsedDiskSpace, "out SpaceSize:s") { backend.used_disk_space }

dbus_method(:Probe) { probe }
dbus_method(:Propose) { propose }
dbus_method(:Install) { install }
Expand All @@ -103,10 +108,15 @@ def select_product(product_id)

def probe
busy_while { backend.probe }

update_validation # probe do force proposal
end

def propose
busy_while { backend.propose }
update_validation

nil # explicit nil as return value
end

def install
Expand Down
12 changes: 12 additions & 0 deletions service/lib/dinstaller/dbus/software/proposal.rb
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ class Proposal < ::DBus::Object
# @param logger [Logger]
def initialize(logger)
@logger = logger
@on_change_callbacks = []

super(PATH)
end
Expand All @@ -54,6 +55,7 @@ def initialize(logger)
dbus_method :AddResolvables,
"in Id:s, in Type:y, in Resolvables:as, in Optional:b" do |id, type, resolvables, opt|
Yast::PackagesProposal.AddResolvables(id, TYPES[type], resolvables, optional: opt)
notify_change!
end

dbus_method :GetResolvables,
Expand All @@ -64,18 +66,28 @@ def initialize(logger)
dbus_method :SetResolvables,
"in Id:s, in Type:y, in Resolvables:as, in Optional:b" do |id, type, resolvables, opt|
Yast::PackagesProposal.SetResolvables(id, TYPES[type], resolvables, optional: opt)
notify_change!
end

dbus_method :RemoveResolvables,
"in Id:s, in Type:y, in Resolvables:as, in Optional:b" do |id, type, resolvables, opt|
Yast::PackagesProposal.RemoveResolvables(id, TYPES[type], resolvables, optional: opt)
notify_change!
end
end

def on_change(&block)
@on_change_callbacks << block
end

private

# @return [Logger]
attr_reader :logger

def notify_change!
@on_change_callbacks.each(&:call)
end
end
end
end
Expand Down
4 changes: 3 additions & 1 deletion service/lib/dinstaller/dbus/software_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,9 @@ def service
def dbus_objects
@dbus_objects ||= [
dbus_software_manager,
DInstaller::DBus::Software::Proposal.new(logger)
DInstaller::DBus::Software::Proposal.new(logger).tap do |proposal|
proposal.on_change { dbus_software_manager.update_validation }
end
]
end

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,10 @@ def main
def GetPartitionInfo; end

# @see https://github.com/yast/yast-packager/blob/master/src/modules/SpaceCalculation.rb#L860
def CheckDiskSize; end
def CheckDiskSize; true; end

# @see https://github.com/yast/yast-packager/blob/master/src/modules/SpaceCalculation.rb#L894
def CheckDiskFreeSpace(*_args); []; end

# @see https://github.com/yast/yast-packager/blob/master/src/modules/SpaceCalculation.rb#L60
def GetFailedMounts
Expand Down
12 changes: 5 additions & 7 deletions service/lib/dinstaller/manager.rb
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,8 @@ def config_phase
installation_phase.config

storage.probe
software.probe

logger.info("Config phase done")
rescue StandardError => e
logger.error "Startup error: #{e.inspect}. Backtrace: #{e.backtrace}"
Expand All @@ -83,13 +85,9 @@ def config_phase
# rubocop:disable Metrics/AbcSize
def install_phase
installation_phase.install
start_progress(7)

start_progress(8)

progress.step("Reading software repositories") do
software.probe
Yast::Installation.destdir = "/mnt"
end
Yast::Installation.destdir = "/mnt"

progress.step("Partitioning") do
storage.install
Expand Down Expand Up @@ -179,7 +177,7 @@ def on_services_status_change(&block)
#
# @return [Boolean]
def valid?
[storage, users].all?(&:valid?)
[storage, users, software].all?(&:valid?)
end

private
Expand Down
60 changes: 56 additions & 4 deletions service/lib/dinstaller/software/manager.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
require "dinstaller/config"
require "dinstaller/helpers"
require "dinstaller/with_progress"
require "dinstaller/validation_error"
require "y2packager/product"
require "yast2/arch_filter"
require "dinstaller/software/callbacks"
Expand Down Expand Up @@ -58,6 +59,7 @@ class Manager

def initialize(config, logger)
@config = config
@probed = false
@logger = logger
@languages = DEFAULT_LANGUAGES
@products = @config.products
Expand All @@ -75,6 +77,7 @@ def select_product(name)

@config.pick_product(name)
@product = name
@probed = false # reset probing when product changed
end

def probe
Expand All @@ -95,6 +98,7 @@ def probe
logger.info "proposal #{proposal["raw_proposal"]}"
end

@probed = true
Yast::Stage.Set("initial")
end

Expand All @@ -114,10 +118,17 @@ def propose
proposal = Yast::Packages.Proposal(force_reset = false, reinit = false, _simple = true)
logger.info "proposal #{proposal["raw_proposal"]}"

solve_dependencies
deps_result = solve_dependencies

# do not return proposal hash, so intentional nil here
nil
proposal_result(proposal, deps_result)
end

def validate
# validation without probing does not make sense and produces false errors
return [] unless @probed

msgs = propose
msgs.map { |m| ValidationError.new(m) }
end

def install
Expand Down Expand Up @@ -166,6 +177,20 @@ def package_installed?(name)
on_target { Yast::Package.Installed(name, target: :system) }
end

# Counts how much disk space installation will use.
# @return [String]
# @note Reimplementation of Yast::Package.CountSizeToBeInstalled
def used_disk_space
return "" unless @probed

size = Yast::Pkg.PkgMediaSizes.reduce(0) do |res, media_size|
media_size.reduce(res, :+)
end

# FormatSizeWithPrecision(bytes, precision, omit_zeroes)
Yast::String.FormatSizeWithPrecision(size, 1, true)
end

private

# adds resolvables from yaml config for given product
Expand All @@ -190,11 +215,23 @@ def solve_dependencies
res = Yast::Pkg.PkgSolve(unused = true)
logger.info "solver run #{res.inspect}"

return if res
return true if res

logger.error "Solver failed: #{Yast::Pkg.LastError}"
logger.error "Details: #{Yast::Pkg.LastErrorDetails}"
logger.error "Solving issues: #{Yast::Pkg.PkgSolveErrors}"

false
end

# messages with reason why solver failed
def solve_messages
last_error = Yast::Pkg.LastError
solve_errors = Yast::Pkg.PkgSolveErrors
res = []
res << last_error unless last_error.empty?
res << "Found #{solve_errors} dependency issues." if solve_errors > 0
res
end

# @return [Logger]
Expand Down Expand Up @@ -259,6 +296,21 @@ def restore_original_repos
logger.info "moving #{REPOS_BACKUP} to #{REPOS_DIR}"
FileUtils.mv(REPOS_BACKUP, REPOS_DIR)
end

def proposal_result(proposal, deps_result)
result = []
# TODO: find if there is a better way to get proposal issue as list
result.concat(process_warnings(proposal)) if proposal["warning_level"] == :blocker
result.concat(solve_messages) unless deps_result

result
end

def process_warnings(proposal)
proposal["warning"]
.split("<br>")
.grep_v(/Please manually select .*/)
end
end
end
end
6 changes: 6 additions & 0 deletions service/package/rubygem-d-installer.changes
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
-------------------------------------------------------------------
Tue Jan 17 10:06:23 UT0 2023 - Josef Reidinger <jreidinger@suse.com>

- Implement validation of software proposal
(gh#yast/d-installer#381)

-------------------------------------------------------------------
Mon Jan 16 17:02:21 UTC 2023 - Imobach Gonzalez Sosa <igonzalezsosa@suse.com>

Expand Down
29 changes: 29 additions & 0 deletions service/test/dinstaller/dbus/software/manager_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
require "dinstaller/dbus/software/manager"
require "dinstaller/dbus/interfaces/progress"
require "dinstaller/dbus/interfaces/service_status"
require "dinstaller/dbus/interfaces/validation"
require "dinstaller/software"

describe DInstaller::DBus::Software::Manager do
Expand All @@ -38,6 +39,8 @@
DInstaller::DBus::Interfaces::ServiceStatus::SERVICE_STATUS_INTERFACE
end

let(:validation_interface) { DInstaller::DBus::Interfaces::Validation::VALIDATION_INTERFACE }

before do
allow_any_instance_of(described_class).to receive(:register_callbacks)
allow_any_instance_of(described_class).to receive(:register_progress_callbacks)
Expand All @@ -52,6 +55,10 @@
expect(subject.intfs.keys).to include(service_status_interface)
end

it "defines Validation D-Bus interface" do
expect(subject.intfs.keys).to include(validation_interface)
end

it "configures callbacks from Progress interface" do
expect_any_instance_of(described_class).to receive(:register_progress_callbacks)
subject
Expand All @@ -63,23 +70,45 @@
end

describe "#probe" do
before do
allow(subject).to receive(:update_validation)
allow(backend).to receive(:probe)
end

it "runs the probing, setting the service as busy meanwhile" do
expect(subject.service_status).to receive(:busy)
expect(backend).to receive(:probe)
expect(subject.service_status).to receive(:idle)

subject.probe
end

it "updates validation" do
expect(subject).to receive(:update_validation)

subject.probe
end
end

describe "#propose" do
before do
allow(subject).to receive(:update_validation)
allow(backend).to receive(:propose)
end

it "calculates the proposal, setting the service as busy meanwhile" do
expect(subject.service_status).to receive(:busy)
expect(backend).to receive(:propose)
expect(subject.service_status).to receive(:idle)

subject.propose
end

it "updates validation" do
expect(subject).to receive(:update_validation)

subject.propose
end
end

describe "#install" do
Expand Down

0 comments on commit 5d045aa

Please sign in to comment.