From 31155bdf1bfbb7f4602d414aff77854f6180d4a6 Mon Sep 17 00:00:00 2001 From: Stefan Hundhammer Date: Tue, 4 Jul 2023 10:34:24 +0200 Subject: [PATCH 1/4] 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 0d0c89379312e5666473b85710d85ad27f909492 Mon Sep 17 00:00:00 2001 From: Stefan Hundhammer Date: Tue, 4 Jul 2023 11:17:13 +0200 Subject: [PATCH 2/4] 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 fb05ebe4ef690ff3acb9db674d21ab956021e252 Mon Sep 17 00:00:00 2001 From: Stefan Hundhammer Date: Tue, 4 Jul 2023 14:22:32 +0200 Subject: [PATCH 3/4] 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 fa5b7f37a0e9bd1b9b903166ff07adb4f9b94546 Mon Sep 17 00:00:00 2001 From: Stefan Hundhammer Date: Tue, 4 Jul 2023 14:43:09 +0200 Subject: [PATCH 4/4] 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