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

Trying to create a PR for not adding recursive folders #3760

Closed
haxwagon opened this issue Jul 1, 2015 · 9 comments
Closed

Trying to create a PR for not adding recursive folders #3760

haxwagon opened this issue Jul 1, 2015 · 9 comments

Comments

@haxwagon
Copy link

haxwagon commented Jul 1, 2015

Hello... so i'm trying to fork and write a PR for the ability to NOT add search paths for every header file. That seems like a crazy use case to want recursive header paths for each dependency issue #1437

in headers_store.rb you have

      def add_files(namespace, relative_header_paths, platform)
        add_search_path(namespace, platform)

I commented out the "add_search_path" and it works great. I wanted to add an option to disable search paths for header files, but I can't figure out how to make it have something like:

      def add_files(namespace, relative_header_paths, platform)
        add_search_path(namespace, platform) if spec_options.add_header_dirs_to_search_path

We are quite blocked and for now are just commenting this out. I know others are suffering from this issue and I believe it's pretty easy to fix if you know the codebase. Can anyone help me??

@segiddins
Copy link
Member

what would "an option to disable search paths for header files" do?

@haxwagon
Copy link
Author

haxwagon commented Jul 6, 2015

Right now folders are added recursively for each subdirectory if
preserve_paths is set (which many frameworks, such as j2objc, need). They
require something like java/util/Time.h. So if you preserve the paths,
cocoapods automatically adds each subdirectory to the include path.

Currently adding that one header would add:

Pods/J2ObjC
Pods/J2ObjC/java
Pods/J2ObjC/java/util

which can have bad side effects... say if you have a file named the same as
a system include file (j2ObjC has several of these... time.h is a bad
offender). This means that any other library that includes <time.h> will
get the j2ObjC one instead of the system time.h that they wanted. The
header search paths had been polluted.

To fix this you really want to be able to only have Pods/J2ObjC added to
the header search paths and still preserve the folder structure of your
headers. Commenting that line out does fix it, but it should be an opt-in
option so existing code isn't affected.

Hope this helps!

On Sun, Jul 5, 2015 at 2:56 PM, Samuel E. Giddins notifications@github.com
wrote:

what would "an option to disable search paths for header files" do?


Reply to this email directly or view it on GitHub
#3760 (comment)
.

@segiddins
Copy link
Member

I think we're agreeing to get rid of the recursive addition of the search paths, full stop?

@haxwagon
Copy link
Author

haxwagon commented Jul 7, 2015

That would suffice for sure. This was a workaround to make it an option,
but recursive paths should never be used IMO

On Mon, Jul 6, 2015, 5:33 PM Samuel E. Giddins notifications@github.com
wrote:

I think we're agreeing to get rid of the recursive addition of the search
paths, full stop?


Reply to this email directly or view it on GitHub
#3760 (comment)
.

@segiddins
Copy link
Member

Agreed

@jcanizales
Copy link

I think we're agreeing to get rid of the recursive addition of the search paths, full stop?

OMG yes. Although that's bound to break some people right?

@segiddins
Copy link
Member

@jcanizales maybe, but at this point it's a choice between breaking people relying on weird behavior, and breaking people who are doing everything right.

@segiddins
Copy link
Member

@twoboxen did you ever get to a point where you could make a PR for this?

@haxwagon
Copy link
Author

We have been using a locally patched version, but here is a PR:
#4042

tonystone added a commit to tonystone/geofeatures that referenced this issue Sep 19, 2015
- The new cocoa pods version 0.39.x (in beta now) has a fix for the adding header file directories recursively (see CocoaPods/CocoaPods/issues/3760).  Doing that was a bad thing, it would add a new search path for each directory and sub directory of the boost implementation.  This is incorrect behavior and needed to be corrected.  We relied on the bug in CocoaPods for our own header files omitting the root of the header search paths (which we could because each directory was it's own search path).  I've corrected that situation in this change
### Simplify namespaces
- I used this opportunity to simplify GeoFeatures internal namespace structure.
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

No branches or pull requests

3 participants