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

Update .spi file to remove internal symbol for SVD2Swift #104

Merged
merged 2 commits into from
Aug 24, 2024

Conversation

Kyle-Ye
Copy link
Contributor

@Kyle-Ye Kyle-Ye commented May 29, 2024

Replace this paragraph with a description of your changes and rationale. Provide links to an existing issue or external references/discussions, if appropriate.

Checklist

  • I've added at least one test that validates that my change is working, if appropriate
  • I've followed the code style of the rest of the project
  • I've read the Contribution Guidelines
  • I've updated the documentation if necessary

@Kyle-Ye Kyle-Ye marked this pull request as ready for review May 29, 2024 06:48
@Kyle-Ye Kyle-Ye requested a review from rauhul as a code owner May 29, 2024 06:48
@Kyle-Ye
Copy link
Contributor Author

Kyle-Ye commented May 29, 2024

I've verified the .spi.yml file by validate-spi-manifest tool. Do you know is there any way we can test the new ".spi.yml" result locally? cc @finestructure

@finestructure
Copy link

Yes, we have an online validator: https://swiftpackageindex.com/validate-spi-manifest

@Kyle-Ye
Copy link
Contributor Author

Kyle-Ye commented May 29, 2024

Do you know is there any way we can test the new ".spi.yml" result locally?

I mean consuming .spi.yml and see the result locally.

We can see the doc build with the doc icon one in SwiftPackageIndex but the log does not include the build command.

Anyway, following the knowledge I got from this issue.

I assume SPI will use the following command to build SVD2Swift.

bash -c shopt -s nullglob && for manifest in Package.swift ; do
if ! grep -i "https://github.com/apple/swift-docc-plugin" "$manifest" ; then
cat <<EOF >> "$manifest"

package.dependencies.append(
    .package(url: "https://github.com/apple/swift-docc-plugin", from: "1.0.0")
)
EOF
fi
done
mkdir -p .docs/Test
env DEVELOPER_DIR=/Applications/Xcode-15.3.0.app xcrun swift package \
    --allow-writing-to-directory .docs/Test \
    generate-documentation \
    --emit-digest \
    --disable-indexing \
    --output-path .docs/Test \
    --target SVD2Swift

And after the new .spi change, it will become to use the following command.

env DEVELOPER_DIR=/Applications/Xcode-15.3.0.app xcrun swift package \
    --allow-writing-to-directory .docs/Test \
    generate-documentation \
    --emit-digest \
    --disable-indexing \
    --output-path .docs/Test \
    --target SVD2Swift \
    --minimum-access-level public

But sadly swift-docc-plugin does not support customize it currently.

https://github.com/apple/swift-docc-plugin/blob/3aad4aabc42835f09f600bb17070f6a11bc84180/Plugins/SharedPackagePluginExtensions/Target%2BdefaultSymbolGraphOptions.swift#L14-L39

@Kyle-Ye
Copy link
Contributor Author

Kyle-Ye commented Aug 12, 2024

The upstream has landed "minimum-access-level" support but have not released a version yet.

We need to add swift-docc-plugin main branch dependency manually in this repo's Package.swift or wait for a new release since SPI is using .package(url: "https://github.com/apple/swift-docc-plugin", from: "1.0.0").

cc @rauhul

After merging the PR and waiting the SPI pipeline update, you can see the updated doc hosted on https://swiftpackageindex.com/apple/swift-mmio/main/documentation/svd2swift

@Kyle-Ye Kyle-Ye marked this pull request as ready for review August 12, 2024 16:09
@Kyle-Ye
Copy link
Contributor Author

Kyle-Ye commented Aug 23, 2024

https://github.com/swiftlang/swift-docc-plugin/releases/tag/1.4.0 is released. And I believe this PR is ready for a merge.

@finestructure
Copy link

FYI, that syntax with two documentation_targets is almost certainly not going to work the way you intend. Only one of them will be decoded, I believe.

@Kyle-Ye
Copy link
Contributor Author

Kyle-Ye commented Aug 23, 2024

@finestructure
Copy link

The config is an array, yes. It's so you can have different configs per Swift version / platform.

@Kyle-Ye
Copy link
Contributor Author

Kyle-Ye commented Aug 23, 2024

The config is an array, yes. It's so you can have different configs per Swift version / platform.

All the targets will use the same Swift version / platform.

The use case here is that we'd like to give "SVD2Swift" target some custom_documentation_parameters while not effecting other targets.

Do you know what is the proper way to do it now? Or is it a feature currently not supported by SPI? @finestructure

