yonaskolb / Mint

A package manager that installs and runs executable Swift packages
MIT License
2.27k stars 122 forks source link

Add support for SPM resource bundles #224

Closed tid-kijyun closed 2 years ago

tid-kijyun commented 2 years ago

Added support for resouce in SPM version 5.3 or later. #223 SPM outputs a file named [package name]_[target name].bundle (or *.resources on linux) when resources is specified for a target in Package.swift. If resources is specified for multiple targets, multiple bundles will be output, so I had to figure out the dependency graph and copy only what I need.
For example, SwiftGen outputs the following two bundles in the build, but mint should only copy one of them. (The other one is for testing, so it is not needed).

We also had to take into consider the case where resources are included in external packages that we depend on, so I separated the commits.

I used the following repository to test this.

About Unit Tests

I haven't written any unit tests because I didn't think I should use my personal repository for testing. If you can prepare a sample repository of packages that contain some resources, we can write unit tests.

kkharji commented 2 years ago

Any plans to merge this one?. btw @tid-kijyun thanks for solving my issue ❤️

bosbefok commented 2 years ago

Please can we get this merged. This issue is also affecting me.

yonaskolb commented 2 years ago

The solution in the PR https://github.com/yonaskolb/Mint/pull/248 to just copy the whole build directory seems to me to be a bit safer, simpler, and more forwards compatible. Any objections to that @tid-kijyun?

tid-kijyun commented 2 years ago

This is a matter of taste. As you know, that method(#248) also copies unnecessary files (test bundles, etc.). I avoided it because I wanted to avoid wasting user storage.

yonaskolb commented 2 years ago

Yep I hear you. As SPM has build plugins now though, it's harder and harder to statically determine the proper build outputs, so this may be the safer option. In the case of https://github.com/apple/swift-format, I don't know that there is a way to determine that lib_InternalSwiftSyntaxParser.dylib needs to be included without traversing the whole dependency tree.

yonaskolb commented 2 years ago

There would be a way to accomplish things and make this perfect, but I just don't have the time to maintain all that code with newer SPM releases

tid-kijyun commented 2 years ago

I understand. I think #248 is also a good solution as it has no side effects. Feel free to close this PR.😉

yonaskolb commented 2 years ago

https://github.com/yonaskolb/Mint/pull/248 released in 0.17.2. Closing this for now, but I appreciate the work that went into this. I'm not opposed to this approach in theory, and if people complain of disk space, we perhaps just copy all dylibs, bundles, and resources over.

yonaskolb commented 1 year ago

@tid-kijyun https://github.com/yonaskolb/Mint/pull/253 implements what this PR was trying to achieve, but in a simpler way without reading the package info. Let me know what you think!

tid-kijyun commented 1 year ago

@yonaskolb I think #251 and #253 are simpler and better than my way. I don't think there are any side effects.