Bug 194795

Created attachment 362345 [details] Patch

Comment on attachment 362345 [details] Patch r=me!

We should reconsider removing the '...' without a replacement for it. The '...' is used to ferry some logging across the "Xcode barrier". generate-xcfilelist runs in two contexts: standalone, and from within Xcode. To achieve this, when running standalone, generate-xcfilelist can launch Xcode and then invoke itself to continue some operations. If, when debugging, it's necessary to enable logging, both the standalone generate-xcfilelist and the one sub-launched under Xcode both emit logging. But, at the same time, Xcode will spew out a lot of logging. We want the generate-xcfilelist logging, but not the Xcode logging. In order to distinguish between the two generate-xcfilelist will scan all of the logging from the Xcode process, keep the logging starting with '...' and discarding the rest. There are also cases where we need to ferry error messages across this Xcode barrier. We we like to receive this logging without also receiving Xcode's native logging. For examples of where we filter based on ..., see: sed -E -e 's/^/... /' < "${GX_TEMP}/std.err" and: WK_SUBLAUNCH_SCRIPT_PARAMETERS=("${GX_ARGS[@]}") xcodebuild "${GX_XCODE_PARAMETERS[@]}" | grep '^\.\.\. ' I'll be the first to admit that this is a bit of a hack. If anyone has a better approach to pick apart the generate-xcfilelist logging from Xcode's native logging, we can use that instead.

Will filter-webkit-build handle "Generating all .xcfilelists"? This logging doesn't seem to match the regex added in the patch. Is plain old "Generating .+" handled elsewhere?

(In reply to Keith Rollin from comment #4) > Will filter-webkit-build handle "Generating all .xcfilelists"? This logging > doesn't seem to match the regex added in the patch. Is plain old "Generating > .+" handled elsewhere? It won't. Well noticed. It's not such a big deal since a fbwk "failure" will just let the output go through. But I'll fix it.

(In reply to Keith Rollin from comment #3) > We should reconsider removing the '...' without a replacement for it. The > '...' is used to ferry some logging across the "Xcode barrier". > > generate-xcfilelist runs in two contexts: standalone, and from within Xcode. > To achieve this, when running standalone, generate-xcfilelist can launch > Xcode and then invoke itself to continue some operations. > > If, when debugging, it's necessary to enable logging, both the standalone > generate-xcfilelist and the one sub-launched under Xcode both emit logging. > But, at the same time, Xcode will spew out a lot of logging. We want the > generate-xcfilelist logging, but not the Xcode logging. In order to > distinguish between the two generate-xcfilelist will scan all of the logging > from the Xcode process, keep the logging starting with '...' and discarding > the rest. > > There are also cases where we need to ferry error messages across this Xcode > barrier. We we like to receive this logging without also receiving Xcode's > native logging. > > For examples of where we filter based on ..., see: > > sed -E -e 's/^/... /' < "${GX_TEMP}/std.err" > > and: > > WK_SUBLAUNCH_SCRIPT_PARAMETERS=("${GX_ARGS[@]}") xcodebuild > "${GX_XCODE_PARAMETERS[@]}" | grep '^\.\.\. ' > > I'll be the first to admit that this is a bit of a hack. If anyone has a > better approach to pick apart the generate-xcfilelist logging from Xcode's > native logging, we can use that instead. OK. I definitely shouldn't land my patch in this state then. Maybe we could prefix generate-xcfilelist output with something other than "..." and have fbwk identify it.

(In reply to Dean Jackson from comment #6) > > Maybe we could prefix generate-xcfilelist output with something other than > "..." and have fbwk identify it. I had a similar thought during development of the script. I was thinking of keeping the ==='s and ...'s and filtering based on them. But you see a need to remove them and replace them with some other prefix? Personally, I'm fine with anything. We could use "GXCF:", for example.

Comment on attachment 362345 [details] Patch r- based on the comment thread

Committed r241771: <https://trac.webkit.org/changeset/241771>

<rdar://problem/48213591>