Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[#43] Use system gtest #102

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

[#43] Use system gtest #102

wants to merge 3 commits into from

Conversation

iblislin
Copy link
Contributor

The self-shipped gmockremoved!

Hi @Chocobo1 !
Here is a patch for AUR.

diff --git a/PKGBUILD b/PKGBUILD
index a2c4d70..de2da0c 100644
--- a/PKGBUILD
+++ b/PKGBUILD
@@ -1,14 +1,14 @@
 # Maintainer: Chocobo1

 pkgname=chewing-editor-git
-pkgver=0.0.2.r17.gd9b7f53
-pkgrel=2
+pkgver=0.0.2.r33.g633b2b0
+pkgrel=1
 pkgdesc="Cross platform chewing user phrase editor"
 arch=('i686' 'x86_64')
 url="http://chewing.im"
 license=('GPL')
 depends=('libchewing' 'qt5-base' 'hicolor-icon-theme')
-makedepends=('git' 'cmake' 'qt5-tools' 'help2man')
+makedepends=('git' 'cmake' 'qt5-tools' 'help2man' 'gtest')
 install=$pkgname.install
 source=("git://github.com/chewing/chewing-editor.git")
 md5sums=('SKIP')
@@ -29,13 +29,13 @@ build() {
     cd "chewing-editor/build"

     cmake ../ -DCMAKE_INSTALL_PREFIX='/usr'
-    make
+    make -j
 }

 check() {
     cd "chewing-editor/build"

-    ./run-test
+    make -j check
 }

 package() {

@coveralls
Copy link

Coverage Status

Coverage remained the same at 94.382% when pulling dcbcbda on iblis17:sys-gtest into 633b2b0 on chewing:master.

@Chocobo1
Copy link
Member

Thanks for the AUR patch!
some notes:

  1. make -j: using -j without limit is dangerous, could exhaust the memory and lead to race conditions under certain circumstances
    also see: https://bbs.archlinux.org/viewtopic.php?pid=1391531#p1391531
  2. I plan to update chewing-editor-git after Use gzip to compress the manpage #69 is merged.
  3. not familiar with gtest, I'll let other admins review that...

@czchen
Copy link
Member

czchen commented Mar 21, 2016

Is it possible to use https://cmake.org/cmake/help/v3.0/module/FindGTest.html to find gtest in ubuntu build?

@iblislin
Copy link
Contributor Author

Hi @czchen !
Accutally, using the cmake module do not fix the problem in ubuntu. I made anothor branch to test that:
commit and travis.
The problem in ubuntu is that the package do not ship the share library.

But i think using the cmake module is still a goo idea, i will amend my commit later.

If you really want to hide this problem behind users, i will investigate on static linking.

@czchen
Copy link
Member

czchen commented Mar 22, 2016

@iblis17 I think the problem is that cmake FindGTest script cannot find the source only gtest in system. We need to patch the FindGTest script so that it can find the source only gtest in system.

@iblislin
Copy link
Contributor Author

:-o ok, i will make it, but i do not patch module before.
Any tips for that?

We need to make a custom module right?

@yan12125
Copy link
Collaborator

I think the problem is that cmake FindGTest script cannot find the source only gtest in system. We need to patch the FindGTest script so that it can find the source only gtest in system.

I managed to handle source-only gtest in system without patching FindGTest: yan12125@d199ecb. It can handle source-only /usr/src/gtest on Ubuntu 16.04, source-only /usr/src/googletest on Ubuntu 18.04 and static library /usr/lib/x86_64-linux-gnu/libgtest.a on Ubuntu 20.04. See Travis CI test results at https://travis-ci.org/github/yan12125/chewing-editor/builds/706409508 for build logs for various systems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants