[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: Bug#406231: ppp_2.4.4rel-5 patch



-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Frans Pop wrote:
> On Thursday 18 January 2007 10:33, Eddy Petrișor wrote:
>>>  db_input high ppp/username || true
>>> -db_go || true
>>> +db_go || exit 30
>> What is the difference induced by return code 30? Will it make the
>> installation fail if ran non-interactively and this data is not
>> provided?
>>
>> (I don't seem to be able to find again the codes and their meaning for
>> main-menu, although I found it in the past.)
> 
> With "true" you are saying: just continue whatever the user does, which is 
> broken. The || exit 30 should make the script exit to the main menu if 
> the user selects the <GoBack> button, but will leave ppp marked as 
> unconfigured. It would be better to have a state engine in the script 

Thanks for the clarification.

> that goes back to a previous question, but that is something for 
> post-Etch.

I agree.

>>> +# PPPoE connection
>>> +auto provider
>>> +iface provider inet ppp
>>> +	pre-up /sbin/ifconfig $ETH up
> 
> Hmm. This should be $IFACE, not $ETH. Please fix that.

Done

>> (Just checking)
>> In order for this to work, the interface name should be preserved. This
>> happens, AFAIK, even for interfaces which are not configured, right?
> 
> I'm not completely sure. If I understand you correctly, preserving the 
> name is something that is done by udev and I'm not sure if that covers 
> your use case too. Marco can probably tell you.

Yes, he did. Is preserved.

>> Why change the name of the script in the source package _and_ the name
>> in the d-i environment? Wouldn't it make more sense to rename it
>> directly in the source? (I suggest 30ppp-udeb-config.) This would also
>> save some source code reader (later) the trouble to understand where
>> did the config-target-ppp file went.
> 
> Because it is somewhat more flexible this way. Not including the number in 
> the source file means it is easier to change the number if needed.
> You can still see what went where by grepping in the source for whatever 
> current name. However, it all makes no real difference. The name "30ppp" 

I chose ppp-udeb-postbaseinst in the source, and will be copied to 30ppp in
the binary.

> is most consistent with other scripts already in post-base-installer.d. 

Indeed.

> IMO the layout in the source file (dumping everything in the "extra" 
> directory) is not really the clearest solution...
> 
>> Not sure if copying the whole /etc/ppp/peers/ directory isn't a better
>> idea (in the style of what is done with the configuration files from
> 
> Why? What other files are there that need to be copied?

I think the options file is a good one, if the user hacks on it during
install.  I don't remember exactly, but there might be others. I'll look into
this.

>> d-i). Having that in mind, aren't the configuration files already
>> copied in the target *with* the correct permissions when this script is
>> ran?
> 
> No. The ppp dir is copied nowhere else and copying this way _only_ works 
> because the files pap/chap-secrets and peers/provider already exist in 
> _/target_ because the ppp deb installs default files there that already 
> have the correct permissions. The provider file in the d-i environment 
> does not have the correct premissions (wrong group).

Wouldn't it make more sense to fix that in the target and make sure the
permissions and group is ok? Is there a way to create groups in the target
(something in the style of apt-install)?

Also creating the target directory and quitting seems more intelligent than
just quitting if not found?


I merged your changes in my darcs repo[1]. I'll test these changes and report
back.


[1] http://users.alioth.debian.org/~eddyp-guest/darcs/ppp/ppp-2.4.4rel/debian/
[2] can be duplicated with:

darcs get --set-scripts-executable
http://users.alioth.debian.org/~eddyp-guest/darcs/ppp/ppp-2.4.4rel/debian/

- --
Regards,
EddyP
=============================================
"Imagination is more important than knowledge" A.Einstein
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)

iD8DBQFFr9O2Y8Chqv3NRNoRAgo1AKDTdd5dRpoebjpnH9FPAeX2GWiEYQCgyC0d
D33EyRtkcjSDg0lhrxDJoIs=
=4h0T
-----END PGP SIGNATURE-----



Reply to: