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

Library install location is wrong #1161

Closed
mwoehlke-kitware opened this issue Jul 27, 2017 · 9 comments · Fixed by #1160
Closed

Library install location is wrong #1161

mwoehlke-kitware opened this issue Jul 27, 2017 · 9 comments · Fixed by #1160

Comments

@mwoehlke-kitware
Copy link
Contributor

There are two issues with the library install location:

  • It doesn't install to the correct arch-dependent location.
  • .dlls are not installed to bin (Windows-only issue).

#1160 fixes the first. For the second, just using DESTINATION is insufficient. The correct invocation is e.g.:

install(TARGETS gtest gtest_main
  RUNTIME DESTINATION bin
  LIBRARY DESTINATION lib${LIB_SUFFIX}
  ARCHIVE DESTINATION lib${LIB_SUFFIX}
)
mwoehlke-kitware added a commit to mwoehlke-kitware/googletest that referenced this issue Jul 27, 2017
Modify library install destinations to install .dll's to the correct
location (`bin`, not `lib`), and to install other artifacts to the
correct platform-dependent location by honoring LIB_SUFFIX. This is
required for some distributions (e.g. Fedora) and will fix an issue that
otherwise requires those distributions to patch the upstream sources.

Fixes google#1161.
@mwoehlke-kitware
Copy link
Contributor Author

I updated #1160 to fix both issues...

d3x0r added a commit to d3x0r/googletest that referenced this issue Jul 31, 2017
Apply changes from google#1161 to support windows better (dll goes to runtime, .exp/.,lib to lib and .lib to lib )
mwoehlke-kitware added a commit to mwoehlke-kitware/googletest that referenced this issue Jul 31, 2017
Modify library install destinations to install .dll's to the correct
location (`bin`, not `lib`), and to install other artifacts to the
correct platform-dependent location by using GNUInstallDirs. This is
required for some distributions (e.g. Fedora) and will fix an issue that
otherwise requires those distributions to patch the upstream sources.
Also, add options to suppress installation, which may be useful for
projects that embed Google Test.

Fixes google#1161.

Co-Authored-By: d3x0r <d3x0r@users.noreply.github.com>
d3x0r added a commit to d3x0r/googletest that referenced this issue Jul 31, 2017
…irs for correct lib target name.

Apply changes from google#1161 to support windows better (dll goes to runtime, .exp/.,lib to lib and .lib to lib )
d3x0r added a commit to d3x0r/googletest that referenced this issue Jul 31, 2017
…irs for correct lib target name.

Apply changes from google#1161 to support windows better (dll goes to runtime, .lib(export) to lib and .lib(static) to lib )
d3x0r added a commit to d3x0r/googletest that referenced this issue Jul 31, 2017
…irs for correct lib target name.

Apply changes from google#1161 to support windows better (dll goes to runtime, .lib(export) to lib and .lib(static) to lib )
d3x0r added a commit to d3x0r/googletest that referenced this issue Jul 31, 2017
…irs for correct lib target name.

Apply changes from google#1161 to support windows better (dll goes to runtime, .lib(export) to lib and .lib(static) to lib )
d3x0r added a commit to d3x0r/googletest that referenced this issue Jul 31, 2017
…irs for correct lib target name.

Apply changes from google#1161 to support windows better (dll goes to runtime, .lib(export) to lib and .lib(static) to lib )
mwoehlke-kitware added a commit to mwoehlke-kitware/googletest that referenced this issue Jul 31, 2017
Modify library install destinations to install .dll's to the correct
location (`bin`, not `lib`), and to install other artifacts to the
correct platform-dependent location by using GNUInstallDirs. This is
required for some distributions (e.g. Fedora) and will fix an issue that
otherwise requires those distributions to patch the upstream sources.
Also, add options to suppress installation, which may be useful for
projects that embed Google Test.

Fixes google#1161.

Co-Authored-By: d3x0r <d3x0r@users.noreply.github.com>
@mwoehlke-kitware
Copy link
Contributor Author

mwoehlke-kitware commented Aug 7, 2017

how you run the install before the fix and how you run the install after the proposed fix.

