Bug 175803 – [CMake] Use find_package for SQLite

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Don Olmstead]

Should use find_package for SQLite installations.

[Don Olmstead]

Created attachment 318712 [details]
Patch

[Alex Christensen]

Comment on attachment 318712 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=318712&action=review

> Source/WebKitLegacy/PlatformWin.cmake:-6
> -        "${WEBKIT_LIBRARIES_DIR}/include/sqlite"

AppleWin needs sqlite, too, I think.

[Don Olmstead]

Comment on attachment 318712 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=318712&action=review

>> Source/WebKitLegacy/PlatformWin.cmake:-6
>> -        "${WEBKIT_LIBRARIES_DIR}/include/sqlite"
> 
> AppleWin needs sqlite, too, I think.

Its in a if (${WTF_PLATFORM_WIN_CAIRO}) block.

I can have both AppleWin and WinCairo use find_package. Wasn't entirely sure if it would behave properly with SQLite3${DEBUG_SUFFIX}.

[Alex Christensen]

Comment on attachment 318712 [details]
Patch

Ah, you're right.  This is fine.

[Konstantin Tokarev]

Comment on attachment 318712 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=318712&action=review

> Source/WebCore/PlatformWinCairo.cmake:7
> +    ${SQLITE_INCLUDE_DIR}

I think it should be in WebCore/CMakeLists.txt, as all ports use sqlite

[Alex Christensen]

All ports use sqlite, but AppleWin needs the debug suffix in the internal debug build.

[Don Olmstead]

Created attachment 318895 [details]
Patch

[Don Olmstead]

Created attachment 318896 [details]
Patch

[Konstantin Tokarev]

Comment on attachment 318896 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=318896&action=review

> Source/WebKitLegacy/PlatformWin.cmake:-29
> -        PRIVATE SQLite3${DEBUG_SUFFIX}

Does find_package(Sqlite) handle debug suffix correctly? If no, it makes sense to use just

 set(SQLITE_LIBRARIES SQLite3${DEBUG_SUFFIX})

here

[Don Olmstead]

(In reply to Konstantin Tokarev from comment #9)
> Comment on attachment 318896 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=318896&action=review
> 
> > Source/WebKitLegacy/PlatformWin.cmake:-29
> > -        PRIVATE SQLite3${DEBUG_SUFFIX}
> 
> Does find_package(Sqlite) handle debug suffix correctly? If no, it makes
> sense to use just
> 
>  set(SQLITE_LIBRARIES SQLite3${DEBUG_SUFFIX})
> 
> here

I'm looking into that. I'm not sure why the bot failed as I tried it locally and it finds SQLite3. There is no suffix in the release.

[Don Olmstead]

Created attachment 318921 [details]
Patch

[Don Olmstead]

Comment on attachment 318921 [details]
Patch

Changed this so its now for all ports.

The whole SQLite3${DEBUG_SUFFIX} seems to throw a wrench in using find_package for AppleWin so set(SQLITE_LIBRARIES) was used instead. ${SQLITE_INCLUDE_DIR} doesn't need to be set since its ignored when its just empty. If find_package were used without REQUIRED then it would be set to NOTFOUND which would error.

I think this is probably what we want to do with the other WebCore libraries that are common between all ports, zlib, xml2, xslt.

[Konstantin Tokarev]

>The whole SQLite3${DEBUG_SUFFIX} seems to throw a wrench in using find_package for AppleWin

Not surpising actually, as FindSqlite.cmake makes no effort to find suffixed library

[WebKit Commit Bot]

Comment on attachment 318921 [details]
Patch

Clearing flags on attachment: 318921

Committed r221135: <http://trac.webkit.org/changeset/221135>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/34057821>

[Csaba Osztrogonác]

(In reply to WebKit Commit Bot from comment #14)
> Comment on attachment 318921 [details]
> Patch
> 
> Clearing flags on attachment: 318921
> 
> Committed r221135: <http://trac.webkit.org/changeset/221135>

It broke the WinCairo build:
https://build.webkit.org/builders/WinCairo%2064-Bit%20Release/builds/5504

[Don Olmstead]

(In reply to Csaba Osztrogonác from comment #17)
> (In reply to WebKit Commit Bot from comment #14)
> > Comment on attachment 318921 [details]
> > Patch
> > 
> > Clearing flags on attachment: 318921
> > 
> > Committed r221135: <http://trac.webkit.org/changeset/221135>
> 
> It broke the WinCairo build:
> https://build.webkit.org/builders/WinCairo%2064-Bit%20Release/builds/5504

I'm just going to rejigger the directory structure in WinCairoRequirements so it hopefully likes things better, https://bugs.webkit.org/show_bug.cgi?id=175972