-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
Feature/nsync #6085
Feature/nsync #6085
Conversation
This reverts commit 5f1419d.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
This comment has been minimized.
This comment has been minimized.
More information from
https://github.com/google/nsync/blob/master/internal/dll.c#L61 - do be investigated ... |
@CAMOBAP All DLL files should be copied to bin folder, not lib. |
@uilianries yep just fixed this. But how those DLL will be found in runtime? Should we add |
This comment has been minimized.
This comment has been minimized.
864c257
to
53643f2
Compare
This comment has been minimized.
This comment has been minimized.
53643f2
to
17bd7b2
Compare
This comment has been minimized.
This comment has been minimized.
@uilianries thanks a lot for your help! looks like all done now |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Almost good to go. only small suggestions
Co-authored-by: Uilian Ries <[email protected]>
This comment has been minimized.
This comment has been minimized.
Done |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One more 🙃
Move base_path to conandata.yaml Co-authored-by: Chris Mc <[email protected]>
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM thank you!!
ar_dest = \ | ||
"ARCHIVE DESTINATION ${CMAKE_INSTALL_LIBDIR} " \ | ||
"COMPONENT Development" | ||
rt_dest = 'RUNTIME DESTINATION "${CMAKE_INSTALL_BINDIR}"' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd apply this change regardless of the OS and build type, even if only makes sense there.
Also keep in mind you're removing the COMPONENT
, I don't know the implications of this in this particular case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Per documentation in https://cmake.org/cmake/help/latest/command/install.html:
COMPONENT
Specify an installation component name with which the install rule is associated, such as "runtime" or "development". During component-specific installation only install rules associated with the given component name will be executed. During a full installation all components are installed unless marked withEXCLUDE_FROM_ALL
. IfCOMPONENT
is not provided a default component "Unspecified" is created. The default component name may be controlled with theCMAKE_INSTALL_DEFAULT_COMPONENT_NAME
variable.
Because we don't perform component-specific installation probably this is not the case for us.
As another solution, I think we can safely add COMPONENT RuntimeLibraries
Fix typo in receipt description Co-authored-by: Eric Riff <[email protected]>
b48c151
All green in build 38 (
|
Specify library name and version: nsync/1.23.0
nsync
is a dependency foronnxruntime
receipt, which is in progressconan-center hook activated.