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

Fix OS X dependencies, remove ubuntu disco, update C++ standard #570

Merged
merged 10 commits into from
May 16, 2020

Conversation

egordon
Copy link
Contributor

@egordon egordon commented May 15, 2020

Currently, all tests segfault after main on OS X. This was due to a missing dart dependency: tinyxml.

In addition, Ubuntu disco is no longer supported, so that has been updated to the latest LTS release (focal).

Finally, due to defaults in both bionic and focal, this updates the standard to C++14. This matches the DART standard, allows for now-idiomatic function (e.g. std::make_unique), and its where we should be heading for a stable release.


Before creating a pull request

  • [ N/A ] Document new methods and classes
  • [ N/A ] Format code with make format

Before merging a pull request

  • Set version target by selecting a milestone on the right side
  • Summarize this change in CHANGELOG.md
  • Add unit test(s) for this change

@egordon egordon added this to the Aikido 0.3.0 milestone May 15, 2020
@egordon egordon changed the title Fix travis errors (remove disco and fix OS X segfaults) Fix OS X dependencies, remove ubuntu disco, update C++ standard May 15, 2020
Copy link
Contributor

@brianhou brianhou left a comment

Choose a reason for hiding this comment

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

Minor nits. I think we should also update the README to reflect the new C++ standard.

.ci/install_linux_catkin.sh Outdated Show resolved Hide resolved
.ci/install_linux_cmake.sh Outdated Show resolved Hide resolved
@@ -3,7 +3,7 @@ brew 'dartsim'
brew 'eigen'
brew 'libmicrohttpd'
brew 'ompl'
brew 'tinyxml2'
brew 'tinyxml'
brew 'yaml-cpp'

# Install dartsim dependencies explicitly because, for some unknown reasons,
Copy link
Contributor

Choose a reason for hiding this comment

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

Not being able to automatically pull in transitive dependencies is incredibly annoying. It really seems like there should be an obvious way to fix this. Can we add an issue to do so?

@jslee02 do you remember whether this problem was just with Travis CI + Homebrew? Or whether it's a plain Homebrew issue?

In the worst case, it seems like we should be able to do something like brew deps dartsim from Travis to generate this list of dependencies that should be installed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added issue #571

Maybe we can unblock PRs by pushing this and continue discussion there?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yep, absolutely.

Copy link
Member

Choose a reason for hiding this comment

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

@jslee02 do you remember whether this problem was just with Travis CI + Homebrew? Or whether it's a plain Homebrew issue?

I didn't investigate further but just this workaround. Probably it was just Homebrew. Not sure if it's fixed or now. 😞

@egordon egordon merged commit e3ab367 into master May 16, 2020
@egordon egordon deleted the egordon/fix_travis branch May 16, 2020 00:20
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.

3 participants