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

ninja-build is not listed as a build dependency? #956

Closed
sillydan1 opened this issue Jun 28, 2017 · 3 comments
Closed

ninja-build is not listed as a build dependency? #956

sillydan1 opened this issue Jun 28, 2017 · 3 comments
Labels
question

Comments

@sillydan1
Copy link

sillydan1 commented Jun 28, 2017 •
edited
Loading

Why isn't ninja listed as a build dependency? If one was following the described build process and doesn't have the ninja-build package installed (GNU/LINUX) the build process will complain about ninja is missing.

Linux devuan 3.16.0-4-amd64 #1 SMP Debian 3.16.43-2+deb8u1 (2017-06-18) x86_64 GNU/Linux
@sillydan1 sillydan1 changed the title ninja-build is not listed as a dependency? ninja-build is not listed as a build dependency? Jun 28, 2017
@ghost
Copy link

ghost commented Jul 1, 2017 •
edited by ghost
Loading

I wonder if that might be down to some specific local configuration. I'm on Ubuntu 16.04, don't have ninja installed (I've just barely even heard of it), and all is well. As Devuan is also a Debian fork, it seems it wouldn't be needed there either.

$ dpkg-query --list '*ninja*'
Desired=Unknown/Install/Remove/Purge/Hold
| Status=Not/Inst/Conf-files/Unpacked/halF-conf/Half-inst/trig-aWait/Trig-pend
|/ Err?=(none)/Reinst-required (Status,Err: uppercase=bad)
||/ Name                             Version               Architecture          Description
+++-================================-=====================-=====================-=====================================================================
un  backupninja                      <none>                <none>                (no description available)
un  ninja-build                      <none>                <none>                (no description available)

Is there any chance you might have pointed something to it explicitly? Some versions of cmake can optionally use it, if you throw the right sorts of switches.

@johnkslang
Copy link
Member

Is the question just about the manual instructions (in the readme), which use ninja? And, if so, to match, it just needs mention in the manual instructions? Or, is this really about what's happening internally in cmake?

@cgmb
Copy link
Contributor

cgmb commented Nov 13, 2017

Ninja is optional. If you drop -G Ninja from the cmake command and replace ninja install with make install, the project builds just fine. The only reason its mentioned is because ninja is faster.

I always build with ninja, but I would be tempted to drop it from the build instructions. People familiar with CMake on Linux know it's an option anyway, and there's no point in confusing people who are new to this stuff.

Though, that would still leave a few unmentioned dependencies: make (on Linux) and a C++ compiler. It might be nice to mention what version of C++ is being targeted. That seems to be C++11.

cgmb added a commit to cgmb/glslang that referenced this issue Nov 13, 2017
@cgmb
Simplify build instructions
912906f 
Fixed Windows build instructions:
- ./update_glslang_sources.py is not a valid command on Windows.
  Just call python explicitly.
- -DCMAKE_INSTALL_PREFIX=`pwd`/install creates a directory named `pwd`
  Replaced with %CD%.
- The section mentioning cygwin is unclear and doesn't seem necessary
  to build.

Simplified:
- Remove SSH clone instructions. Instructions on how to clone are for
  those unfamiliar with GitHub, so fewer options are better and SSH
  requires additional configuration to make work.
- Replace ninja with make, because more people are familiar with it and
  we probably don't need to explain how to install it. Fixes KhronosGroup#956.

Update:
- Mention minimum language version in the Dependencies section.
  Though perhaps we should elaborate on exactly which compilers are
  supported? Issue KhronosGroup#42 discusses what compilers to support, but that
  information doesn't seem to be communicated anywhere.
cgmb added a commit to cgmb/glslang that referenced this issue Nov 13, 2017
@cgmb
Improve build instructions
781791c 
Fixed Windows build instructions:
- ./update_glslang_sources.py is not a valid command on Windows.
  Just call python explicitly.
- -DCMAKE_INSTALL_PREFIX=`pwd`/install creates a directory named `pwd`
  Replaced with %CD%.

Simplified:
- Remove SSH clone instructions. Instructions on how to clone are for
  those unfamiliar with GitHub, so fewer options are better and SSH
  requires additional configuration to make work.
- Replace ninja with make, because more people are familiar with it and
  we probably don't need to explain how to install it. Fixes KhronosGroup#956.
- The section mentioning cygwin is unclear and doesn't seem necessary
  to build.

Expanded:
- Mention minimum language version in the Dependencies section.
  Though perhaps we should elaborate on exactly which compilers are
  supported? Issue KhronosGroup#42 discusses what compilers to support, but that
  information doesn't seem to be communicated anywhere.
@cgmb cgmb mentioned this issue Nov 13, 2017
Merged
cgmb added a commit to cgmb/glslang that referenced this issue Dec 5, 2017
@cgmb
Improve build instructions
384b207 
- Mention that the instructions are written for Bash.
- Quote the CMAKE_INSTALL_PREFIX to handle spaces in $(pwd).
- Remove SSH clone instructions. Instructions on how to clone are for
  those unfamiliar with GitHub, so fewer options are better and SSH
  requires additional configuration to make work.
- Replace ninja with make, because more people are familiar with it and
  we probably don't need to explain how to install it. Fixes KhronosGroup#956.
- Remove the section mentioning cygwin, as it's unclear and doesn't
  seem necessary.
- Mention minimum language version in the Dependencies section.
- Move `cd $BUILD_DIR` out of the Linux configuration section because
  it's needed on both platforms. Add a line about creating the build
  directory, too.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
question
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@sillydan1 @cgmb @johnkslang