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.

Sign up for GitHub

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

Jump to bottom

Add all dependencies in CI #43

Closed
knatten opened this issue Nov 29, 2019 · 8 comments · Fixed by #92
Closed

Add all dependencies in CI #43

knatten opened this issue Nov 29, 2019 · 8 comments · Fixed by #92
Labels
ci Continuous integration / devops

Comments

@knatten
Copy link
Contributor

knatten commented Nov 29, 2019 •
edited
Loading

Currently we only have PCL, not Eigen nor OpenCV. And when adding 16.04, we'll not even have PCL on that one.

Consider using Conan like we do in zivid-python. Think about the experience for people who just want to clone the repo and check out the samples, need to still be easy to get started.

Here's a summary of which versions we currently use (according to README.md on tip of master), and which are available in Ubuntu 16 / 18 / 20:

Dependency We use Ubuntu 16.04 Ubuntu 18.04 Ubuntu 20.04
Eigen 3.3.7 3.3~beta1-2 3.3.4 3.3.7
OpenCV 4.0.1 2.4.9 3.2.0 4.2.0
PCL ??? 1.7.2 1.8.1 1.10.0
@knatten knatten mentioned this issue Nov 29, 2019
Closed
@nedrebo
Copy link

nedrebo commented Nov 29, 2019 •
edited
Loading

Can probably do the conon stuff only if USE_PCL or USE_EIGEN is ON.

CI of course sets everything to ON.

@knatten
Copy link
Contributor Author

knatten commented Nov 29, 2019

Yeah that would be good.

The main work for this issue I think is to decide whether to ask users to use Conan as well. We currently have a quite long KB article for how to install everything correctly on Windows, and no instructions for Linux. We need to figure out if it's simpler to instruct people to install conan instead, and allow dependencies to be installed automatically.

If we do that, it should still be possible to clone this repo and build the samples which have no dependencies, without bothering about conan. (Like it is today.)

Also need to figure out if we should allow people to override these settings and use an installation of Eigen not from conan etc.

@nedrebo
Copy link

nedrebo commented Nov 29, 2019

Also need to figure out if we should allow people to override these settings and use an installation of Eigen not from conan etc.

I would say no to this one. Sounds like advanced usage, they figure it out anyway. Keep it as simple as possible, optimize for the casual user.

@knatten
Copy link
Contributor Author

knatten commented Nov 29, 2019 •
edited
Loading

Blocked by #44 That one's been fixed now.

@knatten knatten self-assigned this Nov 29, 2019
@knatten
Copy link
Contributor Author

knatten commented Dec 2, 2019

When this is fixed, search the code base for #43 and remove any workarounds which have been introduced in the mean time.

@knatten knatten mentioned this issue Dec 3, 2019
Merged
@knatten knatten added the ci Continuous integration / devops label Dec 3, 2019
@knatten knatten removed their assignment Dec 3, 2019
@knatten knatten mentioned this issue Dec 11, 2019
Merged
@SatjaSivcev
Copy link
Contributor

Ubuntu 20.04 LTS is out, with these versions:
https://www.ubuntuupdates.org/package/core/focal/universe/base/opencv
https://www.ubuntuupdates.org/package/core/focal/universe/base/pcl
https://www.ubuntuupdates.org/package/core/focal/universe/base/libeigen3-dev

@knatten
Copy link
Contributor Author

knatten commented Apr 24, 2020

Nice! I've updated the issue with those version numbers. Would be good if you could figure out which PCL version you depend on. Looks like we can do CI with Ubuntu 20.04 without any external dependencies! \o/

@SatjaSivcev
Copy link
Contributor

Yes, @torbsorb confirmed the same. :)

@torbsorb torbsorb mentioned this issue Apr 29, 2020
Merged
torbsorb added a commit that referenced this issue Apr 29, 2020
@torbsorb
Merge pull request #92 from zivid/clang-tidy-on-all-samples
929b236 
This PR adds Ubuntu:20.4 to CI, clang-tidy-10, clang-format-10 and enables both build and lint on all samples. This includes samples dependent on Eigen3 and OpenCV, which was previously neither built nor linted.

This closes #17, closes #43, and closes #96
knatten added a commit that referenced this issue Jun 4, 2020
@knatten
Remove outdated TODO
598b43f 
Issue #43 was closed in PR #92. As of then, it seems the decision was
that it's enough to build all samples on Ubuntu 20.04. On older distros,
we skip samples where the dependencies which are shipped with the distro
are too old.
torbsorb pushed a commit that referenced this issue Jun 5, 2020
@knatten @torbsorb
Remove outdated TODO
03e54cf 
Issue #43 was closed in PR #92. As of then, it seems the decision was
that it's enough to build all samples on Ubuntu 20.04. On older distros,
we skip samples where the dependencies which are shipped with the distro
are too old.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
ci Continuous integration / devops
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Clang tidy on all samples zivid/zivid-cpp-samples
3 participants
@knatten @nedrebo @SatjaSivcev