Am Samstag, den 18.10.2008, 22:18 +0000 schrieb Louis. Lenders:

> Spent loads of time for more than 3 years trying to help triaging bugs
> and helping out in appdb, only to find out that all my patches are
> ignored silently

Sorry, what do you mean by *all* of your patches?

This is from the wine git log:

2008-09-25      Louis Lenders   msi: Add stub for MsiSetExternalUIRecord.
2008-09-25      Louis Lenders   shdocvw: Create default App Paths key for 
iexplore... 
2008-09-04      Louis Lenders   wine.inf: Add default Directx registry key for 
InstalledVersion.
2008-08-28      Louis Lenders   shobjidl.idl: Add Taskbarlist interface 
definitions.
2008-08-28      Louis Lenders   shlwapi: Fix UrlUnEscape to expand URLs 
in-place even... 
2008-08-28      Louis Lenders   shlwapi: Add test showing UrlUnEscape should 
convert... 
2008-06-23      Louis Lenders   d3dx9_*: Add version resources.
2008-06-21      Louis Lenders   advapi32: Add stub for 
GetAuditedPermissionsFromAcl... 
2008-06-20      Louis Lenders   kernel32: Fix typo in SetProcessAffinityMask.
2008-06-10      Louis Lenders   mscoree: Add stub for CorBindToCurrentRuntime.
2008-05-29      Louis Lenders   wine.inf: Add fake glu32.
2008-04-18      Louis Lenders   wininet: Improve stub for 
FindNextUrlCacheEntryW a... 
2008-04-17      Louis Lenders   urlmon: Add stub for 
CoInternetSetFeatureEnabled.
2008-03-12      Louis Lenders   oleacc: Add GetOleaccVersionInfo.
2008-03-07      Louis Lenders   shdocvw: Return something more useful for 
WebBrowser_get_Rea... 
2008-02-29      Louis Lenders   programs: Add a stubbed out secedit.exe.
2008-02-21      Louis Lenders   shdocvw: Pretend success in 
WebBrowser_get_RegisterAsDropTarget.
2008-01-11      Louis Lenders   shdocvw: Change return value for 
PersistMemory_Load.
2008-01-10      Louis Lenders   user32: Add stub for GetLayeredWindowAttributes.
[further patches back to 2005-12-06]


> If people spent quite an amount of time trying to get a patch written,
> it shouldn't be too much trouble to just drop a note on wine-devel,
> what the reason is the patch got rejected.

In the case of the FOF_MULTIDESTFILES test patch that Vitaly didn't get
in, I don't see an obvious problem in the last version, although that
patch could be extended.

I suspect (but I am definitely not an expert in this area), that the
patch Vitaly Perov sent:

| +    /* move many files into directory with FOF_MULTIDESTFILES */
| +    set_curr_dir_path(from, "test?.txt\0");
| +    set_curr_dir_path(to, "testdir2\0");
| +    retval = SHFileOperationA(&shfo2);
| +    todo_wine
| +    {
| +        ok(retval == ERROR_SUCCESS, "Expected ERROR_SUCCESS, got %d\n", 
retval);
| +        ok(file_exists("testdir2\\test2.txt"), "Expected the file 
'test2.txt' to exist\n");
| +        ok(file_exists("testdir2\\test4.txt"), "Expected the directory 
'test4.txt' to exist\n");
| +    }

works because FOF_MULTIDESTFILES makes SHFileOperation pair from entries
with to entries. As in this case, the from entry only has one component,
namely "test?.txt" (even if it expands to multiple files), only one
component of the destination is used regardless of FOF_MULTIDESTFILES.

It would interesting to see what happens in the case that from contains
two wildcard patterns, like "test?.txt\0test_?.txt\0" and dest contains
two destinations like "testdir2\0testdir2\\nested\0". If my intuition is
right, without FOF_MULTIDESTFILES everything is moved into testdir2, and
with FOF_MULTIDESTFILES, test1.txt, test2.txt, test3.txt and the
directory test4.txt is moved to testdir2, whereas test_5.txt is moved to
testdir2\nested.

But in the issue that started the thread I have to agree with James.
FOF_MULTIDESTFILES is not to be set just because the destination is a
directory. I quote Vitaly quoting MSDN:
| FOF_MULTIDESTFILES
|    The pTo member specifies multiple destination files (one for each source 
| file in pFrom) rather than one directory where all source files are to be 
| deposited.

In this sentence "rather than" is the same as "instead of". So
FOF_MULTIDESTFILES indicates that the pTo member does *not* specify one
destination directory, but a list of destination files instead. As the
test that did not get committed seems to show (if it was tested in
Windows, stating that helps getting a patch committed; sending a patch
that does not work on any Windows version is risky, as it might put you
on the infamous s**t list temporarily), the "multiple destination files"
MSDN talks about might also be multiple destination directories.

> >Ok, I try to resend my old tests, and write tests to other patches.
> >Also there are some functions and stubs implemented, wchich doesn't require 
> >(as I think) a tests.
Even if you submit a stub, you usually do that because an application
needs that function. In this case, it might be helpfull for people
trying to implement that function to find an example use case in the
testcase, so adding a todo_wine test is a nice thing, even if you just
add a stub (it also shows in the test statistics that something needs to
be done).

The tests do not only serve the purpose to test that Wine does what we
expect wine to do (in fact, that is only a minor point of the tests),
but more importantly, they should show that Wine and Windows do the
*same* thing, so writing a testcase that passes after your patch has
been applied to wine is a no-brainer unless you tested that this test
also passes on Windows.

If a patch that contains only a test did not get committed and you
resubmit it, please add a note on which Windows version you tested it,
if you didn't include it in your first try. General hint: If you can't
test on windows for some reason (like you propose a direct3d test that
needs a graphics card you don't own), do not send the testcase to
wine-patches, but to wine-devel, and explain briefly why you can't test,
and ask other developers to test it for you. This is usually not a
problem.

Regards,
  Michael Karcher




Reply via email to