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

Unintentional but unacceptable diagnostic language #5968

Closed
dabrahams opened this issue Dec 11, 2022 · 12 comments
Closed

Unintentional but unacceptable diagnostic language #5968

dabrahams opened this issue Dec 11, 2022 · 12 comments
Labels

Comments

@dabrahams
Copy link
Contributor

Description

Not sure this bug really belongs on SPM. I just built swift-format and the build ended with:

remark: Incremental compilation has been disabled: it is not compatible with whoremark: Incremental compilation has been disabled: it is not compatible with whoremark: Incremental compilation has been disabled: it is not compatible with whoremark: Incremental compilation has been disabled: it is not compatible with whoremark: Incremental compilation has been disabled: it is not compatible with whoremark: Incremental compilation has been disabled: it is not compatible with whoremark: Incremental compilation has been disabled: it is not compatible with who[11/16] Co[25/25] Compiling SwiftFormatTestSupport DiagnosingTestCase.swift
Build complete! (103.43s)

Something needs to be inserting a newline in the output before all remarks.

Expected behavior

No response

Actual behavior

No response

Steps to reproduce

Build swift-format from the CLI with swift 5.7

Swift Package Manager version/commit hash

No response

Swift & OS version (output of swift --version && uname -a)

No response

@dabrahams dabrahams added the bug label Dec 11, 2022
@abertelrud
Copy link
Contributor

abertelrud commented Dec 21, 2022

Almost certainly the remark refers to "whole module optimization". Seems to be two bugs:

  1. the message cut off
  2. it needs a newline (as you point out)

Taking a look.

@abertelrud
Copy link
Contributor

abertelrud commented Dec 21, 2022

I do see the missing newline, but not the cutting off, so I get remark: Incremental compilation has been disabled: it is not compatible with whole module optimizationremark: Incremental compilation has been disabled: it is not compatible with whole module optimizationremark: Incremental compilation has been disabled: it is not compatible with whole module optimizationremark: Incremental compilation has been disabled: it is not compatible with whole module optimizationremark: Incremental compilation has been disabled: it is not compatible with whole module optimization[4/14] Compiling TSCLibc libc.swift

I only get this if I set TERM to something that doesn't support ANSI (or pipe the output), otherwise I don't.

Still not good, so checking what's going on.

@abertelrud
Copy link
Contributor

Also seems unfortunate to be getting that same message so many times; either SwiftPM should dedupe or maybe that's something in the driver. In any case the missing newline seems to be the first thing to address here, and that is almost certainly on the SwiftPM side.

@abertelrud
Copy link
Contributor

abertelrud commented Dec 21, 2022

❯ So this looks like a combination of two things:

  1. swift-driver emits both its JSON output and raw remarks to stderr (this seems unfortunate)
  2. swift-package tries to interpret the JSON packets from the driver but when it can't it just emits the raw bytes, and ends up losing the newline (still looking into that one)

For example:

