Skip to content

Conversation

@maxhora
Copy link
Contributor

@maxhora maxhora commented Jul 26, 2017

…ag for lz4 and zstd libs

if (MSVC)
set(ZSTD_STATIC_LIB "${ZSTD_BUILD_DIR}/build/VS2010/bin/x64_${CMAKE_BUILD_TYPE}/libzstd_static.lib")
set(ZSTD_BUILD_COMMAND BUILD_COMMAND msbuild ${ZSTD_BUILD_DIR}/build/VS2010/zstd.sln /t:Build /v:minimal /p:Configuration=${CMAKE_BUILD_TYPE} /p:Platform=x64 /p:PlatformToolset=v140 /p:OutDir=${ZSTD_BUILD_DIR}/build/VS2010/bin/x64_${CMAKE_BUILD_TYPE}/ /p:SolutionDir=${ZSTD_BUILD_DIR}/build/VS2010/ )
set(ZSTD_PATCH_COMMAND PATCH_COMMAND git --git-dir=. apply --verbose ${CMAKE_SOURCE_DIR}/build-support/zstd_msbuild_wholeprogramoptimization_param.patch)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@wesm , just setting of --git-dir= to specify root directory of repository is enough to make git apply workable to apply patches. It works at list for Windows :)

Copy link
Member

@xhochy xhochy left a comment

Choose a reason for hiding this comment

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

I'm a bit confused that we patch here using git although we have a non-git source tarball. Why cannot we use the patch command. Is there no equivalent on Windows/AppVeyor?

@wesm
Copy link
Member

wesm commented Jul 26, 2017

Right, there's no patch equivalent on Windows AFAIK

@wesm
Copy link
Member

wesm commented Jul 26, 2017

We should probably report these issues upstream?

@maxhora
Copy link
Contributor Author

maxhora commented Jul 27, 2017

@xhochy @wesm patch tool is available on Appveyor, but on clean Windows installation it's not. Usage of patch for Windows will create additional dependency to install.
But conda, for example, has patch tool for Windows.

@xhochy
Copy link
Member

xhochy commented Jul 27, 2017

@Maxris good to know. Then we can keep it this way as long as users don't complain for the git dependency ;)

Copy link
Member

@wesm wesm left a comment

Choose a reason for hiding this comment

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

+1, thanks @Maxris!

@asfgit asfgit closed this in 7b3378f Jul 27, 2017
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.

3 participants