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

minor docs updates #5879

Merged
merged 3 commits into from
Aug 16, 2016
Merged

minor docs updates #5879

merged 3 commits into from
Aug 16, 2016

Conversation

incanus
Copy link
Contributor

@incanus incanus commented Aug 5, 2016

  1. iOS docs bug reported via user.
  2. Update dependencies for CMake since Switch build system to CMake #5359 landed. (look ok @kkaefer?)

@mention-bot
Copy link

@incanus, thanks for your PR! By analyzing this pull request, we identified @1ec5, @tmcw and @friedbunny to be potential reviewers.

@1ec5
Copy link
Contributor

1ec5 commented Aug 5, 2016

👍

@@ -26,6 +26,7 @@ targets.
- Modern C++ compiler that supports `-std=c++14`
- clang++ 3.5 or later _or_
- g++-5 or later
- CMake (for build only)
- Python 2.x (for build only)
Copy link
Member

Choose a reason for hiding this comment

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

We don't need python anymore afaik.

@friedbunny
Copy link
Contributor

It would also be great to verify that the test instructions still work and are accurate.

@mikemorris
Copy link
Contributor

mikemorris commented Aug 8, 2016

It appears that we specifically require CMake v3.x+, which on Ubuntu 12.04 requires adding a backport ppa (discovered this by digging around in .travis.yml):

sudo apt-add-repository ppa:george-edison55/precise-backports
sudo apt-get update

Attempting to install with CMake 2.8.7 yields the following error:

(cd build/linux-x86_64/Debug && cmake -G Ninja ../../.. \
                -DCMAKE_BUILD_TYPE=Debug \
                -DCMAKE_EXPORT_COMPILE_COMMANDS=ON \
                -DWITH_CXX11ABI=OFF \
                -DWITH_COVERAGE=)
CMake Error: Could not create named generator Ninja
make: *** [build/linux-x86_64/Debug/build.ninja] Error 1

@@ -26,6 +26,7 @@ targets.
- Modern C++ compiler that supports `-std=c++14`
- clang++ 3.5 or later _or_
- g++-5 or later
- CMake (for build only)
- Python 2.x (for build only)
- [Node.js](https://nodejs.org/) (for build only)
- [`pkg-config`](https://wiki.freedesktop.org/www/Software/pkg-config/) (for build only)
Copy link
Contributor

@mikemorris mikemorris Aug 8, 2016

Choose a reason for hiding this comment

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

It appears that we're no longer using pkg-config either? Oops, looks like we're using it indirectly via find_package in CMake now

I was previously using this to ensure we were compiling and linking against a mason-installed version of Mesa, ran into trouble rebasing 04fc3cb onto master because now it can't find EGL/egl.h (which it previously found through pkg-config --cflags)

Copy link
Member

Choose a reason for hiding this comment

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

We are not using find_package in CMake. However, we are using it indirectly because we are invoking mason (which could call pkg-config based on the script.sh) when there is no mason.ini file yet in the package. As we move on and more of the packages we use contain a mason.ini in the precompiled tarball download, we can remove the on-demand mason.ini creation, which removes the pkg-config requirement for Mapbox GL Native.

For what it's worth, we only require Python for creating the shader files and version headers, but we can easily move this to node.js, which we require anyway because of our use of npm.

Copy link
Member

Choose a reason for hiding this comment

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

created ticket for Python removal: #6010

@kkaefer
Copy link
Member

kkaefer commented Aug 15, 2016

Yes, we require CMake 3.1 due to features we used that are not present before 3.1.

@1ec5
Copy link
Contributor

1ec5 commented Aug 16, 2016

@bleege committed the CMake change directly to master in bbd6cfc.

@bleege
Copy link
Contributor

bleege commented Aug 16, 2016

I rebased the development branch onto master and then added an update to explicitly say CMake 3.1 or later per @kkaefer's note. Once CI 👍 I'll merge this into master.

@bleege bleege merged commit 74b70bc into master Aug 16, 2016
@bleege
Copy link
Contributor

bleege commented Aug 16, 2016

CI Tested and Approved. We're documented once again!

@bleege bleege deleted the docs-updates branch August 16, 2016 22:20
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants