From 95bca984c649eea23b7a8f3ea76d6e0aeccff224 Mon Sep 17 00:00:00 2001 From: Stefan Hundhammer Date: Tue, 4 Jul 2023 10:34:24 +0200 Subject: [PATCH 1/7] Factored out required vs. optional used features --- src/lib/y2storage/devicegraph.rb | 17 +++++++++++++++++ test/y2storage/devicegraph_test.rb | 30 ++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+) diff --git a/src/lib/y2storage/devicegraph.rb b/src/lib/y2storage/devicegraph.rb index cc0fade22..5c6c006d3 100644 --- a/src/lib/y2storage/devicegraph.rb +++ b/src/lib/y2storage/devicegraph.rb @@ -615,6 +615,23 @@ def used_features(required_only: false) StorageFeaturesList.from_bitfield(storage_used_features(type)) end + # List of required (mandatory) storage features used by the devicegraph + # + # @return [StorageFeaturesList] + def required_used_features + used_features(required_only: true) + end + + # List of optional storage features used by the devicegraph + # + # @return [StorageFeaturesList] + def optional_used_features + all = storage_used_features(Storage::UsedFeaturesDependencyType_SUGGESTED) + required = storage_used_features(Storage::UsedFeaturesDependencyType_REQUIRED) + # Using binary XOR in those bit fields to calculate the difference + StorageFeaturesList.from_bitfield(all ^ required) + end + private # Copy of a device tree where hashes have been substituted by sorted diff --git a/test/y2storage/devicegraph_test.rb b/test/y2storage/devicegraph_test.rb index 2600a836b..2050ad9e0 100755 --- a/test/y2storage/devicegraph_test.rb +++ b/test/y2storage/devicegraph_test.rb @@ -1331,4 +1331,34 @@ def with_sda2_deleted(initial_graph) end end end + + describe "#required_used_features" do + before { fake_scenario(scenario) } + + context "with local devices combining several filesystem types" do + let(:scenario) { "mixed_disks" } + + it "returns only the features for mounted filesystems" do + features = fake_devicegraph.required_used_features + expect(features).to be_a Y2Storage::StorageFeaturesList + expect(features.map(&:id)) + .to contain_exactly(:UF_BTRFS, :UF_XFS, :UF_SWAP) + end + end + end + + describe "#optional_used_features" do + before { fake_scenario(scenario) } + + context "with local devices combining several filesystem types" do + let(:scenario) { "mixed_disks" } + + it "returns only the features for filesystems that are not mounted" do + features = fake_devicegraph.optional_used_features + expect(features).to be_a Y2Storage::StorageFeaturesList + expect(features.map(&:id)) + .to contain_exactly(:UF_EXT4, :UF_NTFS) + end + end + end end From 7d03f853afc67b4286b398c2e9f33ce273c9d28f Mon Sep 17 00:00:00 2001 From: Stefan Hundhammer Date: Tue, 4 Jul 2023 11:17:13 +0200 Subject: [PATCH 2/7] Factored out storage pkg proposal handling to PackageHandler --- .../y2storage/clients/inst_disk_proposal.rb | 18 +----------------- .../y2storage/clients/partitions_proposal.rb | 5 +++-- src/lib/y2storage/package_handler.rb | 15 +++++++++++++++ 3 files changed, 19 insertions(+), 19 deletions(-) diff --git a/src/lib/y2storage/clients/inst_disk_proposal.rb b/src/lib/y2storage/clients/inst_disk_proposal.rb index 606f5dd5d..b6f29de0f 100755 --- a/src/lib/y2storage/clients/inst_disk_proposal.rb +++ b/src/lib/y2storage/clients/inst_disk_proposal.rb @@ -162,23 +162,7 @@ def actions_after_partitioner(devicegraph, dialog_result) # Add storage-related software packages (filesystem tools etc.) to the # set of packages to be installed. def add_storage_packages - features = storage_manager.staging.used_features - required_features = storage_manager.staging.used_features(required_only: true) - - required_packages = required_features.pkg_list - optional_packages = features.pkg_list - required_packages - - set_proposal_packages(required_packages, false) - set_proposal_packages(optional_packages, true) - end - - # @see #add_storage_packages - # - # @param pkgs [Array] list of packages - # @param optional [Boolean] whether the packages in the list are optional - def set_proposal_packages(pkgs, optional) - pkg_handler = Y2Storage::PackageHandler.new(pkgs, optional: optional) - pkg_handler.set_proposal_packages + Y2Storage::PackageHandler.set_proposal_packages_for(storage_manager.staging) end # Save the list of filesystem to /etc/sysconfig/storage. diff --git a/src/lib/y2storage/clients/partitions_proposal.rb b/src/lib/y2storage/clients/partitions_proposal.rb index eadcc21d4..43a528e5b 100755 --- a/src/lib/y2storage/clients/partitions_proposal.rb +++ b/src/lib/y2storage/clients/partitions_proposal.rb @@ -120,8 +120,9 @@ def update_state staging = storage_manager.staging actiongraph = staging ? staging.actiongraph : nil self.actions_presenter = ActionsPresenter.new(actiongraph) - Y2Storage::DumpManager.dump(staging) - Y2Storage::DumpManager.dump(actions_presenter) + DumpManager.dump(staging) + DumpManager.dump(actions_presenter) + PackageHandler.set_proposal_packages_for(staging) end end diff --git a/src/lib/y2storage/package_handler.rb b/src/lib/y2storage/package_handler.rb index 2c59f5832..afb6157d4 100755 --- a/src/lib/y2storage/package_handler.rb +++ b/src/lib/y2storage/package_handler.rb @@ -116,6 +116,21 @@ def set_proposal_packages success end + # Add the proposal packages for storage that are needed for the specified + # devicegraph's used features. This marks the packages for installation; + # it does not install them yet. + # + # @param devicegraph [Devicegraph] usually StorageManager.instance.staging + # @param optional + def self.set_proposal_packages_for(devicegraph, optional: true) + required_packages = devicegraph.required_used_features.pkg_list + PackageHandler.new(required_packages, optional: false).set_proposal_packages + return unless optional + + optional_packages = devicegraph.optional_used_features.pkg_list + PackageHandler.new(optional_packages, optional: true).set_proposal_packages + end + private # Whether the packages should be considered as optional when adding them to the From c6769261eb4e9f07a4a70987f08003b99ff95964 Mon Sep 17 00:00:00 2001 From: Stefan Hundhammer Date: Tue, 4 Jul 2023 14:22:32 +0200 Subject: [PATCH 3/7] Extended unit test for fringe cases --- test/y2storage/devicegraph_test.rb | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/test/y2storage/devicegraph_test.rb b/test/y2storage/devicegraph_test.rb index 2050ad9e0..98aaeeaf3 100755 --- a/test/y2storage/devicegraph_test.rb +++ b/test/y2storage/devicegraph_test.rb @@ -1345,6 +1345,16 @@ def with_sda2_deleted(initial_graph) .to contain_exactly(:UF_BTRFS, :UF_XFS, :UF_SWAP) end end + + context "with an empty disk" do + let(:scenario) { "empty_disks" } + + it "Survives not having any storage features" do + features = fake_devicegraph.required_used_features + expect(features).to be_a Y2Storage::StorageFeaturesList + expect(features.map(&:id)).to be == [] + end + end end describe "#optional_used_features" do @@ -1360,5 +1370,15 @@ def with_sda2_deleted(initial_graph) .to contain_exactly(:UF_EXT4, :UF_NTFS) end end + + context "with an empty disk" do + let(:scenario) { "empty_disks" } + + it "Survives not having any storage features" do + features = fake_devicegraph.optional_used_features + expect(features).to be_a Y2Storage::StorageFeaturesList + expect(features.map(&:id)).to be == [] + end + end end end From 0247f7e4be4eac609513c2ffa253833de24cbfe4 Mon Sep 17 00:00:00 2001 From: Stefan Hundhammer Date: Tue, 4 Jul 2023 14:43:09 +0200 Subject: [PATCH 4/7] Added unit test for new PackageHandler.set_proposal_packages_for method --- test/y2storage/package_handler_test.rb | 42 ++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/test/y2storage/package_handler_test.rb b/test/y2storage/package_handler_test.rb index dc7bb7bb0..a2efb1ba2 100755 --- a/test/y2storage/package_handler_test.rb +++ b/test/y2storage/package_handler_test.rb @@ -135,4 +135,46 @@ end end end + + describe "#set_proposal_packages_for" do + before do + fake_scenario(scenario) + allow(Yast::Mode).to receive(:installation).and_return(installation) + allow(Yast::Package).to receive(:Installed).and_return(false) + allow(Yast::Package).to receive(:Available).and_return(true) + end + + context "with local devices combining several filesystem types" do + PROPOSAL_ID = "storage_proposal" + let(:scenario) { "mixed_disks" } + let(:installation) { true } + + it "Adds the correct required and optional storage packages to the proposal" do + expect(Yast::PackagesProposal).to receive(:SetResolvables) + .with(PROPOSAL_ID, :package, ["btrfsprogs", "e2fsprogs", "xfsprogs"], optional: false) + .and_return true + expect(Yast::PackagesProposal).to receive(:SetResolvables) + .with(PROPOSAL_ID, :package, ["e2fsprogs", "ntfs-3g", "ntfsprogs"], optional: true) + .and_return true + described_class.set_proposal_packages_for(fake_devicegraph) + end + + it "Adds only required storage packages to the proposal if 'optional' is 'false" do + expect(Yast::PackagesProposal).to receive(:SetResolvables) + .with(PROPOSAL_ID, :package, ["btrfsprogs", "e2fsprogs", "xfsprogs"], optional: false) + .and_return true + described_class.set_proposal_packages_for(fake_devicegraph, optional: false) + end + end + + context "with empty disks" do + let(:scenario) { "empty_disks" } + let(:installation) { true } + + it "Does not add any storage packages to the proposal" do + expect(Yast::PackagesProposal).not_to receive(:SetResolvables) + described_class.set_proposal_packages_for(fake_devicegraph, optional: true) + end + end + end end From 1a8c57249d02085a8cf70a4e20dc031e06d0345d Mon Sep 17 00:00:00 2001 From: Stefan Hundhammer Date: Wed, 5 Jul 2023 15:47:00 +0200 Subject: [PATCH 5/7] Version bump and change log --- package/yast2-storage-ng.changes | 9 +++++++++ package/yast2-storage-ng.spec | 2 +- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/package/yast2-storage-ng.changes b/package/yast2-storage-ng.changes index 255f14bae..fc4a0b1b2 100644 --- a/package/yast2-storage-ng.changes +++ b/package/yast2-storage-ng.changes @@ -1,3 +1,12 @@ +------------------------------------------------------------------- +Wed Jul 5 13:42:12 UTC 2023 - Stefan Hundhammer + +- Ensure adding storage support software packages for MicroOS + which uses its custom partitions_proposal client, not the + standard inst_disk_proposal client (bsc#1212452) + https://github.com/yast/yast-storage-ng/pull/1346 +- 4.4.45 + ------------------------------------------------------------------- Thu Jun 29 11:42:21 UTC 2023 - Ancor Gonzalez Sosa diff --git a/package/yast2-storage-ng.spec b/package/yast2-storage-ng.spec index 2ba93bca6..8323ba9fe 100644 --- a/package/yast2-storage-ng.spec +++ b/package/yast2-storage-ng.spec @@ -16,7 +16,7 @@ # Name: yast2-storage-ng -Version: 4.4.44 +Version: 4.4.45 Release: 0 Summary: YaST2 - Storage Configuration License: GPL-2.0-only OR GPL-3.0-only From f2e67b5fd14b1c1bbc6683eab3fd1f96cb2b6a6e Mon Sep 17 00:00:00 2001 From: Stefan Hundhammer Date: Mon, 10 Jul 2023 10:40:02 +0200 Subject: [PATCH 6/7] Use class method notation for class method unit tests --- test/y2storage/package_handler_test.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/y2storage/package_handler_test.rb b/test/y2storage/package_handler_test.rb index a2efb1ba2..48cc36582 100755 --- a/test/y2storage/package_handler_test.rb +++ b/test/y2storage/package_handler_test.rb @@ -136,7 +136,7 @@ end end - describe "#set_proposal_packages_for" do + describe ".set_proposal_packages_for" do before do fake_scenario(scenario) allow(Yast::Mode).to receive(:installation).and_return(installation) From fcc76021e62e3d11040930b5a4cf68ecade02ccd Mon Sep 17 00:00:00 2001 From: Stefan Hundhammer Date: Mon, 10 Jul 2023 13:04:00 +0200 Subject: [PATCH 7/7] Drop cache for reproducable results for parallel unit tests --- src/lib/y2storage/storage_feature.rb | 8 ++++++++ test/y2storage/package_handler_test.rb | 2 ++ test/y2storage/storage_features_list_test.rb | 2 ++ 3 files changed, 12 insertions(+) diff --git a/src/lib/y2storage/storage_feature.rb b/src/lib/y2storage/storage_feature.rb index 55cd999e9..ca80de157 100755 --- a/src/lib/y2storage/storage_feature.rb +++ b/src/lib/y2storage/storage_feature.rb @@ -124,6 +124,14 @@ def self.all end end + # Drop the cache of storage packages that are available. + # + # This is only ever needed if the available packages might have changed + # since the last use of this class. + def self.drop_cache + @all = nil + end + # Constructor # # This looks up a constant in the ::Storage namespace to make sure the id diff --git a/test/y2storage/package_handler_test.rb b/test/y2storage/package_handler_test.rb index 48cc36582..92eb4aa78 100755 --- a/test/y2storage/package_handler_test.rb +++ b/test/y2storage/package_handler_test.rb @@ -23,6 +23,7 @@ require_relative "spec_helper" require "y2storage/package_handler" +require "y2storage/storage_feature" describe Y2Storage::PackageHandler do let(:feature_pkg) { ["lvm2", "btrfsprogs", "e2fsprogs"] } @@ -139,6 +140,7 @@ describe ".set_proposal_packages_for" do before do fake_scenario(scenario) + Y2Storage::StorageFeature.drop_cache allow(Yast::Mode).to receive(:installation).and_return(installation) allow(Yast::Package).to receive(:Installed).and_return(false) allow(Yast::Package).to receive(:Available).and_return(true) diff --git a/test/y2storage/storage_features_list_test.rb b/test/y2storage/storage_features_list_test.rb index 8f53fd49b..ffa61b556 100755 --- a/test/y2storage/storage_features_list_test.rb +++ b/test/y2storage/storage_features_list_test.rb @@ -22,6 +22,7 @@ # find current contact information at www.suse.com. require_relative "spec_helper" +require "y2storage/storage_feature" require "y2storage/storage_features_list" describe Y2Storage::StorageFeaturesList do @@ -84,6 +85,7 @@ let(:bits) { Storage::UF_NTFS | Storage::UF_EXT3 } before do + Y2Storage::StorageFeature.drop_cache allow(Yast::Package).to receive(:Available).and_return false allow(Yast::Package).to receive(:Available).with("ntfsprogs").and_return true end