Skip to content

Conversation

@tiennou
Copy link
Contributor

@tiennou tiennou commented Dec 10, 2017

Otherwise Xcode will happily not-link our git2 target, resulting in a "missing file" error when building eg. examples

Otherwise Xcode will happily not-link our git2 target, resulting in a "missing file" error when building eg. examples
# when using only object libraries.
FILE(WRITE ${CMAKE_CURRENT_BINARY_DIR}/dummy.c "")
LIST(APPEND LIBGIT2_OBJECTS ${CMAKE_CURRENT_BINARY_DIR}/dummy.c)
ENDIF()
Copy link
Member

Choose a reason for hiding this comment

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

You're only doing this after already exporting LIBGIT2_OBJECTS into the parent scope. So I guess all callers of this CMakeLists.txt file will not get that dummy file. Is this intended?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure what is achieved by those PARENT_SCOPE, is it so that clar can static-link against the whole library code ?

AFAICT, this makes the dummy file appear, it shouldn't impact anything other than the Xcode generator, and if my point above is true then I don't think it matters because libgit2_clar is an executable, and it has .c files of its own (which sidesteps the underlying Xcode issue).

If you're concerned, I can move the block just above those SET lines.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, exactly. We do not want to link against the .so or .a file but instead to the object files directly such that we can use some internal functions.

I think your reasoning makes sense, so it should be a non-issue. Thanks.

@pks-t pks-t merged commit 30455a5 into libgit2:master Jan 3, 2018
@pks-t
Copy link
Member

pks-t commented Jan 3, 2018

Thanks for the workaround!

@tiennou
Copy link
Contributor Author

tiennou commented Jan 3, 2018

Sorry for the missed fix : since I was originally prodding Xcode, and CMake didn't actually cleanup the expected dylib, it appeared to work when I tested the first time.

@tiennou tiennou deleted the fix/4352 branch January 3, 2018 13:47
@tiennou tiennou mentioned this pull request Jan 17, 2018
@tiennou tiennou mentioned this pull request Oct 27, 2019
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.

2 participants