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

Added 'pre' and 'post' setup blocks #174

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

colinta
Copy link
Member

@colinta colinta commented Oct 14, 2014

  • easier for gems to add files, because @files is just an empty array at this point
  • no default setup is done, so sdk_version and so on can still be set by the user
  • pre_setup / setup / post_setup are a good trifecta for a compiler 😃
# in your gem
Motion::Project::App.pre_setup do |app|
  Dir.glob(File.join(File.dirname(__FILE__), 'lib/**/*.rb')).each do |file|
    app.files << file
  end

  # other settings as well, it's still a "good ol' setup block"
  app.libs += ['/usr/lib/libicucore.dylib', '/usr/lib/libc++.dylib']
  app.frameworks += ['CFNetwork', 'Security', 'SystemConfiguration']
  app.weak_frameworks += ['Accounts', 'Social']
end

@colinta
Copy link
Member Author

colinta commented Oct 14, 2014

Also, since post_setup is available, gems could detect system configuration and add/remove files and settings as necessary. For instance, it might be a good place to add cocoapods?

end

# should we include a check here for "already included app/ files"? it's
# not necessary since this operation is idempotent.
Copy link
Member

Choose a reason for hiding this comment

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

I’m unclear as to why not to check it, especially since you do it after all :)

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh shoot, I just realized I refactored this in /Library/RubyMotion, and didn't copy that in. I'm gonna push that up (with comments to hopefully answer some of these questions)

@alloy
Copy link
Member

alloy commented Oct 15, 2014

Also, since post_setup is available, gems could detect system configuration and add/remove files and settings as necessary. For instance, it might be a good place to add cocoa pods?

Can you elaborate on this? Why should motion-cocoapods use that hook instead? E.g. what config access should it be using?

@colinta
Copy link
Member Author

colinta commented Oct 15, 2014

I didn't mean motion-cocoapots would use that hook. I was imagining something like this:

Motion::Project::App.pre_setup do |app|
  # add my gem files to app.files
end
Motion::Project::App.post_setup do |app|
  installed_pods = app.pods
  # I just went looking for a method that could return true/false for "did they include this cocoapod", but
  # couldn't find one... but if it DID exist, this is what I was imagining:
  unless installed_pods.include?('AFNetworking')
    raise StandardError, 'The AFNetworking library is required for this gem, please add it to your cocoapods'
  end
end

Also, code analyzing tools like dbt are ideal for using the post_setup hook:

Motion::Project::App.post_setup do |app|
  DBT.analyze(app)
end

config.setup_blocks << block
config_without_setup.setup_blocks << block
config.setup
end
Copy link
Member Author

Choose a reason for hiding this comment

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

calling config here is what causes some troubles, because config ends up determining sdk and api versions. For instance, on android, I need to set app.api_version = '19', otherwise I get an error about the "L" SDK not being installed.

Which is fine until I include a gem, which calls setup, which calls config, and goes looking for the api_version. My Rakefile's setup hasn't been called, so it detects the wrong one, and errors out.

end
should_validate = true
@app_dir = nil
end
Copy link
Member Author

Choose a reason for hiding this comment

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

Ok, ignore the "controversial" stuff above, this restores the previous behavior, where app.files is set before the Rakefile setup block is called.

@colinta
Copy link
Member Author

colinta commented Oct 15, 2014

Now that I think more on it, the most important features of pre_setup are:

  • app.files has not been set to the app/ files, so appending them to the end is safe
  • config.setup is not called, and so validate is not called, which prevents the gems from inadvertently causing errors

@colinta
Copy link
Member Author

colinta commented Oct 15, 2014

Btw, @alloy, when I was looking through cocoapods code, I noticed this line: https://github.com/HipByte/motion-cocoapods/blob/master/lib/motion/project/cocoapods.rb#L74

This assumes App.template will be something sane, like :ios or :osx, but with extensions this template can also be :ios_extension and other values. Just a heads up on that.

@alloy
Copy link
Member

alloy commented Oct 31, 2014

@colinta Do you have an example gem that has been updated to use this so we can easily test and verify it?

@colinta
Copy link
Member Author

colinta commented Oct 31, 2014

Yes, the android branch of sugarcube. It also has a "support" file that adds pre/post setup methods if they're not defined.

On Oct 31, 2014, at 7:21 AM, Eloy Durán notifications@github.com wrote:

@colinta Do you have an example gem that has been updated to use this so we can easily test and verify it?


Reply to this email directly or view it on GitHub.

* master: (157 commits)
  3.2
  use quoted string instead of #shellescape
  [iOS] When stripping symbols, ignore symbols from the symbols file that are not present in the executable.
  revert deleting i386 for simulator
  Fixes the extra-packages arg to work with multiple packages
  [iOS] Silence `strip` in non-verbose mode.
  introduce :native parameter for app.vendor_project to include 3rd-party shared libraries
  [iOS] Do not strip symbols from host that a framework requires.
  [RM-696] escape Watch app's Entitlements.plist path because it contains white spaces.
  [RM-695] add 'application-identifier' to entitlements in order to deploy app to iOS 8.2 beta 3 device
  remove `i386' at :build:simulator task
  remove 'i386' only if build 'x86_64'
  reduce compile time by removing `i386' for iOS simulator
  [iOS] Pick-up new certificates prefixed with `iOS` instead of `iPhone`.
  [WatchKit] Honor `xcode_dir` setting from host app.
  [log] Show backtrace on `App.fail` and in verbose mode.
  3.1
  [iOS Ext] Don't use private API, try to replicate Xcode closely.
  [iOS Ext] Always pass on the full env to the target.
  [iOS Ext] Fix startup crashes by reverting to previous behaviour.
  ...
@jamonholmgren
Copy link
Contributor

I would love to have these methods. 👍

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