After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 773657 - [PATCH] Fix JHBuild issues on FreeBSD after switching to cmake
[PATCH] Fix JHBuild issues on FreeBSD after switching to cmake
Status: RESOLVED FIXED
Product: evolution-data-server
Classification: Platform
Component: general
3.23.x (obsolete)
Other FreeBSD
: Normal normal
: ---
Assigned To: Evolution Shell Maintainers Team
Evolution QA team
Depends on:
Blocks:
 
 
Reported: 2016-10-29 05:10 UTC by Ting-Wei Lan
Modified: 2016-11-02 19:04 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Read DB_CFLAGS and DB_LIBS from cmake options instead of environment variables (3.27 KB, patch)
2016-10-29 05:11 UTC, Ting-Wei Lan
none Details | Review
Add several versioned names of db_load (920 bytes, patch)
2016-10-29 05:11 UTC, Ting-Wei Lan
none Details | Review
Unset HAVE_KRB5 before running another CHECK_C_SOURCE_COMPILES test (1.25 KB, patch)
2016-10-29 05:11 UTC, Ting-Wei Lan
none Details | Review
Bug 773657 - Fix JHBuild issues on FreeBSD after switching to cmake (3.46 KB, patch)
2016-10-31 17:44 UTC, Ting-Wei Lan
reviewed Details | Review

Description Ting-Wei Lan 2016-10-29 05:10:12 UTC
Evolution-data-server cannot be successfully built on FreeBSD after switching to cmake. I will attach some patches here to fix errors or warnings found when running JHBuild.
Comment 1 Ting-Wei Lan 2016-10-29 05:11:09 UTC
Created attachment 338765 [details] [review]
Read DB_CFLAGS and DB_LIBS from cmake options instead of environment variables

Unlike the configure script generated by autotools, cmake doesn't allow
setting environment variables on its command line. This means we can no
longer use environment variables as the only way to specify required
settings because adding environment variables is not supported in
JHBuild modulesets.

Besides, autotools automatically caches used variables in generated
makefiles, while cmake never caches environment variables. As these
project-specific environment variables may be unset most of the time
and re-configuration can be triggered without user confirmation, values
specified with environment variables are likely to lose when cmake is
re-run at unexpected time.

This commit also makes libdb detection failure be fatal, so the build
will stop immediately after failing the test, preventing unexpected
compiling or linking error from happening.
Comment 2 Ting-Wei Lan 2016-10-29 05:11:15 UTC
Created attachment 338766 [details] [review]
Add several versioned names of db_load

Some distributions don't have a default version of Berkeley DB and the
name of db_load is always versioned. We only add names for version 5 to
the list because most distributions use that version as their default.
Comment 3 Ting-Wei Lan 2016-10-29 05:11:21 UTC
Created attachment 338767 [details] [review]
Unset HAVE_KRB5 before running another CHECK_C_SOURCE_COMPILES test

CHECK_C_SOURCE_COMPILES does nothing if the result variable is already
defined, so we must unset the variable from the cache if we want to
reuse the same variable name.
Comment 4 Milan Crha 2016-10-31 14:20:23 UTC
Review of attachment 338765 [details] [review]:

Thanks for a bug report and patches. If we are going away from the environment variables, then I'll call the options WITH_LIBDB_CFLAGS and WITH_LIBDB_LIBS, simply because there are too many databases (the 'DB' prefix is too ambiguous). I wanted to keep he behaviour as close to the autotools as possible, but I didn't think of the environment variables being such a hassle.
Comment 5 Milan Crha 2016-10-31 14:28:19 UTC
The other patches look fine. Do you want me to do the change, or you'll do it? If you'll do, please merge the patches into one. Thanks in advance.
Comment 6 Ting-Wei Lan 2016-10-31 17:44:57 UTC
Created attachment 338834 [details] [review]
Bug 773657 - Fix JHBuild issues on FreeBSD after switching to cmake
Comment 7 Ting-Wei Lan 2016-10-31 17:48:06 UTC
I think making it possible to specify BDB options with -D arguments is important because we need it to work in JHBuild. I will update the JHBuild moduleset after the patch gets accepted and committed.
Comment 8 Milan Crha 2016-10-31 20:28:26 UTC
Review of attachment 338834 [details] [review]:

