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

-parseable-output being dumped by driver or SPM with non-traditional package layout. #5482

Open
dabrahams opened this issue May 9, 2022 · 23 comments
Assignees
Labels

Comments

@dabrahams
Copy link
Contributor

Describe the bug
Sometimes I see JSON output from SPM when invoking swift test.

To Reproduce

  1. Get v. 0.1.0 of https://github.com/loftware/ArrayOfOptional (it is tagged)

  2. Go into ArrayOfOptional.swift and change the line

        if isOccupied[i] { (p + i).deinitialize(count: 1) }
    

    to

        if isOccupied[i] { (p + i).deinitialize(count: 1, 3) }
    
  3. rm -rf .build .swiftpm

  4. swift test; I get normal output as expected below

  5. swift test again; observe embedded json

swift test
3056
{
  "command_arguments" : [
    "-frontend",
    "-c",
    "-primary-file",
    "\/Users\/dave\/src\/loft\/ArrayOfOptional\/ArrayOfOptional.swift",
    "-emit-dependencies-path",
    "\/Users\/dave\/src\/loft\/ArrayOfOptional\/.build\/arm64-apple-macosx\/debug\/LoftDataStructures_ArrayOfOptional.build\/ArrayOfOptional.d",
    "-emit-reference-dependencies-path",
    "\/Users\/dave\/src\/loft\/ArrayOfOptional\/.build\/arm64-apple-macosx\/debug\/LoftDataStructures_ArrayOfOptional.build\/ArrayOfOptional.swiftdeps",
    "-target",
    "arm64-apple-macosx10.10",
    "-Xllvm",
    "-aarch64-use-tbi",
    "-enable-objc-interop",
    "-sdk",
    "\/Applications\/Xcode.app\/Contents\/Developer\/Platforms\/MacOSX.platform\/Developer\/SDKs\/MacOSX12.3.sdk",
    "-I",
    "\/Users\/dave\/src\/loft\/ArrayOfOptional\/.build\/arm64-apple-macosx\/debug",
    "-I",
    "\/Applications\/Xcode.app\/Contents\/Developer\/Platforms\/MacOSX.platform\/Developer\/usr\/lib",
    "-F",
    "\/Applications\/Xcode.app\/Contents\/Developer\/Platforms\/MacOSX.platform\/Developer\/Library\/Frameworks",
    "-enable-testing",
    "-g",
    "-module-cache-path",
    "\/Users\/dave\/src\/loft\/ArrayOfOptional\/.build\/arm64-apple-macosx\/debug\/ModuleCache",
    "-swift-version",
    "5",
    "-Onone",
    "-D",
    "SWIFT_PACKAGE",
    "-D",
    "DEBUG",
    "-new-driver-path",
    "\/Applications\/Xcode.app\/Contents\/Developer\/Toolchains\/XcodeDefault.xctoolchain\/usr\/bin\/swift-driver",
    "-resource-dir",
    "\/Applications\/Xcode.app\/Contents\/Developer\/Toolchains\/XcodeDefault.xctoolchain\/usr\/lib\/swift",
    "-enable-anonymous-context-mangled-names",
    "-module-name",
    "LoftDataStructures_ArrayOfOptional",
    "-target-sdk-version",
    "12.3",
    "-parse-as-library",
    "-o",
    "\/Users\/dave\/src\/loft\/ArrayOfOptional\/.build\/arm64-apple-macosx\/debug\/LoftDataStructures_ArrayOfOptional.build\/ArrayOfOptional.swift.o",
    "-index-store-path",
    "\/Users\/dave\/src\/loft\/ArrayOfOptional\/.build\/arm64-apple-macosx\/debug\/index\/store",
    "-index-system-modules"
  ],
  "command_executable" : "\/Applications\/Xcode.app\/Contents\/Developer\/Toolchains\/XcodeDefault.xctoolchain\/usr\/bin\/swift-frontend",
  "inputs" : [
    "\/Users\/dave\/src\/loft\/ArrayOfOptional\/ArrayOfOptional.swift"
  ],
  "kind" : "began",
  "name" : "compile",
  "outputs" : [
    {
      "path" : "\/Users\/dave\/src\/loft\/ArrayOfOptional\/.build\/arm64-apple-macosx\/debug\/LoftDataStructures_ArrayOfOptional.build\/ArrayOfOptional.swift.o",
      "type" : "object"
    },
    {
      "path" : "\/Users\/dave\/src\/loft\/ArrayOfOptional\/.build\/arm64-apple-macosx\/debug\/LoftDataStructures_ArrayOfOptional.build\/ArrayOfOptional.d",
      "type" : "d"
    },
    {
      "path" : "\/Users\/dave\/src\/loft\/ArrayOfOptional\/.build\/arm64-apple-macosx\/debug\/LoftDataStructures_ArrayOfOptional.build\/ArrayOfOptional.swiftdeps",
      "type" : "swift-dependencies"
    }
  ],
  "pid" : 91709,
  "process" : {
    "real_pid" : 91709
  }
}
3569
{
  "command_arguments" : [
    "-frontend",
    "-emit-module",
    "-experimental-skip-non-inlinable-function-bodies-without-types",
    "\/Users\/dave\/src\/loft\/ArrayOfOptional\/ArrayOfOptional.swift",
    "-target",
    "arm64-apple-macosx10.10",
    "-Xllvm",
    "-aarch64-use-tbi",
    "-enable-objc-interop",
    "-sdk",
    "\/Applications\/Xcode.app\/Contents\/Developer\/Platforms\/MacOSX.platform\/Developer\/SDKs\/MacOSX12.3.sdk",
    "-I",
    "\/Users\/dave\/src\/loft\/ArrayOfOptional\/.build\/arm64-apple-macosx\/debug",
    "-I",
    "\/Applications\/Xcode.app\/Contents\/Developer\/Platforms\/MacOSX.platform\/Developer\/usr\/lib",
    "-F",
    "\/Applications\/Xcode.app\/Contents\/Developer\/Platforms\/MacOSX.platform\/Developer\/Library\/Frameworks",
    "-enable-testing",
    "-g",
    "-module-cache-path",
    "\/Users\/dave\/src\/loft\/ArrayOfOptional\/.build\/arm64-apple-macosx\/debug\/ModuleCache",
    "-swift-version",
    "5",
    "-Onone",
    "-D",
    "SWIFT_PACKAGE",
    "-D",
    "DEBUG",
    "-new-driver-path",
    "\/Applications\/Xcode.app\/Contents\/Developer\/Toolchains\/XcodeDefault.xctoolchain\/usr\/bin\/swift-driver",
    "-resource-dir",
    "\/Applications\/Xcode.app\/Contents\/Developer\/Toolchains\/XcodeDefault.xctoolchain\/usr\/lib\/swift",
    "-enable-anonymous-context-mangled-names",
    "-module-name",
    "LoftDataStructures_ArrayOfOptional",
    "-target-sdk-version",
    "12.3",
    "-emit-module-doc-path",
    "\/Users\/dave\/src\/loft\/ArrayOfOptional\/.build\/arm64-apple-macosx\/debug\/LoftDataStructures_ArrayOfOptional.swiftdoc",
    "-emit-module-source-info-path",
    "\/Users\/dave\/src\/loft\/ArrayOfOptional\/.build\/arm64-apple-macosx\/debug\/LoftDataStructures_ArrayOfOptional.swiftsourceinfo",
    "-emit-objc-header-path",
    "\/Users\/dave\/src\/loft\/ArrayOfOptional\/.build\/arm64-apple-macosx\/debug\/LoftDataStructures_ArrayOfOptional.build\/LoftDataStructures_ArrayOfOptional-Swift.h",
    "-parse-as-library",
    "-o",
    "\/Users\/dave\/src\/loft\/ArrayOfOptional\/.build\/arm64-apple-macosx\/debug\/LoftDataStructures_ArrayOfOptional.swiftmodule",
    "-emit-abi-descriptor-path",
    "\/Users\/dave\/src\/loft\/ArrayOfOptional\/.build\/arm64-apple-macosx\/debug\/LoftDataStructures_ArrayOfOptional.abi.json"
  ],
  "command_executable" : "\/Applications\/Xcode.app\/Contents\/Developer\/Toolchains\/XcodeDefault.xctoolchain\/usr\/bin\/swift-frontend",
  "inputs" : [

  ],
  "kind" : "began",
  "name" : "emit-module",
  "outputs" : [
    {
      "path" : "\/Users\/dave\/src\/loft\/ArrayOfOptional\/.build\/arm64-apple-macosx\/debug\/LoftDataStructures_ArrayOfOptional.swiftmodule",
      "type" : "swiftmodule"
    },
    {
      "path" : "\/Users\/dave\/src\/loft\/ArrayOfOptional\/.build\/arm64-apple-macosx\/debug\/LoftDataStructures_ArrayOfOptional.swiftdoc",
      "type" : "swiftdoc"
    },
    {
      "path" : "\/Users\/dave\/src\/loft\/ArrayOfOptional\/.build\/arm64-apple-macosx\/debug\/LoftDataStructures_ArrayOfOptional.swiftsourceinfo",
      "type" : "swiftsourceinfo"
    },
    {
      "path" : "\/Users\/dave\/src\/loft\/ArrayOfOptional\/.build\/arm64-apple-macosx\/debug\/LoftDataStructures_ArrayOfOptional.build\/LoftDataStructures_ArrayOfOptional-Swift.h",
      "type" : "objc-header"
    },
    {
      "path" : "\/Users\/dave\/src\/loft\/ArrayOfOptional\/.build\/arm64-apple-macosx\/debug\/LoftDataStructures_ArrayOfOptional.abi.json",
      "type" : "abi-baseline-json"
    }
  ],
  "pid" : 91708,
  "process" : {
    "real_pid" : 91708
  }
}
650
{
  "exit-status" : 1,
  "kind" : "finished",
  "name" : "compile",
  "output" : "\/Users\/dave\/src\/loft\/ArrayOfOptional\/ArrayOfOptional.swift:20:29: error: cannot convert value of type 'UnsafeMutablePointer<T>' to expected argument type 'Int'\n        if isOccupied[i] { (p + i).deinitialize(count: 1, 3) }\n                            ^\n\/Users\/dave\/src\/loft\/ArrayOfOptional\/ArrayOfOptional.swift:20:36: error: value of type 'Int' has no member 'deinitialize'\n        if isOccupied[i] { (p + i).deinitialize(count: 1, 3) }\n                           ~~~~~~~ ^~~~~~~~~~~~\n",
  "pid" : 91709,
  "process" : {
    "real_pid" : 91709
  }
}
133
{
  "exit-status" : 0,
  "kind" : "finished",
  "name" : "emit-module",
  "pid" : 91708,
  "process" : {
    "real_pid" : 91708
  }
}
[0/2] Compiling Swift Module 'LoftDataStructures_ArrayOfOptional' (1 sources)
Building for debugging...
[1/13] Compiling LoftDataStructures_ArrayOfOptional ArrayOfOptional.swift
/Users/dave/src/loft/ArrayOfOptional/ArrayOfOptional.swift:20:29: error: cannot convert value of type 'UnsafeMutablePointer<T>' to expected argument type 'Int'
        if isOccupied[i] { (p + i).deinitialize(count: 1, 3) }
                            ^
