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

Better handling of STB when not installed #2033

Closed
wants to merge 3 commits into from

Conversation

@rcurtin
Copy link
Member

commented Sep 27, 2019

I tried to build mlpack 3.2.0 for Fedora and found that it actually didn't compile when I disabled downloading of STB. This is an attempt to fix that. I made sure that the STB headers aren't included unless HAS_STB is defined, and also I made it so that the ImageInfo ABI is the same whether HAS_STB is defined or not. (I think that there could be some very strange linking problems otherwise, like if mlpack was built without HAS_STB but then someone compiled something against mlpack with HAS_STB, for instance.)

@rcurtin rcurtin added this to the mlpack 3.2.1 milestone Sep 27, 2019
@rcurtin rcurtin requested a review from MuLx10 Sep 27, 2019
rcurtin added 2 commits Sep 27, 2019
@zoq
zoq approved these changes Sep 27, 2019
Copy link
Member

left a comment

Thanks, for looking into the issue.

@mlpack-bot
mlpack-bot bot approved these changes Sep 28, 2019
Copy link

left a comment

Second approval provided automatically after 24 hours. 👍

Copy link
Member

left a comment

Looks good to me.

@rcurtin

This comment has been minimized.

Copy link
Member Author

commented Sep 30, 2019

I think I merged this in wrong from the command-line so Github doesn't recognize that it's merged, but it is merged so I'll go ahead and close this issue.

@rcurtin rcurtin closed this Sep 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.