search

SwiftDocOrg / swift-doc

A documentation generator for Swift projects
https://swiftdoc.org
MIT License
1.68k stars 99 forks source link

CI: add Windows to the CI matrix #269

Closed compnerd closed 3 years ago

compnerd commented 3 years ago

This adds the CI support for the Windows platform. Although this may successfully build in CI, some changes are going to be needed beyond getting it to build. Those are already in review.

compnerd commented 3 years ago

The current Windows failure:

error: Dependencies could not be resolved because root depends on 'SwiftSemantics' 0.3.0..<0.4.0.
'SwiftSemantics' >= 0.3.0 cannot be used because package 'swiftsemantics' is required using a stable-version but 'swiftsemantics' depends on an unstable-version package 'swift-syntax' and no versions of 'SwiftSemantics' match the requirement 0.3.1..<0.4.0.

I think that the Package.resolved is preventing the version specific manifest from bumping the version of swift-syntax?

CC: @MaxDesiatov @mattt

mattt commented 3 years ago

@compnerd That seems to be what's breaking things, yeah. Since swift-doc is primarily an executable, my preference would be to keep Package.resolved in history. Could we fix this by bumping the SwiftSemantics dependency to a version that targets your patched swift-syntax branch?

compnerd commented 3 years ago

Yeah, that does work, I have been running with that locally as a workaround.

mattt commented 3 years ago

@compnerd Sounds good. As discussed in https://github.com/SwiftDocOrg/SwiftSemantics/pull/15#issuecomment-831412273, we're momentarily blocked until that new snapshot is created.

compnerd commented 3 years ago

@mattt getting this sorted would be really nice. #258 and #269 together would bring the state of swift-doc to the point where we can start trying to figure out packaging and releasing for Windows. I have ideas on how to address that problem. That problem is related with getting the GHA support for swift-doc, rather than the local use of this package which is what these two PRs are addressing.

compnerd commented 3 years ago

I'll let this run for about an hour. I suspect that there is something going on with file endings thats preventing the tests from passing. Id like to get the CI coverage for building at least, and we can follow up with testing.

compnerd commented 3 years ago

Weird, I wonder why this is now failing. Perhaps a flake?

compnerd commented 3 years ago

I'm not sure what is going on here. But, with #276 I can actually get a build of swift-doc on Windows (see compnerd/swift-build#364, https://github.com/compnerd/swift-build/releases/tag/swift-doc-5.5-DEVELOPMENT-SNAPSHOT-2021-05-02-a).

compnerd commented 3 years ago

With the current version of the manifest:

error: Dependencies could not be resolved because root depends on 'SwiftSemantics' 0.3.2..<0.4.0.
'SwiftSemantics' >= 0.3.2 cannot be used because package 'swiftsemantics' is required using a stable-version but 'swiftsemantics' depends on an unstable-version package 'swift-syntax' and no versions of 'SwiftSemantics' match the requirement 0.3.3..<0.4.0.
compnerd commented 3 years ago

Hmm, the fact that it didn't fail immediately is interesting. I think that there is some cached state that is causing problems with this PR :-(

mattt commented 3 years ago

@compnerd Yeah, I'd gotten my hopes up after it didn't fail immediately. But after 20 minutes, I gave up. Weird log line to call out from that run: https://github.com/SwiftDocOrg/swift-doc/pull/269/checks?check_run_id=2577029218#step:11:55

To clarify your comment https://github.com/SwiftDocOrg/swift-doc/pull/269#issuecomment-840674256, is that with 85eff87? I believe some combination of .revision should get this working, but you're obviously able to validate that more quickly than I can through CI.

compnerd commented 3 years ago

Interesting; I think that is now cached; that occurs sometimes on CI, but I've never been able to catch the output to figure out what is going on :-(

compnerd commented 3 years ago

Okay 85eff879ba7f97682385deb8b7ae853049c59144 does resolve fine. Can you explain to me why that works?

mattt commented 3 years ago

Okay 85eff87 does resolve fine. Can you explain to me why that works?

Apple's documentation for Package.Dependency.Requirement provides an excellent summary of how Swift Package Manager resolves dependencies.

> The dependency requirement can be defined as one of three different version requirements: > > ### A version-based requirement > Decide whether your project accepts updates to a package dependency up to the next major version or up to the next minor version. To be more restrictive, select a specific version range or an exact version. Major versions tend to have more significant changes than minor versions, and may require you to modify your code when they update. > > The version rule requires Swift packages to conform to semantic versioning. To learn more about the semantic versioning standard, visit semver.org. > > Selecting the version requirement is the recommended way to add a package dependency. It allows you to create a balance between restricting changes and obtaining improvements and features. > > ### A branch-based requirement > Select the name of the branch for your package dependency to follow. Use branch-based dependencies when you’re developing multiple packages in tandem or when you don’t want to publish versions of your package dependencies. > > Note that packages which use branch-based dependency requirements can’t be added as dependencies to packages that use version-based dependency requirements; you should remove branch-based dependency requirements before publishing a version of your package. > > ### A commit-based requirement > Select the commit hash for your package dependency to follow. > > Choosing this option isn’t recommended, and should be limited to exceptional cases. While pinning your package dependency to a specific commit ensures that the package dependency doesn’t change and your code remains stable, you don’t receive any updates at all. If you worry about the stability of a remote package, consider one of the more restrictive options of the version-based requirement. > > Note that packages which use commit-based dependency requirements can’t be added as dependencies to packages that use version-based dependency requirements; you should remove commit-based dependency requirements before publishing a version of your package.

The problem stems from an incompatibility of requirement types. If two direct dependencies (SwiftSyntaxHighlighter and SwiftSemantics) share a transitive dependency (swift-syntax), but one requires a branch (release/5.5) and the other requires a commit, Swift Package Manager can't resolve it. (Theoretically it could, if it could determine that the commit belongs to the branch, but that's not happening).

The error message is a bit cryptic, as it mostly discusses version ranges and "stable/unstable versions" (which AFAICT is terminology not found elsewhere). And, honestly, I'm not particularly confident in my assessment. All I know is that the last time I got this kind of message, pinning to specific releases got things building 🤷‍♂️

compnerd commented 3 years ago

I don't understand what is going on with this PR; I say that we merge the manifest changes, discard this change and create a new PR for the CI.

mattt commented 3 years ago

@compnerd Please hold off on that — there's one last thing I want to try first. If this fails, we can try again with a new PR.

My latest change removes the swift clean step that appears to have caused the previous runs to stall.

mattt commented 3 years ago

@compnerd I cancelled the latest run after ~30 minutes. Can you make anything out of these logs? https://github.com/SwiftDocOrg/swift-doc/runs/2577613424

Feel free to close and create a new PR if you think that'd help.

Also, curious to know — how long does a clean build take on your local machine?

compnerd commented 3 years ago

Clean builds on my personal machine take ~10m. It takes ~20m on GitHub: https://github.com/compnerd/swift-build/actions/runs/829993385

compnerd commented 3 years ago

279 seems to get further, Im going to close this off.