/Users/dave/src/loft/ArrayOfOptional/ArrayOfOptional.swift:20:36: error: value of type 'Int' has no member 'deinitialize'
        if isOccupied[i] { (p + i).deinitialize(count: 1, 3) }
                           ~~~~~~~ ^~~~~~~~~~~~
[2/13] Emitting module LoftDataStructures_ArrayOfOptional
error: fatalError

Expected behavior

swift test
Fetching https://github.com/loftware/CheckXCAssertionFailure from cache
Fetching https://github.com/loftware/StandardLibraryProtocolChecks from cache
Fetching https://github.com/loftware/BitVector from cache
Fetched https://github.com/loftware/CheckXCAssertionFailure (0.38s)
Fetched https://github.com/loftware/BitVector (0.38s)
Fetched https://github.com/loftware/StandardLibraryProtocolChecks (0.38s)
Computing version for https://github.com/loftware/BitVector
Computed https://github.com/loftware/BitVector at 0.1.0 (0.01s)
Computing version for https://github.com/loftware/StandardLibraryProtocolChecks
Computed https://github.com/loftware/StandardLibraryProtocolChecks at 0.1.2 (0.01s)
Computing version for https://github.com/loftware/CheckXCAssertionFailure
Computed https://github.com/loftware/CheckXCAssertionFailure at 0.9.6 (0.01s)
Creating working copy for https://github.com/loftware/BitVector
Working copy of https://github.com/loftware/BitVector resolved at 0.1.0
Creating working copy for https://github.com/loftware/CheckXCAssertionFailure
Working copy of https://github.com/loftware/CheckXCAssertionFailure resolved at 0.9.6
Creating working copy for https://github.com/loftware/StandardLibraryProtocolChecks
Working copy of https://github.com/loftware/StandardLibraryProtocolChecks resolved at 0.1.2
Building for debugging...
[1/13] Compiling LoftDataStructures_BitVector BitVector.swift
[2/13] Emitting module LoftDataStructures_BitVector
[3/15] Emitting module LoftDataStructures_ArrayOfOptional
[4/15] Compiling LoftDataStructures_ArrayOfOptional ArrayOfOptional.swift
/Users/dave/src/loft/ArrayOfOptional/ArrayOfOptional.swift:20:29: error: cannot convert value of type 'UnsafeMutablePointer<T>' to expected argument type 'Int'
        if isOccupied[i] { (p + i).deinitialize(count: 1, 3) }
                            ^
