Re: [PATCH] shell32: Fix SHFileOperation(FO_MOVE) for creating subdirectories.

[Olivier F. R. Dierick]

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(-)

[PATCH] shell32: Fix SHFileOperation(FO_MOVE) for creating subdirectories.

[Zhenbo Li]

Hello Olivier,

Thanks for your concern on my patch.

2015-08-15 4:34 GMT+08:00 Olivier F. R. Dierick :
> 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

[PATCH] shell32: Fix SHFileOperation(FO_MOVE) for creating subdirectories.

[Olivier F. R. Dierick]

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 :
> > 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

[PATCH] shell32: Fix SHFileOperation(FO_MOVE) for creating subdirectories.

[Zhenbo Li]

Hello,

2015-08-15 13:39 GMT+08:00 Olivier F. R. Dierick :
> 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

[PATCH] shell32: Fix SHFileOperation(FO_MOVE) for creating subdirectories.

[Olivier F. R. Dierick]

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 :
> > 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