Thanks for the update. It looks fine, except of some typos in the error message. The other comments are for discussion and have nothing to do with this change (except I noticed it when reading the change, and I'd like to know your opinion on it).

::: CMakeLists.txt
@@ +461,2 @@
 if(NOT ("${WITH_LIBDB}" STREQUAL "NO"))
 	if(NOT (("${WITH_LIBDB}" STREQUAL "") OR ("${WITH_LIBDB}" STREQUAL "YES")))

This is nothing with your change, that is fine, I only noticed an inconsistency I caused in this code path. First of all, I claim everywhere "ON" and "OFF" values, but it's checked here for "YES" and "NO", which I think is wrong. Right?

@@ +473,3 @@
 	endif(NOT (("${WITH_LIBDB}" STREQUAL "") OR ("${WITH_LIBDB}" STREQUAL "YES")))
 
 	set(CMAKE_REQUIRED_FLAGS ${LIBDB_CFLAGS})

Is the above line correct? In the light of your other proposals, I'd say it should read:
   set(CMAKE_REQUIRED_INCLUDE_DIRS ${LIBDB_CFLAGS})
instead.

@@ +481,3 @@
+
+	if(NOT HAVE_LIBDB)
+		message(FATAL_ERROR "libdb not found. Use -DWITH_LIBDB=PATH to specify to specify the library prefix, or use -DDB_CFLAGS=-I/path/to/db/include and -DDB_LIBS=/path/to/db/lib to specify arguments for compiling and linking. If you want to disable libdb, please use -DWITH_LIBDB=OFF.")

Here are multiple typos - easy to fix
Comment 9 Milan Crha 2016-10-31 20:29:35 UTC
(In reply to Ting-Wei Lan from comment #7)
> I think making it possible to specify BDB options with -D arguments is
> important because we need it to work in JHBuild.

Sure, I've no problem with the change (I hope it didn't sound like that).
Comment 10 Milan Crha 2016-11-02 12:30:17 UTC
I fixed the typos and committed your change as noted below. I corrected "if(NOT ("${WITH_LIBDB}" STREQUAL "NO"))" and I left the "set(CMAKE_REQUIRED_FLAGS ${LIBDB_CFLAGS})" as it is, that in the second commit.

Created commit b54653c in eds master (3.23.2+)
Created commit dff0abc in eds master (3.23.2+)
Comment 11 Ting-Wei Lan 2016-11-02 19:04:49 UTC
(In reply to Milan Crha from comment #8)
> Review of attachment 338834 [details] [review] [review]:
> 
> Thanks for the update. It looks fine, except of some typos in the error
> message. The other comments are for discussion and have nothing to do with
> this change (except I noticed it when reading the change, and I'd like to
> know your opinion on it).
> 
> ::: CMakeLists.txt
> @@ +461,2 @@
>  if(NOT ("${WITH_LIBDB}" STREQUAL "NO"))
>  	if(NOT (("${WITH_LIBDB}" STREQUAL "") OR ("${WITH_LIBDB}" STREQUAL "YES")))
> 
> This is nothing with your change, that is fine, I only noticed an
> inconsistency I caused in this code path. First of all, I claim everywhere
> "ON" and "OFF" values, but it's checked here for "YES" and "NO", which I
> think is wrong. Right?

Yes, you are right. I didn't find this because I never tried to disable it.

> 
> @@ +473,3 @@
>  	endif(NOT (("${WITH_LIBDB}" STREQUAL "") OR ("${WITH_LIBDB}" STREQUAL
> "YES")))
>  
>  	set(CMAKE_REQUIRED_FLAGS ${LIBDB_CFLAGS})
> 
> Is the above line correct? In the light of your other proposals, I'd say it
> should read:
>    set(CMAKE_REQUIRED_INCLUDE_DIRS ${LIBDB_CFLAGS})
> instead.

I didn't notice this because this patch was made before I started to debug the -D problem. (I have to fix evolution-data-server problems before I can try to build evolution.) The problem reported in that bug report is caused by specifying -D option with a list. If arguments are separated by spaces instead of semicolons, it works fine, so I think this doesn't cause problem here because users never pass CFLAGS as a list.

(In reply to Milan Crha from comment #10)
> I fixed the typos and committed your change as noted below. I corrected
> "if(NOT ("${WITH_LIBDB}" STREQUAL "NO"))" and I left the
> "set(CMAKE_REQUIRED_FLAGS ${LIBDB_CFLAGS})" as it is, that in the second
> commit.
> 
> Created commit b54653c in eds master (3.23.2+)
> Created commit dff0abc in eds master (3.23.2+)

Thanks for fixing and committing these patches.