/Users/dave/src/loft/ArrayOfOptional/ArrayOfOptional.swift:20:36: error: value of type 'Int' has no member 'deinitialize'
        if isOccupied[i] { (p + i).deinitialize(count: 1, 3) }
                           ~~~~~~~ ^~~~~~~~~~~~
error: fatalError

Environment (please complete the following information):

  • OS: macOS 12.3.1
  • Xcode Version/Tag/Branch: Version 13.3.1 (13E500a)

Additional context

Can't reproduce this with a traditional project structure, e.g. this branch: https://github.com/loftware/ArrayOfOptional/tree/traditional-project-structure. Follow the same steps; no JSON.

However, the

error: fatalError

at the end of the build output is confusing and suspicious.

@dabrahams
Copy link
Contributor Author

More info; not sure if it's related, but it seems similar:

Even with a more traditional structure as seen here, an extra comma in the Package file can produce output like this:

'ebnf-citron': error: invalidManifestFormat("/Users/dave/src/ebnf-citron/Package.swift:14:66: error: expected expression in container literal\n      .executable(name: \"ebnf-citron\", targets: [\"ebnf-citron\"]),,\n                                                                 ^", diagnosticFile: nil)

@philipturner
Copy link

Citing #5505 (comment), yes. In that thread, I showed a way to reproduce and bisect the bug. It's too lengthy to put in this issue, though, and may be irrelevant. That's why I put it in the other issue.

