Re: [gnap-dev] Some patches for gnap

[Philipp Riegger]

josé Alberto Suárez López wrote:

> kingtaco told me infra have the new machine, so i hope to have our new
> home soon, so all of us can use the repo to work together and fast.

Nice. With this change, are you also switching to svn or soemthing like
that or changing the gnap repository layout? Making a difference between
gnap_make and the other scripts seems rather artificial, since they are
sharing code now.

>> Note:
>> 1) There are 2 gnap_shared.sh so far, one in the src and one in the toos
>> directory of the gnap svn tree. This should maybe be changed...
>
> shoudl be

Where do you want to have it?

>> 2) The gnap_* scripts do 'source "gnap_shared.sh"', which is probably
>> not
>> good. In which directory will we put gnap_shared.sh?
>
> at moment "/usr/lib/gnap"

Seems to make the most sense, i'll change that.

>> 08-namespace-gnap_shared.patch
>>
>> This cleanes up the namespace, all variables that are used as input
>> parameters in gnap_shared.sh are prefixed with GNAP_ to get a clean
>> usage
>> of the namespace. More to that later.
>
> great

It's not 100% clean so far, i did not work on variables in gnap_overlay
and gnap_remaster so far, i also forgot to check which implications this
has on the config files.

>> Ok, so far, so good. What's next?
>>
>> 1) Configuring gnap
>>
>> At the moment configuring gnap_make (at which i concentrate so far) is
>> quite a mess. We have default parameters (in gnap_shared.sh), we have
>> catalyst.conf and commons.conf which we simply source, we have command
>> line options. At the moment this means the following:
>>
>> Default options are set first, then they are overwritten by command line
>> options, which are overwritten by common.conf, ehich are overwritten by
>> catalyst.conf.
>>
>> This is not really clean. For example, we use CATALYST_TEMP_DIR in
>> common.conf, catalyst uses store_dir in catalyst.conf. This is redundant
>> and does not make it clean. The default common.conf also defines
>> CATALYST_CONF (which might be broken because i renamed it, i have to
>> check), but that means that the command line option for specifying
>> catalyst.conf does not work if this is not commented out.
>>
>> So... do we want to change this? I would like something like:
>>
>> We can use variables from the environment. This will only be valid for
>> GNAP_* variables, so we have a clear namespace and it enables people to
>> set default variables. I'm not sure, how much this will be needed/used,
>> but it sounds like an interesting feature. The catalyst.conf is only
>> used
>> for catalyst configuration (interaction between gnap and catalyst),
>> therefore it does never make sense to overwrite information given there.
>>
>> Default options < environment < common.conf < command line options.
>>
>> This would be my prefered way of configuring gnap, where variables from
>> sources more right overwrite variables from sources more left.
>
> i like the idea, work on it and tell me. I prefer as less config files
> to edit better.

The easiest way to implement this would be:

We use ${:-} or ${:=} (i have to understand the difference between them)
for default parameters, parse the command line for the first time only
extracting parameters concerning external config files or -h, parse theese
config files (we have them or default config files), order will be
common.conf and then catalyst.conf (so common.conf can overwrite the
variable saying where to find catalyst.conf), we parse the command line
options again and use all the info given there to set/overwrite variables.

Advantages:
 - No additional variables needed
 - Quite easy change
 - Should give us what we want
 - No need for a config file parser

Disadvantages:
 - Well... common.conf and catalyst.conf can overwrite lots of things, we
never check which config file is allowed to overwrite what. But if
somebody does strange things there, it's not our fault, i think


>> 2) gnap_make feature: pretend
>>
>> I'd like a --pretend feature for gnap_make which lets it just create the
>> configuration files for catalyst and the source files (portage tree,
>> from
>> tree snapsht and overlays). This might be useful for test runs,
>> development and usage of catalyst features gnap does not support. This
>> could be implemented by simply overwriting CATALYST_BIN with "echo
>> $CATALYSY_BIN", if 1) from above is clear.
>
> i prefer to mantain gnap simple as possible, Really some catalyst option
> is so usefull to use it alone? non-implemented-catalyst options are
> sufficent usefull to implement them in gnap_make?

Hmm, all the reasons why i thought this might be a good idea are only
minor. So... i'll move that off my todo list until i find a valid use case
for it.

>> 3) gnap_make feature: improved overlay handling

[...]

> To improve is ever good :)

An alternative would be to introduce overlay handling to catalyst, but i
think, we don't want that. :-)

>> 4) some small stuff
>>
>> At the moment, if there is a choice (Overwrite/Append, Yes/No) only one
>> possibility is checked and the other is assumed, if the one is not
>> given.
>> I'd like to change this to something like "It is asked in a loop until a
>> valid option is given."

Any comments on this?

Examples:

gconfirm() {
        if [[ "${FORCEYES}" -eq 1 ]]; then
                gwarn "${*} forced to yes"
        else
                read -ep " ${W}*${N} ${*} [N]: " answer
                if [[ "${answer}" != 'y' && "${answer}" != 'Y' ]]; then
                        if [[ -n "${GNAP_TEMPDIR}" ]]; then
                                cleanup
                        fi
                        echo "${GNAP_PRODUCT} aborted !"
                        exit 2
                fi
        fi
}

what if the user mixes up for example german and us keyboard layout, wants
to say y but gets z instead?

if [[ -f "${GNAP_LOGPREFIX}.out" || -f "${GNAP_LOGPREFIX}.err" ]]; then
        if [[ "${FORCEYES}" -ne 1 ]]; then
                read -ep " ${W}*${N} Logfile(s) already exists.
Append/Overwrite
 [A]: " \
                        answer
                if [[ "${answer}" == 'o' || "${answer}" == 'O' ]]; then
                        rm "${GNAP_LOGPREFIX}.out" "${GNAP_LOGPREFIX}.err"
                fi
        fi
fi

Same here, i cannot think of keymap issues but peope sometimes hit the
wrong button. Should we have the questions in a loop until they are
answered correctly?

>> There is a function gwarn, writes to stderr. It is used in some places
>> where ginfo would make more sence, if it would exist. I'd like to
>> implement and use this.
>
> for example?

gwarn 'The following targets will be called:'
gwarn "${TARGETLIST}"

>> That's all i wanted to say about the gnap_scripts at the moment.
>
> you say a lot :)

Hmm... is that good or bad? :)

> nice job

Thanks, unfortunately not what i applied for. :-(

See you,
    Philipp
-- 
gnap-dev@g.o mailing list