New issue
Advanced search Search tips

Issue 852522 link

Starred by 3 users

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: iOS
Pri: 3
Type: Bug



Sign in to add a comment

[ios12] Xcode 10 breaks build console log's ability to directly link to compile failures

Project Member Reported by justincohen@chromium.org, Jun 13 2018

Issue description

Filed 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.
 
Components: Build
Labels: Restrict-View-Google
Cc: dpranke@chromium.org sdefresne@chromium.org rohitrao@chromium.org
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.


Cc: tikuta@chromium.org thakis@chromium.org
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.
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.
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...

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 :).
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.
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.
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?
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.
> 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.
#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"

Project Member

Comment 13 by bugdroid1@chromium.org, 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

+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?
There isn't yet an opt-in toggle. Adding a gn arg that works like msvc_use_absolute_paths sounds fine to me though.
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'
Cc: reed@google.com
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.
justincohen, how did you disable the build system? Is it done in gn in a way that skia could use too?
*the new build system
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, ...).
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).
Labels: -Restrict-View-Google

Sign in to add a comment