[ios12] Xcode 10 breaks build console log's ability to directly link to compile failures |
|||||
Issue descriptionFiled rdar://41098409 to ask that the new build system support this feature. If necessary, we can revert to the old build system to get this feature back.
,
Aug 14
They fixed rdar://41098409, but opened another rdar://43290386 since relative paths don't work with the new build system. dpranke@/sdefresne@ If for some reason Apple says 'no', is there any way to make GN use absolute paths for build output. For example, this would need to be absolute: ../../ios/chrome/browser/ui/browser_view_controller.mm:2094:3: error: use of undeclared identifier 'intentional_failure' intentional_failure; ^ 1 error generated.
,
Aug 14
I apparently can't find either of those bugs in radar, so I don't understand the problem. Can someone explain further, or point me at links? Generally speaking, we don't want to use absolute paths for things, because that breaks build determinism, but we may be able to change things if we have to. +tikuta, thakis, who have been looking at determinism-related things lately.
,
Aug 14
We'd only need the log output to display absolute paths, right? Xcode is reading that line and parsing it to display errors in the UI, but it doesn't care what GN/ninja are doing internally.
,
Aug 14
Xcode appears to be parsing the build output. For "error: " (or "warning: "), they show the error inline in the IDE, for example: https://screenshot.googleplex.com/vuOkby9eBB4 This is very useful for those who are used to Xcode development. With Xcode 10, it seems they don't parse relative paths anymore. We've filed a radar, but incase they opt to not support relative paths...
,
Aug 14
I don't think we can affect the build output in this case without either changing ninja to use absolute paths or injecting a wrapper (much like the xcodefilters we used to have). I'm certainly not going to do the former unconditionally. It's possible we could add a switch to make it use absolute (or possibly enable it automatically when --ide=xcode is passed) but I wouldn't be wild about it. So, I wonder if we could somehow inject a wrapper instead? Ideally, of course, they'll fix XCode :).
,
Aug 14
There is an option to enforce absolute path for compiler warning/error. https://cs.chromium.org/chromium/src/build/config/compiler/BUILD.gn?l=508&rcl=7755fb6cf959514896b8e19556c95eb3735a40fe You can change around here to support this option in mac.
,
Aug 14
Yup, what tikuta says. Xcode folks can set some gn toggle that causes -fdiagnostics-absolute-paths to be passed (xcode_use_absolute_paths?). It will make it so that you no longer use the global goma cache, but that way people can choose if they prefer faster builds or UI-clickable diags.
,
Aug 15
Hmm, losing the goma cache seems bad. I'll wait and see if Apple fixes this before the new build system is required. dpranke@ what were the xcodefilters you mentioned in comment #6?
,
Aug 15
IIRC, long ago pre-Ninja we had scripts that would run xcodebuild and filter the resulting output to be less noisy (or I might be confusing this with WebKit). I would expect we could rig up something similar in the xcode generator so that instead of it invoking ninja directly to build a target, it could invoke a wrapper script that would invoke ninja and then, if an error occurs, rewrite the error message to be an absolute path? I think I didn't know about msvc_use_absolute_paths; interesting to know that there's a precedent in a different IDE.
,
Aug 15
> Hmm, losing the goma cache seems bad. It's "just" the global goma cache where if a build bot already compiled a file at a revision you're compiling at, goma doesn't need to do work for you. The global goma cache hasn't been working (due to abs paths) until like a month ago, so it's not really even a regression -- you're just missing out on a progression.
,
Aug 15
#11, As thakis said, you won't get compile result from goma's cache by introducing abs path. But if you are using different args.gn from buildbot, you may not get benefit of global goma cache currently. You can check cache hit ratio of your chrome build from http://localhost:8088/statz See lines like "goma: finished=371 cache_hit=0 local_cachehit=0 aborted=181 retry=4 fail=0"
,
Aug 21
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ab6d480f5172331eba5fe66236480f2600e41d18 commit ab6d480f5172331eba5fe66236480f2600e41d18 Author: Justin Cohen <justincohen@google.com> Date: Tue Aug 21 13:31:10 2018 [ios] Disable new build system in Xcode 10 for convert_gn_xcodeproj. Xcode 10 obscures compile failures when build output includes relative paths, and our xctest-build-sources fail to compile. Disable build system for now. Bug: 852522 Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs;master.tryserver.chromium.mac:ios-simulator-cronet Change-Id: Ice0e5c7e6734270f804ae0f8a21cf5621ff13f58 Reviewed-on: https://chromium-review.googlesource.com/1182143 Reviewed-by: Olivier Robin <olivierrobin@chromium.org> Reviewed-by: Rohit Rao <rohitrao@chromium.org> Commit-Queue: Justin Cohen <justincohen@chromium.org> Cr-Commit-Position: refs/heads/master@{#584722} [modify] https://crrev.com/ab6d480f5172331eba5fe66236480f2600e41d18/ios/build/tools/convert_gn_xcodeproj.py
,
Oct 1
+1 to try to solve this. Is there a manual way to sneak -fdiagnostics-absolute-paths into my xcode project, or is there no user-workaround yet?
,
Oct 1
There isn't yet an opt-in toggle. Adding a gn arg that works like msvc_use_absolute_paths sounds fine to me though.
,
Oct 1
We aren't missing out on anything yet with the new build system, so we've disabled it so far. Are you seeing issues now? I'm hoping Xcode 12 will fix this. If not, we have some options (e.g. | perl -pe 's/\.\.\/\.\.\//\/path\/to\/your\/bling\/src\//g'
,
Oct 1
,
Oct 1
The issue we see is just as developers, not our build-bots. This issue makes it far harder/slower to develop using the Xcode IDE.
,
Oct 1
justincohen, how did you disable the build system? Is it done in gn in a way that skia could use too?
,
Oct 1
*the new build system
,
Oct 2
The fix in comment #21 would only apply if you use "setup-gn.py" script, not "gn gen --ide=xcode". It should relatively be straight-forward to implement in gn xcode project generator however. This would allow the fix to be shared by skia, and not just chromium. Another option would be a wrapper around clang rewriting paths on stderr to convert relative to absolute paths. This is quite easy to write, and can be controlled by a flag to avoid invocation of one more wrapper for use cases that do not need rewriting (bots, ...).
,
Oct 2
CL to implement the creation of WorkspaceSettings.xcsettings available for review at https://gn-review.googlesource.com/c/gn/+/2900 (BTW, it looks like "git cl format" does not work for this repository).
,
Oct 17
|
|||||
►
Sign in to add a comment |
|||||
Comment 1 by justincohen@chromium.org
, Jun 13 2018Labels: Restrict-View-Google