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.

Sign up for GitHub

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

Jump to bottom

Public access to all targets declared in packages #932

Closed
sewerynplazuk opened this issue Feb 21, 2024 · 11 comments
Closed

Public access to all targets declared in packages #932

sewerynplazuk opened this issue Feb 21, 2024 · 11 comments

Comments

@sewerynplazuk
Copy link
Contributor

In the latest release - 0.28.0 - the underlying targets declared by the packages were hidden. As I understand the change was introduced in #883. I generally agree with the view that you need to rely on the library products rather than the underlying targets but not in all cases.

I use these rules to generate BUILD files for SPM packages, so I can wrap them in XCFrameworks and use them for internal distribution. To do this, access to the underlying targets is crucial. Thus my question, can we restore public access to these targets, perhaps under a feature toggle?
@cgrindel
Copy link
Owner

I don't feel strongly about the visibility of the generated targets.

@brentleyjones Do you have any thoughts?

@brentleyjones
Copy link
Collaborator

The visibility is to enforce the same restriction that SPM has: you depend on the products of Swift packages, nothing else. So, in an SPM world, I don't believe you are able to do what you are trying to do, can you? If I'm wrong there, then yes, let's open them up. Or, if you can give a concrete example of how the current way fails, maybe there is a different solution than opening up the visibility.

@brentleyjones
Copy link
Collaborator

If this is just because you need a product that is a subset of another product, you could patch the package to add that product in. But I have a feeling that's not the root issue here.

@acecilia
Copy link

acecilia commented Feb 21, 2024 •
edited
Loading

I don't believe you are able to do what you are trying to do, can you?

Agree. This is actually a bazel-only feature.

I can give some of my context of the usecase:

  • AFAIK when using SPM is only supported to create dynamic frameworks from a swift package. But not static frameworks. I think the issue is very well explained here
  • A workaround is to rewrite the dependency-tree with another system such as cocoapods/xcodegen that allows to obtain a xcodeproj, and use the usual scripts/carthage/xcodebuild to create a prebuilt framework. The problem is when the dependency-tree is so complex/big that rewriting it is unattainable effort (example: GRPC only supports SPM in recent versions)
  • But thanks to rules_swift_package_manager + bazel, is possible to generate prebuilt dynamic/static framework from swift packages. rules_swift_package_manager is able to auto-generate the dependency-tree as bazel targets, and bazel is able to create prebuilt frameworks for them. So far I have not found any other tool that can do so

For extra context, this PR is based on the same usecase: need to disable library evolution in order to create the prebuilt frameworks

@sewerynplazuk
Copy link
Contributor Author

@acecilia described the problem very well. In short, a framework can contain only one module (aka SPM target), a product can depend on multiple targets. With only access to the product, I can't create all the necessary artifacts to distribute internally prebuilt binaries.

If this is just because you need a product that is a subset of another product, you could patch the package to add that product in. But I have a feeling that's not the root issue here.

This technically may work (manually creating products for all the targets), but it does not solve the problems with complex dependency trees that @acecilia mentioned (they need to be maintained).

@brentleyjones
Copy link
Collaborator

Could you provide an example repro for what you are trying to do. It would help me fully wrap my head around this. In the end we might allow changing the visibility, but I want to make sure there isn't another option first.

@cgrindel
Copy link
Owner

I agree with @brentleyjones. If we proceed with adjusting the visibility, I would like to add it as an option set in MODULE.bazel. Also, we will want to document the option as "use at your own risk". After the 1.0 release, we should ensure that the publicly visible targets are stable. Anything that is not public by default is fair game to change without a major version bump.

@sewerynplazuk
Copy link
Contributor Author

sewerynplazuk commented Feb 22, 2024 •
edited
Loading

@brentleyjones spm-to-xcframework, let me know if that helps.

@brentleyjones
Copy link
Collaborator

That does help. I need to think on this more.

@brentleyjones
Copy link
Collaborator

I think we can do this with a flag, preferably set per-package, with a disclaimer that the format of the labels can change between rules_swift_package_manager releases without us having to bump our major version (really that the targets themselves are an implementation detail), and that SPM counts those targets as non-public API as well.

@sewerynplazuk could you take on the work to implement this PR?

@sewerynplazuk
Copy link
Contributor Author

@brentleyjones Great, that should work for us. I'll prepare the change.

@sewerynplazuk sewerynplazuk mentioned this issue Nov 18, 2024
Merged
cgrindel pushed a commit that referenced this issue Jan 26, 2025
@sewerynplazuk
feat: add publicly_expose_all_targets flag (#1345)
d91c628 
Attempt to solve
#932.

Following the example given in the linked issue, with the new flag
`publicly_expose_all_targets `, it is possible to granularly expose the
build files of individual packages, as an example:

```
swift_deps.configure_package(
    name = "swift-nio-transport-services",
    publicly_expose_all_targets = True,
)
swift_deps.configure_package(
    name = "swift-protobuf",
    publicly_expose_all_targets = True,
)
use_repo(
    swift_deps,
    "swift_deps_info",
    "swift_package",
    "swiftpkg_swift_nio_transport_services",
    "swiftpkg_swift_protobuf",
)
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@brentleyjones @cgrindel @acecilia @sewerynplazuk