swiftc -parseable-output -whole-module-optimization -incremental -c foo.swift
remark: Incremental compilation has been disabled: it is not compatible with whole module optimization
1201
{
  "command_arguments" : [
    "-frontend",
    "-c",
    "foo.swift",
    "-target",
    "arm64-apple-macosx12.0",
    "-Xllvm",
    "-aarch64-use-tbi",
    "-enable-objc-interop",
    "-stack-check",
. . .

@abertelrud
Copy link
Contributor

Filed apple/swift-driver#1241 on the Swift Driver issue and will see what we do in SwiftPM about the current state of affairs. Since the output is always line-based, we should be able to do better in the JSON parser than we are currently doing.

@compnerd
Copy link
Collaborator

This is fairly reminiscent of the llbuild issue that we had on Windows - we would not buffer things correctly and interleave messages: #4192

@abertelrud
Copy link
Contributor

That's a good point — thanks for pointing this out. In this case it seems that this is actually handled fairly well already by the JSON parser in SwiftPM, and an additional newline is all that's needed. That doesn't explain the truncation but does at least properly emit the diagnostics. I still think the driver should emit JSON for its own diagnostics, though, when -parseable-output is in effect.

@abertelrud
Copy link
Contributor

I'm preparing a SwiftPM PR for the newline at least.

@abertelrud
Copy link
Contributor

rdar://103608636

abertelrud added a commit to abertelrud/swift-package-manager that referenced this issue Dec 21, 2022
…lines

SwiftPM uses the Swift Driver's `-parseable-output` flag to get a sequence of length-prefixed JSON-encoded messages that it can read.  Unfortunately the Swift Driver doesn't JSON-encode its own diagnostics, leading to things like apple#5968.

I filed apple#5968 on the Swift Driver to get it to encode its JSON messages, but meanwhile, it turns out that we can fairly easily fix the fallback logic that SwiftPM uses when it emits raw output.  It was simply missing a newline.  It's safe to always add one, since the logic that parses JSON splits by newlines, so we won't find any terminating newlines.

rdar://103608636
abertelrud added a commit to abertelrud/swift-package-manager that referenced this issue Dec 21, 2022
…lines

SwiftPM uses the Swift Driver's `-parseable-output` flag to get a sequence of length-prefixed JSON-encoded messages that it can read.  Unfortunately the Swift Driver doesn't JSON-encode its own diagnostics, leading to things like apple#5968.

I filed apple#5968 on the Swift Driver to get it to encode its JSON messages, but meanwhile, it turns out that we can fairly easily fix the fallback logic that SwiftPM uses when it emits raw output.  It was simply missing a newline.  It's safe to always add a newline to the raw driver output, since the logic that parses JSON splits by newlines, so we won't find any terminating newlines.  Note that we don't add newlines to the output coming from messages from the compiler frontend, since that output already has trailing newlines.

rdar://103608636
abertelrud added a commit to abertelrud/swift-package-manager that referenced this issue Dec 21, 2022
…lines

SwiftPM uses the Swift Driver's `-parseable-output` flag to get a sequence of length-prefixed JSON-encoded messages that it can read.  Unfortunately the Swift Driver doesn't JSON-encode its own diagnostics, leading to things like apple#5968.

I filed apple#5968 on the Swift Driver to get it to encode its JSON messages, but meanwhile, it turns out that we can fairly easily fix the fallback logic that SwiftPM uses when it emits raw output.  It was simply missing a newline.  It's safe to always add a newline to the raw driver output, since the logic that parses JSON splits by newlines, so we won't find any terminating newlines.  Note that we don't add newlines to the output coming from messages from the compiler frontend, since that output already has trailing newlines.

rdar://103608636
abertelrud added a commit to abertelrud/swift-package-manager that referenced this issue Dec 22, 2022
…lines

SwiftPM uses the Swift Driver's `-parseable-output` flag to get a sequence of length-prefixed JSON-encoded messages that it can read.  Unfortunately the Swift Driver doesn't JSON-encode its own diagnostics, leading to things like apple#5968.

I filed apple#5968 on the Swift Driver to get it to encode its JSON messages, but meanwhile, it turns out that we can fairly easily fix the fallback logic that SwiftPM uses when it emits raw output.  It was simply missing a newline.  It's safe to always add a newline to the raw driver output, since the logic that parses JSON splits by newlines, so we won't find any terminating newlines that shouldn't be on the string.

Note that we do not add newlines to the output coming from messages from the compiler frontend, since that output already has trailing newlines.  So in essence this change makes the `.unparsableOutput()` case the same as the cases of regular output, from a newline perspective.

rdar://103608636
abertelrud added a commit that referenced this issue Dec 22, 2022
…lines (#5987)

SwiftPM uses the Swift Driver's `-parseable-output` flag to get a sequence of length-prefixed JSON-encoded messages that it can read.  Unfortunately the Swift Driver doesn't JSON-encode its own diagnostics, leading to things like #5968.

I filed #5968 on the Swift Driver to get it to encode its JSON messages, but meanwhile, it turns out that we can fairly easily fix the fallback logic that SwiftPM uses when it emits raw output.  It was simply missing a newline.  It's safe to always add one, since the logic that parses JSON splits by newlines, so we won't find any terminating newlines.

rdar://103608636
abertelrud added a commit to abertelrud/swift-package-manager that referenced this issue Dec 22, 2022
…ng newlines (apple#5987)

SwiftPM uses the Swift Driver's `-parseable-output` flag to get a sequence of length-prefixed JSON-encoded messages that it can read.  Unfortunately the Swift Driver doesn't JSON-encode its own diagnostics, leading to things like apple#5968.

I filed apple/swift-driver#1241 on the Swift Driver to get it to encode its JSON messages, but meanwhile, it turns out that we can fairly easily fix the fallback logic that SwiftPM uses when it emits raw output.  It was simply missing a newline.  It's safe to always add one, since the logic that parses JSON splits by newlines, so we won't find any terminating newlines.

rdar://103608636
(cherry picked from commit 14d05cc)
@abertelrud
Copy link
Contributor

Fixed in #5987

@abertelrud
Copy link
Contributor

Also nominating for SwiftPM 5.8 as it's a safe fix.

@abertelrud
Copy link
Contributor

This fix adds the requisite newlines. I wonder if the original truncation had something to do with #5960.

abertelrud added a commit that referenced this issue Jan 3, 2023
…ng newlines (#5987) (#5996)

SwiftPM uses the Swift Driver's `-parseable-output` flag to get a sequence of length-prefixed JSON-encoded messages that it can read.  Unfortunately the Swift Driver doesn't JSON-encode its own diagnostics, leading to things like #5968.

I filed apple/swift-driver#1241 on the Swift Driver to get it to encode its JSON messages, but meanwhile, it turns out that we can fairly easily fix the fallback logic that SwiftPM uses when it emits raw output.  It was simply missing a newline.  It's safe to always add one, since the logic that parses JSON splits by newlines, so we won't find any terminating newlines.

rdar://103608636
(cherry picked from commit 14d05cc)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants