Opened 3 years ago

Closed 3 years ago

#5057 closed enhancement (complete)

configure script does not detect gtest-1.8.0

Reported by: tomek Owned by: fdupont
Priority: low Milestone: Kea1.2
Component: build system Version: git
Keywords: Cc:
CVSS Scoring: Parent Tickets:
Sensitive: no Defect Severity: N/A
Sub-Project: DHCP Feature Depending on Ticket:
Estimated Difficulty: 0 Add Hours to Ticket: 0
Total Hours: 0 Internal?: no

Description (last modified by tomek)

After several years, gtest-1.8.0 has been published. However, Kea does not detect it, because the code layout has changed in gtest-1.8.0. Instead of having their scripts in scripts/, they now reside in googletest/scripts/. As a result, the Kea's configure script fails to find it and refuses to compile.

An easy workaround for the configure check is to pass --with-gtest-source=/path/to/gtest-1.8.0/googletest, but the knowledge of internal gtest source code structure should not be necessary. This allows user to start building procedure, but then it fails to link with the following error:

Making all in tests
make[5]: Wejście do katalogu '/home/thomson/devel/kea3/src/lib/exceptions/tests'
  CXX      run_unittests-run_unittests.o
  CXX      run_unittests-exceptions_unittest.o
  CXXLD    run_unittests
/usr/bin/ld: cannot find -lgtest
collect2: error: ld returned 1 exit status

when it first tries to link with gtest library.

Subtickets

Change History (15)

comment:1 Changed 3 years ago by tomek

  • Description modified (diff)

comment:2 Changed 3 years ago by tomek

  • Description modified (diff)

comment:3 Changed 3 years ago by fdupont

I don't understand what is exactly the problem: usually either the system packages gtest (note it should not according to gtest documentation), or gtest is used from sources.
In the second case (I often use it because the package is incomplete, is not available or provides a too old version) with the correct -with-gtest-source it works well even the static library is built and can give some minor warnings from libtool. So IMHO the ticket is about the first case.
If the package layout changed with 1.8.0 we need to know what is the system (BTW perhaps it is one I believe the package is wrong when the problem is different, cf the last Ubuntu?)
To finish IMHO this ticket should be in 1.2.

comment:4 Changed 3 years ago by hschempf

  • Milestone changed from Kea-proposed to Kea1.2

Per Oct 27 Kea team meeting, accept 1.2

comment:5 Changed 3 years ago by tomek

  • Priority changed from medium to low

I'm not sure whether the issue was coming from me using --with-gtest rather than --with-gtest-source or perhaps it was Marcin's commit (cac049fc04914a52e88a2247a6afaab272aef09d) that improved the situation, but now it's possible to build with gtest 1.8.0.

There are some outstanding issues, though. In particular, --with-gtest pointed to 1.8.0/googletest directory completes configure, but then fails to link. Also, gtest version is reported as unknown.

As such, decreasing priority to low.

comment:6 Changed 3 years ago by fdupont

  • Owner set to fdupont
  • Status changed from new to accepted

Trying to reproduce a bug (without success, i.e., the bug was fixed) on Ubuntu 16.04 I got a similar error so I take the ticket. BTW about google test the Makefile.am in ext/gtest builds only the static/archive library so libtool complains (without bad consequences but this should be fixed, BTW there is a flag to set too for dynamic builds). I'll try to fix the version report too.

comment:7 Changed 3 years ago by fdupont

A first look shows configure looks for the include but not the library. For the version configure relies only on gtest-config.

comment:8 Changed 3 years ago by fdupont

  • Owner changed from fdupont to UnAssigned
  • Status changed from accepted to reviewing

I added a check for the library. The ticket is available for review but perhaps I'll try to fix some other points too so the commit is 8d3603d5ab4ce60566e62d8f827d5b4bfd579cdc

comment:9 Changed 3 years ago by fdupont

I give up about the version as gtest 1.8.0 has 1.7.0 set in its configure.ac... So it is not reliable.

comment:10 Changed 3 years ago by fdupont

I propose 2 changes:

  • use LDADD (vs LDFLAGS) for the libgtest.a built from sources
  • use the KEA_CXXFLAGS to build this library in order to be sure -fPIC is included.

comment:11 Changed 3 years ago by stephen

  • Owner changed from UnAssigned to stephen

comment:12 follow-up: Changed 3 years ago by stephen

  • Owner changed from stephen to fdupont

Reviewed commit 8d3603d5ab4ce60566e62d8f827d5b4bfd579cdc

I can see that the extra check has been added, but to complete this ticket, we can do more.

Since we know that gtest-1.8 is now in a "goggletest" subdirectory, the configure.ac should be changed to look for files first in the directory specified in the --with-gtest or --with-gtest-source and then, if the files are not found, in the "googletest" subdirectory of those directories.

comment:13 in reply to: ↑ 12 Changed 3 years ago by fdupont

  • Owner changed from fdupont to stephen

Replying to stephen:

Reviewed commit 8d3603d5ab4ce60566e62d8f827d5b4bfd579cdc

I can see that the extra check has been added, but to complete this ticket, we can do more.

Since we know that gtest-1.8 is now in a "goggletest" subdirectory, the configure.ac should be changed to look for files first in the directory specified in the --with-gtest or --with-gtest-source and then, if the files are not found, in the "googletest" subdirectory of those directories.

=> I agree for --with-gtest-source but not for --with-gtest because in the second Google test was packaged. Done.

comment:14 Changed 3 years ago by stephen

  • Owner changed from stephen to fdupont

Reviewed commit 72961fc6df644c74ce2b6359234a2ddb6f24dc6c

All OK, please merge.

comment:15 Changed 3 years ago by fdupont

  • Resolution set to complete
  • Status changed from reviewing to closed

Merged. Closing.

Note: See TracTickets for help on using tickets.