trunks package failing to compile due to broken #include <gmock/gmock.h> |
||||
Issue descriptionThe last two builds of reef-chrome-pfq at https://uberchromegw.corp.google.com/i/chromeos/builders/reef-chrome-pfq have failed in BuildPackages due in part to problems with the chromeos-base/trunks package: trunks-0.0.1-r1909: FAILED: aosp/system/tpm/trunks/trunks_test.mock_authorization_delegate.o trunks-0.0.1-r1909: x86_64-cros-linux-gnu-clang++ -MMD -MF aosp/system/tpm/trunks/trunks_test.mock_authorization_delegate.o.d -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -I/build/reef/tmp/portage/chromeos-base/trunks-0.0.1-r1909/work/trunks-0.0.1/aosp/system/tpm -Iaosp/system/tpm/trunks/trunks_test.gen/include -Igen/include -I/build/reef/tmp/portage/chromeos-base/trunks-0.0.1-r1909/work/trunks-0.0.1/platform2 -I/build/reef/tmp/portage/chromeos-base/trunks-0.0.1-r1909/work/trunks-0.0.1/platform -I/build/reef/usr/include -Wall -Wno-psabi -Wunused -Wno-unused-parameter -ggdb3 -fstack-protector-strong -Wformat=2 -fvisibility=internal -Wa,--noexecstack -Werror --sysroot=/build/reef -DUSE_RTTI_FOR_TYPE_TAGS -Wno-c++11-extensions -Wno-unused-local-typedefs -DBASE_VER=395517 -pthread -I/build/reef/usr/include/chromeos -I/build/reef/usr/include/base-395517 -I/build/reef/usr/include/glib-2.0 -I/build/reef/usr/lib64/glib-2.0/include -I/build/reef/usr/include/nss -I/build/reef/usr/include/nspr -I/build/reef/usr/include/dbus-1.0 -I/build/reef/usr/lib64/dbus-1.0/include -fPIE -std=gnu++11 -DVCSID='"0.0.1-r1909-52b19713e49d1867a2c44ea11d7089e900d8730e"' -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -O2 -pipe -O2 -pipe -O2 -pipe -march=corei7 -g -fno-exceptions -fno-unwind-tables -fno-asynchronous-unwind-tables -clang-syntax -clang-syntax -fno-exceptions -fno-unwind-tables -fno-asynchronous-unwind-tables -c ../../../../../../../tmp/portage/chromeos-base/trunks-0.0.1-r1909/work/trunks-0.0.1/aosp/system/tpm/trunks/mock_authorization_delegate.cc -o aosp/system/tpm/trunks/trunks_test.mock_authorization_delegate.o trunks-0.0.1-r1909: In file included from ../../../../../../../tmp/portage/chromeos-base/trunks-0.0.1-r1909/work/trunks-0.0.1/aosp/system/tpm/trunks/mock_authorization_delegate.cc:17: trunks-0.0.1-r1909: /build/reef/tmp/portage/chromeos-base/trunks-0.0.1-r1909/work/trunks-0.0.1/aosp/system/tpm/trunks/mock_authorization_delegate.h:23:10: fatal error: 'gmock/gmock.h' file not found trunks-0.0.1-r1909: #include <gmock/gmock.h> trunks-0.0.1-r1909: ^~~~~~~~~~~~~~~ trunks-0.0.1-r1909: 1 error generated. I think that the problem is that trunks.gyp defines a trunks_test target that isn't hidden behind a check for the test USE flag.
,
Jul 10 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/ba2feb1f0a849d21fae99e4fe222025d2c103859 commit ba2feb1f0a849d21fae99e4fe222025d2c103859 Author: Daniel Erat <derat@chromium.org> Date: Mon Jul 10 18:26:22 2017 trunks: Add gmock to DEPEND. The trunk package unconditionally installs libtrunks_test.a, which includes code that depends on gmock. BUG= chromium:740549 TEST=none Change-Id: I807dbab91e38f9332585df38eefd86625a3a048e Reviewed-on: https://chromium-review.googlesource.com/565043 Commit-Queue: Dan Erat <derat@chromium.org> Tested-by: Dan Erat <derat@chromium.org> Trybot-Ready: Dan Erat <derat@chromium.org> Reviewed-by: Eric Caruso <ejcaruso@chromium.org> Reviewed-by: Mike Frysinger <vapier@chromium.org> Reviewed-by: Greg Levin <glevin@chromium.org> [modify] https://crrev.com/ba2feb1f0a849d21fae99e4fe222025d2c103859/chromeos-base/trunks/trunks-9999.ebuild
,
Jul 20 2017
libtrunks_test.a is used by aosp/system/tpm/{attestation,tpm_manager}. They link to it for common things like TrunksFactoryForTest when building attestation_testrunner/tpm_manager_testrunner.
Since attestation/tpm_manager can be tested independently from trunks, they can have USE=test when trunks was built w/o USE=test.
Is there a better way to achieve having a common test lib built by one package and used by another?
,
Jul 20 2017
,
Jul 20 2017
My understanding is that setting FEATURES=test when running emerge both sets the test USE flag and results in the src_test phase being run. Can we make the attestation and tpm_manager ebuilds require that trunks was built with USE=test when they themselves are built with USE=test? Alternately, it's probably cleaner to introduce a new trunks-test package that builds and installs libtrunks_test.a (or better, make it into a shared library). Then attestation and tpm_manager can just depend on that when they're built with USE=test.
,
Jul 20 2017
> Can we make the attestation and tpm_manager ebuilds require that trunks was built with USE=test when they themselves are built with USE=test? Should that work with UnitTest stage run by builders? Do they just emerge everything with FEATURES=test? > Alternately, it's probably cleaner to introduce a new trunks-test package that builds and installs libtrunks_test.a (or better, make it into a shared library). Then attestation and tpm_manager can just depend on that when they're built with USE=test. Trunks own unit tests also need libtrunks_test.a. We could also move them (trunks_testrunner target in trunks.gyp) to that trunks-test package, but that itself may be confusing to those who want to run those tests manually against the trunks package after making a change.
,
Jul 20 2017
Btw, I do see mocks (though, just .h) installed as a part of non-test builds in other cases. Any ebuild that uses platform_install_dbus_client_lib (e.g power_manager-client): does
doins "${OUT}/gen/include/${libname}/dbus-proxy-mocks.h"
Maybe it's just a question of choosing a better name? Something like libtrunks_test -> libtrunks_client_mocks.
libtrunks_test consists of mocks for various interface classes + an implementation of TrunksFactory that returns those mocks (or lets the tests inject their own instances).
,
Jul 21 2017
No, I don't think it's a naming problem. We shouldn't be including code that's only used for testing (including mock implementations) in non-test images. It's a waste of space, especially when it means we also pull in other test-specific libraries like gmock. Headers aren't a problem as far as I'm aware since we strip them out from the image.
,
Jul 31 2017
in the attestation/tpm_manager ebuilds, write the dependency like: chromeos-base/trunks[test?] that should force portage to DTRT here. then you can make all the code in trunks conditional upon USE=test.
,
Aug 30 2017
A related issue 760657 - at least the "tpm_manager missing gmock/gmock.h" part. Btw, trunks is not built for tpm 1.2 ("+tpm -tpm2").
I do see gmock in BuildPackages, though:
[binary N ] dev-cpp/gmock-1.8.0 to /build/whirlwind/ USE="{-test}" 473 KiB
Fetched dev-cpp/gmock-1.8.0 in 6.36s
So, maybe it's the "USE=-test" part, and tpm_manager shouldn't build mocks in this case? That may trigger further missing dependencies on tpm_manager mocks when "-test" (from attestation, cryptohome, ap-daemons, anything else?), so will need to trace all.
,
Aug 30 2017
I tried to fix the dependency in https://chromium-review.googlesource.com/c/chromiumos/overlays/chromiumos-overlay/+/565960, but it produces build errors that I don't fully understand.
,
Oct 6 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/chromite/+/f30e1ea61ffd70600ff4f66ea7885510bbb3ce2a commit f30e1ea61ffd70600ff4f66ea7885510bbb3ce2a Author: Mike Frysinger <vapier@chromium.org> Date: Fri Oct 06 22:51:10 2017 cros_run_unit_tests: drop --nodeps --buildpkgonly This breaks our ability to pull in packages based on USE=test and otherwise change their build time behavior. These options have been enabled since the beginning of this script. Before CL:1684, we used `ebuild` to manually run the test phase on specific packages and avoid building/merging binpkgs. After that, we switched to emerge and --nodeps/--buildpkgonly to largely get the same behavior. By dropping --nodeps, we allow additional packages to get pulled in that otherwise would require manual listing in the deptree, either as an unconditional dep in the package, or in a common virtual which doesn't scale. It also means we run the test phases for those deps, but we don't have any packages (yet?) that this matters to. We can improve this later on by manually building the depgraph of test deps as part of the main build_packages phase. By dropping --buildpkgonly, we allow packages to adjust things they install based on the USE=test flag. This adds a some overhead where we are merging the packages into the sysroot (again). This may lead to a slight desync in that the sysroot has files that aren't in the release image, but we're already in that situation now with all the install masked files we strip out. BUG= chromium:740549 TEST=precq run rebuilds packages as needed and pulls in new deps Change-Id: I7a8b1eb1792fe6b6d0b80b53020fac2eff952311 Reviewed-on: https://chromium-review.googlesource.com/698948 Commit-Ready: Mike Frysinger <vapier@chromium.org> Tested-by: Mike Frysinger <vapier@chromium.org> Reviewed-by: Dan Erat <derat@chromium.org> [modify] https://crrev.com/f30e1ea61ffd70600ff4f66ea7885510bbb3ce2a/lib/chroot_util.py
,
Oct 7 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/258c5851889feb057bb79f048799b439c25deba0 commit 258c5851889feb057bb79f048799b439c25deba0 Author: Daniel Erat <derat@chromium.org> Date: Sat Oct 07 22:33:38 2017 trunks: Only install libtrunks_test.a for test USE flag. This test library was getting installed unconditionally, seemingly as part of now-outdated Brillo stuff (http://b/24099462). It only appears to be used by other packages when the "test" USE flag is set. Stop installing it by default so a followup change to trunks.gyp can make us stop building it by default, so another change to this ebuild can make the trunks package stop unconditionally depending on gmock. Also make the attestation, chaps, cryptohome, and tpm_manager packages, all of which depend on libtrunks_test.a when built for testing, require that trunks be built with the "test" USE flag when they themselves are built with it. BUG= chromium:740549 TEST=built a dev image for a board with tpm2 Change-Id: I824547721d6fc60d215473fac9f50856165dcc64 Reviewed-on: https://chromium-review.googlesource.com/565960 Commit-Ready: Dan Erat <derat@chromium.org> Tested-by: Dan Erat <derat@chromium.org> Reviewed-by: Mike Frysinger <vapier@chromium.org> [modify] https://crrev.com/258c5851889feb057bb79f048799b439c25deba0/chromeos-base/tpm_manager/tpm_manager-9999.ebuild [modify] https://crrev.com/258c5851889feb057bb79f048799b439c25deba0/chromeos-base/chaps/chaps-9999.ebuild [modify] https://crrev.com/258c5851889feb057bb79f048799b439c25deba0/chromeos-base/trunks/trunks-9999.ebuild [modify] https://crrev.com/258c5851889feb057bb79f048799b439c25deba0/chromeos-base/attestation/attestation-9999.ebuild [modify] https://crrev.com/258c5851889feb057bb79f048799b439c25deba0/chromeos-base/cryptohome/cryptohome-9999.ebuild
,
Oct 9 2017
The following revision refers to this bug: https://chromium.googlesource.com/aosp/platform/system/tpm/+/8abfbae298ce3689a6c6be68d7094e2841968aec commit 8abfbae298ce3689a6c6be68d7094e2841968aec Author: Daniel Erat <derat@chromium.org> Date: Mon Oct 09 01:30:13 2017 trunks: Only build libtrunks_test.a for test USE flag. BUG= chromium:740549 TEST=none Change-Id: I3ea20d8532667429fe8038b1746401365276871a Reviewed-on: https://chromium-review.googlesource.com/706702 Commit-Ready: Dan Erat <derat@chromium.org> Tested-by: Dan Erat <derat@chromium.org> Reviewed-by: Mike Frysinger <vapier@chromium.org> [modify] https://crrev.com/8abfbae298ce3689a6c6be68d7094e2841968aec/trunks/trunks.gyp
,
Oct 9 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/503da065b9303a99c7d4748e84dd0c95e4adfa8f commit 503da065b9303a99c7d4748e84dd0c95e4adfa8f Author: Daniel Erat <derat@chromium.org> Date: Mon Oct 09 16:19:34 2017 trunks: Remove unconditional gmock dependency. chromeos-base/trunks used to depend unconditionally on dev-cpp/gmock. Now that libtrunks_test.a is only built when the "test" USE flag is set, remove the unconditional gmock dependency. (The dependency is still added by platform.eclass when the test USE flag is set.) BUG= chromium:740549 TEST=none Change-Id: I32c89186928de472c6c1974ac961a2dc7104b86c Reviewed-on: https://chromium-review.googlesource.com/706308 Commit-Ready: Dan Erat <derat@chromium.org> Tested-by: Dan Erat <derat@chromium.org> Reviewed-by: Mike Frysinger <vapier@chromium.org> [modify] https://crrev.com/503da065b9303a99c7d4748e84dd0c95e4adfa8f/chromeos-base/trunks/trunks-9999.ebuild
,
Oct 9 2017
,
Oct 13 2017
Looks like after https://crrev.com/c/706702 we now can't unittest cryptohome for tpm2 (without first building a test version of trunks): # cros_workon_make --board=eve --test cryptohome ... /usr/x86_64-pc-linux-gnu/x86_64-cros-linux-gnu/binutils-bin/2.27.0/ld.gold.real: error: cannot find -ltrunks_test ...
,
Oct 13 2017
"cros_run_unit_tests --board eve --packages cryptohome" succeeds. After all, it emerges dependencies with USE="{test*}". It doesn't run the unittests themselves, though. At least, not for cryptohome. Is that expected?
And running "cros_workon_make --board=eve --test cryptohome" after "cros_run_unit_tests --board eve --packages cryptohome" also works (no surprise here as well).
So, what's the right way to run unittests for cryptohome with these changes?
And does running unit tests now interferes with build_packages - should I rebuild packages after doing unittests to make sure that the resulting image contains non-test-versions of those packages?
,
Oct 14 2017
cros_workon_make ignores the dep tree. it's got sharp edges like this. I don't see any conflict with build_packages.
,
Feb 20 2018
|
||||
►
Sign in to add a comment |
||||
Comment 1 by derat@chromium.org
, Jul 10 2017