search

openSUSE / openSUSE-release-tools

Tools to aid in staging and release work for openSUSE/SUSE
GNU General Public License v2.0
61 stars 95 forks source link

Fix handling of preinstall images #2935

Closed dirkmueller closed 1 year ago

dirkmueller commented 1 year ago

These have special name handling in _preinstallimage. The package name has to be "preinstallimage-$name" where $name is what is given in the _preinstallimage description

Vogtinator commented 1 year ago

Looks like preinstallimage-minimal already violates that - it has Name: base! I wonder whether the name is actually used anywhere, it doesn't appear to be in the preinstallimage info.

Vogtinator commented 1 year ago

Looks like this is broken in more cases. If the Name does not really matter, can we just ignore it?

dirkmueller commented 1 year ago

@Vogtinator all existing preinstallimages were checked in outside the bot, so the bot was never able to do any checking on it. this PR is improving over that situation.

what we can do is enforce that "name" is set to "preinstallimage" always (which is the default if it isn't set). patch updated.

Vogtinator commented 1 year ago

@Vogtinator all existing preinstallimages were checked in outside the bot, so the bot was never able to do any checking on it. this PR is improving over that situation.

what we can do is enforce that "name" is set to "preinstallimage" always (which is the default if it isn't set). patch updated.

Is there any advantage of having the name set to preinstallimage or match the source container name outside of consistency?

dirkmueller commented 1 year ago

@Vogtinator the easiest would be to not set the name, in which case the spec file parser sets the name to "preinstallimage". this is what this PR is doing (now, didn't do before).

dirkmueller commented 1 year ago

@Vogtinator all existing preinstallimages were checked in outside the bot, so the bot was never able to do any checking on it. this PR is improving over that situation. what we can do is enforce that "name" is set to "preinstallimage" always (which is the default if it isn't set). patch updated.

Is there any advantage of having the name set to preinstallimage or match the source container name outside of consistency?

currently the name is not used by anything. it used to be required to be set because some logic in the OBS crashed when no Name was available. This has been fixed to set the name to "preinstallimage" if it isn't set.

Vogtinator commented 1 year ago

Ok, so this is just a cosmetical change. Considering that all previous preinstallimage changes were done skipping the bot, this PR won't break anything...

dirkmueller commented 1 year ago

@Vogtinator its not cosmetical for SLE. the sle release managers require the "bot" to pass and currently there is no way to pass it.

Vogtinator commented 1 year ago

@Vogtinator its not cosmetical for SLE. the sle release managers require the "bot" to pass and currently there is no way to pass it.

I'm referring to the Name of preinstallimages. Is that not just cosmetical in SLE?

dirkmueller commented 1 year ago

@Vogtinator its not cosmetical for SLE. the sle release managers require the "bot" to pass and currently there is no way to pass it. I'm referring to the Name of preinstallimages. Is that not just cosmetical in SLE?

I think we can do withotu the "Name" in _preinstallimage. but we still need this PR to fix it