Skip to content

chore: Replace platform requirement with @available markup#326

Open
adam-fowler wants to merge 1 commit into
ordo-one:mainfrom
adam-fowler:remove-platform-requirement
Open

chore: Replace platform requirement with @available markup#326
adam-fowler wants to merge 1 commit into
ordo-one:mainfrom
adam-fowler:remove-platform-requirement

Conversation

@adam-fowler

@adam-fowler adam-fowler commented Jun 7, 2025

Copy link
Copy Markdown

Description

Replaced platform requirement in Package.swift with @available checks. This allows package-benchmark to be included in other packages without them requiring the additional platform requirements of package-benchmark.

Unfortunately we still need a 10.15 requirement as HdrHistogram requires it.

How Has This Been Tested?

I included package-benchmark in another package not requiring macOS 13 and verified everything compiled fine.

Minimal checklist:

  • I have performed a self-review of my own code
  • I have added DocC code-level documentation for any public interfaces exported by the package
  • I have added unit and/or integration tests that prove my fix is effective or that my feature works

@hassila

hassila commented Jun 7, 2025

Copy link
Copy Markdown
Contributor

Thanks!

@dimlio wdyt of updating hdr histogram similarly too?

@hassila hassila changed the title Replace platform requirement with @available markup chore: Replace platform requirement with @available markup Jun 8, 2025
@hassila

hassila commented Jul 1, 2025

Copy link
Copy Markdown
Contributor

@adam-fowler - most consumers of Benchmark simply puts a subdirectory with a separate Package.swift for the benchmarks that runs with the required platforms - have a look at e.g. Foundation - was this something you considered?

@adam-fowler

Copy link
Copy Markdown
Author

@adam-fowler - most consumers of Benchmark simply puts a subdirectory with a separate Package.swift for the benchmarks that runs with the required platforms - have a look at e.g. Foundation - was this something you considered?

Yeah I do that with Hummingbird. But it is much nicer to be able to include it in the main package.swift as we can ensure we don't break the benchmarks with code changes.

@hassila

hassila commented Jul 1, 2025

Copy link
Copy Markdown
Contributor

Oh, we just run the benchmarks as one CI step that is required to build too, but maybe that is not feasible?

@MahdiBM

MahdiBM commented Jul 1, 2025

Copy link
Copy Markdown
Contributor

I think using subdirectories is the better way to go so a package is not pulling this package unnecessarily, but I also do think this PR is fine to merge since a lot of different first-party packages have started doing this (and for good reasons) so package-benchmark would be consistent with those, and also the fact that it's not the easiest to figure out how to properly have a sub-package so someone might still want to do that anyway from a UX perspective.

See https://github.com/MahdiBM/swift-dns/Benchmarks for example, where I had to do a bunch of symlinking and then manually put together the Package.swift of the benchmarking sub-module like this to avoid needing to expose some APIs as public.

@adam-fowler

Copy link
Copy Markdown
Author

I think using subdirectories is the better way to go so a package is not pulling this package unnecessarily

SwiftPM does not pull the unused Benchmark dependency. I've just verified that

@MahdiBM

MahdiBM commented Jul 1, 2025

Copy link
Copy Markdown
Contributor

@adam-fowler that's news to me. I just tried MultipartKit's Benchmarks sub-module and even if i do swift build --target MultipartKit it still does pull package-benchmark although that target has no dependency on package-benchmark.
How did you test to confirm it doesn't pull package-benchmark in your case? is that package public?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants