[Libosinfo] [PATCH 1/4] debian: Add templates to JEOS script

[Lasse Schuirmann]

2014-07-08 21:08 GMT+02:00 Zeeshan Ali (Khattak) <zeeshanak at gnome.org>:
> On Tue, Jul 8, 2014 at 2:35 PM, Lasse Schuirmann
> <lasse.schuirmann at gmail.com> wrote:
>> This introduces default values so that no questions are asked if
>> localization or target disk are not given or 'C'.
>
> Description is good but shortlog is vague. 'templates' is just a
> means. How about: debian: Assume defaults for l10n & disk

I'll take that.

>> ---
>>  data/install-scripts/debian.xml | 52 +++++++++++++++++++++++++++++++++--------
>>  1 file changed, 42 insertions(+), 10 deletions(-)
>>
>> diff --git a/data/install-scripts/debian.xml b/data/install-scripts/debian.xml
>> index c5c8592..e344bda 100644
>> --- a/data/install-scripts/debian.xml
>> +++ b/data/install-scripts/debian.xml
>> @@ -1,4 +1,5 @@
>>  <libosinfo version="0.0.1">
>> +  <!-- JEOS PROFILE -->
>>    <install-script id='http://debian.org/scripts/debian/jeos'>
>>      <profile>jeos</profile>
>>      <expected-filename>preseed.cfg</expected-filename>
>> @@ -8,6 +9,7 @@
>>        <param name="l10n-keyboard" policy="optional" value-map="http://libosinfo.fedorahosted.org/x11-keyboard"/>
>>        <param name="l10n-timezone" policy="optional"/>
>>        <param name="l10n-language" policy="optional"/>
>> +      <param name="target-disk" policy="optional"/>
>>      </config>
>>      <injection-method>initrd</injection-method>
>>      <template>
>> @@ -17,33 +19,64 @@
>>
>>          <xsl:output method="text"/>
>>
>> +        <xsl:template name="target-disk">
>> +          <xsl:choose>
>> +            <xsl:when test="config/target-disk != ''">
>> +              <xsl:value-of select="config/target-disk"/>
>> +            </xsl:when>
>> +            <xsl:otherwise>
>> +              <xsl:text>/dev/vda</xsl:text>
>
> Shouldn't this be based on OS version, like in fedora's script?

I wasn't able to grab an older debian ISO to test the script on them.
Despite of this, assuming this was necessary in fedora for some
specific changes they made from one version to the other, I don't see
any reason we'd need it for debian too. Target -disk is hard to guess
anyway because it hardly depends on the usage of the VM. You could
also use libosinfo for generating scripts for real machines, I assume
half of the installations would fail if you don't give target-disk
manually.

>> +            </xsl:otherwise>
>> +          </xsl:choose>
>> +        </xsl:template>
>> +
>> +        <xsl:template name="l10n-language">
>> +          <xsl:choose>
>> +            <xsl:when test="config/l10n-language != '' and config/l10n-language != 'C'">
>
> * Don't we have the same issue in fedora and rhel scripts?

I have not made much experience with the fedora scripts but I assume
that kickstart is designed to assume defaults if values are empty or
invalid. (At least I had no problems installing fedora and I had "C"
in there.) Preseed is just very bad at this, if anything is wrong with
the script it'll ask the user.

> * It can be lowercase 'c' too:
> https://www.gnu.org/savannah-checkouts/gnu/gettext/manual/html_node/Locale-Names.html

Diddn't find the lowercase c on the page but it's no problem to add it.

Lasse