Re: Includes [4] shlwapi

[Juan Lang]

Jon, are you sure about removing the winnls.h includes?  This patch looks
suspect:
> diff -ur wine/dlls/shlwapi/url.c wine-develop/dlls/shlwapi/url.c
> --- wine/dlls/shlwapi/url.c	2004-12-03 23:27:08.000000000 -0500
> +++ wine-develop/dlls/shlwapi/url.c	2004-12-06 15:42:34.000000000 -0500
> @@ -25,8 +25,6 @@
>  #include <stdlib.h>
>  #include "windef.h"
>  #include "winbase.h"
> -#include "winnls.h"
> -#include "winerror.h"
>  #include "wine/unicode.h"
>  #include "wininet.h"
>  #include "winreg.h"
> @@ -1416,7 +1414,7 @@
>     * return the same digests for the same URL.
>     */
>    WideCharToMultiByte(0, 0, pszUrl, -1, szUrl, MAX_PATH, 0, 0);
> - HashData((PBYTE)szUrl, (int)strlen(szUrl), lpDest, nDestLen);
> - HashData((const BYTE*)szUrl, (int)strlen(szUrl), lpDest, nDestLen);
> return S_OK;
>
> }

winnls.h declares WideCharToMultiByte, and we just saw a compilation error
on some systems because it was missing [1].  I haven't checked others, but
this caught my eye.

[1] http://www.winehq.org/hypermail/wine-cvs/2004/12/0091.html

--Juan

__________________________________ 
Do you Yahoo!? 
Yahoo! Mail - Find what you need with new enhanced search.
http://info.mail.yahoo.com/mail_250

Includes [4] shlwapi

[Jon Griffiths]

Hi Juan,
> winnls.h declares WideCharToMultiByte, and we just saw a
> compilation error
> on some systems because it was missing [1].

winnls.h is included already by wine/unicode.h in url.c, so its not
needed again there. The script I wrote basically comments out the
includes one at a time, and restores any that caused a compile
failure when missing. The worst that could happen is that a new
warning could be generated, but I rebuilt all the files, check the
build logs and believe I got them all.

Cheers,
Jon

P.S. I will be trying your patch as soon as I can clean up my tree a
bit :-)

=====
"Don't wait for the seas to part, or messiahs to come;
 Don't you sit around and waste this chance..." - Live

[e-mail]

__________________________________ 
Do you Yahoo!? 
Yahoo! Mail - Find what you need with new enhanced search.
http://info.mail.yahoo.com/mail_250

Includes [4] shlwapi

[Alexandre Julliard]

Jon Griffiths  writes:
> winnls.h is included already by wine/unicode.h in url.c, so its not
> needed again there. The script I wrote basically comments out the
> includes one at a time, and restores any that caused a compile
> failure when missing. The worst that could happen is that a new
> warning could be generated, but I rebuilt all the files, check the
> build logs and believe I got them all.

The problem with that approach is that you remove a lot of headers
that are included anyway so it doesn't make any difference in build
time, and many of them will have to be added again once we finish the
header cleanup. For instance you remove winuser.h everywhere win.h is
included because win.h brings it in already, but once we get rid of
win.h we'll have to go back and re-add winuser.h.

What you should do is limit the changes to the cases where it makes an
actual difference in the dependency tree. You'll get the same
performance gain with a much smaller set of changes.

Includes [4] shlwapi

[Jon Griffiths]

Hi,
> The problem with that approach is that you remove a lot of headers
> that are included anyway so it doesn't make any difference in build
> time, and many of them will have to be added again once we finish
> the header cleanup. For instance you remove winuser.h everywhere
> win.h is included because win.h brings it in already, but once we
> get rid of win.h we'll have to go back and re-add winuser.h.

It might be better to clean up the headers first then. By clean up I
take it you mean removing any header that aren't part of the platform
SDK? If that is the long term plan then it could be handled in 2
steps a) remove all #includes from non-sdk headers and add them to
the files that need them, then b) start removing the non-standard
headers (possibly much later).

If so, thats probably a worthwhile project.
> What you should do is limit the changes to the cases where it makes
> an
> actual difference in the dependency tree. You'll get the same
> performance gain with a much smaller set of changes.

I will update my script to do this and resend.

Cheers,
Jon

=====
"Don't wait for the seas to part, or messiahs to come;
 Don't you sit around and waste this chance..." - Live

[e-mail]

__________________________________ 
Do you Yahoo!? 
Yahoo! Mail - now with 250MB free storage. Learn more.
http://info.mail.yahoo.com/mail_250

Includes [4] shlwapi

[Alexandre Julliard]

Jon Griffiths  writes:
> It might be better to clean up the headers first then. By clean up I
> take it you mean removing any header that aren't part of the platform
> SDK? If that is the long term plan then it could be handled in 2
> steps a) remove all #includes from non-sdk headers and add them to
> the files that need them, then b) start removing the non-standard
> headers (possibly much later).

No, I don't think we want to do that in two steps, since we don't want
to add headers that are only needed to build a private header. For
instance many private headers still contain 16-bit stuff, and we
definitely don't want to start including Win16 headers all over the
place.

