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

[Specification] Allow specifying a custom module map file #218

Merged
merged 8 commits into from
Apr 9, 2015

Conversation

segiddins
Copy link
Member

\c @kylef @mrackwitz

@orta
Copy link
Member

orta commented Feb 14, 2015

So this is a breaking change that would require a Specs repo update if someone pushes a pod from a beta/master to the repo, are there any other DSL attributes we should add if we're doing this?

@segiddins
Copy link
Member Author

Didn't we already add the module_name attribute?

-Samuel E. Giddins

On Feb 14, 2015, at 9:42 AM, Orta notifications@github.com wrote:

So this is a breaking change that would require a Specs repo update if someone pushes a pod from a beta/master to the repo, are there any other DSL attributes we should add if we're doing this?


Reply to this email directly or view it on GitHub.

@orta
Copy link
Member

orta commented Feb 14, 2015

Legit point, and it's being used, ignore me.

screen shot 2015-02-14 at 12 47 48 pm

@segiddins
Copy link
Member Author

I think this was one of the huge benefits of switching to JSON

@mrackwitz
Copy link
Member

So with subspecs, I see the following two cases:

  • Without custom modulemap: The generated modulemap will inject the generated umbrella header, which takes care of importing all public headers, including those of subspecs. Importing the full Clang module, will give you everything. But you can also import implicitly derived submodules by using e.g. MyPod.MySubspecHeader.
  • With custom modulemap: The custom modulemap will be used and injected by the build setting in the generated target. Because this is given, there is also no need to generate an umbrella header. All public headers, even those of the subspecs, need to be transitively imported or known to the modulemap by explicitly enumerating them. In consequence, the pod authors would need to make sure, that they provide a public header with precompiler-guards with conditions for header-availability for each subspec (and eventually have something like a sub-umbrella-header for each subspec), so that the resulting Clang module does include the fulll public interface, including the subspecs.

@segiddins
Copy link
Member Author

@mrackwitz sure. I think specifying your own module map is like 'hard mode' -- you're responsible for it doing the things it should. I don't think we'll recommend that anyone use it -- it'll just be an option for those who need it. As to dealing with subspecs -- sure, that'd be nice, but (a) that would all be inside CocoaPods/CocoaPods and (b) it's 1/2 unrelated to this -- it can be done separately.

@mrackwitz
Copy link
Member

but (a) that would all be inside CocoaPods/CocoaPods and (b) it's 1/2 unrelated to this -- it can be done separately.

We could i.e. go a whole different approach and allow customizing the generated modulemap in the DSL, so that you could add modulemap-declarations also on the subspec level.
It's a bit of intentional overthinking as it isn't based yet on actual issues. Intentional because changing the DSL back and forth is limited, so we should take care and allow an extensible design, which is compatible with existing features.
After all, this seems to be given, so from my side, we're definitively good to go for this approach. 👍

@mrackwitz mrackwitz force-pushed the seg-module-map branch 2 times, most recently from 00415bf to 8467526 Compare April 2, 2015 11:51
mrackwitz added a commit that referenced this pull request Apr 6, 2015
@@ -2,6 +2,12 @@

## Master

##### Enhancements

* Allow specifying a custom module map file.
Copy link
Member Author

Choose a reason for hiding this comment

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

two spaces

@segiddins
Copy link
Member Author

Other than the one CHANGELOG comment and the linting, this should be good to go once the work in CocoaPods is done and #233 has been finished.

@orta
Copy link
Member

orta commented Apr 9, 2015

This'll need a rebase 👍

@mrackwitz
Copy link
Member

Done! With the change of 1375f0f, which affects how the linter is tested, I discovered a duplicated warning, introduced with #233, which I fixed right here on this branch.

@@ -210,10 +210,6 @@ def _validate_authors(a)
'from default')
end
end

if a.empty?
Copy link
Member Author

Choose a reason for hiding this comment

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

why is this removed?

Copy link
Member

Choose a reason for hiding this comment

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

238a282: The author attribute is specified in the DSL as :required => true, so it's linted automatically and we can rely on that. :)

@mrackwitz
Copy link
Member

This should be then ready now. Anything else left?

@segiddins
Copy link
Member Author

Nope, this is good! 👍

mrackwitz added a commit that referenced this pull request Apr 9, 2015
[Specification] Allow specifying a custom module map file
@mrackwitz mrackwitz merged commit 971a8c4 into master Apr 9, 2015
@mrackwitz mrackwitz deleted the seg-module-map branch April 9, 2015 15:34
@wjk wjk mentioned this pull request Apr 18, 2015
Ashton-W pushed a commit to Ashton-W/Core that referenced this pull request Nov 2, 2015
Ashton-W pushed a commit to Ashton-W/Core that referenced this pull request Nov 2, 2015
[Specification] Allow specifying a custom module map file
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants