Specification refactor

[fabiopelosin]

Attempt to refactor the specification class. It is still a work in progress.

Objective

Have a clean hierarchy of specifications nodes:

• each specification inherits all its subspecs.
• for specification dryness common attributes can be specified by a node and will be inherited by all its subspecs.
• simplify code removing download only pods and wrappers [Pod subspec not adding other subspec dependencies to the project #223].
• If a specification contains incompatible subspecs it can specify a main subspec

---

Discusion

This is an experiment performed in total freedom (even in code style) and might have unwanted features that can be rolled back.

SubSpec class

The subspec class was decoupling inheritance logic from attributes specification and thus was removed and replaced with meta programming. Every specification can have a parent. If no parent is provided it is a top level specification.

Moreover, the subspec class was requiring its parent and this logic has been removed, in favor of the inheritance logic specified at the attribute level.

Inheritance

dependency 'RestKit'

Will require result in the following specs being loaded:

 - RestKit
     - RestKit/Network
     - RestKit/ObjectMapping
       - RestKit/ObjectMapping/CoreData
       - RestKit/ObjectMapping/JSON
       - RestKit/ObjectMapping/XML
     - RestKit/UI

However, to handle incompatible different strategies it is possible to specify a main subspec:

dependency do |s|
        s.main_subspec = 'JSON'
        s.subspec 'JSON' do |js|
          js.dependency 'RestKit/Network'
          js.dependency 'RestKit/UI'
          js.dependency 'RestKit/ObjectMapping/JSON'
          js.dependency 'RestKit/ObjectMapping/CoreData'
        end
        s.subspec 'XML' do |js|
          js.dependency 'RestKit/Network'
          js.dependency 'RestKit/UI'
          js.dependency 'RestKit/ObjectMapping/XML'
          js.dependency 'RestKit/ObjectMapping/CoreData'
        end
[...]
end

Is not fully implemented yet but this will result in the RestKit spec returning RestKit/JSON instead of itself so pod installs tells to newbies that they are using a json specific strategy.

Attributes inheritance

Some attributes can be specified only at the top level. They are what uniquely identifies a pod, can't be assigned by pods that have a parent (raises) and always return the top level value.

Other attributes are resolved back in the chain. For example every spec inherits all the frameworks required by its parents. The same logic will be applied to source files and if there are any duplicates local pod will filter the unique values. Looking at the code should be clear how every attribute is handled.

---

Todo

• deprecate clean paths
  • add readme_file attribute to specification.
  • nuke any file that is not specified in the podspec.
  • will dry podpecs.
  • in case if needed add a preserve_files attribute.
• local pod will receive a all the specifications sharing a common parent
  • it will be able to remove duplicates (source files, frameworks, xconfig, ...).
  • it will be able to clean all unused files.
• Speficiation#activate_for_platform activates a given platform so the attr readers return the value for the platform and clients will not need to be aware of which attributes are multiplatform.
• update the 0.6 branch of the master repo

---

Related Issues

• [Pod subspec not adding other subspec dependencies to the project #223] Pod subspec not adding other subspec dependencies to the project
• [Specifying a podspec which contains subspecs throws exception #141] Specifying a podspec which contains subspecs throws exception
• [Think of a good way to do clean_paths for a spec with subspecs #87] Think of a good way to do clean_paths for a spec with subspecs

[alloy]

Sweet.

I’ve at least read specification.rb and I think you’re on the right track.

One question, do you have an estimate of the timeframe for this? Can we do it in 0.6, or will that push the first RC release back too long?

If a specification contains incompatible subspecs it can specify a main subspec

dependency do |s|
        s.main_subspec = 'JSON'
        s.subspec 'JSON' do |js|
        end
        s.subspec 'XML' do |js|
        end
end

In your example, the value for main_subspec is more of a preference. Can we call that something like preferred_dependency?

add readme_file attribute to specification.

Is that for doc generation purposes too? Otherwise I would opt for just preserve_files, because we will need that anyways.

[fabiopelosin]

I’ve at least read specification.rb and I think you’re on the right track.

Cool i was really looking for confirmation at least for that one.

One question, do you have an estimate of the timeframe for this? Can we do it in 0.6, or will that push the first RC release back too long?

Lets see at what's left tomorrow night, but I think that it could be done for 0.6. What worries me are regressions that might arise by cases covered in the specs.

In your example, the value for main_subspec is more of a preference. Can we call that something like preferred_dependency?

Agreed on the preferred part. However this subspec is not a dependency but it is the specification that you get, which naturally inherits from the parent. If you require RestKit but you get RestKit/JSON. This is so because in this way the installer, and all the clients list it as RestKit/JSON. In this way it is intuitive that there is RestKit/XML.

So preferred_subspec?

Is that for doc generation purposes too? Otherwise I would opt for just preserve_files, because we will need that anyways.

My idea was that LocalPod checks for [license, license.txt] and [readme.*] so you need to specify those files only if they are not available. The linter will warn if those files are not found and if not available the push will need to be done with allow-warnings.

For this reason I was tempted to specify the license file as license_file instead of using the current hash.

[fabiopelosin]

State

The branch still needs a good clean up, specs, and some good testing. But the logic should be there.

I have tested it with ShareKit and it seems to be working. Here is the modified version of the spec. It doesn't require a Core subspec and it doesn't require an include all subspec.

dependency 'ShareKit'

$ pod install --no-update --no-integrate
Installing Facebook-iOS-SDK (1.2)
Installing JSONKit (1.5pre)
Installing Reachability (3.0.0)
Installing SBJson (2.2.3)
Installing SSKeychain (0.1.2)
Installing ShareKit (2.0)
 - Evernote
 - Facebook
 - Flickr
 - Foursquare
 - GoogleReader
 - Instapaper
 - LinkedIn
 - Pinboard
 - ReadItLater
 - Tumblr
 - Twitter
 - Vkontakte
Installing objectiveflickr (0.0.1)
Generating support files

dependency 'ShareKit/Facebook'

$ pod install --no-update --no-integrate
Installing Facebook-iOS-SDK (1.2)
Installing SBJson (2.2.3)
Installing ShareKit (2.0)
 - Facebook
Generating support files

---

Still open questions

• preferred_spec name, and actually if it is a good idea to substitute it to the parent.
  • Another, alternative would be to have a simple attribute like exclude_from_parent_deps for the subspec that should not be automatically inherited.
• license and preserve_paths

[alloy]

Yeah that ShareKit spec does look good.

For this reason I was tempted to specify the license file as license_file instead of using the current hash.

Which hash? The Specification#license hash?

preferred_spec name

Actually, what do you think of default_spec?

and actually if it is a good idea to substitute it to the parent.

I’m not following. Is this about the question below?

Another, alternative would be to have a simple attribute like exclude_from_parent_deps for the subspec that should not be automatically inherited.

I don’t see a benefit in blacklisting vs whitelisting in this case. Naming one makes it much clearer, to the user of the spec, which specs are going to be used.

license and preserve_paths

Like with the hash, I’m not sure what the concrete question is regarding the license attribute. The preserve_paths attribute is definitely needed.

[fabiopelosin]

Which hash? The Specification#license hash?

Yes, but I don't think it is a good idea anymore.

Actually, what do you think of default_spec?
I’m not following. Is this about the question below?

Now that I have better comprehension of the problem I see less need for substituting the parent with the default_spec. It adds a non necessary layer of complexity and doesn't adhere to the principle of least surprise. To keep it KISS, I'll change that part and simply make Specifications#dependencies return the default_spec if declared.

Taking into account that, your original suggestion (preferred_dependency) seems the best name.

I don’t see a benefit in blacklisting vs whitelisting in this case. Naming one makes it much clearer, to the user of the spec, which specs are going to be used.

Blacklisting does not require to keep the list of subspecs, but this is a minor nuisance.

Related: To be fair the RestKit example was approximative, as would result in a broken require RestKit/ObjectMapping. A more precise one would be:

dependency do |s|
        s.main_subspec = 'JSON'
        s.subspec 'ObjectMapping' do |s|
          preferred_dependency = 'JSON-Strategy'
          s.subspec 'JSON-Strategy' do |s|
            s.dependency = 'RestKit/ObjectMapping/JSON'
            s.dependency = 'RestKit/ObjectMapping/CoreData'
            ...
          end
          s.subspec 'XML-Strategy'

          s.subspec 'JSON'
          s.subspec 'XML'
          s.subspec 'CoreData'
          ...
        end
...
end

To present that there is a specific strategy being used I was planning to implement some logic in LocalPod#to_s or in Installer.
But is not perfect, yet. Any feedback?

Like with the hash, I’m not sure what the concrete question is regarding the license attribute. The preserve_paths attribute is definitely needed.

Good to know that preserve paths are needed. What I plan to do is simply avoid to clean any file matching [license*, readme*] in the root.

---

@alloy do you have any troublesome podspec to suggest for testing?

[alloy]

Which hash? The Specification#license hash?

Yes, but I don't think it is a good idea anymore.

So what do you propose?

Taking into account that, your original suggestion (preferred_dependency) seems the best name.

Agreed, let's just go with that.

Related: To be fair the RestKit example was approximative, as would result in a broken require RestKit/ObjectMapping. A more precise one would be:

Do you have the actual specification somewhere? The condensed versions are making it harder to see what's going on.

To present that there is a specific strategy being used I was planning to implement some logic in LocalPod#to_s or in Installer.
But is not perfect, yet. Any feedback?

Something to the effect of “installing preferred JSON subspec”?

I wonder if it's really needed. If you know what you are doing such warnings only become noise. And besides, you can see from the list of specs being installed that it chose the JSON subspec, right?

@alloy do you have any troublesome podspec to suggest for testing?

I think RestKit is definitely one of the most advanced ones. Also note how the callbacks are being handled by extending it with a module. Is that already fixed by your simplification of a spec and its subspecs?

[jeanregisser]

Good to know that preserve paths are needed. What I plan to do is simply
avoid to clean any file matching [license*, readme*] in the root.

We should also avoid cleaning *.podspec at the root.

On master, if you use an external dependency (:git => 'GIT-URL') for a spec that contains s.clean_paths = FileList['*'].exclude(/(Source|README.markdown|MIT.LICENSE)$/), pod install works the firs time, but then fails the second time with an error saying it can't clone to a dir that's not empty.

The reason is that the second time it doesn't see the previously fetched (and cleaned) podspec, so it will try to fetch the dependency again to the same dir which is now not empty.

[fabiopelosin]

We should also avoid cleaning *.podspec at the root.

@jeanregisser Thanks for pointing this out.

[fabiopelosin]

The refactor is almost complete the remaining issues are:

• it needs a good code review and testing ;-).
• some specs are still missing (LocalPod, Installer) and the specs related to ASIHTTPRequest need to be restored.
• RestKit has issues (apparently related to the header files and the extension).

---

Which hash? The Specification#license hash?
So what do you propose?

I was tempted to remove the file key from the Specification#license hash and have a separated attribute (Specification#license_file). However, now I think that the current implementation is good. (related comment)

Do you have the actual specification somewhere? The condensed versions are making it harder to see what's going on.

I changed my mind also on this one. The current proposals are available here.

Something to the effect of “installing preferred JSON subspec”?
I wonder if it's really needed. If you know what you are doing such warnings only become noise. And besides, you can see from the list of specs being installed that it chose the JSON subspec, right?

Good points that I missed on this update... however the current implementation should be fine. The installer output is Installing RestKit/JSON (0.10.0) while in the rest of the app it is referred as RestKit.

[fabiopelosin]

MacRuby is not used anymore, so this is not needed... right?

[alloy]

Indeed! You can just merge the code with the normal #initialize method.

[alloy]

Docs

The documentation is generated for all the known subspecs and must be created before cleaning the local pod.

• The --doc-force option is unsustainable because the dir is expected to be cleaned and would result in a documentation encompassing only the active subspecs.
• The docs can still be generated once and work for all projects.

Fair enough.

I think the issue I had was that I’m (currently) checking the Pods directory into SCM, which means that when I work on another machine, I don’t get the docs on a pod install. However, since we should try to actually use the Podfile.lock file ASAP anyways, this point becomes moot.

Compiler flags

The compiler flags are resolved per specification and can be used to silence compiler warnings. An example is available here. This is further discussed in #209.

Nice, that’s definitely the right approach.

Restkit

Restkit now compiles and lints. To avoid the need of creating an override two attributes have been added to Specification in 9027f20 and in Specs/3d7810d.

• header_mappings_dir: preserves the headers name spacing from a given folder. ('.' can be passed -- SSZipArchive).
• exclude_headers: specifies the header that should not be added to the search paths (a suggestion for a better name is appreciated).

I think we should totally get rid of the notion that people can override these methods, is that now the case?

Regarding the exclude_headers attribute name, I think it should be something like exclude_header_search_paths.

[alloy]

Man, this PR is epic :D

Kudos!

[fabiopelosin]

Regarding the documentation I have removed the force_doc option.

I think we should totally get rid of the notion that people can override these methods, is that now the case?

It should be as far I'm concerned.

Kudos!

:-)

---

The refactor is missing some specs for the LocalPod, needs to update the fixtures, and to update the specs repo. @alloy How do you suggest to proceed?

[alloy]

The refactor is missing some specs for the LocalPod, needs to update the fixtures, and to update the specs repo. @alloy How do you suggest to proceed?

How about you write some disabled stub spec descriptions so that we know what has to be done? Then I can possibly work on them too.

I would do that in a separate merge, though, let's merge this bad boy asap.

[fabiopelosin]

Ok, I've added the disabled specs.

[alloy]

Cool. So, time to merge? :)

[fabiopelosin]

Yes 🎉

If you don't have any objections I will also merge the sister branch in the specs repo to in the 0.6 branch.

[alloy]

✨ 🌟 ✨

[alloy]

Go ahead. You don’t need to ask me anymore about these things, you are as much responsible for these areas of the code as much as I am nowadays :)