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

Improve build #55

Open
wants to merge 10 commits into
base: dev/UpdatedHeaders
Choose a base branch
from

Conversation

rpavlik
Copy link

@rpavlik rpavlik commented Oct 5, 2020

The peculiarities of the build threw me off: I kept getting errors, etc. and it seemed to only be supported as an "in-source" build which is unusual for CMake. I have adjusted things so that build dir and source dir don't have to be the same, the resulting binaries (and deps) can still be copied as a post-build step (optional, however, adjustable from CMake), and the script is otherwise simplified by using things built in to CMake and removing unneeded options. I did not touch CI or the README here.

Note: I was a bit confused by the fact that some Linux binaries are in a sub-sub directory. so I put a TODO in the CMake for it. after further review, I'm pretty confident files were in there as an accident, since the RPATH wouldn't find them there, and they may conflict at build time, so I removed them.

@rpavlik
Copy link
Author

rpavlik commented Oct 5, 2020

FYI, I am pretty sure the ucrtbased files are not intended to be redistributable (at least other debug runtimes aren't) and they're not actually needed here (since we're building against the release CRT), so those could also be removed

CMake seems to not always copy (at least on Windows) if the file isn't removed first.
Copy link
Contributor

@1runeberg 1runeberg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@zite for the Linux SteamVR Input plugin, does it still require pre-built binaries for the auto-install?

That's the only reason this was added I believe, similar to the windows binaries.

@zite
Copy link
Collaborator

zite commented Oct 7, 2020

Not sure, suppose we could try to remove them and see what happens

@rpavlik
Copy link
Author

rpavlik commented Oct 8, 2020

The binaries for Linux are still there, they just aren't located in two different places in the plugin.

@basilfamer
Copy link

basilfamer commented Oct 15, 2020

This absolutely fixed an issue I had, and I'm extremely grateful! I did have to enable LinuxStandalone64 on the assembly definition though

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.

4 participants