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

ocamlbuild should pass -package flags when building C files #6794

Closed
vicuna opened this Issue Feb 25, 2015 · 8 comments

Comments

Projects
None yet
2 participants
@vicuna
Copy link
Collaborator

vicuna commented Feb 25, 2015

Original bug ID: 6794
Reporter: @whitequark
Assigned to: @diml
Status: closed (set by @xavierleroy on 2017-02-16T14:15:08Z)
Resolution: fixed
Priority: normal
Severity: minor
Target version: 4.02.2+dev / +rc1
Category: -for ocamlbuild use https://github.com/ocaml/ocamlbuild/issues
Related to: #6809
Monitored by: @hcarty

Bug description

The -package flags would carry the corresponding -I flags, which may be necessary to build the C components. E.g. cstubs needs this.

@vicuna

This comment has been minimized.

Copy link
Collaborator Author

vicuna commented Mar 5, 2015

Comment author: @diml

I attempted a patch: #150

@vicuna

This comment has been minimized.

Copy link
Collaborator Author

vicuna commented Mar 5, 2015

Comment author: @gasche

I don't have the use-cases or time to test this feature myself. If you tell me that you or whitequark tested it and it works as you expect, I would be glad to merge it after code review. I would be slightly happier with a test for it in the ocamlbuild testsuite, but I won't make it a strong requirement.

(I'm rather thinking of "trunk only". Is anyone dead sure that this cannot break existing projects?)

@vicuna

This comment has been minimized.

Copy link
Collaborator Author

vicuna commented Mar 5, 2015

Comment author: @diml

I checked on an example that the -I flags were passed correctly.

I don't mind it being committed to trunk only, I was just looking at issues targeted 4.02.2 that could be fixed easily. It could potentially break exsiting projects as C stubs might be passed more -I options. Although it's quite rare for ocaml projects to install header files.

@vicuna

This comment has been minimized.

Copy link
Collaborator Author

vicuna commented Mar 5, 2015

Comment author: @gasche

whitequark, could you confirm that it works as expected in the case of cstubs?

@vicuna

This comment has been minimized.

Copy link
Collaborator Author

vicuna commented Mar 6, 2015

Comment author: @whitequark

Confirmed.

@vicuna

This comment has been minimized.

Copy link
Collaborator Author

vicuna commented Mar 6, 2015

Comment author: @gasche

Onto the merge list then. Thanks!

@vicuna

This comment has been minimized.

Copy link
Collaborator Author

vicuna commented Mar 8, 2015

Comment author: @gasche

Merged in 4.02 and master. Thanks to you both.

@vicuna

This comment has been minimized.

Copy link
Collaborator Author

vicuna commented Jul 17, 2015

Comment author: @damiendoligez

This fix was reverted in 4.02. See the followup #6809.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.