Skip to content

Commit

Permalink
[libjpeg-turbo] Disable GETENV/SETENV in UWP. See libjpeg-turbo/libjp…
Browse files Browse the repository at this point in the history 
…eg-turbo#203
  • Loading branch information
@ras0219-msft
ras0219-msft committed Mar 18, 2018
1 parent adb8edd commit bae7c95
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 1 deletion.
2 changes: 1 addition & 1 deletion ports/libjpeg-turbo/CONTROL
View file
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
Source: libjpeg-turbo
Version: 1.5.3
Version: 1.5.3-1
Description: libjpeg-turbo is a JPEG image codec that uses SIMD instructions (MMX, SSE2, NEON, AltiVec) to accelerate baseline JPEG compression and decompression on x86, x86-64, ARM, and PowerPC systems.
11 changes: 11 additions & 0 deletions ports/libjpeg-turbo/portfile.cmake
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,17 @@ vcpkg_from_github(
HEAD_REF master
)

vcpkg_download_distfile(GETENV_PATCH
URLS "https://github.com/libjpeg-turbo/libjpeg-turbo/commit/bd96b30b74fe166fc94218cfc64a097fafdcc05f.diff"
FILENAME "libjpeg-turbo-bd96b30b74fe166fc94218cfc64a097fafdcc05f.diff"
SHA512 4cd064521b5e4baba4adf972f9f574f6dd43a2cd3e3ad143ca2cdf0f165024406d4fd2ed094124d0c17c5370394140e82fdd892d3cdc49609acdf8f79db1758c
)

vcpkg_apply_patches(
SOURCE_PATH ${SOURCE_PATH}
PATCHES "${CMAKE_CURRENT_LIST_DIR}/add-options-for-exes-docs-headers.patch"
"${CMAKE_CURRENT_LIST_DIR}/linux-cmake.patch"
"${GETENV_PATCH}"
)

if(VCPKG_TARGET_ARCHITECTURE STREQUAL "arm" OR VCPKG_TARGET_ARCHITECTURE STREQUAL "arm64" OR (VCPKG_CMAKE_SYSTEM_NAME AND NOT VCPKG_CMAKE_SYSTEM_NAME STREQUAL "WindowsStore"))
Expand All @@ -22,6 +29,10 @@ else()
set(ENV{PATH} "$ENV{PATH};${NASM_EXE_PATH}")
endif()

if(VCPKG_CMAKE_SYSTEM_NAME STREQUAL "WindowsStore")
set(ENV{_CL_} "-DNO_GETENV -DNO_PUTENV")
endif()

string(COMPARE EQUAL "${VCPKG_LIBRARY_LINKAGE}" "dynamic" ENABLE_SHARED)
string(COMPARE EQUAL "${VCPKG_LIBRARY_LINKAGE}" "static" ENABLE_STATIC)
string(COMPARE EQUAL "${VCPKG_CRT_LINKAGE}" "dynamic" WITH_CRT_DLL)
Expand Down

6 comments on commit bae7c95

@Tryum
Copy link

@Tryum Tryum commented on bae7c95 Mar 18, 2018

Choose a reason for hiding this comment

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

Please let people contribute to VCPKG, you're too quick :)

On the other hand I now have learnt that the distfile/patch trick, and the "direct-to-compiler" preprocessor definition !

@ras0219-msft
Copy link
Contributor Author

@ras0219-msft ras0219-msft commented on bae7c95 Mar 18, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

The _CL_ trick is a bit unfortunate; I hope @dcommander adds cmake options for the two preprocessor defs :)

@Tryum
Copy link

@Tryum Tryum commented on bae7c95 Mar 18, 2018

Choose a reason for hiding this comment

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

Ok ! I currently have a patch for the CMake file in my vcpkg working copy. I have to refine it a bit and submit another PR !

@dcommander
Copy link

Choose a reason for hiding this comment

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

I have no idea why it's necessary to set the _CL_ environment variable here, or what that does. You should be able to simply call add_definitions(-DNO_GETENV -DNO_PUTENV), and that will affect the whole project.

@ras0219-msft
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The script here is actually isolated from the underlying cmake build itself. This is unfortunately needed for compatibility because most libraries accidentally assume that they are the project root.

@dcommander
Copy link

@dcommander dcommander commented on bae7c95 Mar 18, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

OK, but bearing in mind that this is the first I've heard of vcpkg, does it eventually invoke CMake to perform the actual build? If so, then why can't it just manipulate CMAKE_C_FLAGS as I demonstrated in libjpeg-turbo/libjpeg-turbo#203? There is a lot more to a Windows Store-compliant build than simply setting NO_GETENV and NO_PUTENV. The rest of that process is, from an open source developer's point of view (hint: OSS developers don't always like using the Visual Studio IDE, as a general rule), a little esoteric. I need to understand how and why vcpkg is doing things like setting CMAKE_SYSTEM_NAME=WindowsStore, since that affects other aspects of our build. Currently, if you set CMAKE_SYSTEM_NAME=WindowsStore, it breaks CMake's CPU detection, and thus the libjpeg-turbo build cannot figure out which SIMD extensions (x86, x64) it should use. If I can nail down a less fragile way of handling all of this, I'd be happy to introduce a new CMake variable that serves as a one-stop shop for making libjpeg-turbo Windows Store-compatible (that could involve simply handling CMAKE_SYSTEM_NAME=WindowsStore, if we could figure out how to fix CPU detection), but also bear in mind that I'm not being paid for any of this work to support a proprietary operating system, so there is only a limited amount I can do.

Please sign in to comment.