search

alcatraz / Alcatraz

Package manager for Xcode
alcatraz.io
MIT License
9.88k stars 1.15k forks source link

Allow installing plugins with git submodule dependencies #454

Open tjarratt opened 8 years ago

tjarratt commented 8 years ago

Hello!

I'm an author and maintainer of a couple few Xcode plugins (notably Better Console, Cedar Shortcuts and Better Refactor Tools). A few people have asked me to make these installable via Alcatraz, and I was pretty excited to do this.

Unfortunately I learned that my plugins aren't installable because they have dependencies that I track via git submodules. I took a look at a few other plugins and it looks like none of them link against frameworks, so maybe I'm unique in having this problem?

I believe that my plugins could be installed by Alcatraz if the following changed...

  1. issue the git submodule update --init --recursive command after cloning a repo
    • for projects with submodules, these need to be updated before building
  2. xcode clean build ... should be able to take in the scheme name
    • since build settings such as FRAMEWORK_SEARCH_PATHS are stored on the scheme, rather than the target, any plugin that wants to link against a framework or have other build settings will need to specify the scheme.

Would a PR adding the optional uses_submodules and scheme_name flag to the package JSON format, along with some changes to the ATZPluginInstaller class to support this be well received? I haven't dug into the code too deeply, perhaps there's a better place where this behavior could be implemented?

Also, I'm a strong believer in TDD. Are there tests for the Alcatraz plugin that I can refer to? I'd love to be able to contribute tests for these changes.

guillaumealgis commented 8 years ago

git submodules

1 . issue the git submodule update --init --recursive command after cloning a repo

I see no reason not to include a quick git submodule update --init --recursive after each clone, and one git submodule update after each fetch; reset --hard (plugin update) given that these do nothing in git repos not including submodules.

This would prevent adding another field to the JSON, which is a good thing to keep the entry bar low for plugin maintainers.

xcodebuild schemes

2 . xcode clean build ... should be able to take in the scheme name

I'm not following you on this one, though. FRAMEWORK_SEARCH_PATHS is stored either at the target or the project level, but schemes do not influences the build settings. Schemes are just an bundle of target(s) + build configuration(s).

I took a look at your projects and I'm gonna guess the one for which the build fails is Xcode-Better-Refactor-Tools.

And indeed, this fails:

xcodebuild clean build -project XcodeBetterRefactorTools.xcodeproj -target XcodeBetterRefactorTools

while this works:

xcodebuild clean build -project XcodeBetterRefactorTools.xcodeproj -scheme XcodeBetterRefactorTools

The failure happens when building the BetterRefactorToolsKit dependency, because the compiler cannot find the framework SwiftXPC.

Upon further examination, the reason for this is that when building with -scheme, xcodebuild defines a environment variable BUILD_DIR (couldn't find any documentation on this, just an observation from the build logs). This directory will contain all compilation intermediate and product files (including .frameworks generated by dependency projects). This isn't the case when building with -target, and each target (yours and the dependencies) use their own build dir (named build). So you end up with something like this (skipped a lot of files to keep it readable):

Xcode-Better-Refactor-Tools/
├── BetterRefactorToolsKit
│   ├── build                            <----
│   ├── Dependency\ Injection
│   ├── Entities
│   ├── Logging
│   ├── UI
│   └── Use\ Cases
├── BetterRefactorToolsKitSpecs
│   ├── build                            <----
│   └── Use\ Cases
├── Externals
│   ├── Blindside
│   │   └── build                        <----
│   ├── Cedar
│   │   └── build                        <----
│   * snip *
│   ├── SWXMLHash
│   │   └── build                        <----
│   ├── SourceKitten
│   │   └── build                        <----
│   ├── SourceKittenFramework.framework
│   │   └── build                        <----
│   └── SwiftXPC
│   │   └── build                        <----
* snip *

FRAMEWORK_SEARCH_PATH is automatically defined to include the BUILD_DIR. However, in this configuration, each target defines BUILD_DIR relative to its own SRCROOT, so when building the BetterRefactorToolsKit target, BUILD_DIR is /path/to/Xcode-Better-Refactor-Tools/BetterRefactorToolsKit/build/, so the SwiftXPC.framework built earlier cannot be found.

Proposed solution: Let Alcatraz define BUILD_DIR when compiling the plugins, so everything end up in one unique directory and the compiler doesn't get completely lost.

This would look like:

xcodebuild clean build -project MyAwesomePlugin.xcodeproj BUILD_DIR=/some/alcatraz/build/dir/plugin-name/

(I tested this for Xcode-Better-Refactor-Tools, it works)

This has two benefits:

  1. Make plugins with complex dependencies build as intended 😉
  2. Move all build artefacts currently polluting ~/Library/Application Support/Alcatraz into a temporary build dir (/tmp/com.mneor.alcatraz/build or something of the sort), as we don't need to keep them around after the build completed.

Binary release

Having said all that, one of the next steps for Alcatraz is to move to binary distribution, where the author use whatever build system they feel confortable with, and just submit the built .xcplugin bundle to Alcatraz. So moving to this system would make your issues disappear.

However, this has been planned for a long time (at least since 2014), and (afaik) we haven't made much progress on it. So for the time being, I think PRs fixing the two problems as discussed earlier would be welcomed 😇. Otherwise I'll try to find the time to tackle these soon (it shouldn't be too much work).

Testing

Also, I'm a strong believer in TDD. Are there tests for the Alcatraz plugin that I can refer to? I'd love to be able to contribute tests for these changes.

I think originally @supermarin added tests to the project (the Specs/ folder), and I think they were kinda left behind as Alcatraz grew. To be honest, I am (to my shame) really inexperienced with testing so I'm in a bad position to drive this forward. But having up-to-date tests (and a passing build on travis) would be awesome and I'd definitely make a big effort to get up to speed!

tjarratt commented 8 years ago

Wow, thanks for the feedback and research @guillaume-algis. I'm a little overwhelmed.

I'm pretty excited about the notion of being able to release my .xcplugin directly, but I think you're right that implementing this features as a set of PRs first would be helpful and benefit plugin authors today.

I can relate to your sense of shame and frustration around TDD. I'm very lucky and privileged to work in an environment where my coworkers all practice TDD and teach each other every day through pairing. Before I joined my company, I was very frustrated with how I approached testing, but it's hard for me to remember that after so long.

So, as a concrete proposal, I will write a PR for the git submodule update --init --recursive after git clone. That seems pretty reasonable to implement on the existing controllers. The tests will probably be rather simple XCTest cases, as I'd rather not impose a new dependency on this project.

guillaumealgis commented 8 years ago

Little update: https://github.com/alcatraz/Alcatraz/pull/465 fixed a part of the submodule problem, we probably still need to run git submodule update (with --force ?) after each plugin update (fetch; reset --hard).