Skip to content

Append the input header to the end of the clang args, instead of the front #45

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

Merged
merged 1 commit into from
Aug 30, 2016

Conversation

fitzgen
Copy link
Member

@fitzgen fitzgen commented Aug 30, 2016

@emilio
Copy link
Contributor

emilio commented Aug 30, 2016

@bors-servo: r+

@bors-servo
Copy link

📌 Commit 82b166d has been approved by emilio

bors-servo pushed a commit that referenced this pull request Aug 30, 2016
Append the input header to the end of the clang args, instead of the front

r @emilio
@bors-servo
Copy link

⌛ Testing commit 82b166d with merge 1e26a8a...

@bors-servo
Copy link

☀️ Test successful - status-travis

@bors-servo bors-servo merged commit 82b166d into rust-lang:master Aug 30, 2016
@Manishearth
Copy link
Member

This causes clang to complain with error: language not recognized: ''

Manishearth added a commit to Manishearth/servo that referenced this pull request Aug 31, 2016
@fitzgen
Copy link
Member Author

fitzgen commented Aug 31, 2016

This causes clang to complain with error: language not recognized: ''

With what command? I verified manually that the rust-mozjs/etc/bindings.sh script works with this change.

@emilio
Copy link
Contributor

emilio commented Aug 31, 2016

Yes, that's correct. What Stylo was doing (to workaround the behavior before this patch), was using the filename after clang-flags.

I think this confused docopt, that thought that the input header was other thing, and when we pushed it to clang_args, well, clang got confused.

But it's not this patch's bug per se.

@bholley
Copy link

bholley commented Aug 31, 2016

Yes, but stylo binding generation is still broken. What is the fix?

@emilio
Copy link
Contributor

emilio commented Aug 31, 2016

Manishearth added a commit to Manishearth/servo that referenced this pull request Sep 1, 2016
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.

6 participants