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

Add YAJL recipe #813

Merged
merged 2 commits into from
Apr 10, 2020
Merged

Add YAJL recipe #813

merged 2 commits into from
Apr 10, 2020

Conversation

christopher-dG
Copy link
Contributor

for use in YAJL.jl

@giordano
Copy link
Member

Is this supposed to work on Windows at all?

@christopher-dG
Copy link
Contributor Author

ah yes it should, i need to look at this evidently.

@giordano
Copy link
Member

Yes, I'm reading the same file, but I don't think that helps, it's about using Visual Studio

@giordano
Copy link
Member

I think it's creating broken Makefiles, /D NDEBUG /O2 makes me think they're hardcoding these flags with / instead of -

@giordano
Copy link
Member

@christopher-dG
Copy link
Contributor Author

Oh well... I'm just gonna remove Windows then.

@giordano
Copy link
Member

giordano commented Apr 10, 2020

I have the feeling that we can patch the CMake file to just remove entire W32 branch, it's just wrong (more precisely, it assumes that Visual Studio is the only compiler for Windows)

@giordano
Copy link
Member

@christopher-dG
Copy link
Contributor Author

oh cool!

@christopher-dG
Copy link
Contributor Author

It works locally now, so all the builds should succeed.

@giordano giordano merged commit 8007167 into JuliaPackaging:master Apr 10, 2020
@giordano
Copy link
Member

I opened a PR upstream to fix the CMake file: lloyd/yajl#228

@christopher-dG
Copy link
Contributor Author

It's very unlikely to be merged i think, development has long since stopped on that repo 😬

@giordano
Copy link
Member

Oh, I didn't even check the activity 🤦

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.

2 participants