@tomerd
Copy link
Contributor

tomerd commented May 14, 2022

Even with a more traditional structure as seen here, an extra comma in the Package file can produce output like this:

'ebnf-citron': error: invalidManifestFormat("/Users/dave/src/ebnf-citron/Package.swift:14:66: error: expected expression in container literal\n .executable(name: "ebnf-citron", targets: ["ebnf-citron"]),,\n ^", diagnosticFile: nil)

@dabrahams this is indeed a separate issue and iirc was already fixed in 5.7

@abertelrud
Copy link
Contributor

Tracked by rdar://93982172. Thanks for the detailed information on how to repro! I wasn't able to do so earlier when this was being discussed in the Slack thread, but will try again with these instructions.

@abertelrud
Copy link
Contributor

Oh I do see it now! Either I was doing it wrong before, or was testing with the wrong version. Thanks!

@abertelrud
Copy link
Contributor

I thought we had quashed this one. The problem is that because there are no changes to the structure, swift test (and indeed swift build) doesn't spend any time recreating the build description that it sends to llbuild. The code in buildPackageStructure calls:

        let buildSystem = try self.createBuildSystem(buildDescription: .none)

which means that BuildOperationBuildSystemDelegateHandler.init() creates an empty directory of "Swift parsers".

And yet llbuild decided that it needs to invoke a Swift compiler anyway, and so when BuildOperationBuildSystemDelegateHandler hears about it, it doesn't get a hit in the swiftParsers dictionary for that task and emits the output verbatim instead of parsing as Swift output.

