Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1966983002:
Fix it2me_standalone_host compilation. (Closed)
|
Created:
4 years, 7 months ago by Dirk Pranke Modified:
4 years, 7 months ago CC:
chromium-reviews, chromoting-reviews_chromium.org, stevenjb Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionFix it2me_standalone_host compilation.
This newly-added binary doesn't compile on ChromeOS; I think
it might be desktop-linux-only?
R=zijiehe@chromium.org, sergeyu@chromium.org
BUG=607213
Committed: https://crrev.com/b91b7557f1fd7e44a00231d040a42f2928520ffc
Cr-Commit-Position: refs/heads/master@{#393435}
Patch Set 1 #
Total comments: 1
Patch Set 2 : update condition from review feedback #Patch Set 3 : fix import #Patch Set 4 : fix gtk dependency #Messages
Total messages: 22 (8 generated)
Hi, Should this new binary have been desktop-linux-only? If not, what platforms should it work on? Also, is this binary intentionally GN-only?
On 2016/05/11 02:30:29, Dirk Pranke wrote: > Hi, > > Should this new binary have been desktop-linux-only? If not, what platforms > should it work on? > > Also, is this binary intentionally GN-only? This binary is designed to work for all platforms, and presubmit test covered linux_chromium_gn_chromeos_rel. Would you mind to share the error log with me, so I can propose a change to resolve the build break.
On 2016/05/11 18:09:26, Hzj_jie wrote: > On 2016/05/11 02:30:29, Dirk Pranke wrote: > > Hi, > > > > Should this new binary have been desktop-linux-only? If not, what platforms > > should it work on? > > > > Also, is this binary intentionally GN-only? > > This binary is designed to work for all platforms, and presubmit test covered > linux_chromium_gn_chromeos_rel. Would you mind to share the error log with me, > so I can propose a change to resolve the build break. And yes, this target is GN-only. I believe we are moving from GYP to GN, so we do not need to add GYP rules for new targets. Is that correct?
On 2016/05/11 18:18:38, Hzj_jie wrote: > On 2016/05/11 18:09:26, Hzj_jie wrote: > > On 2016/05/11 02:30:29, Dirk Pranke wrote: > > > Hi, > > > > > > Should this new binary have been desktop-linux-only? If not, what platforms > > > should it work on? > > > > > > Also, is this binary intentionally GN-only? > > > > This binary is designed to work for all platforms, and presubmit test covered > > linux_chromium_gn_chromeos_rel. Would you mind to share the error log with me, > > so I can propose a change to resolve the build break. it2me_standalone_host_main.cc:10 includes gtk.h and xlib if OS_LINUX is set; this should probably be conditional on use_x11 and !is_chromeos. > And yes, this target is GN-only. I believe we are moving from GYP to GN, so we > do not need to add GYP rules for new targets. Is that correct? that's not unconditionally true. It kinda depends on the target. For a minor target like this, gn-only is probably fine.
On 2016/05/11 19:07:06, Dirk Pranke wrote:
> On 2016/05/11 18:18:38, Hzj_jie wrote:
> > On 2016/05/11 18:09:26, Hzj_jie wrote:
> > > On 2016/05/11 02:30:29, Dirk Pranke wrote:
> > > > Hi,
> > > >
> > > > Should this new binary have been desktop-linux-only? If not, what
> platforms
> > > > should it work on?
> > > >
> > > > Also, is this binary intentionally GN-only?
> > >
> > > This binary is designed to work for all platforms, and presubmit test
> covered
> > > linux_chromium_gn_chromeos_rel. Would you mind to share the error log with
> me,
> > > so I can propose a change to resolve the build break.
>
> it2me_standalone_host_main.cc:10 includes gtk.h and xlib if OS_LINUX is set;
> this should probably be conditional on use_x11 and !is_chromeos.
>
> > And yes, this target is GN-only. I believe we are moving from GYP to GN, so
we
> > do not need to add GYP rules for new targets. Is that correct?
>
> that's not unconditionally true. It kinda depends on the target. For a minor
> target
> like this, gn-only is probably fine.
Here is the specific failure with the following args.gn:
~/Work/chrome/src (master) $ cat out/Release/args.gn
target_os = "chromeos"
use_goma = true
~/Work/chrome/src (master) $ ninja -j 1000 -l 10 -C out/Release
it2me_standalone_host_main
ninja: Entering directory `out/Release'
[1/2] CXX
obj/remoting/test/it2me_standalone_host_main/it2me_standalone_host_main.o
FAILED:
obj/remoting/test/it2me_standalone_host_main/it2me_standalone_host_main.o
/usr/local/google/home/stevenjb/goma/gomacc
../../third_party/llvm-build/Release+Asserts/bin/clang++ -MMD -MF
obj/remoting/test/it2me_standalone_host_main/it2me_standalone_host_main.o.d
-DV8_DEPRECATION_WARNINGS -DENABLE_MDNS=1 -DENABLE_NOTIFICATIONS
-DENABLE_PEPPER_CDMS -DENABLE_PLUGINS=1 -DENABLE_PDF=1 -DENABLE_PRINTING=1
-DENABLE_PRINT_PREVIEW=1 -DENABLE_SPELLCHECK=1 -DUSE_UDEV
-DUI_COMPOSITOR_IMAGE_TRANSPORT -DUSE_ASH=1 -DUSE_AURA=1 -DUSE_PANGO=1
-DUSE_CAIRO=1 -DUSE_CLIPBOARD_AURAX11=1 -DUSE_DEFAULT_RENDER_THEME=1
-DUSE_GLIB=1 -DUSE_NSS_CERTS=1 -DUSE_X11=1 -DENABLE_WEBRTC=1
-DENABLE_EXTENSIONS=1 -DENABLE_TASK_MANAGER=1 -DENABLE_THEMES=1
-DENABLE_CAPTIVE_PORTAL_DETECTION=1 -DENABLE_SESSION_SERVICE=1
-DENABLE_APP_LIST=1 -DENABLE_SUPERVISED_USERS=1 -DENABLE_SERVICE_DISCOVERY=1
-DIMAGE_LOADER_EXTENSION=1 -DENABLE_TOPCHROME_MD=1 -DENABLE_WAYLAND_SERVER=1
-DFULL_SAFE_BROWSING -DSAFE_BROWSING_CSD -DSAFE_BROWSING_DB_LOCAL
-DCHROMIUM_BUILD -DENABLE_MEDIA_ROUTER=1 -DFIELDTRIAL_TESTING_ENABLED
-DCR_CLANG_REVISION=268813-1 -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE
-D_LARGEFILE64_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS
-DOS_CHROMEOS -D_DEBUG -DDYNAMIC_ANNOTATIONS_ENABLED=1
-DWTF_USE_DYNAMIC_ANNOTATIONS=1 -D_GLIBCXX_DEBUG=1 -DGL_GLEXT_PROTOTYPES
-DUSE_GLX -DUSE_EGL -DTOOLKIT_VIEWS=1 -DGTEST_HAS_POSIX_RE=0
-DGTEST_LANG_CXX11=0 -DGTEST_HAS_RTTI=0 -DGOOGLE_PROTOBUF_NO_RTTI
-DGOOGLE_PROTOBUF_NO_STATIC_INITIALIZER -DHAVE_PTHREAD -DFEATURE_ENABLE_SSL
-DFEATURE_ENABLE_VOICEMAIL -DEXPAT_RELATIVE_PATH -DGTEST_RELATIVE_PATH
-DNO_MAIN_THREAD_WRAPPING -DNO_SOUND_SYSTEM -DLINUX -DWEBRTC_LINUX
-DWEBRTC_POSIX -DCHROMEOS -DWEBRTC_CHROMIUM_BUILD -DWEBRTC_POSIX -DWEBRTC_LINUX
-I../.. -Igen -I/usr/include/glib-2.0
-I/usr/lib/x86_64-linux-gnu/glib-2.0/include -I../../third_party/khronos
-I../../gpu -I../../testing/gtest/include -I../../third_party/protobuf/src
-I../../third_party/webrtc_overrides -I../../third_party/libjingle/overrides
-I../../third_party/libjingle/source -I../../testing/gtest/include
-I../../third_party -I../../third_party/webrtc_overrides -I../../third_party
-I../../testing/gmock/include -fno-strict-aliasing -funwind-tables -fPIC -pipe
-B../../third_party/binutils/Linux_x64/Release/bin -fcolor-diagnostics
-fdebug-prefix-map=/usr/local/google/home/stevenjb/Work/chrome/src=. -pthread
-m64 -march=x86-64 -Wall -Werror -Wextra -Wno-missing-field-initializers
-Wno-unused-parameter -Wno-c++11-narrowing -Wno-covered-switch-default
-Wno-deprecated-register -Wno-unneeded-internal-declaration
-Wno-inconsistent-missing-override -Wno-shift-negative-value
-Wno-undefined-var-template -O0 -g2 -gsplit-dwarf -fvisibility=hidden -Xclang
-load -Xclang
../../third_party/llvm-build/Release+Asserts/lib/libFindBadConstructs.so -Xclang
-add-plugin -Xclang find-bad-constructs -Xclang -plugin-arg-find-bad-constructs
-Xclang check-templates -Xclang -plugin-arg-find-bad-constructs -Xclang
follow-macro-expansion -Xclang -plugin-arg-find-bad-constructs -Xclang
check-implicit-copy-ctors -Xclang -plugin-arg-find-bad-constructs -Xclang
check-ipc -Wheader-hygiene -Wstring-conversion -fno-threadsafe-statics
-fvisibility-inlines-hidden -Wno-undefined-bool-conversion
-Wno-tautological-undefined-compare -std=gnu++11
-Wno-reserved-user-defined-literal -fno-rtti -fno-exceptions -c
../../remoting/test/it2me_standalone_host_main.cc -o
obj/remoting/test/it2me_standalone_host_main/it2me_standalone_host_main.o
../../remoting/test/it2me_standalone_host_main.cc:13:10: fatal error:
'gtk/gtk.h' file not found
#include <gtk/gtk.h>
^
1 error generated.
ninja: build stopped: subcommand failed.
Thank you to let me know. https://codereview.chromium.org/1966983002/diff/1/remoting/test/BUILD.gn File remoting/test/BUILD.gn (right): https://codereview.chromium.org/1966983002/diff/1/remoting/test/BUILD.gn#newc... remoting/test/BUILD.gn:146: if (is_desktop_linux) { Please change the condition to if (!is_chromeos && !is_android && enable_remoting_host) Thank you.
Description was changed from ========== Fix it2me_standalone_host compilation. This newly-added binary doesn't compile on ChromeOS; I think it might be desktop-linux-only? R=zijiehe@chromium.org BUG= ========== to ========== Fix it2me_standalone_host compilation. This newly-added binary doesn't compile on ChromeOS; I think it might be desktop-linux-only? R=zijiehe@chromium.org, sergeyu@chromium.org BUG=607213 ==========
dpranke@chromium.org changed reviewers: + sergeyu@chromium.org
Done. Please take another look? sergeyu@, can I get an OWNERS approval?
lgtm
On 2016/05/12 22:58:30, Sergey Ulanov wrote: > lgtm Thanks! I'm going to go ahead and land this w/o waiting for zijiehe@ , to unbreak the CrOS build.
The CQ bit was checked by dpranke@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1966983002/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1966983002/40001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: win_clang on tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_clang/builds/...)
The CQ bit was checked by dpranke@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from sergeyu@chromium.org Link to the patchset: https://codereview.chromium.org/1966983002/#ps60001 (title: "fix gtk dependency")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1966983002/60001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1966983002/60001
Message was sent while issue was closed.
Description was changed from ========== Fix it2me_standalone_host compilation. This newly-added binary doesn't compile on ChromeOS; I think it might be desktop-linux-only? R=zijiehe@chromium.org, sergeyu@chromium.org BUG=607213 ========== to ========== Fix it2me_standalone_host compilation. This newly-added binary doesn't compile on ChromeOS; I think it might be desktop-linux-only? R=zijiehe@chromium.org, sergeyu@chromium.org BUG=607213 ==========
Message was sent while issue was closed.
Committed patchset #4 (id:60001)
Message was sent while issue was closed.
Description was changed from ========== Fix it2me_standalone_host compilation. This newly-added binary doesn't compile on ChromeOS; I think it might be desktop-linux-only? R=zijiehe@chromium.org, sergeyu@chromium.org BUG=607213 ========== to ========== Fix it2me_standalone_host compilation. This newly-added binary doesn't compile on ChromeOS; I think it might be desktop-linux-only? R=zijiehe@chromium.org, sergeyu@chromium.org BUG=607213 Committed: https://crrev.com/b91b7557f1fd7e44a00231d040a42f2928520ffc Cr-Commit-Position: refs/heads/master@{#393435} ==========
Message was sent while issue was closed.
Patchset 4 (id:??) landed as https://crrev.com/b91b7557f1fd7e44a00231d040a42f2928520ffc Cr-Commit-Position: refs/heads/master@{#393435} |
