From 38224a67d9d87f92e8455743aa2d0599bba78c79 Mon Sep 17 00:00:00 2001 From: Martin Vidner Date: Fri, 24 Mar 2017 10:16:37 +0100 Subject: [PATCH 1/8] Better coverage tracking with SimpleCov.track_files --- test/test_helper.rb | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/test/test_helper.rb b/test/test_helper.rb index a57fc8266..ff0305d1e 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -20,15 +20,8 @@ add_filter "/test/" end - # For coverage we need to load all ruby files - # Note that clients/ are excluded because they run too eagerly by design - fs = Dir["#{srcdir}/**/*.rb"] - fs.delete_if { |f| f.start_with? "#{srcdir}/clients/" } - # HACK: this would BuildRequire yast2-slp so defer this - fs.delete_if { |f| f.end_with? "/SourceManagerSLP.rb" } - fs.each do |f| - require_relative f - end + # track all ruby files under src + SimpleCov.track_files("#{srcdir}/**/*.rb") # use coveralls for on-line code coverage reporting at Travis CI if ENV["TRAVIS"] From 6a6b1764feabec3c102f7df1f05ae231880495c5 Mon Sep 17 00:00:00 2001 From: Martin Vidner Date: Fri, 24 Mar 2017 11:01:59 +0100 Subject: [PATCH 2/8] Based Yast::SoftwareProposalClient on ::Installation::ProposalClient --- src/Makefile.am | 3 +- src/clients/software_proposal.rb | 159 +----------------- src/lib/packager/clients/software_proposal.rb | 156 +++++++++++++++++ 3 files changed, 160 insertions(+), 158 deletions(-) create mode 100644 src/lib/packager/clients/software_proposal.rb diff --git a/src/Makefile.am b/src/Makefile.am index 7ee695a31..1c7aac179 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -72,7 +72,8 @@ desktop_DATA = \ ylibclientdir = "${yast2dir}/lib/packager/clients" ylibclient_DATA = \ - lib/packager/clients/pkg_finish.rb + lib/packager/clients/pkg_finish.rb \ + lib/packager/clients/software_proposal.rb ylibcfadir = "${yast2dir}/lib/packager/cfa" ylibcfa_DATA = \ diff --git a/src/clients/software_proposal.rb b/src/clients/software_proposal.rb index c38273a3f..cdf3aad82 100644 --- a/src/clients/software_proposal.rb +++ b/src/clients/software_proposal.rb @@ -1,157 +1,2 @@ -# encoding: utf-8 - -# Module: software_proposal.ycp -# -# Author: Klaus Kaempf -# -# Purpose: Proposal function dispatcher - software. -# -# See also file proposal-API.txt for details. -# -# $Id$ -# -module Yast - class SoftwareProposalClient < Client - def main - Yast.import "Pkg" - textdomain "packager" - - Yast.import "Packages" - Yast.import "Language" - Yast.import "Installation" - - @func = Convert.to_string(WFM.Args(0)) - @param = Convert.to_map(WFM.Args(1)) - @ret = {} - - if @func == "MakeProposal" - @force_reset = Ops.get_boolean(@param, "force_reset", false) - @language_changed = Ops.get_boolean(@param, "language_changed", false) - - @reinit = false - @partition_changed = false - - if Installation.dirinstall_installing_into_dir - # check the target directory in dirinstall mode - if Packages.timestamp != Installation.dirinstall_target_time - @partition_changed = true - end - # save information about target change time in module Packages - Packages.timestamp = Installation.dirinstall_target_time - else - @storage_timestamp = Convert.to_integer( - WFM.call("wrapper_storage", ["GetTargetChangeTime"]) - ) - - # check the partitioning in installation - if Packages.timestamp != @storage_timestamp - # don't set flag partition_changed if it's the first "change" - @partition_changed = true if Packages.timestamp != 0 - end - # save information about target change time in module Packages - Packages.timestamp = @storage_timestamp - end - - if Pkg.GetPackageLocale != Language.language - @language_changed = true - Pkg.SetPackageLocale(Language.language) - end - if !Builtins.contains(Pkg.GetAdditionalLocales, Language.language) - # FIXME this is temporary fix - # language_changed = true; - Pkg.SetAdditionalLocales( - Builtins.add(Pkg.GetAdditionalLocales, Language.language) - ) - end - - # if only partitioning has been changed just return the current state, - # don't reset to default (bnc#450786, bnc#371875) - if @partition_changed && !@language_changed && !@force_reset && !Packages.PackagesProposalChanged - return Packages.Summary([ :product, :pattern, :selection, :size, :desktop ], false); - end - - @reinit = true if @language_changed - Builtins.y2milestone( - "package proposal: force reset: %1, reinit: %2, language changed: %3", - @force_reset, - @reinit, - @language_changed - ) - @ret = Packages.Proposal( - @force_reset, # user decision: reset to default - @reinit, # reinitialize due to language or partition change - false - ) # simple version - - if @language_changed && !@force_reset - # if the language has changed the software proposal is reset to the default settings - if !Builtins.haskey(@ret, "warning") - # the language_changed flag has NOT been set by the NLD frame - @ret = Builtins.add( - @ret, - "warning", - _("The software proposal is reset to the default values.") - ) - end - end - if Ops.greater_than(Packages.solve_errors, 0) - # the proposal for the packages requires manual intervention - @ret = Builtins.union( - @ret, - { - # warning text - "warning" => _( - "Cannot solve dependencies automatically. Manual intervention is required." - ), - "warning_level" => :blocker - } - ) - end - elsif @func == "AskUser" - @has_next = Ops.get_boolean(@param, "has_next", false) - - # call some function that displays a user dialog - # or a sequence of dialogs here: - # - # sequence = DummyMod::AskUser( has_next ); - - @chosen_id = Ops.get(@param, "chosen_id") - if @chosen_id == "mediacheck" - @result = Convert.to_symbol(WFM.CallFunction("checkmedia", WFM.Args)) - @ret = { "workflow_sequence" => @result } - else - @result = :again - @client_to_call = "inst_sw_select" - - while @result == :again - @result = Convert.to_symbol( - WFM.CallFunction(@client_to_call, [true, true]) - ) - end - - # Fill return map - - @ret = { "workflow_sequence" => @result } - end - elsif @func == "Description" - # disable proposal if doing image-only installation - return nil if Installation.image_only - # Fill return map. - # - # Static values do just nicely here, no need to call a function. - - @ret = { - # this is a heading - "rich_text_title" => _("Software"), - # this is a menu entry - "menu_title" => _("&Software"), - "id" => "software_stuff" - } - end - - deep_copy(@ret) - end - end -end - -Yast::SoftwareProposalClient.new.main +require "packager/clients/software_proposal" +Yast::SoftwareProposalClient.new.run diff --git a/src/lib/packager/clients/software_proposal.rb b/src/lib/packager/clients/software_proposal.rb new file mode 100644 index 000000000..15176d5a5 --- /dev/null +++ b/src/lib/packager/clients/software_proposal.rb @@ -0,0 +1,156 @@ +# encoding: utf-8 + +# ------------------------------------------------------------------------------ +# Copyright (c) 2017 SUSE LLC +# +# +# This program is free software; you can redistribute it and/or modify it under +# the terms of version 2 of the GNU General Public License as published by the +# Free Software Foundation. +# +# This program is distributed in the hope that it will be useful, but WITHOUT +# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS +# FOR A PARTICULAR PURPOSE. See the GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License along with +# this program; if not, contact SUSE. +# +# To contact SUSE about this file by physical or electronic mail, you may find +# current contact information at www.suse.com. +# ------------------------------------------------------------------------------ + +require "installation/proposal_client" + +module Yast + class SoftwareProposalClient < ::Installation::ProposalClient + def initialize + Yast.import "Pkg" + textdomain "packager" + + Yast.import "Packages" + Yast.import "Language" + Yast.import "Installation" + end + + def make_proposal(flags) + @force_reset = flags.fetch("force_reset", false) + @language_changed = flags.fetch("language_changed", false) + + @reinit = false + @partition_changed = false + + if Installation.dirinstall_installing_into_dir + # check the target directory in dirinstall mode + if Packages.timestamp != Installation.dirinstall_target_time + @partition_changed = true + end + # save information about target change time in module Packages + Packages.timestamp = Installation.dirinstall_target_time + else + @storage_timestamp = Convert.to_integer( + WFM.call("wrapper_storage", ["GetTargetChangeTime"]) + ) + + # check the partitioning in installation + if Packages.timestamp != @storage_timestamp + # don't set flag partition_changed if it's the first "change" + @partition_changed = true if Packages.timestamp != 0 + end + # save information about target change time in module Packages + Packages.timestamp = @storage_timestamp + end + + if Pkg.GetPackageLocale != Language.language + @language_changed = true + Pkg.SetPackageLocale(Language.language) + end + if !Builtins.contains(Pkg.GetAdditionalLocales, Language.language) + # FIXME this is temporary fix + # language_changed = true; + Pkg.SetAdditionalLocales( + Builtins.add(Pkg.GetAdditionalLocales, Language.language) + ) + end + + # if only partitioning has been changed just return the current state, + # don't reset to default (bnc#450786, bnc#371875) + if @partition_changed && !@language_changed && !@force_reset && !Packages.PackagesProposalChanged + return Packages.Summary([ :product, :pattern, :selection, :size, :desktop ], false); + end + + @reinit = true if @language_changed + Builtins.y2milestone( + "package proposal: force reset: %1, reinit: %2, language changed: %3", + @force_reset, + @reinit, + @language_changed + ) + @ret = Packages.Proposal( + @force_reset, # user decision: reset to default + @reinit, # reinitialize due to language or partition change + false + ) # simple version + + if @language_changed && !@force_reset + # if the language has changed the software proposal is reset to the default settings + if !Builtins.haskey(@ret, "warning") + # the language_changed flag has NOT been set by the NLD frame + @ret = Builtins.add( + @ret, + "warning", + _("The software proposal is reset to the default values.") + ) + end + end + if Ops.greater_than(Packages.solve_errors, 0) + # the proposal for the packages requires manual intervention + @ret = Builtins.union( + @ret, + { + # warning text + "warning" => _( + "Cannot solve dependencies automatically. Manual intervention is required." + ), + "warning_level" => :blocker + } + ) + end + @ret + end + + def ask_user(params) + chosen_id = params["chosen_id"] + if chosen_id == "mediacheck" + @result = Convert.to_symbol(WFM.CallFunction("checkmedia", WFM.Args)) + @ret = { "workflow_sequence" => @result } + else + @result = :again + @client_to_call = "inst_sw_select" + + while @result == :again + @result = Convert.to_symbol( + WFM.CallFunction(@client_to_call, [true, true]) + ) + end + + # Fill return map + + @ret = { "workflow_sequence" => @result } + end + @ret + end + + def description + # disable proposal if doing image-only installation + return nil if Installation.image_only + + { + # this is a heading + "rich_text_title" => _("Software"), + # this is a menu entry + "menu_title" => _("&Software"), + "id" => "software_stuff" + } + end + end +end From 1196ee70eeddd6d3c186f2ffde0bc3152f9dd8c4 Mon Sep 17 00:00:00 2001 From: Martin Vidner Date: Fri, 24 Mar 2017 11:10:37 +0100 Subject: [PATCH 3/8] A trivial test for Yast::SoftwareProposalClient --- test/Makefile.am | 3 ++- test/lib/clients/software_proposal_test.rb | 10 ++++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) create mode 100644 test/lib/clients/software_proposal_test.rb diff --git a/test/Makefile.am b/test/Makefile.am index a3c9c5367..eb5de4aa5 100644 --- a/test/Makefile.am +++ b/test/Makefile.am @@ -9,7 +9,8 @@ TESTS = \ product_patterns_test.rb \ source_dialogs_test.rb \ space_calculation_test.rb \ - lib/cfa/zypp_conf_test.rb + lib/cfa/zypp_conf_test.rb \ + lib/clients/software_proposal_test.rb TEST_EXTENSIONS = .rb RB_LOG_COMPILER = rspec diff --git a/test/lib/clients/software_proposal_test.rb b/test/lib/clients/software_proposal_test.rb new file mode 100644 index 000000000..4c32c85b0 --- /dev/null +++ b/test/lib/clients/software_proposal_test.rb @@ -0,0 +1,10 @@ +#!/usr/bin/env rspec + +require_relative "../../test_helper" +require "packager/clients/software_proposal" + +describe Yast::SoftwareProposalClient do + it "can be constructed" do + expect { subject }.to_not raise_error + end +end From bf2eaf6029f5b8db11f6551983c79c4ca158df29 Mon Sep 17 00:00:00 2001 From: Martin Vidner Date: Fri, 24 Mar 2017 13:23:23 +0100 Subject: [PATCH 4/8] Test #ask_user #description --- test/lib/clients/software_proposal_test.rb | 35 ++++++++++++++++++++-- 1 file changed, 33 insertions(+), 2 deletions(-) diff --git a/test/lib/clients/software_proposal_test.rb b/test/lib/clients/software_proposal_test.rb index 4c32c85b0..46a75c228 100644 --- a/test/lib/clients/software_proposal_test.rb +++ b/test/lib/clients/software_proposal_test.rb @@ -3,8 +3,39 @@ require_relative "../../test_helper" require "packager/clients/software_proposal" +RSpec.shared_examples "Installation::ProposalClient" do + describe "#description" do + it "contains 3 string keys (or is nil or {})" do + d = subject.description + next if d.nil? + expect(d).to be_a Hash + expect(d["id"]).to be_a String + expect(d["menu_title"]).to be_a String + expect(d["rich_text_title"]).to be_a String + end + end + + describe "#ask_user" do + it "returns a Hash with workflow_sequence" do + r = subject.ask_user({}) + expect(r).to be_a Hash + expect(r["workflow_sequence"]).to be_a Symbol + end + end +end + describe Yast::SoftwareProposalClient do - it "can be constructed" do - expect { subject }.to_not raise_error + before do + allow(Yast::WFM).to receive(:CallFunction).and_return(:next) + end + + include_examples "Installation::ProposalClient" + + describe "#ask_user(mediacheck)" do + it "returns a Hash with workflow_sequence" do + r = subject.ask_user({"chosen_id" => "mediacheck"}) + expect(r).to be_a Hash + expect(r["workflow_sequence"]).to be_a Symbol + end end end From 42ac527e003d65968b938c4f73cb5a3072d672d8 Mon Sep 17 00:00:00 2001 From: Martin Vidner Date: Fri, 24 Mar 2017 13:51:42 +0100 Subject: [PATCH 5/8] Factored out #partitioning_changed? #adjust_locales --- src/lib/packager/clients/software_proposal.rb | 89 +++++++++++-------- 1 file changed, 51 insertions(+), 38 deletions(-) diff --git a/src/lib/packager/clients/software_proposal.rb b/src/lib/packager/clients/software_proposal.rb index 15176d5a5..6b20d6b13 100644 --- a/src/lib/packager/clients/software_proposal.rb +++ b/src/lib/packager/clients/software_proposal.rb @@ -34,51 +34,17 @@ def initialize def make_proposal(flags) @force_reset = flags.fetch("force_reset", false) - @language_changed = flags.fetch("language_changed", false) - @reinit = false - @partition_changed = false - - if Installation.dirinstall_installing_into_dir - # check the target directory in dirinstall mode - if Packages.timestamp != Installation.dirinstall_target_time - @partition_changed = true - end - # save information about target change time in module Packages - Packages.timestamp = Installation.dirinstall_target_time - else - @storage_timestamp = Convert.to_integer( - WFM.call("wrapper_storage", ["GetTargetChangeTime"]) - ) - - # check the partitioning in installation - if Packages.timestamp != @storage_timestamp - # don't set flag partition_changed if it's the first "change" - @partition_changed = true if Packages.timestamp != 0 - end - # save information about target change time in module Packages - Packages.timestamp = @storage_timestamp - end - - if Pkg.GetPackageLocale != Language.language - @language_changed = true - Pkg.SetPackageLocale(Language.language) - end - if !Builtins.contains(Pkg.GetAdditionalLocales, Language.language) - # FIXME this is temporary fix - # language_changed = true; - Pkg.SetAdditionalLocales( - Builtins.add(Pkg.GetAdditionalLocales, Language.language) - ) - end + @language_changed = adjust_locales + @language_changed ||= flags.fetch("language_changed", false) # if only partitioning has been changed just return the current state, # don't reset to default (bnc#450786, bnc#371875) - if @partition_changed && !@language_changed && !@force_reset && !Packages.PackagesProposalChanged + if partitioning_changed? && !@language_changed && !@force_reset && !Packages.PackagesProposalChanged return Packages.Summary([ :product, :pattern, :selection, :size, :desktop ], false); end - @reinit = true if @language_changed + @reinit = @language_changed Builtins.y2milestone( "package proposal: force reset: %1, reinit: %2, language changed: %3", @force_reset, @@ -152,5 +118,52 @@ def description "id" => "software_stuff" } end + + private + + def partitioning_changed? + changed = false + + if Installation.dirinstall_installing_into_dir + # check the target directory in dirinstall mode + if Packages.timestamp != Installation.dirinstall_target_time + changed = true + end + # save information about target change time in module Packages + Packages.timestamp = Installation.dirinstall_target_time + else + storage_timestamp = Convert.to_integer( + WFM.call("wrapper_storage", ["GetTargetChangeTime"]) + ) + + # check the partitioning in installation + if Packages.timestamp != storage_timestamp + # don't set changed if it's the first "change" + changed = true if Packages.timestamp != 0 + end + # save information about target change time in module Packages + Packages.timestamp = storage_timestamp + end + + changed + end + + # Adjust package locales + # @return [Boolean] has the language changed + def adjust_locales + language_changed = false + if Pkg.GetPackageLocale != Language.language + language_changed = true + Pkg.SetPackageLocale(Language.language) + end + if !Builtins.contains(Pkg.GetAdditionalLocales, Language.language) + # FIXME this is temporary fix + # language_changed = true; + Pkg.SetAdditionalLocales( + Builtins.add(Pkg.GetAdditionalLocales, Language.language) + ) + end + language_changed + end end end From 200032e25f794cdb1ed0df4d078b531a8b6c0d06 Mon Sep 17 00:00:00 2001 From: Martin Vidner Date: Fri, 24 Mar 2017 14:15:41 +0100 Subject: [PATCH 6/8] Do not ignore solver problems after touching the partitioning (bsc#1029306) --- src/lib/packager/clients/software_proposal.rb | 28 +++++++++---------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/src/lib/packager/clients/software_proposal.rb b/src/lib/packager/clients/software_proposal.rb index 6b20d6b13..ca5f5c5a7 100644 --- a/src/lib/packager/clients/software_proposal.rb +++ b/src/lib/packager/clients/software_proposal.rb @@ -41,22 +41,22 @@ def make_proposal(flags) # if only partitioning has been changed just return the current state, # don't reset to default (bnc#450786, bnc#371875) if partitioning_changed? && !@language_changed && !@force_reset && !Packages.PackagesProposalChanged - return Packages.Summary([ :product, :pattern, :selection, :size, :desktop ], false); + @ret = Packages.Summary([ :product, :pattern, :selection, :size, :desktop ], false); + else + @reinit = @language_changed + Builtins.y2milestone( + "package proposal: force reset: %1, reinit: %2, language changed: %3", + @force_reset, + @reinit, + @language_changed + ) + @ret = Packages.Proposal( + @force_reset, # user decision: reset to default + @reinit, # reinitialize due to language or partition change + false + ) # simple version end - @reinit = @language_changed - Builtins.y2milestone( - "package proposal: force reset: %1, reinit: %2, language changed: %3", - @force_reset, - @reinit, - @language_changed - ) - @ret = Packages.Proposal( - @force_reset, # user decision: reset to default - @reinit, # reinitialize due to language or partition change - false - ) # simple version - if @language_changed && !@force_reset # if the language has changed the software proposal is reset to the default settings if !Builtins.haskey(@ret, "warning") From 18741c21d47f109dfc41d25db55889bb2cf8e946 Mon Sep 17 00:00:00 2001 From: Martin Vidner Date: Fri, 24 Mar 2017 14:50:09 +0100 Subject: [PATCH 7/8] Test #make_proposal --- test/lib/clients/software_proposal_test.rb | 24 ++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/test/lib/clients/software_proposal_test.rb b/test/lib/clients/software_proposal_test.rb index 46a75c228..b9c550b3a 100644 --- a/test/lib/clients/software_proposal_test.rb +++ b/test/lib/clients/software_proposal_test.rb @@ -38,4 +38,28 @@ expect(r["workflow_sequence"]).to be_a Symbol end end + + describe "#make_proposal" do + before do + allow(Yast::Packages).to receive(:PackagesProposalChanged).and_return false + end + + it "reports solver problems if partitioning unchanged" do + expect(subject).to receive(:adjust_locales).and_return true + expect(subject).to receive(:partitioning_changed?).and_return false + expect(Yast::Packages).to receive(:Proposal).and_return({foo: :bar}) + expect(Yast::Packages).to receive(:solve_errors).and_return(1) + + expect(subject.make_proposal({})).to include("warning_level" => :blocker) + end + + it "reports solver problems if partitioning changed" do + expect(subject).to receive(:adjust_locales).and_return false + expect(subject).to receive(:partitioning_changed?).and_return true + expect(Yast::Packages).to receive(:Summary).and_return({foo: :bar}) + expect(Yast::Packages).to receive(:solve_errors).and_return(1) + + expect(subject.make_proposal({})).to include("warning_level" => :blocker) + end + end end From 1c633dbe4f8e0fba5fe54f9c1a8ae0f4cca214b3 Mon Sep 17 00:00:00 2001 From: Martin Vidner Date: Fri, 24 Mar 2017 14:54:38 +0100 Subject: [PATCH 8/8] version + changelog --- package/yast2-packager.changes | 7 +++++++ package/yast2-packager.spec | 2 +- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/package/yast2-packager.changes b/package/yast2-packager.changes index 57198cbb7..b71619a40 100644 --- a/package/yast2-packager.changes +++ b/package/yast2-packager.changes @@ -1,3 +1,10 @@ +------------------------------------------------------------------- +Fri Mar 24 13:53:40 UTC 2017 - mvidner@suse.com + +- Do not ignore solver problems after touching the partitioning + (bsc#1029306) +- 3.1.121.3 + ------------------------------------------------------------------- Mon Mar 20 12:33:35 UTC 2017 - jreidinger@suse.com diff --git a/package/yast2-packager.spec b/package/yast2-packager.spec index de6d913be..ef1cf8f45 100644 --- a/package/yast2-packager.spec +++ b/package/yast2-packager.spec @@ -17,7 +17,7 @@ Name: yast2-packager -Version: 3.1.121.2 +Version: 3.1.121.3 Release: 0 BuildRoot: %{_tmppath}/%{name}-%{version}-build