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

headers with a standard name like string.h and limits.h should be renamed #61

Open
m6w6 opened this issue Jan 20, 2020 · 4 comments
Open

headers with a standard name like string.h and limits.h should be renamed #61

m6w6 opened this issue Jan 20, 2020 · 4 comments
Milestone
2.0

Comments

@m6w6
Copy link
Collaborator

m6w6 commented Jan 20, 2020 •
edited
Loading

Imported from Launchpad using lp2gh.

$ find . -name string.h
./third_party/libmemcached/libmemcached-1.0/struct/string.h
./third_party/libmemcached/libhashkit-1.0/string.h
./third_party/libmemcached/libhashkit/string.h
./third_party/libmemcached/tests/string.h

"string.h" is a header from the C runtime library. Project headers should not use this name.
@m6w6 m6w6 added the New label Jan 20, 2020
@m6w6
Copy link
Collaborator Author

m6w6 commented Jan 20, 2020

These headers should be renamed, preferably each to a distinct name.

@m6w6 m6w6 removed the New label Jan 20, 2020
@m6w6 m6w6 closed this as completed Jan 22, 2020
@m6w6 m6w6 added this to the 2.0 milestone Oct 19, 2020
@m6w6 m6w6 reopened this Oct 19, 2020
@m6w6 m6w6 changed the title string.h should be renamed headers with a standard name like string.h and limits.h should be renamed Dec 27, 2020
@cmb69
Copy link

cmb69 commented Mar 9, 2022

Due to this, Windows in-tree builds of PECL/memcached fail, because other extensions include these files:

C:\php-snap-build\dep-aux\vs16\x64\libmemcached\include\libhashkit-1.0\string.h(27): error C2054: expected '(' to follow 'HASHKIT_API' (compiling source file ext\bz2\bz2.c)
…

cmb69 added a commit to php/web-rmtools that referenced this issue Mar 9, 2022
@cmb69
Prepare for memcached builds
822f808 
In theory, this should work, but in practice it fails since libmemcached
ships headers with standard names[1] which are erroneously included.  We
still make the change, if only as a reminder that libmemcached needs to
be fixed.

We shall have a look at building with `--enable-memcached-igbinary` when
the header issue has been resolved.

[1] <awesomized/libmemcached#61 (comment)>
@m6w6
Copy link
Collaborator Author

m6w6 commented Mar 9, 2022

Yeah, libmemcached/include/libhashkit-1.0 and siblings must not be in the include paths

@cmb69
Copy link

cmb69 commented Mar 10, 2022

And that is even possible. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
2.0
Development

No branches or pull requests
2 participants
@m6w6 @cmb69