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

WIP Fix install. #556

Merged
merged 3 commits into from Jan 29, 2019
Merged

WIP Fix install. #556

merged 3 commits into from Jan 29, 2019

Conversation

fjhenigman
Copy link
Contributor

A couple header files were not being installed.
Fixes #551.

A couple header files were not being installed.
Fixes google#551.
Copy link
Collaborator

@dneto0 dneto0 left a comment

Choose a reason for hiding this comment

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

I find it confusing to have shaderc/visibility.h written in libshaderc_util/include/....
I think I prefer them to moved into source tree at libshaderc/include/shaderc/visibility.h

Also, the include guards are going to be out of date.

Please split common.h into two files: status.h has the status enum, and env.h has the two target environment enums.

@fjhenigman
Copy link
Contributor Author

I find it confusing to have shaderc/visibility.h written in libshaderc_util/include/....
I think I prefer them to moved into source tree at libshaderc/include/shaderc/visibility.h

I left them in libshaderc_util in case in future we wanted a switch to disable libshaderc, i.e. if someone wanted libshaderc_spvc but not libshaderc. And all other shared stuff is in libshaderc_util. Up to you.

@dneto0
Copy link
Collaborator

dneto0 commented Jan 29, 2019

I find it confusing to have shaderc/visibility.h written in libshaderc_util/include/....
I think I prefer them to moved into source tree at libshaderc/include/shaderc/visibility.h

I left them in libshaderc_util in case in future we wanted a switch to disable libshaderc, i.e. if someone wanted libshaderc_spvc but not libshaderc. And all other shared stuff is in libshaderc_util. Up to you.

I can buy that. Right now libshaderc_spvc doesn't have a library or include dependency on libshaderc. So keeping the content in the libshaderc_util tree has merit.

@dneto0
Copy link
Collaborator

dneto0 commented Jan 29, 2019

Remaining two requests:

  • please split "common.h" into env.h and status.h
  • please fix the include guards.

Split common.h into env.h and status.h.
Fix include guards.
@fjhenigman fjhenigman merged commit e7a1910 into google:master Jan 29, 2019
@dneto0
Copy link
Collaborator

dneto0 commented Jan 29, 2019

Thanks for resolving.

@fjhenigman fjhenigman deleted the install branch March 28, 2019 01:47
dneto0 added a commit to dneto0/shaderc that referenced this pull request Nov 14, 2019
Includes:
419517b spvc is optional, off by default, in Android.mk (google#563)
6099206 Remove commas for googltest test suite macro (google#561)
9e1151a Add NV_EXTENSIONS shader types on util compiler conversion. (google#559)
f5abb5c Move public headers to libshaderc. (google#558)
e7a1910 WIP Fix install. (google#556)
6ff17fe Fix cmake build for libshaderc_spvc. (google#554)
8e5a205 Remove use of deprecated googletest macro (google#541)
634dd35 Flesh out for Dawn fuzzers. (google#552)
ce7d921 Use the full spirv-cross dependency. (google#550)
cabee68 BUILD.gn: fix build error on is_component_build=true (google#549)
2dc76ac BUILD.gn: Make libraries Chromium-style "components" (google#548)
fdecc63 Disable spvc build by default.  (google#542) (google#547)
026d4e3 Fix build with exceptions off. (google#546)
d93ef8b Fix MinGW: Format %lu must be unsigned long. (google#544)
5c97fca Fix leaks in tests (google#545)
003c82f Fix ASAN build. (google#543)
a0a4695 Add libshaderc_spvc interface to SPIRV-Cross. (google#524)
fcb2d4e Fix NDK build. (google#540)
14ae0de Android.mk: better fallback for SPIRV-Tools dir (google#538)
6733f17 Roll RE2 Dep (google#537)
1079643 Fixup source matching test (google#534)
b9e151b Fix Windows test script return value. (google#533)
355fe6a Switch GLSLang to upstream (google#515)
d6c89a6 Generate source text debug info if requested (google#455) (google#532)
3faf34a [kokoro] Stop running tests on VS 2013 builds. (google#531)
a516b67 Update Android NDK. (google#530)
4727ede Test shader uniforms need descriptor set and binding (google#523)
32b6d90 Add git clone of re2 and effcee to the Dockerfile (google#527)
53c776f Update to use Android.mk from glslang (google#520)
2898ad7 Add unit tests for mesh and task shaders
44f7dd2 Add support for NV_EXTENSIONS in #pragma shader_stage()
7544155 Build file updates. (google#514)
eb743ec Fixup shadowed variables (google#513)
823901b Unpin googletest. It has been fixed upstream
a8bbd80 Add effcee and re2 when building spirv-tools tests (google#510)
92efe45 Only include third_party directories if needed. (google#507)
909f0d3 BUILD.gn: remove the static_library target.
818a37b [kokoro] Report error if ctest fails
a4116bc [kokoro] Compile for x86_64 and print CMake version on Windows
72512db VS builds: Pin an older googletest to avoid around a CMake problem
b806cd2 Disable layout and relax logical pointer for optimizer validation
196d38f BUILD.gn: Make source_set not depend on static_libraries
c9da865 BUILD.gn: only use complete_static_lib on mac
a1f4027 Start Shaderc v2018.1-dev development
7d8582b Finalize Shaderc v2018.0
ed3c8e3 Update CHANGES
69413e3 Fix -Wfallthrough warnings
6503446 Return *this in move-assign operator
0e81b8b Add default ctor and move assignment to CompilationResult
7193d2e Adjust to glslang's removal of NV_EXTENSIONS from headers
41d15ff BUILD.gn: make the static_lib complete to fix the build on OSX
fc4ff0e Pass environment version into SPIRV-Tools properly
5804d8c Add SONAME version to the library

Test: checkbuild.py on Linux; unit tests on Windows
Change-Id: Ib255e3b16d7e96ce4bcc5ccaff9c178c472d8bb7
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.

None yet

2 participants