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

Introduce SwiftBundler library target #66

Merged
merged 19 commits into from
Mar 10, 2025

Conversation

ctreffs
Copy link
Contributor

@ctreffs ctreffs commented Mar 4, 2025

Motivation

While swift-bundler being an executable is nice and useful, some use-cases require the core functionality of swift-bundler to be available as a library. This allows for example other ArgumentParser based executables to incorporate swift-bundler commands and execute them as part of their CLI.

This PR enables that through introducing a SwiftBundler library target.
All swift-bundler files where moved to that target, except Main.swift which remains the sole file in the swift-bundler executable.

Nothing changes in regards to functionality only the essential types like SwiftBundler where made public.

Library usage

For now we don't expose every type in the public interface, but provide the main entry point to library users.
A library user can then just call

SwiftBundler.main(arguments: ["run", ...])

to invoke swift-bundler as usual.
This ensures no deviation between CLI interface and library functionality while allowing swift-bundler being used inside other CLI code.

CHANGES

  • Use official Swift Argument Parser v1.5.0 (makes interface compatible to be imported form different target)
  • Move swift-bundler coder to library target SwiftBundler
  • Refactor Main.swift to main.swift to support Swift 5.10 main function creation on Windows

Copy link
Owner

@stackotter stackotter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for doing this 🙏 I replied to your comments and I agree that things should be changed to avoid polluting the global namespace.

I believe that the renaming of SwiftBundler to SwiftBundlerDocumentation will break the update-docsworkflow (which will only run once this is merged so you won't see it in the PR CI results). DocC has been able to generate docs for executable targets for a while now (I think?), so we can probably just move the docs to the CLI target and update the CI action accordingly. The alternative is leaving them inSwiftBundlerDocumentationand updating the action to reflect that, but then the documentation title will beSwiftBundlerDocumentationrather thanSwiftBundlerorswift-bundler`, unless DocC has introduced finer control over that since I set up these docs.

@ctreffs
Copy link
Contributor Author

ctreffs commented Mar 7, 2025

I believe that the renaming of SwiftBundler to SwiftBundlerDocumentation will break the update-docsworkflow (which will only run once this is merged so you won't see it in the PR CI results). DocC has been able to generate docs for executable targets for a while now (I think?), so we can probably just move the docs to the CLI target and update the CI action accordingly. The alternative is leaving them inSwiftBundlerDocumentationand updating the action to reflect that, but then the documentation title will beSwiftBundlerDocumentationrather thanSwiftBundlerorswift-bundler`, unless DocC has introduced finer control over that since I set up these docs.

I've recreated what happens on CI locally and as far as I can tell it does not break anything.

Screenshot 2025-03-07 at 08 54 06

I've therefore updated the ./generate_docs.sh script to match whats happening on CI to verify this. Please tell me if this is still as expected.

@ctreffs ctreffs requested a review from stackotter March 7, 2025 07:56
Copy link
Owner

@stackotter stackotter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good now, just the docs issue which I'll finish my comment on.

@stackotter
Copy link
Owner

I believe that the renaming of SwiftBundler to SwiftBundlerDocumentation will break the update-docsworkflow (which will only run once this is merged so you won't see it in the PR CI results). DocC has been able to generate docs for executable targets for a while now (I think?), so we can probably just move the docs to the CLI target and update the CI action accordingly. The alternative is leaving them inSwiftBundlerDocumentationand updating the action to reflect that, but then the documentation title will beSwiftBundlerDocumentationrather thanSwiftBundlerorswift-bundler`, unless DocC has introduced finer control over that since I set up these docs.

I've recreated what happens on CI locally and as far as I can tell it does not break anything.

Screenshot 2025-03-07 at 08 54 06

I've therefore updated the ./generate_docs.sh script to match whats happening on CI to verify this. Please tell me if this is still as expected.

The documentation is meant to contain a bunch of usage articles rather than code documentation. Here's what it looks like at the moment; https://swiftbundler.dev/documentation/swiftbundler/

The documentation is currently located at Documentation/SwiftBundler.docc and after your changes would be compiled under the SwiftBundlerDocumentation target. The main issue I see with that (assuming the github workflow gets updated) is that the url would be /documentation/swiftbundlerdocumentation and that the documentation title would probably be SwiftBundlerDocumentation (I seem to remember trying to change the title of a DocC site in the past to no avail).

Might be worth trying to move the docc archive to swift-bundler and then generating docs for that target and seeing how it goes.

@ctreffs
Copy link
Contributor Author

ctreffs commented Mar 7, 2025

#65 should be merged as basis for this PR

Copy link
Owner

@stackotter stackotter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Happy to merge this once the Windows CI gets fixed (looks like one of the Signal changes didn't propagate to the #if os(Windows)-gated implementation)

@ctreffs
Copy link
Contributor Author

ctreffs commented Mar 10, 2025

The current Windows CI build failure is swift version dependent as it seems. Running this on my local machine everything is fine:

PS C:\Users\ctref\Developer\swift-bundler> swift build --product swift-bundler
Building for debugging...
[1/1] Write auxiliary file C:\Users\ctref\Developer\swift-bundler\.build\x86_64-unknown-windows-msvc\debug\swift-version--4DE95AB4C5047989.txt
Build of product 'swift-bundler' complete! (63.45s)
PS C:\Users\ctref\Developer\swift-bundler> swift --version
Swift version 6.0.2 (swift-6.0.2-RELEASE)
Target: x86_64-unknown-windows-msvc
PS C:\Users\ctref\Developer\swift-bundler>

Have to look into it more in-depth

@ctreffs ctreffs requested a review from stackotter March 10, 2025 08:28
@stackotter
Copy link
Owner

The current Windows CI build failure is swift version dependent as it seems. Running this on my local machine everything is fine:

Yeah I've run into that issue before (in SwiftCrossUI's CI), it's such a frustrating bug, no clue why it happens.

Copy link
Owner

@stackotter stackotter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lgtm! I'll merge this into a new branch, figure out docs stuff, then merge into main.

@stackotter stackotter changed the base branch from main to library_target March 10, 2025 14:36
@stackotter stackotter merged commit 9a2466e into stackotter:library_target Mar 10, 2025
4 checks passed
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.

2 participants