Skip to content

Remove empty strings from AutoYaST values#3302

Merged
imobachgs merged 3 commits intomasterfrom
clean-ay-profile
Mar 19, 2026
Merged

Remove empty strings from AutoYaST values#3302
imobachgs merged 3 commits intomasterfrom
clean-ay-profile

Conversation

@imobachgs
Copy link
Copy Markdown
Contributor

@imobachgs imobachgs commented Mar 18, 2026

It is really uncommon to use empty strings as values in AutoYaST. Those values are not a problem in AutoYaST, but they can cause problems in Agama. For instance, an empty element is ignored in AutoYaST, but not in Agama.

In order to make the migration from AutoYaST to Agama easier, the conversion removes empty string values. The exception is the section, which is handled by yast2-storage-ng. Actually, this section contains the <subvolumes_prefix />, which can be set to an empty string as described in the AutoYaST documentation.

@imobachgs imobachgs marked this pull request as ready for review March 19, 2026 07:03
Comment thread service/lib/agama/autoyast/profile_fetcher.rb Outdated
new_value = clean_value(value)
all[key] = new_value unless new_value.nil?
end
new_hash unless new_hash.empty?
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

if my internal parser works correctly this leads to filtering the key / value pair out in clean_profile so in general empty hashes are not converted. I'm fine with that but do not understand why the same doesn't happen for arrays (clean_array doesn't care about empty arrays)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Because an empty array can be used to remove some predefined (like a list of subvolumes).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

that is already covered by not doing filtering on storage section at all, isn't it? Any other example? I'm just curious.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

That's the point. I am not 100% sure if it happens in any other place. So I prefer to be conservative.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Having said that, we could change that, but for sure not now (it needs further testing and we are already late :-)).

#
# @param profile [Hash]
# @return [Hash]
def clean_profile(profile)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

NP: in theory there is place for de-duplication ... the profile is implemented as a Hash ... so clean_profile and clean_hash are kind of duplicates (I'm aware of that "partitioning" condition)

Copy link
Copy Markdown
Contributor

@mchf mchf Mar 19, 2026

Choose a reason for hiding this comment

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

I mean in theory clean_profile is something like:

clean_hash(profile.except("partitioning")).merge(profile["partitioning"])

or something like that

Copy link
Copy Markdown
Contributor Author

@imobachgs imobachgs Mar 19, 2026

Choose a reason for hiding this comment

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

Well, that code will crash (as profile["partitioning"] could be nil), but I get the idea. However, you still need some place to implement that logic. Not to mention that we would like to avoid hashes that turn into nil (because they are empty).

Copy link
Copy Markdown
Contributor

@joseivanlopez joseivanlopez left a comment

Choose a reason for hiding this comment

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

LGTM

@imobachgs imobachgs merged commit 2db95cd into master Mar 19, 2026
21 checks passed
@imobachgs imobachgs deleted the clean-ay-profile branch March 19, 2026 13:29
@imobachgs imobachgs mentioned this pull request Apr 14, 2026
imobachgs added a commit that referenced this pull request Apr 14, 2026
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.

3 participants