Um... ninja install? How else would I "run the install"?

What platforms did you test this on?

Tested on Fedora 26, with BUILD_SHARED_LIBS=ON. I compared output of find on the install trees: as expected, the only change is that libraries installed to lib64 (correct on Fedora) instead of lib (wrong on Fedora).

@gennadiycivil
Copy link
Contributor

Thank you for the prompt response. Since this fix is also relevant on Windows we need to make sure that Windows testing has been included.

@mwoehlke-kitware
Copy link
Contributor Author

I tried on Windows...

Without #1160:

-- Installing: .../build/googletest/install/lib/gtest.lib
-- Installing: .../build/googletest/install/lib/gtest.dll
-- Installing: .../build/googletest/install/lib/gtest_main.lib
-- Installing: .../build/googletest/install/lib/gtest_main.dll

This is wrong; the .dlls should be in bin.

With #1160:

-- Installing: .../build/googletest/install/lib/gtest.lib
-- Installing: .../build/googletest/install/bin/gtest.dll
-- Installing: .../build/googletest/install/lib/gtest_main.lib
-- Installing: .../build/googletest/install/bin/gtest_main.dll

This is correct; the .libs are in lib, the .dlls are in bin.

mwoehlke-kitware added a commit to mwoehlke-kitware/googletest that referenced this issue Aug 9, 2017
Modify library install destinations to install .dll's to the correct
location (`bin`, not `lib`), and to install other artifacts to the
correct platform-dependent location by using GNUInstallDirs. This is
required for some distributions (e.g. Fedora) and will fix an issue that
otherwise requires those distributions to patch the upstream sources.
Also, add options to suppress installation, which may be useful for
projects that embed Google Test.

Since Google Test is trying to support archaic versions of CMake, a
brain-dead fallback (which requires that the user set either LIB_SUFFIX
or CMAKE_INSTALL_LIBDIR themselves) is included for versions that
predate GNUInstallDirs.

Fixes google#1161.

Co-Authored-By: d3x0r <d3x0r@users.noreply.github.com>
@SylvainCorlay
Copy link

@mwoehlke-kitware why not use include(GNUInstallDirs) instead of using manually set bin, lib, lib.

@mwoehlke-kitware
Copy link
Contributor Author

@SylvainCorlay, #1160 does do that...

@SylvainCorlay
Copy link

My bad. Sorry for the noise.

@Cylix
Copy link

Cylix commented Aug 16, 2017

FYI, as reported in #1198 , this change break the installation of gtest by not installing the headers anymore.

@gennadiycivil
Copy link
Contributor

@mwoehlke-kitware could you please take a look, @Cylix reported

FrankieLi added a commit to lunasolutions/googletest that referenced this issue Dec 6, 2017
* Correct some typos in a comment

* fix small typo in comment

* add note about different definitions of Test Case

There are contradictory definitions of the term "test case", so prepare
new users in Primer.md to avoid confusion.

* Samples changes upstreaming

* Samples changes upstreaming

* WIP

* WIP, windows testing

* WIP, windows testing

* WIP, windows testing

* WIP, win testing

* WIP, win testing

* Punctuation

Missing periods

* be more specific on Test Case

* WIP

* Added googlemock tests

* Added googlemock tests

* Infinite Loop when calling a mock function that takes boost::filesystem::path as parameter google#521: Add is_same type trait

* Infinite Loop when calling a mock function that takes boost::filesystem::path as parameter google#521: Add is_same type trait and prevent infinite loops for recursive containers

* WIP

* Fix library install destinations

Modify library install destinations to install .dll's to the correct
location (`bin`, not `lib`), and to install other artifacts to the
correct platform-dependent location by using GNUInstallDirs. This is
required for some distributions (e.g. Fedora) and will fix an issue that
otherwise requires those distributions to patch the upstream sources.
Also, add options to suppress installation, which may be useful for
projects that embed Google Test.

Since Google Test is trying to support archaic versions of CMake, a
brain-dead fallback (which requires that the user set either LIB_SUFFIX
or CMAKE_INSTALL_LIBDIR themselves) is included for versions that
predate GNUInstallDirs.

Fixes google#1161.

