Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Port to generalized log parser #5

Merged
merged 1 commit into from
Jul 1, 2018
Merged

Conversation

jerrymarino
Copy link
Owner

@jerrymarino jerrymarino commented Jun 30, 2018

This patch ports the program to a simple log parser. It is orders of
magnitude easier to maintain, reason about, debug, and use.

Previously, it was relying on internal data structures of Xcode, which
integrated nice into Xcode's build system. While novel, there are several pitfalls:

  • Keeping up with Apple's changes
  • The new llbuild based build system doesn't use the same data structures
  • It only works with some of the build
  • It was too hard to debug

Parsing logs has several pitfalls too but is better than other evils
like:

  • using Xcode or Swift internals ( prev impl )
  • swapping out compilers in the build system and writing
    argv to a file ( very bad for many reasons! )

This patch implements a very generalized log parser that accepts input
from virtually any log containing swift frontend actions. In the future,
we'll get away from this, and that will be a follow up.

Testing:

  • Unit tests
  • Integration tests

Copy link
Owner Author

@jerrymarino jerrymarino left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Horray for cleaning this up! This should fix: jerrymarino/SwiftVimTestHost#1

}

/// Check if the first non " " argument is clang
func isCC(lexed: [String]) -> Bool {
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

None of these naive checks will work with custom compilers ( for clang and swiftc ), that is an edge case that this program doesn't need to handle ATM. It should be at least called out.

return swiftSources.filter { $0.hasSuffix(swiftFile) }.first ?? swiftSources.first
}

func workingDirectoryInSwiftInvocation(lexed: [String]) -> String? {
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could probably just use the cd action, as clang. The previous program did not, so continue to parse this.

}
}

if objectFile == "" {
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a comment: this assumes -primary-file is being used and the first file is reasonable.


class LogParsingTests: XCTestCase {

func atestFrontendInvocation() {
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove prefix a

}
}

func isspace(_ c: String) -> Bool {
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can probably be inlined.


echo "Installing to /usr/local/bin"
ditto build/Release/XCCompilationDB /usr/local/bin
ditto .build/release/XcodeCompilationDatabase /usr/local/bin/XCCompilationDB
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should probably just be in the makefile as the default target.

@jerrymarino jerrymarino force-pushed the jmarino_log_parsing branch 2 times, most recently from 9cd7f77 to 1f5eef7 Compare July 1, 2018 05:29
This patch ports the program to a simple log parser. It is orders of
magnitude to maintain, reason about, debug, and use.

Previously, it was relying on internal data structures of Xcode, which
integrated nice into Xcode's build system. While novel, there are several pitfalls:
- Keeping up with Apple's changes
- The new llbuild based build system doesn't use the same data structures
- It only works with some of the build
- It was too hard to debug

Parsing logs has *several pitfalls too* but is better than other evils
like:
- using Xcode or Swift internals ( prev impl )
- swapping out compilers in the build system and writing
argv to a file ( very bad for many reasons! )

This patch implements a very generalized log parser that accepts input
from *virtually any log containing swift frontend invocations*.

In the future, it may get away from using frontend invocations, and that
would be a follow up.

Testing:
- Unit tests
- Integration tests
@jerrymarino jerrymarino force-pushed the jmarino_log_parsing branch from 1f5eef7 to 20121a2 Compare July 1, 2018 05:57
@jerrymarino jerrymarino changed the title WIP: Port to generalized log parser Port to generalized log parser Jul 1, 2018
@jerrymarino jerrymarino merged commit 598725f into master Jul 1, 2018
@jerrymarino jerrymarino deleted the jmarino_log_parsing branch July 1, 2018 05:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant