-
-
Notifications
You must be signed in to change notification settings - Fork 47
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
--symbol-graph-minimum-access-level is not working for swift-mmio #3374
Comments
I am still confused about this.
|
The command we're essentially running is
If you run this locally, does it produce an archive with the expected symbol visibility? |
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. |
One thing perhaps worth trying is specifying the parameter as |
I've generated a doc set with our process for current 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 I'm attaching the archive below in case you have a way of telling the difference. |
Actually, I think I see the difference:
vs
|
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 |
Pipeline: 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'
...
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":[]} |
This is baffling. We're clearly passing along the flags correctly, because
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:
|
Is there any chance that |
I can produce the correct result using
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. |
The
|
Yeah. The access level information is consumed by swift-docc-plugin and produce You need to debug swift-docc-plugin instead of swift-docc. |
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. |
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. |
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:
whereas my test produces
I'm baffled. The test is as follows and essentially ends up running a 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 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? |
Running the checkout.zip and it works fine for me with
Is there any way I can reproduce it to help debug it? (eg. make
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. |
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! |
I've pushed a standalone test repo here: https://github.com/finestructure/Issue3374 If you run the test it shows the problem: 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. |
I've pointed the test repo at my fork of swift-mmio just for convenience while I was still relying on |
The next step I'm doing:
|
I might know the problem here. It is 10 elements now.
But it should be 11 elements as follows.
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. |
What do you think of adding a logic of spliting the arguments by " " and flat them in SPI by Anyway, I'll fix it on swift-mmio side via apple/swift-mmio#122. |
Ah, that's great, well spotted! I suspected something like this and that's why I suggested trying I actually went ahead and tried it in my fork first thing - to no effect - and I think I know why now. I suspect the underlying command just silently ignores parameters it doesn't recognise rather than throw an error. It's really good to know that that's the case for future debugging sessions (although it would help if there was at least a warning). I'll leave the issue open until we've verified the docs are correct with your upstream fix. Thanks for your help fixing this! |
That's probably something I'll add, yes, just to remove that particular sharp edge. As I said, what would really help is a warning from the upstream commands (not sure which one that is - probably |
I believe it will land on swift-docc-plugin if it should have a warning. I can add it for unrecognized warning here. What do you think about it? @d-ronnqvist |
We explicitly don't want to warn about unrecognized arguments in the DocC plugin because we have a feature where the plugin forwards all unrecognized flags to The plugin documentation even has examples of flags that the plugin doesn't "recognize", like |
Is there a way a warning mode could be enabled? The problem is that we are picking a certain combination of plugin and docc when we generate docs, which might be different to what users are testing their setup with. If we don't get any warnings it's quite tricky for us to help diagnose the issue - and users have no chance to diagnose it themselves via our build logs (except @Kyle-Ye now 😆) if it doesn't show any warnings. |
If you're asking if the plugin could have an opt-in behavior where it warns about any "unknown" flags then the answer is yes, but it would cause a lot of noise since it would warn about all the passthrough flags. For example, in the example command from earlier:
these flags are "unknown" flags that the plugin forwards to DocC; The way to remedy this would be to make the plugin aware of all passthrough flags but this introduces two problems:
If each plugin version is associated with a certain DocC version, then this effectively robs both you and the developer of their choice of combinations of DocC and Plugin. This leads me to think that the only way for both you and the developer to be able to chose a combinations of DocC and Plugin versions and not have a large amount of warning noise is to make the plugin know of all possible flags for all previous DocC versions. This is what I was referring to when I said that we wouldn't want to add this type of warning. |
That's unfortunate, thanks for the explanation, @d-ronnqvist ! I presume that not all flag are destined for Is there any way you can see how we could better surface issues with doc gen invocations? For example, initially I didn't even know what result Kyle was looking for in the doc pages, because I didn't know how Perhaps one option would be to run with |
The latest main branch of swift-mmio (https://swiftpackageindex.com/apple/swift-mmio/main/documentation/svd2swift) still has the internal symbol.
And I can't get enough information from the log to continue investigating.
https://swiftpackageindex.com/builds/4046999D-F3D9-40C6-B7AC-19DEBF6DFE7E
The text was updated successfully, but these errors were encountered: