http://bugs.winehq.org/show_bug.cgi?id=17195
--- Comment #124 from Luke Kenneth Casson Leighton lkcl@lkcl.net 2013-01-03 12:48:16 CST ---
(In reply to comment #120)
there's a way round the "requirement" that tests be run, which is to add unnecessary code that checks - at runtime - whether the old functionality is to be used or the new functionality is to be used, and to have the choice be made either as a compile-time switch or even as a configuration option.
I don't think Alexandre would go for that without some compelling reason.
well, alexandre already asked for tests to be carried out, and you've already explained that tests cannot be carried out unless the *entire* patch set is applied and accepted, because it's so fundamental.
the two things are therefore mutually exclusively incompatible, therefore a third way is essential to devise. and accept that it is necessary to devise.
a runtime switch or a compile time switch is such a third way.
if that third way also turns out to be unacceptable then, round the loop we go: one of the other two choices must be chosen... or a fourth way found.
(In reply to comment #121)
btw adam did you also check that this patch works on FreeBSD?
No, but there's no reason it shouldn't. I'm not using any system calls, etc that Wine doesn't already use.
ok great! last time you mentioned using some weird feature of TCP that was only available to linux.
(In reply to comment #122)
oh wait! event is also being renamed to event_empty which is highly significant, and buried within the renaming.
Why is that significant? Empty events are the only thing it's used for. In a later patch an event_avail gets added, I just renamed it to avoid confusion. The structs in here are just for the server, the renames don't affect anything outside of that file.
it's not obvious just from the patch what's going on. is it *only* renaming? if so, what gets renamed to what? if you only renamed one at a time, the number of lines being changed would be smaller, so comparisons would be easy.
the sheer number of things being renamed here actually removes *entire* functions and replaces them with ones that are... identical? are they? i don't know, and i can't tell.
later on in the patch, the renaming becomes sufficiently severe that it looks like you're removing create_pipe_server. whereas what it looks like is that you've swapped the function order of find_available_[server/instance] and create_pipe_[server/instance]. i don't know, because it's hard to tell.
my recommendation would be - pain in the ass as it is - to do this as completely separate renames. and for each patch to contain the renaming of the header file as well as all files in which each renamed function/member variable is reused.
this would at least allow those to be done as entirely independent separate patches that would clearly have nothing to do with the actual functionality.
they could in fact be done under a separate bug report, so as to shorten the length of this one.