[CXX-1049] Install target fails to install headers when source absolute path contains the substring "test" Created: 26/Sep/16 Updated: 15/Nov/16 Resolved: 07/Oct/16 |
|
| Status: | Closed |
| Project: | C++ Driver |
| Component/s: | Build |
| Affects Version/s: | 3.0.1 |
| Fix Version/s: | 3.0.3 |
| Type: | Bug | Priority: | Major - P3 |
| Reporter: | Nicholas J Hathaway | Assignee: | David Golden |
| Resolution: | Done | Votes: | 0 |
| Labels: | None | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Description |
|
I noticed something very odd when installing the driver and the install location path had the word in all lower case "test" at the beginning of any of the parent directories of the install location. The only headers that get installed are export.hpp, version.hpp, and config.hpp even though when it goes to install it says "Install configuration: "Release"". I've noticed this on both Ubuntu and Mac and with different compilers so it doesn't seem to be a OS or compiler problem. If I use the same exact cmake commands and just change the install location so that it doesn't contain the word "test" at the beginning of the parent directories it works. And so it fails when the path is one of the following And it works with the following paths So I'm not sure what's happening but I'm guessing it's a cmake issue but I don't know cmake very well so I couldn't tell. Of course the work around is to not install to location that contains the word "test" but I thought it was odd and probably not intended behavior so I thought I would bring it up. NIck |
| Comments |
| Comment by Githook User [ 08/Oct/16 ] |
|
Author: {u'username': u'jrassi', u'name': u'J. Rassi', u'email': u'rassi@10gen.com'}Message: |
| Comment by Githook User [ 08/Oct/16 ] |
|
Author: {u'username': u'jrassi', u'name': u'J. Rassi', u'email': u'rassi@10gen.com'}Message: |
| Comment by Githook User [ 07/Oct/16 ] |
|
Author: {u'username': u'xdg', u'name': u'David Golden', u'email': u'xdg@xdg.me'}Message: The prior approach to selecting headers was to include all headers, then The new approach renames all private headers to .hh, which are excluded |
| Comment by Githook User [ 07/Oct/16 ] |
|
Author: {u'username': u'xdg', u'name': u'David Golden', u'email': u'xdg@xdg.me'}Message: The prior approach to selecting headers was to include all headers, then The new approach renames all private headers to .hh, which are excluded |
| Comment by David Golden [ 06/Oct/16 ] |
|
The Google C++ Style Guide for headers suggests .inc for files included that aren't headers. Since private headers aren't available as headers to end-users, they are effectively just "non header includes" needed for compilation only. How about renaming to .inc? A different header suffix is going to be much cleaner in Cmake than other approaches, I think. |
| Comment by Andrew Morrow (Inactive) [ 06/Oct/16 ] |
|
I'd be opposed to renaming the private header files to .h. The .h suffix is for C files, and many environments expect and act on the file suffix. We were quite deliberate in our use of .hpp everywhere for C++ files that we wrote. |
| Comment by David Golden [ 06/Oct/16 ] |
|
It looks like cmake doesn't give us full regular expressions, so we're a bit handicapped in how we do this. What if we changed all the private headers from .hpp to .h and then just excluded *.h rather than trying to exclude directories with regular expression matches? Then we're not doing directory path matching at all (which is notoriously hard to do correctly and portably). We currently have only one .h header and it's already private. acm, any objections to this change? |
| Comment by David Golden [ 04/Oct/16 ] |
| Comment by David Golden [ 26/Sep/16 ] |
|
Great catch. Thanks. |