Conversation
Signed-off-by: Matthew A Johnson <matjoh@microsoft.com>
mjp41
left a comment
There was a problem hiding this comment.
Is there a way this can be tested in CI? I just want to add something so this can't be broken in the future?
| file(REMOVE_RECURSE "${CURRENT_PACKAGES_DIR}/debug/include") | ||
| file(REMOVE_RECURSE "${CURRENT_PACKAGES_DIR}/debug/share") |
There was a problem hiding this comment.
I don't understand what these lines are doing.
There was a problem hiding this comment.
vcpkg only allows you to have one copy of the headers etc. under the install directory. If it didn't remove the debug copies, then the post-validation step would fail.
Signed-off-by: Matthew A Johnson <matjoh@microsoft.com>
Signed-off-by: Matthew A Johnson <matjoh@microsoft.com>
Signed-off-by: Matthew A Johnson <matjoh@microsoft.com>
Signed-off-by: Matthew A Johnson <matjoh@microsoft.com>
Signed-off-by: Matthew A Johnson <matjoh@microsoft.com>
Great suggestion, I've added CI tests for this which simulate using vcpkg to install the library. |
mjp41
left a comment
There was a problem hiding this comment.
LGTM, just wondering if the test is actually testing that snmalloc is there in the static case.
| void* p = malloc(64); | ||
| if (p == nullptr) | ||
| return 1; | ||
| free(p); | ||
| return 0; |
There was a problem hiding this comment.
It would be nice to check if actually uses snmalloc. I think something like:
| void* p = malloc(64); | |
| if (p == nullptr) | |
| return 1; | |
| free(p); | |
| return 0; | |
| char* p = (char*)malloc(64); | |
| if (p == nullptr) | |
| return 1; | |
| if ((p + 64) != __malloc_endpointer(p)) | |
| return 1; | |
| free(p); | |
| return 0; |
where you add a prototype of
void* __malloc_end_pointer(void*);
This will then call an snmalloc specific feature.
Not sure if what you are building will have a prefix of sn_ for all the methods in the static shim.
| target_include_directories(snmalloc | ||
| INTERFACE | ||
| $<INSTALL_INTERFACE:include/snmalloc> | ||
| $<INSTALL_INTERFACE:include> |
There was a problem hiding this comment.
Is this the bit that has been wrong as an install target.
Signed-off-by: Matthew A Johnson <matjoh@microsoft.com>
Signed-off-by: Matthew A Johnson <matjoh@microsoft.com>
This PR proposes a vcpkg integration for snmalloc in both header-only library mode and static-shim configurations. Once accepted, it will be possible to add snmalloc to vcpkg repositories.