Frans Pop wrote: > On Tuesday 09 January 2007 19:07, Eddy Petrişor wrote: >> Attached is a patch that closes a few open issues with the ppp-udeb. >> The most important are #402450 (not bringing up the interface after >> install and not saving a correspondent section in the target >> /etc/network/interfaces) and the handling of bad login information >> being typed. > > Patch looks better than previous version. Still some comments.
Ok. > Cheers, > FJP > > I think these earlier comments are still valid: > ! I still have some reservations regarding this patch. > ! - netcfg's base-installer script already copies the /etc/network/* > ! files to the target system; what does the interfaces file in the d-i > ! environment contain if ppp-udeb is run? That file is not touched by ppp-udeb. > ! as the netcfg base-installer script will still be run, this is > ! relevant. Should I consider adding the pppoe related information in there? > ! - The 50config-target-ppp script is not idempotent: it will > ! add the same section again and again if it is run more than once. Yes, I know, I am aware of this issue and was aware of it from the start. The main problem is that the interfaces file does not allow comments at the end of lines with useful information, so doing something like I did for the /etc/resolv.conf file is not possible. I am not 100% sure, but I think I tested di's sed if it supports deleting sections like : sed '/^# Start of ppp-udeb section$/,/^# End of ppp-udeb section$/d' And it didn't. So I preferred a working solution, although not 100% clean. > In combination these two comments are deadly: you are potentially _adding_ > to an existing /target/etc/network/interfaces that was copied from the > d-i environment. If you are certain that no /etc/network/interfaces is > created in the d-i environment if ppp-udeb is used, this is a bit less of > an issue. Sorry, but I don't see what is the problem. This it is the desired behaviour, after all, so the pppoe connection is brought up and configured after restart. >> Issues still open: >> - hostname is not set (I think I can force netcfg to run the mandatory >> steps, but I didn't had the time to do it during the winter holidays) >> and does not yet blend well with netcfg > > I don't think you should want to use netcfg for that in the current > situation. ppp-udeb should be more or less self-sufficient. > IMO this issue is important. > > What about the /etc/hosts file? Is that being created? I was thinking of setting netcfg's templates to choose "Do not configure the network at this time" and make it skip directly to the hostname question. I did some tests this morning, but they are inconclusive yet. Still I am confident I can do this. >> - finish-install.d was not working at some point in the past, but the >> current situation is uncertain > > Huh? We would have installation failures all over the place if > finish-install.d was not working... Err, I meant the ppp-udeb finnish-install.d script seemed to have an uncertain behaviour. But I think the issue was the missing serction in /etc/network/interfaces. > Other comments > ============== > As you are introducing new templates, you should allow translators the > opportunity to update their translations! Yes, this is a issue I am aware of, but, to be honest, I was put off by your initial comments regarding these latest changes we are talking about and I wasn't sure it would worth the effort since I felt they might not end up in Etch anyway. > Is the lo interface now being activated for the installation itself? If Nope, as I said, I am thinking of letting netcfg do that. > not, that could cause installation problems. Note that the netmask for > the lo interface should have address 127.0.0.1/8! > netcfg uses: > ip link set lo up > ip addr flush dev lo > ip addr add 127.0.0.1/8 dev lo > > > From the patch > ============== > + * ppp-udeb: the interface is now raised in the installed system > Interfaces are not "raised", but "brought up" or maybe "activated". ok, I'll correct that. > + * Added myself to Uploades with maintainer's consent. > s/Uploades/Uploaders/ > > +RET= > +log-output -t ppp-udeb pppd call provider || RET=$? > + > +if [ 0$RET -eq 19 ]; then > Why not just initialize RET=0, then you can replace the ugly "0$RET" by > "$RET" for both tests. Because I didn't thought of it :-D that way from the start (I built that up from some different code logic). -- Regards, EddyP ============================================= "Imagination is more important than knowledge" A.Einstein -- To UNSUBSCRIBE, email to [EMAIL PROTECTED] with a subject of "unsubscribe". Trouble? Contact [EMAIL PROTECTED]
