Skip to content
This repository has been archived by the owner on Feb 8, 2022. It is now read-only.

build.rs is too platform specific #10

Closed
mtmiller opened this issue Apr 21, 2017 · 7 comments · Fixed by #49
Closed

build.rs is too platform specific #10

mtmiller opened this issue Apr 21, 2017 · 7 comments · Fixed by #49

Comments

@mtmiller
Copy link
Collaborator

I can build the project by deleting the --target=... and changing the -I/path/to/my/octave options. What options or environment variables are available to cargo or bindgen to make this file less platform specific so anyone can build for their system?

@ctaggart
Copy link
Owner

ctaggart commented Apr 21, 2017

@mtmiller Looking at the stylo example, it looks like we can do something like:

if cfg!(target_os = "windows") {

} else if cfg!(target_os = "macos") {

}

And there is more documentation at http://rustbyexample.com/attribute/cfg.html. I haven't done a ton of Rust coding either. Please do enhance it to work on your computer. Accepting PRs. :-)

@mtmiller
Copy link
Collaborator Author

That's not really what I'm reporting though. There is not one hardcoded location for Octave include files. If you and I are on the exact same distro, and my Octave header files are in /usr/local/octave/include and yours are in /opt/octave4.2/include, there must be a way to work that without every user having to edit this file. I would expect CPPFLAGS=-I/opt/octave/include/octave-4.2.1/octave cargo build to work, but it seems to be ignored. Maybe bindgen offers no way to do this.

@ctaggart
Copy link
Owner

@mtmiller, are you reporting that Octave header files may be installed in different locations? How do you recommend we look for them? How about an environment variable like this:
https://github.com/servo/servo/blob/master/components/style/build_gecko.rs#L49

@ctaggart
Copy link
Owner

#22 shows where to look for the headers on Linux.

@ctaggart
Copy link
Owner

Well, that's unfortunate. It looks like committing the generated file is not a good idea. The binding end up different between Windows and Linux as you can see here with 5.1.0:

image

@ctaggart
Copy link
Owner

I added a switch statement for different targets in #32.

@ctaggart ctaggart reopened this Nov 21, 2019
@ctaggart
Copy link
Owner

Probably should support environment variables for the various paths. We can search for the existence of files in well known locations if not set.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants