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

Comments: It's time to use Swift Package Manager #517

Open
orta opened this issue Jan 6, 2019 · 14 comments
Open

Comments: It's time to use Swift Package Manager #517

orta opened this issue Jan 6, 2019 · 14 comments

Comments

@orta
Copy link
Contributor

orta commented Jan 6, 2019

It's been three years, and [Swift Package Manager][spm] (SPM) is at a point where it can be useful for iOS projects. It'll take a bit of sacrifice and a little bit of community spirit to fix some holes probably but in my opinion, it's time for teams to start adopting SPM for their 3rd party dev tools.

http://artsy.github.io/blog/2019/01/05/its-time-to-use-spm/

@DmitryBespalov
Copy link

DmitryBespalov commented Jan 11, 2019

@orta imo it's long time for the community to modify the SPM to the iOS package management. I'm willing to participate in such a project.

@orta
Copy link
Contributor Author

orta commented Jan 11, 2019

I don't see it happening for iOS package management in a reasonable time-frame (because a lot of that work needs to happen in xcbuild/Xcode (closed source) and Swift PM (which you could improve 👍)

My last post on Swift PM had comments that mentioned a few hacks that people are using to try work around this which is a good start, maybe you could contribute to those?

@dive
Copy link

dive commented Jan 11, 2019

Check SE-0236: Package Manager Platform Deployment Settings thread. This proposal is already accepted (with some changes) but it seems like the right direction for full iOS support. Of course, there are a lot of things that should be done but the future seems reasonable for iOS.

@orta
Copy link
Contributor Author

orta commented Jan 12, 2019

Nice! Maybe Xcode 11 could be the one 👍

@sey
Copy link

sey commented Jan 22, 2019

Did you experience any issues while running Git pre-commit hook with Komondor?
I have reproduced your setup. It works fine when I run the swiftlint command manually but when it runs through Komondor it gets stuck most of the time.
Even when I run the swiftlint directly from the pre-commit hook it works as expected.

@dawilliams-gpsw
Copy link

Great article Orta. I have a few questions:

  1. What exactly is the target section of the Package.swift? I'm not sure I understand the comment included. Are you telling the package manager that it cannot build the arbitrary Swift file unless Danger is included?

  2. Do you place the four commands listed in separate run script phases? For example, setting swift run swiftlint --path MyProject as a run script, and if it's not installed, SPM will auto-install it for us as long as the Package.swift is in the project's root?

  3. If the above assumption is correct, is there a way to determine if a command is already installed, eg if which komono >/dev/null; then, or something similar? I suppose I could write a bash script to create a temporary file and skip the run script if that temporary file exists. Would that work?

  4. I keep seeing this message in my warnings: PackageDescription API v3 is deprecated and will be removed in the future; used by package(s): Unbox -- How can I find out what dependency is using this to see if there is an updated version? I seem to get it after every swift run command

@orta
Copy link
Contributor Author

orta commented Jan 23, 2019

1

The target section is to make sure that libDanger is built, so that it can be dynamically linked at runtime during the evaluation of Danger Swift (otherwise there's no easy way to connect the two) - plus you have to have a target or all commands fail.

2

Yep, it will download/build/run. in that one command. I prefer to have the commands as separate scripts mainly for readability, you could make it a one-liner with && between the commands

3

There kinda isn't really, you'd have to do swift run komondor --version or something which doesn't trigger to see if it raises, that would detect that it's in the package manifest. You could detect if the binary has been compiled though ( by looking to see if it exists in .build )

4

That's the Unbox dependency, it's in the error message 😸 - I have a PR for this but it's been hard to get a release out. I brought it back up.

@nicholascross
Copy link

Thanks for this!

I prefer to do any autocorrection as a pre commit hook to avoid killing xcode's undo stack and this shows me an easy way to integrate it.

For full control I prefer to review changes rather than just staging them and committing them. I modified the example you provided so it will abort the commit if there are any modifications to source files due to formatting.

Here it is in case it is useful to anyone.

#if canImport(PackageConfig)
    import PackageConfig

    let config = PackageConfig([
        "komondor": [
            // When someone has run `git commit`, first run
            // run SwiftFormat and the auto-correcter for SwiftLint
            // If there are any modifications then cancel the commit
            // so changes can be reviewed
            "pre-commit": [
                "find Injectable -type f -name '*.swift' -exec md5 {} ';' | md5 > .pre_format_hash",
                "swift run swiftformat .",
                "swift run swiftlint autocorrect --path Injectable/",
                "find Injectable -type f -name '*.swift' -exec md5 {} ';' | md5 > .post_format_hash",
                "diff .pre_format_hash .post_format_hash > /dev/null || { echo \"File formatting modified\" ; rm .pre_format_hash ; rm .post_format_hash ; exit 1; }",
                "rm .pre_format_hash ; rm .post_format_hash",
            ],
        ],
    ])
#endif

It could be prohibitive on large codebases but I am not sure if there is a more straightforward way. IIRC either swiftformat or swiftlint don't provide simple exit codes but that might have changed since I last checked.

@orta
Copy link
Contributor Author

orta commented Feb 16, 2019

@nicholascross - Nice work - while I want Komondor to be pretty generic, I'd be open to baking something like this into the tool itself considering it's a good pattern that ideally everyone would want

@nicholascross
Copy link

I thought about it some more and I think we can make it more generic and relevant by relying on git for the diff.

I have updated my package file so that it only cares if staged files have been modified during the commit.

git diff --cached --name-only | xargs git diff | md5

"Your staged files were modified during commit" kind of behaviour could be handy to support in Komondor directly!

@ajanuar
Copy link

ajanuar commented Jul 12, 2019

Hi @orta, I found this issue in Catalina OS with Swift 5.0. Any idea how to solve this? Thanks

swift run komondor install
warning: dependency 'Realm' is not used by any target
This copy of libswiftCore.dylib requires an OS version prior to 10.14.4.
[3]    52236 abort      swift run komondor install

@MainasuK
Copy link

Xcode 11 support SPM finally. However, it's still a pain to use the resource in SPM. (Also Post-Script)

https://bugs.swift.org/browse/SR-2866

@diogot
Copy link

diogot commented Feb 28, 2020

It seems that isn't possible to run post-scripts to SPM packages on Xcode 11.

@evanjmg
Copy link

evanjmg commented Dec 22, 2020

Did you get this working with React Native? If so can you open source your solution?

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

No branches or pull requests

10 participants