Co-Authored-By: d3x0r <d3x0r@users.noreply.github.com>

* clarify distinction regarding Test Case

* Adding a flag option to change the default mock type

* Initial Revision, review 164634031

* Added Copyright

* Minor style fixes

Typos, punctuation & broken links

* say "former version" instead of "released version"

* Update WORKSPACE

Remove comments

* Addressing comments

* Addressing Comments

* Add support for pkgconfig

* Add documentation for pkg-config

* Use GTEST_LOG instead of printf

* Removed extra colon in error log

* Change AppVeyor Status Badge to point to new AppVeyor Project Location

* Change AppVeyor Status Badge to point to new AppVeyor Project Location

* Fix problem installing gtest when gmock enabled

Fix a bug deciding whether to enable the option to install Google Test
caused by one of the dependent option dependencies not being defined
yet.

Fixes google#1198; impossible to install Google Test if Google Mock is built.

* Add function name to exception if there's no default action

* Handling invalid flag values

* Update README.md

Another AppVeyor move

* adds test for NiceMock with unknown return value

* Fix test if exceptions are not supported

* Switch return type to class without default constructor

* Change tabs to spaces in test case

* Adding CMake visibility policy setting

This policy setting will silence a warning when using with a visibility settings on targets. Due to the forced `cmake_minimum_version`, policy settings in CMakeLists calling this one (including the main CMakeLists) are lost, forcing the change to be made here.

* Proposing these changes, please review

Slightly better names and cleaner tests.
Please review

* Added "explicit" as per compiler suggestion

* Remove unused variable

* Support ref-qualified member functions in Property().

* Support x64 configuration for old VS2010 projects

VS2010 solution only to simplify old users (who used these solutions) upgrading to new gtest/gmock, new users should use CMake generated solutions. VS2010 solution can be opened in any new VS.

* Remove gtest VS2005 projects

* Support x64 configuration for old VS2015 projects

* Applying lint checks from upstream google3

* fix typo in comment and string (SetUpTestCase)

* remove unused TestCase import

* fix typo: xUnit

* remove non-existing gmock_build_samples switch

* switch on verbose make

* run combined build only

There is no need for separate 'googlemock' and 'googletest' builds,
as the 'googlemock' build includes 'googletest' and it's unit tests.

* remove Yob's comma mentioned in issue google#1105

* use plural verb as mentioned in issue google#1105

* Detect Fuchsia, and set GTEST_HAS_PTHREAD on GTEST_OS_FUCHSIA

* use build type set in .travis.yml

The BUILD_TYPE variable of .travis.yml was ignored up to now.

* use upper-case build type

While cmake does not complain on build type 'debug', the cmake
documentation always spells it 'Debug', so take this.

* fix SetUp/TearDownTestCase() in AdvancedGuide

fixes issue google#1087

* remove obsolete comment regarding python tests on linux

* ask cmake for per-configuration output subdir

On single-configuration build systems as Makefile Generators, there is
no subdirectory for the configuration in the build tree - therefore ask
cmake for the subdir by using CMAKE_CFG_INTDIR, which is just '.' on
single-configuration build systems (Linux et al.).

* Revert "ask cmake for per-configuration output subdir"

This reverts commit 73d58dd.

Unfortunately, ${CMAKE_CFG_INTDIR} is set during build only and doesn't
help here.

* create different python based tests for single and multi configuration build generators

* Note that it is preferable for Googlers to create a CL internally first

* removed internal link ( not allowed in OSS)

* Removed "Trivial" 

Who knows? may not be very trivial given the code drift between internal and OSS

* AppVeyor MinGW-w64 test build

* fix example's comment

* change links from former code.google.com to current github repository

* CMake: use threads if allowed and found, not just if found.

If the user's cmakelists.txt first look for threads using
find_package(Threads), then set(gtest_disable_pthreads ON),
and then include googletest. GoogleTest will not look for
threads. But since they have already been found before in
user's cmakelists, it will use them regardless.

This helped me fix build issue in darktable-org/rawspeed
on windows/MSYS2, even though there are threads, and they
are usable, googletest build was failing with issues
about AutoHandle. I was first looking for threads, and only
then including googletest, so no matter the value of
gtest_disable_pthreads, it failed.