Includes [4] shlwapi

[Jon Griffiths]

Hi,
> No, I don't think we want to do that in two steps, since we don't
> want to add headers that are only needed to build a private header.
> For instance many private headers still contain 16-bit stuff, and

we
> definitely don't want to start including Win16 headers all over the
> place.

Right, but the 16 bit header are non SDK headers. what I'm talking
about as the first step is removing SDK headers included from the non
SDK headers in include/. With your example of not removing winuser.h
since it will need to be added in again to the files that use the
private headers, I am suggesting removing the inclusion of winuser.h
from e.g. winpos.h and win.h, and including them in the files that
need them directly. If we hide the source files dependencies in
header files its impossible to remove any uneeded dependencies from
them.

I'll post the script I'm using to clean up the headers and as the
private headers are removed (or periodically) it can be run to weed
out any uneeded dependencies remaining.

In the meantime, I've updated my script to only remove headers that
reduce a files dependencies and will be posting the revised patches
shortly.

Cheers,
Jon

=====
"Don't wait for the seas to part, or messiahs to come;
 Don't you sit around and waste this chance..." - Live

[e-mail]

__________________________________ 
Do you Yahoo!? 
Jazz up your holiday email with celebrity designs. Learn more. 
http://celebrity.mail.yahoo.com

Includes [4] shlwapi

[Alexandre Julliard]

Jon Griffiths  writes:
> Right, but the 16 bit header are non SDK headers. what I'm talking
> about as the first step is removing SDK headers included from the non
> SDK headers in include/. With your example of not removing winuser.h
> since it will need to be added in again to the files that use the
> private headers, I am suggesting removing the inclusion of winuser.h
> from e.g. winpos.h and win.h, and including them in the files that
> need them directly.

It's the same issue, I took 16-bit headers as example but you can have
the same problem with SDK headers, it's quite possible that a file
doesn't need winuser.h by itself. So you would first have to move the
includes out of the non-sdk header into the source files, and then
once you get rid of the non-sdk header go back and remove the unneeded
headers again. It's a lot easier to wait until the non-sdk header gets
removed and then do everything in one step.

Includes [4] shlwapi

[Jon Griffiths]

Hi,
> It's the same issue, I took 16-bit headers as example but you can
> have
> the same problem with SDK headers, it's quite possible that a file
> doesn't need winuser.h by itself. So you would first have to move
> the
> includes out of the non-sdk header into the source files, and then
> once you get rid of the non-sdk header go back and remove the
> unneeded headers again. 
> It's a lot easier to wait until the non-sdk header
> gets removed and then do everything in one step.

Since the process of removing the unneeded headers is automated, and
most files that include non-sdk headers already have the missing
includes, I can't see that its any more work one way or the other
(well, it takes several hours to run the script, but thats the
computer working, not me).

However I'm happy to run the script periodically as the headers are
reorganised/removed.

Meanwhile I'll try to reduce the dependencies on dll internal
headers.

Cheers,
Jon

=====
"Don't wait for the seas to part, or messiahs to come;
 Don't you sit around and waste this chance..." - Live

[e-mail]

__________________________________ 
Do you Yahoo!? 
Yahoo! Mail - You care about security. So do we. 
http://promotions.yahoo.com/new_mail

Includes [4] shlwapi

[Alexandre Julliard]

Jon Griffiths  writes:
> Since the process of removing the unneeded headers is automated, and
> most files that include non-sdk headers already have the missing
> includes, I can't see that its any more work one way or the other
> (well, it takes several hours to run the script, but thats the
> computer working, not me).

Well, it's more work for the poor slob who has to review, test, and
commit the resulting patches <g>  (not to mention for people who
maintain local changes and need to fix merge conflicts after each
commit).

Includes [4] shlwapi

[Jon Griffiths]

Hi,
> Well, it's more work for the poor slob who has to review, test, and
> commit the resulting patches <g>

gulp, I guess thats me, <sniff> ;-)
> (not to mention for people who
> maintain local changes and need to fix merge conflicts after each
> commit).

I wondered about that; the good news is that all the diffs are at the
very top of the source files, so any existing patches should continue
to apply, albeit offset by a line or two. The only real conflict
possible is if local changes are messing with the includes (which of
course, does/will happen). But of course, everyones future rebuild
times after a cvs update will go down as a result, so the net result
should be a win :-)

Anyway, I've updated my script with the suggestions given and include
it below if anyones interested. I will be posting the (much reduced,
but still significant) patch very shortly to wine-patches.

As mentioned in the script blurb, this only removes headers that
don't cause extra warnings or build errors, that actually reduce
dependencies, and it won't remove a header that provides a prototype
for the function(s) implemented in a source file.

As the private includes get cleaned up I'll re-run this once in a
while to weed out any more that crop up.

Cheers,
Jon

=====
"Don't wait for the seas to part, or messiahs to come;
 Don't you sit around and waste this chance..." - Live

[e-mail]

__________________________________ 
Do you Yahoo!? 
Yahoo! Mail - Find what you need with new enhanced search.
http://info.mail.yahoo.com/mail_250