Re: [PATCH] shell32: Fix SHFileOperation(FO_MOVE) for creating subdirectories.
Hello, I noticed tests failures in shlfileop while trying to add a new test. The failures were in parts that are not touched by my patches (same errors that you got). I have a patch ready for submission that correct the failures in the existing tests. You may want to wait until I submit my patch and it is committed before submitting your patch again. I'll submit my patch as soon as possible. Regards. Le vendredi 14 août 2015 à 21:18 +0800, Zhenbo Li a écrit :
This patch fixes bug 25207. Seems that a dozen of apps are affected by this bug, so please tell me how I can improve this patch if this patch can't be committed directly.
There will be a test error: shlfileop.c:818: Test failed: Expected ERROR_CANCELLED or DE_DIFFDIR
It is not related to my patch.
Hope you enjoy your weekend.
--- dlls/shell32/shlfileop.c | 11 +++++++++-- dlls/shell32/tests/shlfileop.c | 6 +++--- 2 files changed, 12 insertions(+), 5 deletions(-)
-- Olivier F. R. Dierick o.dierick(a)piezo-forte.be
Hello Olivier, Thanks for your concern on my patch. 2015-08-15 4:34 GMT+08:00 Olivier F. R. Dierick <o.dierick(a)piezo-forte.be>:
I noticed tests failures in shlfileop while trying to add a new test. The failures were in parts that are not touched by my patches (same errors that you got). I have a patch ready for submission that correct the failures in the existing tests.
I've sent a patch long before[1] which I think is similar to patch 113659. Alexandre Julliard rejected my patch, and said to me "maybe a difference in the compiler version or something like that. I still would like to understand it instead of hiding it." Can you find out what is the reason of this difference? Or find out how to convince Alexandre Julliard? lol [1]: https://www.winehq.org/pipermail/wine-patches/2015-February/137382.html -- Have a nice day! Zhenbo Li
Hello, It seems that we have been faced by the same problem and found the same solution. ;) The comments in your patch make it sound like there are multiple values returned from Vista. It should not happen, hence the suggestion from Alexandre Julliard to check the compiler version. However, my tests on the Wine Testbot tell me that Vista always returns DE_FILEDESTISFLD. I've not seen Vista return any other value. I suppose you didn't either. So we might be safe and assume that the original code is wrong or dated or that the Vista labeling is a mistake. I can't explain how the original code could pass the testbot. Maybe it was added before the testbot existed, or when Vista had another Service Pack, or the testbot result was overlooked, I don't know. Now Vista returns an unexpected error code and the test fails. The fix we have come up with passes successfully on all testbot VMs. It adds a new condition for the current Vista return value. The old condition remains because it is needed for W7U and W8. If the original code was valid for an older Vista Service Pack, the condition is still there to comply with the requirement of the older Vista. I'll let Alexandre Julliard make up his mind with the provided information. Keep up the good work! Regards, Le samedi 15 août 2015 à 09:05 +0800, Zhenbo Li a écrit :
Hello Olivier,
Thanks for your concern on my patch.
2015-08-15 4:34 GMT+08:00 Olivier F. R. Dierick <o.dierick(a)piezo-forte.be>:
I noticed tests failures in shlfileop while trying to add a new test. The failures were in parts that are not touched by my patches (same errors that you got). I have a patch ready for submission that correct the failures in the existing tests.
I've sent a patch long before[1] which I think is similar to patch 113659. Alexandre Julliard rejected my patch, and said to me "maybe a difference in the compiler version or something like that. I still would like to understand it instead of hiding it."
Can you find out what is the reason of this difference? Or find out how to convince Alexandre Julliard? lol
[1]: https://www.winehq.org/pipermail/wine-patches/2015-February/137382.html
-- Olivier F. R. Dierick o.dierick(a)piezo-forte.be
Hello, 2015-08-15 13:39 GMT+08:00 Olivier F. R. Dierick <o.dierick(a)piezo-forte.be>:
The comments in your patch make it sound like there are multiple values returned from Vista. It should not happen, hence the suggestion from Alexandre Julliard to check the compiler version.
However, my tests on the Wine Testbot tell me that Vista always returns DE_FILEDESTISFLD. I've not seen Vista return any other value. I suppose you didn't either. So we might be safe and assume that the original code is wrong or dated or that the Vista labeling is a mistake.
No. I'm sure that some Vista computers won't return DE_FILEDESTISFLD See http://test.winehq.org/data/tests/shell32:shlfileop.html -- Have a nice day! Zhenbo Li
Hello, Thanks for providing that link to testbot data, I didn't know about it. I guess we are stuck with it until the people in charge of the testbot fix this. The best thing to do at the moment is to work on other parts of the code, and when submitting patches to tests/shlfileop, ask Alexandre to ignore that specific Vista testbot failure. Regards, Le samedi 15 août 2015 à 21:48 +0800, Zhenbo Li a écrit :
Hello,
2015-08-15 13:39 GMT+08:00 Olivier F. R. Dierick <o.dierick(a)piezo-forte.be>:
The comments in your patch make it sound like there are multiple values returned from Vista. It should not happen, hence the suggestion from Alexandre Julliard to check the compiler version.
However, my tests on the Wine Testbot tell me that Vista always returns DE_FILEDESTISFLD. I've not seen Vista return any other value. I suppose you didn't either. So we might be safe and assume that the original code is wrong or dated or that the Vista labeling is a mistake.
No. I'm sure that some Vista computers won't return DE_FILEDESTISFLD
See http://test.winehq.org/data/tests/shell32:shlfileop.html
-- Olivier F. R. Dierick o.dierick(a)piezo-forte.be
participants (2)
-
Olivier F. R. Dierick -
Zhenbo Li