RESOLVED FIXED Bug 130615
More preparation for using Soup on Windows
https://bugs.webkit.org/show_bug.cgi?id=130615
Summary More preparation for using Soup on Windows
Alex Christensen
Reported 2014-03-21 13:55:18 PDT
Similar to https://bugs.webkit.org/show_bug.cgi?id=130472 I've got a build compiling, but there is some problem with malloc and gmalloc. I'm planning to upstream my changes to make debugging it have more localized changes.
Attachments
Patch (28.41 KB, patch)
2014-03-21 14:07 PDT, Alex Christensen
no flags
Patch (16.89 KB, patch)
2014-03-27 10:58 PDT, Alex Christensen
no flags
Patch (16.89 KB, patch)
2014-03-27 14:55 PDT, Alex Christensen
no flags
Alex Christensen
Comment 1 2014-03-21 14:07:32 PDT
Carlos Garcia Campos
Comment 2 2014-03-22 01:27:23 PDT
Comment on attachment 227488 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=227488&action=review > Source/WebCore/loader/soup/CachedRawResourceSoup.cpp:22 > + > +#if USE(SOUP) > + Why do you need this? you are not supposed to build this file if not using soup > Source/WebCore/loader/soup/SubresourceLoaderSoup.cpp:22 > + > +#if USE(SOUP) > + Ditto. > Source/WebCore/platform/soup/SharedBufferSoup.cpp:22 > + > +#if USE(SOUP) > + Ditto. > Source/WebCore/platform/soup/URLSoup.cpp:29 > + > +#if USE(SOUP) > + Ditto.
Alex Christensen
Comment 3 2014-03-27 10:58:35 PDT
Alex Christensen
Comment 4 2014-03-27 11:02:09 PDT
(In reply to comment #2) > (From update of attachment 227488 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=227488&action=review > > > Source/WebCore/loader/soup/CachedRawResourceSoup.cpp:22 > > + > > +#if USE(SOUP) > > + > > Why do you need this? you are not supposed to build this file if not using soup Like I said in https://bugs.webkit.org/show_bug.cgi?id=130472 I want to make a modified build of WinCairo without making a whole new build system in the Visual Studio project files. I think this adds less clutter. I uploaded an updated patch with COMPILER(MSVC) instead of defined _MSC_VER to be more webkit-like instead of glib-like.
Carlos Garcia Campos
Comment 5 2014-03-27 11:06:09 PDT
Can't you add the files conditionally to the build system instead? That's possible in all other build systems I think
Alex Christensen
Comment 6 2014-03-27 11:12:14 PDT
That's not possible with Visual Studio as we are using it. Once CMake takes over that will be simple, but I think this is the best solution until then.
Alex Christensen
Comment 7 2014-03-27 13:39:20 PDT
I suppose I could make another all-in-one cpp file, like NetworkingAllInOne.cpp or something that includes all the curl or soup files depending on USE(SOUP) or USE(CURL). What would you think about that?
Alex Christensen
Comment 8 2014-03-27 14:55:00 PDT
Alex Christensen
Comment 9 2014-03-27 16:32:45 PDT
Carlos, would you prefer I make an all-in-one file to conditionally include the soup files, or would you be ok with adding #if USE(SOUP) to the soup files. I'm ok with either, but I've already started down this road, and I'd like to continue unless there is a reason to switch.
Carlos Garcia Campos
Comment 10 2014-03-29 01:45:40 PDT
(In reply to comment #9) > Carlos, would you prefer I make an all-in-one file to conditionally include the soup files, or would you be ok with adding #if USE(SOUP) to the soup files. I'm ok with either, but I've already started down this road, and I'd like to continue unless there is a reason to switch. I personally don't like the idea of adding more unnecessary #ifdefs to the code, but I'm not opposed if there's no other option, I don't want to block your work at all. If using an all in one file is more problematic for you, let's keep the ifdefs approach.
Alex Christensen
Comment 11 2014-03-29 11:50:48 PDT
I think any approach would add some changes, and these ifdefs are pretty minimal. We could always change in the future if this becomes a problem. Could I get an r+?
Carlos Garcia Campos
Comment 12 2014-03-30 03:06:49 PDT
Comment on attachment 227985 [details] Patch Ok.
WebKit Commit Bot
Comment 13 2014-03-31 09:30:26 PDT
Comment on attachment 227985 [details] Patch Clearing flags on attachment: 227985 Committed r166506: <http://trac.webkit.org/changeset/166506>
WebKit Commit Bot
Comment 14 2014-03-31 09:30:33 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.