Bug 93786 – [CMake] Remove glib-related Find modules and write single new one instead.

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Raphael Kubo da Costa (:rakuco)]

[CMake] Remove glib-related Find modules and write single new one instead.

[Raphael Kubo da Costa (:rakuco)]

Created attachment 157910 [details]
Patch

[Thiago Marcos P. Santos]

Comment on attachment 157910 [details]
Patch

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

Overall LGTM, besides the nit bellow.

> Source/cmake/FindGLIB.cmake:26
> +# - Try to find Glib and its components (gio, gobject etc)
> +# Once done, this will define
> +#
> +#  GLIB_FOUND - system has Glib
> +#  GLIB_INCLUDE_DIRS - the Glib include directories
> +#  GLIB_LIBRARIES - link these to use Glib
> +#
> +# Optionally, the COMPONENTS keyword can be passed to FIND_PACKAGE()
> +# and Glib components can be looked for.  Currently, the following
> +# components can be used, and they define the following variables if
> +# found:
> +#
> +#  gio:             GLIB_GIO_LIBRARIES
> +#  gobject:         GLIB_GOBJECT_LIBRARIES
> +#  gmodule:         GLIB_GMODULE_LIBRARIES
> +#  gthread:         GLIB_GTHREAD_LIBRARIES
> +#
> +# Note that the respective _INCLUDE_DIR variables are not set, since
> +# all headers are in the same directory as GLIB_INCLUDE_DIRS.
> +#
> +# Copyright (C) 2012 Raphael Kubo da Costa <[e-mail]>
> +#
> +# Redistribution and use in source and binary forms, with or without
> +# modification, are permitted provided that the following conditions
> +# are met:
> +# 1.  Redistributions of source code must retain the above copyright

I would add the copyright notice on the top followed by the docs. Looks like the docs are not subject of the copyright.

[Raphael Kubo da Costa (:rakuco)]

Comment on attachment 157910 [details]
Patch

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

>> Source/cmake/FindGLIB.cmake:26
>> +# 1.  Redistributions of source code must retain the above copyright
> 
> I would add the copyright notice on the top followed by the docs. Looks like the docs are not subject of the copyright.

Would you change your mind if I said this is the usual form for Find modules in CMake itself (they automatically process this header to produce documentation) and is used in other files here in Source/cmake? :-)

[Thiago Marcos P. Santos]

(In reply to comment #3)
> (From update of attachment 157910 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=157910&action=review
> 
> >> Source/cmake/FindGLIB.cmake:26
> >> +# 1.  Redistributions of source code must retain the above copyright
> > 
> > I would add the copyright notice on the top followed by the docs. Looks like the docs are not subject of the copyright.
> 
> Would you change your mind if I said this is the usual form for Find modules in CMake itself (they automatically process this header to produce documentation) and is used in other files here in Source/cmake? :-)

Changed my mind, LGTM. Sorry for the noise.

[Rob Buis]

Comment on attachment 157910 [details]
Patch

LGTM.

[Raphael Kubo da Costa (:rakuco)]

Comment on attachment 157910 [details]
Patch

Clearing flags on attachment: 157910

Committed r125443: <http://trac.webkit.org/changeset/125443>

[Raphael Kubo da Costa (:rakuco)]

All reviewed patches have been landed.  Closing bug.