It seems to me that the parsing of Swift output needs to be made more robust here — SwiftPM shouldn't have had to create a build description in order to properly deal with Swift output from llbuild. Looking at the code here to see what the minimal risk fix would be, since this should/needs to also go into 5.7.

@abertelrud
Copy link
Contributor

Note that this happens in the self.buildPackageStructure() call in:

                    // if buildPackageStructure returns a valid description we use that, otherwise we perform full planning
                    if try self.buildPackageStructure() {
                        // confirm the step above created the build description as expected
                        // we trust it to update the build description when needed
                        let buildDescriptionPath = self.buildParameters.buildDescriptionPath
                        guard self.fileSystem.exists(buildDescriptionPath) else {
                            throw InternalError("could not find build descriptor at \(buildDescriptionPath)")
                        }
                        // return the build description that's on disk.
                        return try BuildDescription.load(fileSystem: self.fileSystem, path: buildDescriptionPath)
                    }

which is before it even tries to load the build description.

@abertelrud
Copy link
Contributor

We looked at this before, and the mystery is why it tried to compile Swift sources when just trying to create the structure. I don't recall the details but I think one of the problems was that there wasn't a reproducible case, IIRC, means that it was just reading through code to guess what happened and why.

@abertelrud
Copy link
Contributor

So:

  1. SwiftPM needs to be way more robust in detecting and parsing Swift parseable output, but
  2. the question here (and the reason it doesn't always happen) is why llbuild is going off and compiling stuff when just depending on that the output structure is in place

@abertelrud
Copy link
Contributor

The <PackageStructure> node is defined as:

  "PackageStructure":
    tool: package-structure-tool
    inputs: ["/private/tmp/ArrayOfOptional/","/private/tmp/ArrayOfOptional/","/private/tmp/ArrayOfOptional/Package.swift","/private/tmp/ArrayOfOptional/Package.resolved"]
    outputs: ["<PackageStructure>"]
    description: "Planning build"
    allow-missing-inputs: true

in this manifest, and "/private/tmp/ArrayOfOptional/" is defined as:

nodes:
  "/private/tmp/ArrayOfOptional/":
    is-directory-structure: true

The other two files exist. So there should be no work to be done here. The is-directory-structure should mean that only the existence of files and directories, not their contents, should be taken into account.

@abertelrud
Copy link
Contributor

Stepping through llbuild to see if the is-directory-structure is being ignored for some reason. It looks as if this is causing all the source files under /private/tmp/ArrayOfOptional/ to get the default source file treatment by llbuild.

@abertelrud
Copy link
Contributor

Actually, that doesn't look like it. That just affects the scanning for updates. Here the trace file indicates that llbuild thinks that this rule has never been run before. The question is rather why it then proceeds to create rules from the Swift files it finds.

@abertelrud
Copy link
Contributor

@dmbryson, quick question: when a node is marked is-directory-structure: true, should that prevent running of rules to try to product files inside it? Or is a different attribute needed for that?

@abertelrud
Copy link
Contributor

@dabrahams In terms of workaround, I think one would be to use the --build-path option to put the build path somewhere other than inside the source code. There are of course bugs to fix here too in SwiftPM and possibly llbuild (not sure yet). But it's at least a workaround.

@dabrahams
Copy link
Contributor Author

@abertelrud of course I prefer out-of-tree builds anyway, but if you're telling me support for that is buggy, I guess I don't want to chance it.

@dmbryson
Copy link
Contributor

@abertelrud directory structure recursively scans and requests nodes within it. So yes, anything in there that is a produced node will be triggered for scanning and production (if needed).

@philipturner
Copy link

philipturner commented May 29, 2022

I just briefly found the parseable output bug on Swift-Colab. I thought I had worked around it, but maybe I triggered a rare edge case. Swift-Colab currently has the workaround where you delete and re-create the only Swift file in the strangely shaped package. However, this happened:

I was trying to test whether my package swift-reflection-mirror would work on Swift 5.4. I preserved the exact state of the repo when I experienced the bug; just switch to the spm5482 branch of my repository when executing the %install command on Colab.

  1. I compiled a Swift package using the Swift 5.4 toolchain (as an indirect dependency to the oddly shaped, one-file package that actually gets compiled).
  2. I restarted the runtime and downloaded the Swift 5.5 toolchain. This means I swapped out the previous 5.4 toolchain with the new 5.5 toolchain.
  3. I restarted the runtime and ran through the %install command that should compile the Swift package.
  4. The -parseable-output JSON blob shows.
  5. I run the %install command another time.
  6. The -parseable-output JSON does not show.

Repeating steps 5 and 6 any number of times does not get the JSON blob to show again. I do not think it behaved this way in my previous example (#5482 (comment)), but I'll have to double-check. Either way, the fact that it bypassed my workaround is strange.

@abertelrud
Copy link
Contributor

@abertelrud of course I prefer out-of-tree builds anyway, but if you're telling me support for that is buggy, I guess I don't want to chance it.

No, sorry, I was unclear: putting the build directory outside of the source directory should be robust. What I meant to do was to acknowledge that SwiftPM should be able to handle either case (i.e. there is definitely a SwiftPM bug here to fix) but in the meantime a robust workaround should be to use --build-path to put build directory outside the sources.

@abertelrud
Copy link
Contributor

@abertelrud directory structure recursively scans and requests nodes within it. So yes, anything in there that is a produced node will be triggered for scanning and production (if needed).

Thanks for confirming. So in this case I think probably an llbuild fix will be needed in order to fix this (specifically to allow a content filter to be applied so that the build folder isn't consider part of the sources).

@abertelrud
Copy link
Contributor

I just briefly found the parseable output bug on Swift-Colab. I thought I had worked around it, but maybe I triggered a rare edge case. Swift-Colab currently has the workaround where you delete and re-create the only Swift file in the strangely shaped package. However, this happened:

I was trying to test whether my package swift-reflection-mirror would work on Swift 5.4. I preserved the exact state of the repo when I experienced the bug; just switch to the spm5482 branch of my repository when executing the %install command on Colab.

  1. I compiled a Swift package using the Swift 5.4 toolchain (as an indirect dependency to the oddly shaped, one-file package that actually gets compiled).
  2. I restarted the runtime and downloaded the Swift 5.5 toolchain. This means I swapped out the previous 5.4 toolchain with the new 5.5 toolchain.
  3. I restarted the runtime and ran through the %install command that should compile the Swift package.
  4. The -parseable-output JSON blob shows.
  5. I run the %install command another time.
  6. The -parseable-output JSON does not show.

Repeating steps 5 and 6 any number of times does not get the JSON blob to show again. I do not think it behaved this way in my previous example (#5482 (comment)), but I'll have to double-check. Either way, the fact that it bypassed my workaround is strange.

Thanks a lot for the detailed description. Just to make sure I understand correctly, it sounds as if this happens on the first build after switching the toolchains?

@philipturner
Copy link

philipturner commented Jun 2, 2022

Thanks a lot for the detailed description. Just to make sure I understand correctly, it sounds as if this happens on the first build after switching the toolchains?

@abertelrud yes, it happened on the first build. Please let me know if you find any workarounds for that. Even if you fix the bug in Swift 5.8, it won't be fixed on older toolchains. I would like to implement a workaround (if any) into Swift-Colab v2.2 along with several other patches to the kernel's build system.

@philipturner
Copy link

philipturner commented Jun 5, 2022

I found a third reproducer for the bug. Colab notebook:

https://colab.research.google.com/drive/1F6pDngus2tw2I1EYntcgVn2OAc9Eonht?usp=sharing

I force-deleted the "checkouts" directory of ".build", then called "swift build" again with the same build flags. SwiftPM then correctly failed. But then, I ran "swift build" a third time and the massive JSON blob was output. Even wierder, the build proceeded to completion the third time!

@philipturner
Copy link

I ran the test from #5505 using the 2022-08-06 snapshot, and the JSON blob did not appear. I then switched toolchains to the 2022-08-09 snapshot, and the JSON blob did not appear. For reference, the JSON did appear when I started with the release 5.5.2 toolchain, so I was testing it correctly.

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

5 participants