@finestructure
Copy link

Unfortunately that's not supported and I can't think of way to work around that :(

@Kyle-Ye
Copy link
Contributor Author

Kyle-Ye commented Aug 23, 2024

  1. Wait SPI to support the use case.
  2. Or maintain a script to use swift-docc-plugin to build the doc, release doc manually and hosted it under GitHub page.
  • PROS: Full control over swift-docc/swift-docc-plugin. Won't be blocked by SPI.
  • CONS: No multi-version support. Extra effort to maintain the script.

Update:

Or we can just add "--minimum-access-level public" for all targets and keeps using SPI.

@finestructure
Copy link

Just to manage expectations, it's unlikely we'll support per target custom docc arguments anytime soon.

It's more likely that we'd support hosting pre-rendered doc archives that we'd copy and host.

@rauhul
Copy link
Collaborator

rauhul commented Aug 23, 2024

I'm like 95% confident that enabling --minimum-access-level public for all targets is ok for this repo (at least for the foreseeable future). I'll run this through CI and merge after it passes, thank you @Kyle-Ye for sticking through with it!

@rauhul rauhul merged commit 15d368b into apple:main Aug 24, 2024
5 checks passed
@Kyle-Ye
Copy link
Contributor Author

Kyle-Ye commented Aug 28, 2024

Checking the SPI doc build log and latest main doc hosted on SPI.

The internal symbols are still there due to '--minimum-access-level public' is not correctly handled.

The swift-docc-plugin is correctly set to 1.4.1.

✅  Doc result (pending) reported
========================================
GenerateDocs
========================================
Generating docs at path:  $PWD/.docs/apple/swift-mmio/main
Repository:               apple/swift-mmio
Swift version used:       5.10
Target:                   MMIO
Extracting symbol information for 'MMIO'...
Finished extracting symbol information for 'MMIO'. (0.58s)
Building documentation for 'MMIO'...
Error: Unknown option '--minimum-access-level public'
Usage: docc convert [<options>] [<source-bundle-path>]
  See 'docc convert --help' for more information.

@finestructure
Copy link

Does this flag need Swift 6 perhaps?

@Kyle-Ye
Copy link
Contributor Author

Kyle-Ye commented Aug 28, 2024

Spot the issue. The correct key should be --symbol-graph-minimum-access-level instead of --minimum-access-level.

@Kyle-Ye
Copy link
Contributor Author

Kyle-Ye commented Aug 28, 2024

By the way, I also noticed another issue on the log. What should we do to fix it? @finestructure

error: 'docc convert' invocation failed with a nonzero exit code: '64'
error: Error Domain=NSCocoaErrorDomain Code=4 "“MMIO.doccarchive” couldn’t be moved to “swift-mmio” because either the former doesn’t exist, or the folder containing the latter doesn’t exist." UserInfo={NSSourceFilePathErrorKey=/Users/admin/builder/spi-builder-workspace/.build/plugins/Swift-DocC/outputs/intermediates/MMIO.doccarchive, NSUserStringVariant=(
    Move
), NSDestinationFilePath=/Users/admin/builder/spi-builder-workspace/.docs/apple/swift-mmio/main, NSFilePath=/Users/admin/builder/spi-builder-workspace/.build/plugins/Swift-DocC/outputs/intermediates/MMIO.doccarchive, NSUnderlyingError=0x60000315cab0 {Error Domain=NSPOSIXErrorDomain Code=2 "No such file or directory"}}
Error while generating docs: retryLimitExceeded(lastError: Optional(Shell command failed:
    env DEVELOPER_DIR=/Applications/Xcode-15.4.0.app xcrun swift package --allow-writing-to-directory .docs/apple/swift-mmio/main generate-documentation --emit-digest --disable-indexing --output-path .docs/apple/swift-mmio/main --hosting-base-path apple/swift-mmio/main --source-service github --source-service-base-url https://github.com/apple/swift-mmio/blob/main --checkout-path $PWD --target MMIO --minimum-access-level public))

@finestructure
Copy link

If this persists after the flag change please open an issue and I'll take a closer look!

@Kyle-Ye
Copy link
Contributor Author

Kyle-Ye commented Aug 28, 2024

If this persists after the flag change please open an issue and I'll take a closer look!

Yes. I can just reproduce it locally on my machine. Open the issue via SwiftPackageIndex/SwiftPackageIndex-Server#3343

@Kyle-Ye
Copy link
Contributor Author

Kyle-Ye commented Sep 11, 2024

There is still something wrong with it. Tracking the issue on SPI-Server.

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.

3 participants