SwiftPackageIndex / SwiftPackageIndex-Server

The Swift Package Index is the place to find Swift packages!
https://swiftpackageindex.com
Apache License 2.0
542 stars 42 forks source link

--symbol-graph-minimum-access-level is not working for swift-mmio #3374

Open Kyle-Ye opened 1 week ago

Kyle-Ye commented 1 week ago

The latest main branch of swift-mmio (https://swiftpackageindex.com/apple/swift-mmio/main/documentation/svd2swift) still has the internal symbol.

https://github.com/apple/swift-mmio/pull/118

And I can't get enough information from the log to continue investigating.

https://swiftpackageindex.com/builds/4046999D-F3D9-40C6-B7AC-19DEBF6DFE7E

finestructure commented 1 week ago

Looking at the overview I see that the main branch docs are still being generated with Swift 5.10. Only the 0.1.0-pre1 branch is opting into Swift 6 via the

      swift_version: 6.0

key.

CleanShot 2024-09-10 at 11 44 05@2x

If it's working for that tag you should also opt into Swift 6 on the main branch.

finestructure commented 1 week ago

I case it's not clear, the little document icon indicates which builds generated docs:

CleanShot 2024-09-10 at 11 47 18@2x

Kyle-Ye commented 1 week ago

I am still confused about this.

  1. This flag does not require Swift 6, only the latest version of swift-docc-plugin is needed.
  2. I have checked the documentation on 0.1.0-pre1 and main, as well as the build logs of documentation build. The documentation still contains internal symbols and I am unable to obtain valuable compilation command parameters from the logs to further troubleshoot the issue.
finestructure commented 1 week ago

The command we're essentially running is

swift package
--allow-writing-to-directory
$outputPath
generate-documentation
--emit-digest
--disable-indexing
--output-path $outputPath
--hosting-base-path $hostingBasePath
--source-service github
--source-service-base-url https://github.com/$owner/$repo/blob/$reference
--checkout-path $checkoutPath
--target target
--symbol-graph-minimum-access-level public

If you run this locally, does it produce an archive with the expected symbol visibility?

finestructure commented 1 week ago

Other than that I can only note that there are no errors or warnings regarding documentation generation in the logs, so it's hard to diagnose any issues unfortunately.

finestructure commented 1 week ago

One thing perhaps worth trying is specifying the parameter as --symbol-graph-minimum-access-level=public.

Kyle-Ye commented 1 week ago

The command we're essentially running is

swift package
--allow-writing-to-directory
$outputPath
generate-documentation
--emit-digest
--disable-indexing
--output-path $outputPath
--hosting-base-path $hostingBasePath
--source-service github
--source-service-base-url https://github.com/$owner/$repo/blob/$reference
--checkout-path $checkoutPath
--target target
--symbol-graph-minimum-access-level public

If you run this locally, does it produce an archive with the expected symbol visibility?

Yes. The local build or preview works for me.

image

Full command I run locally.

swift package \
--disable-sandbox \
--allow-writing-to-directory \
./docs \
preview-documentation \
--emit-digest \
--disable-indexing \
--output-path ./docs \
--target SVD2Swift \
--symbol-graph-minimum-access-level public

Local diff with the latest main branch of swift-mmio

diff --git a/Package.resolved b/Package.resolved
index 4513f78..fec5896 100644
--- a/Package.resolved
+++ b/Package.resolved
@@ -9,6 +9,24 @@
         "version" : "1.4.0"
       }
     },
+    {
+      "identity" : "swift-docc-plugin",
+      "kind" : "remoteSourceControl",
+      "location" : "https://github.com/apple/swift-docc-plugin",
+      "state" : {
+        "revision" : "0510d9160330025fb5823f7845c26af3cd56a405",
+        "version" : "1.4.1"
+      }
+    },
+    {
+      "identity" : "swift-docc-symbolkit",
+      "kind" : "remoteSourceControl",
+      "location" : "https://github.com/swiftlang/swift-docc-symbolkit",
+      "state" : {
+        "revision" : "b45d1f2ed151d057b54504d653e0da5552844e34",
+        "version" : "1.0.0"
+      }
+    },
     {
       "identity" : "swift-syntax",
       "kind" : "remoteSourceControl",
diff --git a/Package.swift b/Package.swift
index 43bd0e6..15825fc 100644
--- a/Package.swift
+++ b/Package.swift
@@ -35,6 +35,7 @@ let package = Package(
     .package(
       url: "https://github.com/swiftlang/swift-syntax.git",
       from: "509.0.2"),
+    .package(url: "https://github.com/apple/swift-docc-plugin", from: "1.4.1"),
   ],
   targets: [
     // MMIO
finestructure commented 1 week ago

I've generated a doc set with our process for current main (4357d917a0c746ffe2a95d5a1c406095b5389c00) but I can't really tell if it's a "good" doc set or not, because I have no local setup to actually view it.

There must be a way to tell from the archive content (some of the symbol json files) whether it's ok or not. Or to view it without having to host the whole thing somewhere.

I've tried running the preview-documentation command and then replacing the contents of the docs directory but that doesn't work, because the docs generated via our process have different base paths and I just get a blank page.

I'm attaching the archive below in case you have a way of telling the difference.

swift-mmio-docs-main.zip

finestructure commented 1 week ago

Actually, I think I see the difference:

❯ ls -l docs/data/documentation/svd2swift/
total 48
-rw-r--r--@ 1 sas  staff  10839 11 Sep 09:41 usingsvd2swift.json
-rw-r--r--@ 1 sas  staff  11690 11 Sep 09:41 usingsvd2swiftplugin.json

vs

❯ ls -l swift-mmio-docs-main/data/documentation/svd2swift/
total 240
drwxr-xr-x  17 sas  staff    544 11 Sep 09:29 accesslevel/
-rw-r--r--   1 sas  staff   6131 11 Sep 09:29 accesslevel.json
drwxr-xr-x   9 sas  staff    288 11 Sep 09:29 exportcontext/
-rw-r--r--   1 sas  staff   8117 11 Sep 09:29 exportcontext.json
drwxr-xr-x   9 sas  staff    288 11 Sep 09:29 exportoptions/
-rw-r--r--   1 sas  staff   8101 11 Sep 09:29 exportoptions.json
-rw-r--r--   1 sas  staff   1858 11 Sep 09:29 fileheader.json
drwxr-xr-x   5 sas  staff    160 11 Sep 09:29 indentation/
-rw-r--r--   1 sas  staff   3712 11 Sep 09:29 indentation.json
drwxr-xr-x   4 sas  staff    128 11 Sep 09:29 input/
-rw-r--r--   1 sas  staff   3000 11 Sep 09:29 input.json
drwxr-xr-x   5 sas  staff    160 11 Sep 09:29 inputreader/
-rw-r--r--   1 sas  staff   3944 11 Sep 09:29 inputreader.json
drwxr-xr-x   5 sas  staff    160 11 Sep 09:29 output/
-rw-r--r--   1 sas  staff   3757 11 Sep 09:29 output.json
drwxr-xr-x  11 sas  staff    352 11 Sep 09:29 outputwriter/
-rw-r--r--   1 sas  staff   7764 11 Sep 09:29 outputwriter.json
drwxr-xr-x  12 sas  staff    384 11 Sep 09:29 svd/
-rw-r--r--   1 sas  staff   4024 11 Sep 09:29 svd.json
drwxr-xr-x  45 sas  staff   1440 11 Sep 09:29 svd2swift/
-rw-r--r--   1 sas  staff  18311 11 Sep 09:29 svd2swift.json
drwxr-xr-x   7 sas  staff    224 11 Sep 09:29 svd2swifterror/
-rw-r--r--   1 sas  staff   4271 11 Sep 09:29 svd2swifterror.json
drwxr-xr-x   5 sas  staff    160 11 Sep 09:29 svdexportable/
-rw-r--r--   1 sas  staff   6868 11 Sep 09:29 svdexportable.json
drwxr-xr-x   6 sas  staff    192 11 Sep 09:29 swift/
-rw-r--r--   1 sas  staff   2387 11 Sep 09:29 swift.json
-rw-r--r--   1 sas  staff  10839 11 Sep 09:29 usingsvd2swift.json
-rw-r--r--   1 sas  staff  11690 11 Sep 09:29 usingsvd2swiftplugin.json
Kyle-Ye commented 1 week ago

Check $ROOT_DOC/documentation/svd2swift and it should only the following

tree ./docs/documentation/svd2swift
./docs/documentation/svd2swift
├── index.html
├── usingsvd2swift
│   └── index.html
└── usingsvd2swiftplugin
    └── index.html

3 directories, 3 files
image

Left: SPI doc in your last comment, right: my local build

Again, this is docs result I got using the commands I mention above.

swift-mmio-svd2swift.zip

Kyle-Ye commented 1 week ago

Actually, I think I see the difference:

Yeah. That's the bug here.

If swift-docc-plugin has identified "--symbol-graph-minimum-access-level" correctly, it should not produce internal symbol for svd2swift. (The extra ones in your documentation/svd2swift folder)

Kyle-Ye commented 1 week ago

Pipeline: swift-docc-plugin -> trigger SwiftPM to build and extract the symbols.

If you pass --verbose to swift-docc-plugin call, you can see the full docc command.

eg.

swift package \
--disable-sandbox \
--allow-writing-to-directory \
./docs \
preview-documentation \
--emit-digest \
--disable-indexing \
--output-path ./docs \
--target SVD2Swift \
--symbol-graph-minimum-access-level public \
--verbose

# Output information in console
...
symbol graph options: 'SymbolGraphOptions(minimumAccessLevel: PackagePlugin.PackageManager.SymbolGraphOptions.AccessLevel.public, includeSynthesized: true, includeSPI: false, emitExtensionBlocks: true)'
...
docc invocation: '/Applications/Xcode-15.3.0.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/docc preview /Users/kyle/tmp/swift-mmio/Sources/SVD2Swift/Documentation.docc --emit-digest --fallback-display-name SVD2Swift --fallback-bundle-identifier SVD2Swift --additional-symbol-graph-dir /Users/kyle/tmp/swift-mmio/.build/arm64-apple-macosx/extracted-symbols/swift-mmio/SVD2Swift --output-path /Users/kyle/tmp/swift-mmio/docs --fallback-default-module-kind Command-line Tool'
...
  1. Check the SymbolGraphOptions log information in you swift-docc-plugin verbose call.
  2. Check SVD2Swift.symbols.json in additional-symbol-graph-dir.

It should be something like the following with empty symbols

{"metadata":{"formatVersion":{"major":0,"minor":6,"patch":0},"generator":"Apple Swift version 5.10 (swiftlang-5.10.0.13 clang-1500.3.9.4)"},"module":{"name":"SVD2Swift","platform":{"architecture":"arm64","vendor":"apple","operatingSystem":{"name":"macosx","minimumVersion":{"major":10,"minor":15}}}},"symbols":[],"relationships":[]}
finestructure commented 1 week ago

This is baffling. We're clearly passing along the flags correctly, because --verbose is taking effect. Yet --symbol-graph-minimum-access-level public is clearly being ignored:

🖥️ env DEVELOPER_DIR=/Applications/Xcode-15.4.0.app xcrun swift package --allow-writing-to-directory .docs/finestructure/swift-mmio/issue-3374 generate-documentation --emit-digest --disable-indexing --output-path .docs/finestructure/swift-mmio/issue-3374 --hosting-base-path finestructure/swift-mmio/issue-3374 --source-service github --source-service-base-url https://github.com/finestructure/swift-mmio/blob/issue-3374 --checkout-path /var/folders/nk/tlpts6bs799038htr6rx1xnc0000gn/T/873F8D4C-C9B2-4801-B00F-3705D0788AD2/checkout --target SVD2Swift --symbol-graph-minimum-access-level public --verbose
Extracting symbol information for 'SVD2Swift'...
symbol graph options: 'SymbolGraphOptions(minimumAccessLevel: PackagePlugin.PackageManager.SymbolGraphOptions.AccessLevel.internal, includeSynthesized: true, includeSPI: false, emitExtensionBlocks: true)'

Can you think of anything else that might be interfering?

I've made sure the docc-plugin version is constrained to the latest when appending the dependency:

package.dependencies.append(
    .package(url: "https://github.com/swiftlang/swift-docc-plugin", from: "1.4.3")
)
finestructure commented 1 week ago

Is there any chance that preview-documentation and generate-documentation are behaving differently? It's about the only difference I can see at the moment between these two tests we're comparing.

Kyle-Ye commented 1 week ago

Is there any chance that preview-documentation and generate-documentation are behaving differently? It's about the only difference I can see at the moment between these two tests we're comparing.

I can produce the correct result using generate-documentation too.

Can you think of anything else that might be interfering?

I do not know since I can't reproduce your issue.

Can you ssh into your CI machine and using a local swift-docc-plugin to debug it directly?

For debug tips, hope my post can give some help.

finestructure commented 1 week ago

The docc invocation doesn't contain any flags related to access level (but neither does it for yours):

docc invocation: '/Applications/Xcode-15.4.0.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/docc convert /private/var/folders/nk/tlpts6bs799038htr6rx1xnc0000gn/T/C5D10A1C-5F63-45E5-8744-2CCC518E3941/checkout/Sources/SVD2Swift/Documentation.docc --emit-digest --hosting-base-path finestructure/swift-mmio/issue-3374 --source-service github --source-service-base-url https://github.com/finestructure/swift-mmio/blob/issue-3374 --checkout-path /var/folders/nk/tlpts6bs799038htr6rx1xnc0000gn/T/C5D10A1C-5F63-45E5-8744-2CCC518E3941/checkout --fallback-display-name SVD2Swift --fallback-bundle-identifier SVD2Swift --additional-symbol-graph-dir /private/var/folders/nk/tlpts6bs799038htr6rx1xnc0000gn/T/C5D10A1C-5F63-45E5-8744-2CCC518E3941/checkout/.build/arm64-apple-macosx/extracted-symbols/checkout/SVD2Swift --output-path /private/var/folders/nk/tlpts6bs799038htr6rx1xnc0000gn/T/C5D10A1C-5F63-45E5-8744-2CCC518E3941/checkout/.build/plugins/Swift-DocC/outputs/intermediates/SVD2Swift.doccarchive --fallback-default-module-kind Command-line Tool'
Kyle-Ye commented 1 week ago

Yeah. The access level information is consumed by swift-docc-plugin and produce SVD2Swift.symbols.json in additional-symbol-graph-dir.

You need to debug swift-docc-plugin instead of swift-docc.

finestructure commented 1 week ago

Can you ssh into your CI machine and using a local swift-docc-plugin to debug it directly?

That's not an option but also my integration test is running the exact same code as does the live service (and we can see that it produces the same result).

I can't think of anything else to try here but I'll leave the issue open in case we come up with other ideas.

Kyle-Ye commented 1 week ago

🖥️ env DEVELOPER_DIR=/Applications/Xcode-15.4.0.app xcrun swift package --allow-writing-to-directory .docs/finestructure/swift-mmio/issue-3374 generate-documentation --emit-digest --disable-indexing --output-path .docs/finestructure/swift-mmio/issue-3374 --hosting-base-path finestructure/swift-mmio/issue-3374 --source-service github --source-service-base-url https://github.com/finestructure/swift-mmio/blob/issue-3374 --checkout-path /var/folders/nk/tlpts6bs799038htr6rx1xnc0000gn/T/873F8D4C-C9B2-4801-B00F-3705D0788AD2/checkout --target SVD2Swift --symbol-graph-minimum-access-level public --verbose

Can you give me a full reproducible steps for your env? Otherwise I can't reproduce it.

eg. Send me the source directory of your swift-mmio or give the diff.patch compared with the latest main. (I assume you have modified the Package.swift otherwise there is no swift-docc-plugin dependency.)

I've give mine at here.

https://github.com/SwiftPackageIndex/SwiftPackageIndex-Server/issues/3374#issuecomment-2340597139

finestructure commented 6 days ago

I'm attaching the checkout directory below, although I suspect it won't be of much use. I just ran the same command as my integration test in it and it produces:

❯ env DEVELOPER_DIR=/Applications/Xcode-15.4.0.app xcrun swift package --allow-writing-to-directory docs generate-documentation --emit-digest --disable-indexing --output-path docs --hosting-base-path finestructure/swift-mmio/issue-3374 --source-service github --source-service-base-url https://github.com/finestructure/swift-mmio/blob/issue-3374 --checkout-path $PWD --target SVD2Swift --symbol-graph-minimum-access-level public --verbose
Building for debugging...
[1/1] Write swift-version-33747A42983211AE.txt
Build complete! (0.24s)
Building for debugging...
[1/1] Write swift-version-33747A42983211AE.txt
Build complete! (0.45s)
Extracting symbol information for 'SVD2Swift'...
symbol graph options: 'SymbolGraphOptions(minimumAccessLevel: PackagePlugin.PackageManager.SymbolGraphOptions.AccessLevel.public, includeSynthesized: true, includeSPI: false, emitExtensionBlocks: true)'

whereas my test produces

🖥️ env DEVELOPER_DIR=/Applications/Xcode-15.4.0.app xcrun swift package --allow-writing-to-directory .docs/finestructure/swift-mmio/issue-3374 generate-documentation --emit-digest --disable-indexing --output-path .docs/finestructure/swift-mmio/issue-3374 --hosting-base-path finestructure/swift-mmio/issue-3374 --source-service github --source-service-base-url https://github.com/finestructure/swift-mmio/blob/issue-3374 --checkout-path /var/folders/nk/tlpts6bs799038htr6rx1xnc0000gn/T/445584C3-528E-444D-AE10-0E9C3FCB7A2A/checkout --target SVD2Swift --symbol-graph-minimum-access-level public --verbose
Extracting symbol information for 'SVD2Swift'...
symbol graph options: 'SymbolGraphOptions(minimumAccessLevel: PackagePlugin.PackageManager.SymbolGraphOptions.AccessLevel.internal, includeSynthesized: true, includeSPI: false, emitExtensionBlocks: true)'

I'm baffled. The test is as follows and essentially ends up running a Process with the above command:

    func test_issue_3374() async throws {
        // https://github.com/SwiftPackageIndex/SwiftPackageIndex-Server/issues/2504
        let cloneURL = "https://github.com/finestructure/swift-mmio"
        let ref = "issue-3374"
        let pathFragment = try Github.url.parse(cloneURL).path
        try await generateDocs(cloneURL: cloneURL,
                               reference: ref,
                               platform: .macosSpm,
                               swiftVersion: .latestRelease) { tempDir in
            let docsDir = tempDir + "/checkout/.docs/\(pathFragment)/\(ref)"
            print("open \(docsDir)")
            let indexJSON = "\(docsDir)/index/index.json".lowercased()
            XCTAssertTrue(
                Foundation.FileManager.default.fileExists(atPath: indexJSON),
                "not found: \(indexJSON)"
            )
            let index = try XCTUnwrap(DocArchive.Index(path: indexJSON))
            XCTAssertEqual(index.interfaceLanguages.swift.map(\.path), [
                "/documentation/svd2swift"
            ])
            let jsonFiles = try await Current.shell.run(command: .ls("\(docsDir)/data/documentation/svd2swift"))
                .split(separator: "\n")
                .sorted()
            XCTAssertEqual(jsonFiles.count, 2)
            XCTAssertEqual(jsonFiles, ["usingsvd2swift.json", "usingsvd2swiftplugin.json"])
        }
    }

It's pointing to my fork to add the --verbose and trim the target list. You'll see that it's essentially the official main otherwise.

There must be something in the test's environment that prevents it from picking up the flag. We only pass a limited set up env variables, I wonder if it could be that?

checkout.zip

Kyle-Ye commented 6 days ago

Running the checkout.zip and it works fine for me with Applications/Xcode-15.4.0.app xcrun swift package --allow-writing-to-directory docs generate-documentation --emit-digest --disable-indexing --output-path docs --hosting-base-path finestructure/swift-mmio/issue-3374 --source-service github --source-service-base-url https://github.com/finestructure/swift-mmio/blob/issue-3374 --checkout-path $PWD --target SVD2Swift --symbol-graph-minimum-access-level public --verbose.

whereas my test produces

🖥️ env DEVELOPER_DIR=/Applications/Xcode-15.4.0.app xcrun swift package --allow-writing-to-directory .docs/finestructure/swift-mmio/issue-3374 generate-documentation --emit-digest --disable-indexing --output-path .docs/finestructure/swift-mmio/issue-3374 --hosting-base-path finestructure/swift-mmio/issue-3374 --source-service github --source-service-base-url https://github.com/finestructure/swift-mmio/blob/issue-3374 --checkout-path /var/folders/nk/tlpts6bs799038htr6rx1xnc0000gn/T/445584C3-528E-444D-AE10-0E9C3FCB7A2A/checkout --target SVD2Swift --symbol-graph-minimum-access-level public --verbose
Extracting symbol information for 'SVD2Swift'...
symbol graph options: 'SymbolGraphOptions(minimumAccessLevel: PackagePlugin.PackageManager.SymbolGraphOptions.AccessLevel.internal, includeSynthesized: true, includeSPI: false, emitExtensionBlocks: true)'

Is there any way I can reproduce it to help debug it? (eg. make run test_issue_3374 accessible for me)

That's not an option but also my integration test is running the exact same code as does the live service (and we can see that it produces the same result).

It seems to be a panacea for me. I don't know why it is not an option for you.

The difficulty in front of me is that I can't reproduce the bug happening on SPI and your test target.

finestructure commented 1 day ago

I'm working on extracting this into a standalone test that I can share. It's going to take a moment to do that unfortunately!

finestructure commented 16 hours ago

I've pushed a standalone test repo here: https://github.com/finestructure/Issue3374

If you run the test it shows the problem:

CleanShot 2024-09-18 at 11 38 40@2x

The underlying machinery is a stripped down version of what we run on our build machines and seems to reproduce the problem exactly as it does in production.

finestructure commented 16 hours ago

I've pointed the test repo at my fork of swift-mmio just for convenience while I was still relying on .spi.yml to inject the config. This standalone test doesn't look at .spi.yml at all (the values are hardcoded in the builder code) and it doesn't really matter which repo it's targeting.

Kyle-Ye commented 9 hours ago
  1. The package is requiring Swift 6. But the DEVELOPER_DIR is hard coded as 15.4. I'm changing it into 16.0 and I believe it does not matter here.
  2. I can reproduce it in your example. Thanks.

The next step I'm doing:

  1. Replace the remote swift-docc-plugin dependency with my local one.
  2. Try to debug it to figure out what's happening here.
Kyle-Ye commented 9 hours ago

The arguments has passed in successfully at the beginning.

image

But when we actually consume it when symbol-graph generating in the end. It just gone.

image
Kyle-Ye commented 8 hours ago

I might know the problem here.

It is 10 elements now.

▿ 10 elements
  - 0 : "--emit-digest"
  - 1 : "--disable-indexing"
  - 2 : "--output-path"
  - 3 : ".docs/finestructure/swift-mmio/issue-3374"
  - 4 : "--hosting-base-path"
  - 5 : "finestructure/swift-mmio/issue-3374"
  - 6 : "--target"
  - 7 : "SVD2Swift"
  - 8 : "--symbol-graph-minimum-access-level public"
  - 9 : "--verbose"

But it should be 11 elements as follows.

▿ 10 elements
  - 0 : "--emit-digest"
  - 1 : "--disable-indexing"
  - 2 : "--output-path"
  - 3 : ".docs/finestructure/swift-mmio/issue-3374"
  - 4 : "--hosting-base-path"
  - 5 : "finestructure/swift-mmio/issue-3374"
  - 6 : "--target"
  - 7 : "SVD2Swift"
  - 8 : "--symbol-graph-minimum-access-level"
  - 9 : "public"
  - 10 : "--verbose"

Changing the following code in your repo would fix the issue.

        // hard-coding some values here
-        let customParameters = ["--symbol-graph-minimum-access-level public"
+        let customParameters = ["--symbol-graph-minimum-access-level", "public",
                                "--verbose"]

Tested and verified it. Done.

Kyle-Ye commented 8 hours ago

What do you think of adding a logic of spliting the arguments by " " and flat them in SPI by arguments.flatMap { $0.split(separator: " ") }.map { String($0) }? Or should we give a warning here?

Anyway, I'll fix it on swift-mmio side via https://github.com/apple/swift-mmio/pull/122.