Re: GSoC project

[Gaudenz Steinlin <gaudenz@debian.org>]

Hi Sorina

On Sat, 31 Mar 2012 14:32:11 -0400, Sorina - Gabriela Sandu <sandu.sorina@gmail.com> wrote:
> Hello!
> 
> Hope you don't mind, I took some time to dig into d-i internals,
> getting used with the code and the build system.
> 
> I took a look at bug #610752 [0] and made a first attempt of
> a patch. I posted the git diff on pastebin [1], since I wanted to
> be sure it is ok before submitting it. Please tell me if this was
> the right thing to do.

As far as I can see the code is correct. But it's lacking some error
handling. If the user does not enter an integer you just continue with
the default value with any error message. It would be nice to display an
error note, reset the question and reask the question instead. You can
find an example of how to do this in static.c.

And if I'm not mistaken you misinterpreted the value of link_waits. As
it's used as a upper bound in the loop just after your code and inside
this loop the thread sleeps only for 0.25s. So you have to multiply the
value by 4 (like in the original code).

> 
> Firstly, I am not very sure about debconf_go return value. I
> assumed it returns 0 if everything is ok, based on the way it is
> used in other functions, such as netcfg_get_domain
> (netcfg-common.c), but I have not found anything explicitly
> saying so.

The cdebconf client documentation is a bit lacking ;-). I guess the
return codes are from this enum in
d-i/packages/cdebconf/src/debconfclient.h:

/**
 * @brief command codes returned by debconf commands
 */
typedef enum {
        CMD_SUCCESS             = 0,
        CMD_ESCAPEDDATA         = 1,
        CMD_BADQUESTION         = 10, 
        CMD_BADPARAM            = 15, 
        CMD_SYNTAXERROR         = 20, 
        CMD_INPUTINVISIBLE      = 30, // from debconf_input()
        CMD_BADVERSION          = 30, // from debconf_version()
        CMD_GOBACK              = 30, // from debconf_go()
        CMD_PROGRESSCANCELLED   = 30, // from debconf_progress_{set,step,info}()
        CMD_INTERNALERROR       = 100 
} cmdstatus_t;


> 
> Secondly, in the bug report it was suggested to remove
> NETCFG_LINK_WAIT_TIME, but I considered it could be used for
> setting a default value, since I believe getting timeout value
> has a low priority and there wouldn't be necessary to loop until
> a correct value is provided.

I definitely agree about the low priority. For the default value I'd
prefer to have that on the debconf template instead. 
> 
> In addition, could you please tell me what exactly does installation
> manual appendix updating implies?

All debconf templates that are preseedable (for automatic installation)
are documented in the appendix of the installation manual. The source
for the manual is in d-i/manual/en/ in the d-i subversion repository.
It's not (yet) in git.

Regars,
Gaudenz


-- 
Ever tried. Ever failed. No matter.
Try again. Fail again. Fail better.
~ Samuel Beckett ~