Skip to content

0.1 release build system restructuring #376

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 11 commits into from
Sep 24, 2015
Merged

0.1 release build system restructuring #376

merged 11 commits into from
Sep 24, 2015

Conversation

xlz
Copy link
Member

@xlz xlz commented Sep 1, 2015

This implements the source code and build system restructuring in #314 (rename examples/protonect to src), which is intended to the last major PR before 0.1 release. All issues for 0.1 (#233) except x/ztable hardcoding are resolved after this PR.

This PR is rebased on #368 (OpenGL fixes) and #364 (logging system). Commits from #368 and #364 are shown here, but this PR starts from Use BUILD_SHARED_LIBS to control library type. Commits from other PRs can be commented in their respective PRs.

There are some ABI changes. I try to minimize the exposed API before the release for forward compatibility. In the commit 2f186a5 "Separate public and internal API", several headers are moved to an internal header directory and are no longer installed (no change for internal code though). Protonect in the examples directory can be built with installed libfreenect2: cd examples; cmake .. -Dfreenect2_DIR=/tmp/libfreenect2/lib/cmake/freenect2; make; ./Protonect. This will validate the API usage by third-party. And in the commit "Use CMake to generate LIBFREENECT2_API macro", symbols become hidden by default, except those explicitly marked with LIBFREENECT2_API. Inspecting libfreenect2.so with nm -g --defined-only -C libfreenect2.so should reveal only the necessary ones are exported.

This has been tested on Ubuntu/Intel.

I will rebase the PR with updates in the repo.

@xlz
Copy link
Member Author

xlz commented Sep 3, 2015

Rebased to master.

@floe floe added this to the 0.1 milestone Sep 4, 2015
@floe
Copy link
Contributor

floe commented Sep 11, 2015

Sorry, merge conflict with the huge bunch of comments, should be mostly trivial to resolve.

@xlz
Copy link
Member Author

xlz commented Sep 11, 2015

@floe no problem. This comes after #383.

@xlz
Copy link
Member Author

xlz commented Sep 12, 2015

Rebased to master. Good for review again.

Ensure the old generated include/libfreenect2/config.h is removed during testing.

@floe
Copy link
Contributor

floe commented Sep 14, 2015

Seems to work fine for me; can you update the build instructions in the README?

@xlz
Copy link
Member Author

xlz commented Sep 14, 2015

I just removed examples/protonect stuff.

Due to libusb 1.0.20 release (yesterday), there is probably some better way to handle libusb now and some changes about libusb are expected. So I refrained from adding libusb related changes to the docs.

@floe
Copy link
Contributor

floe commented Sep 15, 2015

OK, that's a nice coincidence. I probably won't have a lot of time this week, did you happen to test with libusb-1.0.20 already? Would be great if we could ditch the whole build-this-exact-commit-yourself thing.

@floe
Copy link
Contributor

floe commented Sep 15, 2015

Had a moment to test after all; v1.0.20 does seem to work on Intel USB3 controllers at least. NEC to be tested soon.

@floe
Copy link
Contributor

floe commented Sep 15, 2015

But it's probably a good idea not to mix things up too much; let's merge this soon so we can get it tested and deal with libusb updates at the end.

@xlz
Copy link
Member Author

xlz commented Sep 15, 2015

This goes after #391 bug fix.

Not yet with 1.0.20. I will do it soon. But caution is needed. I found a regression in libusb pre-1.0.20 a while ago #212 (comment). I'll verify if this has been fixed.

I plan to use QEMU with PCI passthrough to test it with Windows and Mac. I'm also busy recently so that still takes some time to set up.

@xlz
Copy link
Member Author

xlz commented Sep 15, 2015

Rebased to master after 391.

@xlz
Copy link
Member Author

xlz commented Sep 18, 2015

I tried to build this on a Windows 8.1 VM with vfio-pci passthrough of xhci controller. The code did build, but I could not run it because I could not install driver for "Xbox NUI Sensor (composite parent)". Zadig could only find 2 "NuiSensor Adapter" devices.

xlz added 11 commits September 22, 2015 12:10
Right now both shared and static libraries are built at once
without options for configuration.

Use CMake standard variable BUILD_SHARED_LIBS to control the build
type. Reusing shared library objects for static one is a bad idea
because -fPIC results in slower static code with more bloat. Thus
the option to build both at once is not provided. Users are free
to rebuild with -DBUILD_SHARED_LIBS=OFF.

This implements requests in #292 and #263, but reverting #276.
Package distributors can use RPATH to specify local libusb.
User reported error with 2.18.12 in #363. It seems before
2.18.12.1 transitive dependencies are not correctly resolved.
It can be found in commit history.
Renaming only commit. Will not build.
example/protonect is no more.
Remove Protonect definitions from the main CMakeLists.txt
to `examples` directory.

Fix *.bin paths.

A few line-end whitespace deletions.
Move generated config.h and resources.inc.h to build directory.
Several LIBFREENECT_API macros are removed from identifiers that
are no longer public. Several headers are moved to internal
directory and no longer exported.

Build for Protonect out-of-tree with public API only. This provides
a demo on how to use the public API.

Protonect will be built by default in libfreenect2, controlled with
BUILD_EXAMPLES.
@floe
Copy link
Contributor

floe commented Sep 22, 2015

I'd say that right now, as long as it builds, we're good to merge.

@floe floe mentioned this pull request Sep 24, 2015
floe added a commit that referenced this pull request Sep 24, 2015
0.1 release build system restructuring
@floe floe merged commit 94826f7 into OpenKinect:master Sep 24, 2015
@xlz xlz deleted the megarefactor branch October 16, 2015 22:48
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