The other obvious solution is for user to first include
googletest, and only then look for threads by himself.

* switch one build to Release mode

This turns on optimization which allows the compiler to discover more
problems and omit some more warnings.

* treat all warnings as errors for GCC (-Werror)

* Allow macros inside of parametrized test names.

This allows doing things like TEST_P(TestFixture, MAYBE(TestName)) for nicer conditional test disabling.

* Add a non-parametrized test.

* avoid -Wshadow warning on GCC

When using INSTANTIATE_TEST_CASE_P with a lambda function which uses
'info' as parameter name, GCC complains that this would shadow
parameter 'info' used in the macro's VA_ARGS call.

* avoid warning about unused variable

* cache ccache

* set MAKEFLAGS to use multiple processors on Travis CI

* install ccache on travis osx build slave

* limit processors to use in Travis build to 4

* remove obsolete link_directories command

It's not necessary, as the target_link_libraries command contains an
absolute path already, and the path given doesn't exist anymore,
leading only to linker warnings like:
ld: warning: directory not found for option
'-L/Users/travis/build/google/googletest/build/googlemock/gtest/src'

* add a cast

* show ccache statistics in log

* call clang via ccache on Linux

* reset ccache statistics at install

* drop unused valgrind package from installation

... and remove explicit gcc installation (will be installed with g++
automatically)

* remove unused variables from travis environment

* Removed flush scopes around GTEST_LOG(FATAL) and exit call since FATAL is expected to abort()

* use GTEST_ATTRIBUTE_UNUSED_ instead of dummy function

* Fix ellipsis position in examples

* Make the failure messages from EXPECT_EQ and friends actually symmetric,

instead of reading more like reversing the former "expected" and "actual"
roles of the LHS and RHS arguments.

This patch is manually applied from internal version (125109873)

* Add explicit `CMAKE_DEBUG_POSTFIX` option

Enable generating different library name to be compatible with CMake's `FindGTest`.

* Remove redundant declaration

TempDir() function is declared twice, once in `internal/gtest-port.h`
and a second time in `gtest.h`.

Fixes a warning with GCC when -Wredundant-decls is given.

* Swap reinterpret_cast for static_cast

Swap reinterpret_cast for static_cast

* Revert "Allow macros inside of parametrized test names."

* Use gender-neutral pronouns in comments and docs

* Updated README with information about C runtime dynamic/static linking issues in Windows

* google#1282: Doc typo fix

* Workaround for Travis issue travis-ci/travis-ci#8552

* Revert "Workaround for Travis issue https://github.com/travis-ci/travis-ci/is…"

* Workaround for Travis issue https://goo.gl/d5eV8o

* googletest: Add GTEST_API_ attribute to ThreadLocal class.

ThreadLocal class needs to be have default visibility.
Root cause is gtest uses typeinfo for the ThreadLocal class.
The problem manifests When gtest/gmock are built as a shared library
with libc++. When a class is used in typeinfo, it must have default
visibility.

There is an explanation about typeinfo and visibility here:
https://developer.apple.com/library/content/documentation/DeveloperTools/Conceptual/CppRuntimeEnv/Articles/SymbolVisibility.html

When libc++ is used with gtest in shared library mode, any tests
that are compiled with -fvisibility=hidden and exercise the
macro EXPECT_CALL, it results in an abort like:
[ FATAL ] /usr/include/gtest/internal/gtest-port.h:1394::
Condition typeid(*base) == typeid(Derived) failed.
This is because the typeinfo for ThreadLocal class is not visible.
Therefore, linker failed to match it to the shared library symbol, creating a
new symbol instead.

This fixes google#1207.

* Enable C++11 features for VS2015 and VS2017

* Fix tests with VS2015 and VS2017

* Fix gmock tests when std::unary_function unavailable

* Remove gcc 6 misleading indentations.

* Enable CI for VS2017

* remove markdown stars (bold) from code examples

* Fixes issue google#826 by treating MinGW as "non-Windows" when determining colored output
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 a pull request may close this issue.

4 participants