Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Build Failure: New libstorage-ng enum #1367

Merged
merged 2 commits into from Feb 12, 2024
Merged

Conversation

shundhammer
Copy link
Contributor

@shundhammer shundhammer commented Feb 12, 2024

Bugzilla

https://bugzilla.suse.com/show_bug.cgi?id=1219804

Problem

Build failure with latest libstorage-ng:

13) Y2Storage::StorageFeature.all contains one entry for each feature from libstorage-ng
    Failure/Error: expect(described_class.all.map(&:to_sym)).to contain_exactly(*constants)

      expected collection contained:  [:UF_BCACHE, :UF_BCACHEFS, :UF_BITLOCKER, :UF_BTRFS, :UF_DASD, :UF_DMRAID, :UF_EXFAT, :UF_EXT2, :UF_E..._ENCRYPTION, :UF_PMEM, :UF_QUOTA, :UF_REISERFS, :UF_SNAPSHOTS, :UF_SWAP, :UF_UDF, :UF_VFAT, :UF_XFS]
      actual collection contained:    [:UF_BCACHE, :UF_BITLOCKER, :UF_BTRFS, :UF_DASD, :UF_DMRAID, :UF_EXFAT, :UF_EXT2, :UF_EXT3, :UF_EXT4,..._ENCRYPTION, :UF_PMEM, :UF_QUOTA, :UF_REISERFS, :UF_SNAPSHOTS, :UF_SWAP, :UF_UDF, :UF_VFAT, :UF_XFS]
      the missing elements were:      [:UF_BCACHEFS]
# ./test/y2storage/storage_feature_test.rb:33:in `block (3 levels) in <top (required)>'

Cause

libstorage-ng now has support for bcachefs, including a new enum value BCACHEFS.

We have a unit test to check if all values of this C++ libstorage-ng enum for storage features are covered on the Ruby side in yast-storage-ng in this map:

https://github.com/yast/yast-storage-ng/blob/master/src/lib/y2storage/storage_feature.rb#L55-L102

The map contains support packages for each storage feature; packages that need to be installed when the feature is used.

Fix

Added a map entry for this new enum value.

What Support Packages are Needed?

Right now none since there is no support for bcachefs in yast-storage-ng for the time being. libstorage may detect it during storage probing, but we will not propose it, and the expert partitioner does not support adding it.

Future Support Packages

There is package bcachefs-tools which will be needed when this is implemented on the Ruby side (in yast-storage-ng), too.

https://build.opensuse.org/package/show/openSUSE:Factory/bcache-tools

Add this Immediately?

No, this might lead to unrecoverable situations if a bcachefs is detected during storage probing, so that package would be requested from the libzypp pool, but it might not be available for the current product.

We would have to add it to OPTIONAL_PACKAGES as well for the time being, but it might be left over there when the feature is implemented, leading to more obscure problems.

https://github.com/yast/yast-storage-ng/blob/master/src/lib/y2storage/storage_feature.rb#L112

For now, it's just in a comment as a reminder to add it in the future.

Related PR

Backport to SLE-15-SP6: PR #1371

@coveralls
Copy link

Coverage Status

coverage: 97.797% (+0.02%) from 97.782%
when pulling 6794fab on huha-enum-bcachefs
into 50263a9 on master.

Copy link
Member

@aschnell aschnell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@shundhammer shundhammer merged commit b700ab0 into master Feb 12, 2024
13 checks passed
@shundhammer shundhammer deleted the huha-enum-bcachefs branch February 12, 2024 15:57
@shundhammer
Copy link
Contributor Author

@ancorgs FYI

I also added a Trello card as a reminder: https://trello.com/c/PT0yG61d

@yast-bot
Copy link

✔️ Internal Jenkins job #1145 successfully finished
✔️ Created OBS submit request #1146228

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants