[Mldonkey-users] [patch #5239] fix bug #16994

[spiralvoice]

Follow-up Comment #1, patch #5239 (project mldonkey):

Yes, the change you observed is correct.
Here is a copy of an IRC conversation about this topic between pango and zet,
zet coded the changes in patch #4810 responsible for the changes.

May 26 13:49:51 <spiralvoice> pango: have investigated it further, some patch
from z some months ago sent those results (same file from several servers I
think) to the GUI
May 26 13:51:03 <spiralvoice> kmldonkey prints each filename as one line
although hash is the same (3 filenames, 1 hash -> kmldonkey 3 search result
lines, sancho 1 line with popup where the other filenames are printed)
May 26 13:51:33 <spiralvoice> this kmldonkey behaviour was already reported
as a bug somewhere
May 26 13:53:40 <pango> spiralvoice: I think mlgui suffers from this too
May 26 13:54:08 <spiralvoice> pango: reason is IMHO the changing of a GUI
opcode instead of introducing a new one
May 26 13:58:28 <pango>
http://mldonkey.sourceforge.net/phpBB2/viewtopic.php?p=22600#22600
May 26 14:01:11 <spiralvoice> patches 4810 or 5088 should be responsible for
that
May 26 14:01:30 <spiralvoice> maybe this new line in commonResults.ml,
IndexedResults.update_result rs rr; - have to test it
May 26 14:01:59 <spiralvoice> 4810 was first in 2.7.3
May 26 14:02:15 <spiralvoice> 2.7.3 and 2.7.5 for sure - contains bug in
searching
May 26 14:04:55 <pango> well, it's not a bug, it's a feature ;) but GUIs need
modifications
May 26 14:05:54 <spiralvoice> no, current opcodes need to be changed back and
new opcodes introduced, GUI protocol assures backward compatibility which is
broken in this case
May 26 14:16:18 <pango> or, as I wrote, a change of protocol version (and
only use new behavior if version is high enough); Because I think it's a
change of behavior, rather than packets contents
May 26 15:15:06 <z|away> GUI protocol was not broken.  It is the design of
the gui protocol to send a structure again when it is updated. Files.
Clients, Options, etc.  Now Results do the same. Adding another opcode would
be incorrect.  
May 26 15:16:22 <pango> but do you agree that protocol version should have
been incremented ?
May 26 15:17:59 <pango> (not that this was not forgotten for some other
protocol changes, I remember when chunks states increased from 3 to 4,...
it's too easy to forget)
May 26 15:18:53 <z|away> Not really. You could set it to not send duplicate
results unless gui prot is the current ver(40) to help, but "should have"
means it was in error.  The error is in guis that didn't take this into
account from the beginning. (sancho was lazy too)
May 26 15:19:42 <pango> not a reason for breaking them if it can be avoided
May 26 15:21:34 <z|away> As it said, if it helps and doesn't hurt it can be
done easily.  But there is no reason to write code specifically to avoid bugs
in other software.
May 26 15:22:15 <pango> no reason ?
May 26 15:24:28 <pango> btw I saw some post from su_blanc on the forum, maybe
he can help with mlgui
May 26 15:24:38 <z|away> No reason = No reason for a new opcode = No reason
to design it specifically for broken software.  Hacks like only sending dups
if guiprot > 39 aren't redesigning anything.
May 26 15:25:45 <pango> that's how backward compatibility is handled by this
protocol scheme
May 26 15:27:18 <z|away> Again, if every gui handled dup options, just like
it handles dup files, clients, etc, it wouldn't be noticed. It isn't a
backwards compatibility issue like changing the format of an opcode, (from a
design perspective).  It is one from a more "hacky" perspective.
May 26 15:27:48 <pango> I think you're often right, but in that case you're
just arrogant
May 26 15:28:28 <z|away> Thanks.  But there remains a difference IMHO.  Maybe
the word hacky bothers you, I don't know..
May 26 15:28:47 <pango> other GUIs exists, they may have their flaws, but we
should try to support them, specially when the cost it's high
May 26 15:28:53 <pango> s/it's/isn't/
May 26 15:29:05 <z|away> Of course.  Where was I arguing against this? 
May 26 15:29:39 <pango> you deny the fact there's any reason to increase
protocol version
May 26 15:30:51 <pango> I don't think a new opcode was called for either
May 26 15:31:40 <z|away> Nope.  Now you're putting words into my mouth.  I
explained what I meant by "No reason" above. 
May 26 15:34:29 <pango> yes, that in theory they're wrong so it's ok to break
them. I agree with the first part of the sentense only
May 26 15:35:05 <z|away> That in theory they are wrong, so there is No Reason
to redesign what a Result is just for them.  
May 26 15:35:33 <pango> but does supporting them require a redesign ?
May 26 15:36:50 <z|away> I've already said this.. (no).  This is what I am
arguing against: a new opcode.
May 26 15:37:44 <pango> and I'm arguing about a protocol version increment,
is the conclusion that we're in violent agreement ? :)
May 26 15:39:14 <z|away> Depends.  I'm in support of a retroactive increment
to only send dups if gui prot is == to the current version, not to increment
the current one to an even higher number.
May 26 15:40:25 <pango> the version number must be higher than the one
"supported" by the GUIs that don't understand search result updates
May 26 15:40:33 <z|away> (or whatever version it was at the time I suppose..
)
May 26 15:40:38 <pango> I don't know when protocol version was last modified
May 26 15:43:49 * pango writing to su_blanc_ atm
May 26 15:48:23 <pango> done
May 26 16:55:25 <Amorphous> man we increase the proto version all the time.
the version is there to be increased...
May 26 16:56:02 <Amorphous> z|away: just changing the behaviour for the
current version is flawed (or not) in the same way it is for older versions.
May 26 16:56:18 <z|away> I don't know what you are arguing.
May 26 16:57:18 <Amorphous> z|away: for inceasing the version and only
sending dups for the newer versions
May 26 16:57:51 <z|away> And? I've said that already. 
May 26 16:58:19 <Amorphous> z|away: no you said: "not to increment the
current one to an even higher number."
May 26 16:58:29 <Amorphous> or did i missunderstood you?
May 26 17:03:44 <z|away> Probably.  The version # at which dups are sent
should match the version#+1 at the time the change was made (a while ago).
Incrementing the current one doesn't make sense.
May 26 17:05:27 <Amorphous> oh mh it seems i missunderstood the whole problem
then...
May 26 17:05:52 <pango> I thought it was a recent change too
May 26 17:08:40 <z|away> Maybe it is.. I don't know what the version# was. If
it was the same as the current, then current can change.  The point is that it
depends on what it was at the time, not what it is now.
May 26 17:09:28 <pango> patch #4810 was applied in January it seems
May 26 17:12:03 <pango> the 19th... proto version changed just before, then
16th ?
http://cvs.savannah.nongnu.org/viewcvs/mldonkey/mldonkey/src/daemon/common/guiProto.ml?only_with_tag=HEAD&r2=1.24&r1=1.23
May 26 17:13:56 <pango> sounds good enough, what about only sending updates
only when negociated proto >= 40 ?
May 26 17:14:12 <SchAmane> huh, thats not so easy if you dont anderstand even
basic of ocaml0
May 26 17:14:12 <z|away> Sure, go for it. :)




    _______________________________________________________

Reply to this item at:

  <http://savannah.nongnu.org/patch/?func=detailitem&item_id=5239>

_______________________________________________
  Nachricht geschickt von/durch Savannah
  http://savannah.nongnu.org/