FS#11203 - pacman doesn't care for NoPassiveFtp anymore

Attached to Project: Pacman
Opened by ramiro blanco (ramiblanco) - Wednesday, 13 August 2008, 19:23 GMT
Last edited by Dan McGee (toofishes) - Wednesday, 20 August 2008, 00:55 GMT
Task Type Bug Report
Category General
Status Closed
Assigned To Xavier (shining)
Architecture All
Severity Medium
Priority Normal
Reported Version 3.2.0
Due in Version 3.2.1
Due Date Undecided
Percent Complete 100%
Votes 0
Private No

Details

Description:
Under pacman v3.2.0 the option NoPassiveFtp is not valid.

Additional info:

This is the error printed on screen:
error: config file /etc/pacman.conf, line 28: directive 'NoPassiveFtp' not recognized.

* package version(s)
v3.2.0
* config and/or log files etc.
/etc/pacman.conf

Steps to reproduce:
uncomment NoPassiveFtp under MiscOptions of pacman.conf and syncronize.
This task depends upon

Closed by  Dan McGee (toofishes)
Wednesday, 20 August 2008, 00:55 GMT
Reason for closing:  Fixed
Additional comments about closing:  Commit fb5c5086e123690a113c0e8dc85848aa2ba61f54
Comment by Xavier (shining) - Wednesday, 13 August 2008, 21:53 GMT
Yeah I know, I just noticed it today :
http://shining.toofishes.net/gitweb/gitweb.cgi?p=pacman.git;a=commitdiff;h=bad2079bbecd2dc6a9a17841dc38131b06f304e5

The correct name has FTP not Ftp.

This bug should be reassigned properly to the proper category and developers, and hopefully it won't be forgotten for the 3.2.1 release.
Comment by Dan McGee (toofishes) - Wednesday, 13 August 2008, 22:05 GMT
This needs to be updated in the manpage as well then? it is 'Ftp' there and not the correct 'FTP'.
Comment by Xavier (shining) - Thursday, 14 August 2008, 07:12 GMT
Indeed, I overlooked that.
Fixed now : http://shining.toofishes.net/gitweb/gitweb.cgi?p=pacman.git;a=commitdiff;h=f105508d66ba1223f2b6ba03d634c4c63eff8611
Comment by Xavier (shining) - Thursday, 14 August 2008, 07:15 GMT
But hmm. If it was already Ftp in both the pacman.conf manpage and pacman.conf, maybe we should just change it in the code then?
Also the Ftp name is more consistent with the CamelCase usage.
Comment by Dan McGee (toofishes) - Thursday, 14 August 2008, 11:21 GMT
What is it in some of the older documentation and code? I would stick with whatever it has always ben. Maybe take a quick look at the pre-3.0 codebase, or wherever we made options case insensitive, and see what it was then?
Comment by Xavier (shining) - Thursday, 14 August 2008, 17:30 GMT
[xavier@nx7400 pacman-2.9.8]$ grep -ri passiveftp .
./ChangeLog: - Added a "NoPassiveFtp" option in pacman.conf
./src/pacman.c:unsigned short pmo_nopassiveftp = 0;
./src/pacman.c: if(!strcmp(key, "NOPASSIVEFTP")) {
./src/pacman.c: pmo_nopassiveftp = 1;
./src/pacman.c: vprint("config: nopassiveftp\n");
./src/pacsync.c:extern unsigned short pmo_nopassiveftp;
./src/pacsync.c: if(!pmo_nopassiveftp) {
./doc/pacman.8.in:.B "NoPassiveFtp"

I don't know when options were made case insensitive, it could have been that way for ages.
But at least in 2.9.8, the doc mentioned Ftp. And the Changelog says Ftp too.
So that looks like a second reason for doing it that way.

Finally I checked in 2.4, and it was already the same as in 2.9.8.

The inconsistency happened when the case sensitive comparison was added, because of the tr_TR locale issue, commit 76f816b9f764434d02e90207ee4656ebae2b6a8c
- if(!strcmp(key, "NOPASSIVEFTP")) {
+ if(strcmp(origkey, "NoPassiveFTP") == 0 || strcmp(key, "NOPASSIVEFTP") == 0) {

Then in commit b3e6cf652c9e989badaf5499abb1d64c1a110927 , I dropped the case insensitive part :
- if(strcmp(key, "NoPassiveFTP") == 0 || strcmp(upperkey, "NOPASSIVEFTP") == 0) {
+ if(strcmp(key, "NoPassiveFTP") == 0) {

So we only find this inconsistency now.

Based on all this, I will edit my patch to use NoPassiveFtp, because it was always documented that way.

Loading...