microsoft / vcpkg

C++ Library Manager for Windows, Linux, and MacOS
MIT License
22k stars 6.12k forks source link

[openimageio] Re-fix the usage #20092

Closed JackBoosY closed 2 years ago

JackBoosY commented 2 years ago

Merging to master caused all my changes in portfile.cmake to be rolled back. Re-fix them:

  1. Restore FindOpenEXR.cmake since it calls find_dependency(OpenEXR CONFIG) and it contains the required macro FOUND_OPENEXR_WITH_CONFIG in OpenImageIOConfig.cmake.
  2. Correct FindLibsquish.cmake and vcpkg-cmake-wrapper.cmake installation path.

Related: #19916.

JackBoosY commented 2 years ago

@jmacey Can you please test this PR?

Thanks.

jmacey commented 2 years ago

@jmacey Can you please test this PR?

Thanks.

This now builds fine (Windows) but still fails to copy squish.dll to the target directory.

JackBoosY commented 2 years ago

but still fails to copy squish.dll to the target directory.

@jmacey Can you provide me a repro step?

jmacey commented 2 years ago

basically when I build under windows using cmake, the following files are copied into the target directory (build/Debug)

-a---- 10/09/2021 09:33 212992 boost_filesystem-vc142-mt-gd-x64-1_76.dll -a---- 10/09/2021 09:33 170496 boost_thread-vc142-mt-gd-x64-1_76.dll -a---- 10/09/2021 08:19 376832 Half-2_5_d.dll -a---- 10/09/2021 08:14 1461248 heif.dll -a---- 10/09/2021 08:19 470016 Iex-2_5_d.dll -a---- 10/09/2021 08:20 5532672 IlmImf-2_5_d.dll -a---- 10/09/2021 08:19 156160 IlmThread-2_5_d.dll -a---- 10/09/2021 08:19 241664 Imath-2_5_d.dll -a---- 10/09/2021 07:52 1008640 jpeg62.dll -a---- 10/09/2021 08:07 1922048 libde265.dll -a---- 10/09/2021 07:53 423424 libpng16d.dll -a---- 10/09/2021 08:12 8110592 libx265.dll -a---- 10/09/2021 09:34 428544 lzmad.dll -a---- 10/09/2021 10:34 15184896 OpenImageIO_d.dll -a---- 10/09/2021 10:33 2268672 OpenImageIO_Util_d.dll -a---- 10/09/2021 09:35 1012224 tiffd.dll

squish.dll is missing and the exe will not run, If I manually copy the file to the target it works fine, I'm guessing this is from the cmake build / link process (it's different under mac and linux as they add relative link paths or use static libs).

Jon


From: Jack·Boos·Yu @.> Sent: 10 September 2021 10:50 To: microsoft/vcpkg @.> Cc: Jon MacEy @.>; Mention @.> Subject: Re: [microsoft/vcpkg] [openimageio] Re-fix the usage (#20092)

but still fails to copy squish.dll to the target directory.

@jmaceyhttps://eur02.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fjmacey&data=04%7C01%7Cjmacey%40bournemouth.ac.uk%7C39ceb932badf4437ee0308d974407b99%7Cede29655d09742e4bbb5f38d427fbfb8%7C0%7C0%7C637668642566979287%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=PPFjfqBuAZF3QNYx9D%2FHFtygqUIq8FdjvbwwknXVOkQ%3D&reserved=0 Can you provide me a repro step?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://eur02.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fmicrosoft%2Fvcpkg%2Fpull%2F20092%23issuecomment-916779222&data=04%7C01%7Cjmacey%40bournemouth.ac.uk%7C39ceb932badf4437ee0308d974407b99%7Cede29655d09742e4bbb5f38d427fbfb8%7C0%7C0%7C637668642566979287%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=EY5eYrcKq0j4Hkj0D2HoCf2nZKbO%2B%2F%2Be%2FJp%2Fd%2FlJOXY%3D&reserved=0, or unsubscribehttps://eur02.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FAACK4G6HKHDJVV3I55MSZX3UBHIH3ANCNFSM5DY3OAKQ&data=04%7C01%7Cjmacey%40bournemouth.ac.uk%7C39ceb932badf4437ee0308d974407b99%7Cede29655d09742e4bbb5f38d427fbfb8%7C0%7C0%7C637668642566989284%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=IzNMLeG%2F%2BYn%2BlDTly4P8d9%2FR2ePU9MsYSFduOyd2izc%3D&reserved=0. Triage notifications on the go with GitHub Mobile for iOShttps://eur02.safelinks.protection.outlook.com/?url=https%3A%2F%2Fapps.apple.com%2Fapp%2Fapple-store%2Fid1477376905%3Fct%3Dnotification-email%26mt%3D8%26pt%3D524675&data=04%7C01%7Cjmacey%40bournemouth.ac.uk%7C39ceb932badf4437ee0308d974407b99%7Cede29655d09742e4bbb5f38d427fbfb8%7C0%7C0%7C637668642566989284%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=6Y0U%2FrzMdCUfyBEOKbt8HvQkaBJW%2FSq6OPTEiJdtEEY%3D&reserved=0 or Androidhttps://eur02.safelinks.protection.outlook.com/?url=https%3A%2F%2Fplay.google.com%2Fstore%2Fapps%2Fdetails%3Fid%3Dcom.github.android%26referrer%3Dutm_campaign%253Dnotification-email%2526utm_medium%253Demail%2526utm_source%253Dgithub&data=04%7C01%7Cjmacey%40bournemouth.ac.uk%7C39ceb932badf4437ee0308d974407b99%7Cede29655d09742e4bbb5f38d427fbfb8%7C0%7C0%7C637668642566989284%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=My5RskdGCSbyKMh3X%2By9Ze2oeMhZo6I9JL%2Bnx1DFptw%3D&reserved=0.


BU is a Disability Confident Employer and has signed up to the Mindful Employer charter. Information about the accessibility of University buildings can be found on the BU AccessAble webpages. This email is intended only for the person to whom it is addressed and may contain confidential information. If you have received this email in error, please notify the sender and delete this email, which must not be copied, distributed or disclosed to any other person. Any views or opinions presented are solely those of the author and do not necessarily represent those of Bournemouth University or its subsidiary companies. Nor can any contract be formed on behalf of the University or its subsidiary companies via email.

JackBoosY commented 2 years ago

@jmacey Did you use the X_VCPKG_APPLOCAL_DEPS_INSTALL feature?

jmacey commented 2 years ago

No and have never had to use this before for any other library.

Jon

On 13 Sep 2021, at 03:05, Jack·Boos·Yu @.***> wrote:



@jmaceyhttps://eur02.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fjmacey&data=04%7C01%7Cjmacey%40bournemouth.ac.uk%7C1da45d5189ab414e822608d9765afbfc%7Cede29655d09742e4bbb5f38d427fbfb8%7C0%7C0%7C637670955416181616%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=bZ54d9wISytAzTmdcyBQ%2BtsfrF5Qls6GCD45clgCqKk%3D&reserved=0 Did you use the X_VCPKG_APPLOCAL_DEPS_INSTALL feature?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://eur02.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fmicrosoft%2Fvcpkg%2Fpull%2F20092%23issuecomment-917777193&data=04%7C01%7Cjmacey%40bournemouth.ac.uk%7C1da45d5189ab414e822608d9765afbfc%7Cede29655d09742e4bbb5f38d427fbfb8%7C0%7C0%7C637670955416191605%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=htoNLYsejN1YYTXeBLfhNjOk9OUrgy3o5xsUUonTFqo%3D&reserved=0, or unsubscribehttps://eur02.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FAACK4GY7KJ6EKGZ5MZM5OL3UBVL7FANCNFSM5DY3OAKQ&data=04%7C01%7Cjmacey%40bournemouth.ac.uk%7C1da45d5189ab414e822608d9765afbfc%7Cede29655d09742e4bbb5f38d427fbfb8%7C0%7C0%7C637670955416191605%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=HbM%2FzfqAluIrbqObH2Wl81Ho0UXBB%2BvHa3mv%2BzDXWLg%3D&reserved=0. Triage notifications on the go with GitHub Mobile for iOShttps://eur02.safelinks.protection.outlook.com/?url=https%3A%2F%2Fapps.apple.com%2Fapp%2Fapple-store%2Fid1477376905%3Fct%3Dnotification-email%26mt%3D8%26pt%3D524675&data=04%7C01%7Cjmacey%40bournemouth.ac.uk%7C1da45d5189ab414e822608d9765afbfc%7Cede29655d09742e4bbb5f38d427fbfb8%7C0%7C0%7C637670955416191605%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=n%2FTvFlBT8D6B65R%2FRxEl7wyzlgeI4BjlXaUhT2SXF%2F8%3D&reserved=0 or Androidhttps://eur02.safelinks.protection.outlook.com/?url=https%3A%2F%2Fplay.google.com%2Fstore%2Fapps%2Fdetails%3Fid%3Dcom.github.android%26referrer%3Dutm_campaign%253Dnotification-email%2526utm_medium%253Demail%2526utm_source%253Dgithub&data=04%7C01%7Cjmacey%40bournemouth.ac.uk%7C1da45d5189ab414e822608d9765afbfc%7Cede29655d09742e4bbb5f38d427fbfb8%7C0%7C0%7C637670955416201602%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=MwFz9yI3WnuRDmVse%2BZHZbkI%2FVtgK6S%2FU2zPQy7pswA%3D&reserved=0.


BU is a Disability Confident Employer and has signed up to the Mindful Employer charter. Information about the accessibility of University buildings can be found on the BU AccessAble webpages. This email is intended only for the person to whom it is addressed and may contain confidential information. If you have received this email in error, please notify the sender and delete this email, which must not be copied, distributed or disclosed to any other person. Any views or opinions presented are solely those of the author and do not necessarily represent those of Bournemouth University or its subsidiary companies. Nor can any contract be formed on behalf of the University or its subsidiary companies via email.

JackBoosY commented 2 years ago

I confirmed that squish.dll was copied into tools/openimageio:

PS F:\vcpkg\packages\openimageio_x86-windows\tools\openimageio> ls

    Directory: F:\vcpkg\packages\openimageio_x86-windows\tools\openimageio

Mode                 LastWriteTime         Length Name
----                 -------------         ------ ----
d-----         9/12/2021  10:56 PM                plugins
-a----         7/19/2021   6:49 PM          94208 boost_filesystem-vc141-mt-x32-1_75.dll
-a----         7/19/2021   6:52 PM          66048 boost_thread-vc141-mt-x32-1_75.dll
-a----        10/19/2020  12:38 AM         135680 brotlicommon.dll
-a----        10/19/2020  12:38 AM          41984 brotlidec.dll
-a----         1/15/2021   1:05 AM          52736 bz2.dll
-a----          2/3/2021  11:18 PM         526848 freetype.dll
-a----          1/6/2021   1:00 AM         274432 Half-2_5.dll
-a----         3/12/2021   8:09 AM         832000 harfbuzz.dll
-a----         8/15/2021   8:15 PM         339968 heif.dll
-a----         9/12/2021  10:55 PM          93696 iconvert.exe
-a----         1/17/2021   6:50 PM       28398592 icudt67.dll
-a----         1/17/2021   6:50 PM        2095104 icuin67.dll
-a----         1/17/2021   6:50 PM        1377792 icuuc67.dll
-a----         9/12/2021  10:55 PM          41984 idiff.exe
-a----          1/6/2021   1:00 AM         181760 Iex-2_5.dll
-a----         9/12/2021  10:55 PM          53760 igrep.exe
-a----         9/12/2021  10:55 PM         119296 iinfo.exe
-a----          1/6/2021   1:00 AM        2668544 IlmImf-2_5.dll
-a----          1/6/2021   1:00 AM          35840 IlmThread-2_5.dll
-a----          1/6/2021   1:00 AM          84992 Imath-2_5.dll
-a----         9/12/2021  10:55 PM         238592 iv.exe
-a----        10/14/2020  10:54 PM         528896 jpeg62.dll
-a----          1/3/2021   9:56 PM         417792 libde265.dll
-a----          2/6/2021   6:42 PM         162304 libpng16.dll
-a----         5/31/2021   3:01 AM        2905600 libx265.dll
-a----         2/18/2021   1:49 AM         132608 lzma.dll
-a----         9/12/2021  10:55 PM          94208 maketx.exe
-a----         9/12/2021  10:55 PM         564736 oiiotool.exe
-a----         9/12/2021  10:55 PM        6078464 OpenImageIO.dll
-a----         9/12/2021  10:54 PM         581120 OpenImageIO_Util.dll
-a----        10/19/2020   1:02 AM         409088 pcre2-16.dll
-a----         9/12/2021  10:56 PM              9 qt.conf
-a----         8/15/2021   8:42 PM        4476416 Qt5Core.dll
-a----         8/15/2021   8:45 PM        5555712 Qt5Gui.dll
-a----         8/15/2021   8:47 PM        4606976 Qt5Widgets.dll
-a----          9/1/2021  11:05 PM          41472 squish.dll
-a----         2/18/2021   1:50 AM         380928 tiff.dll
-a----        10/14/2020  10:55 PM          73216 zlib1.dll
-a----        12/23/2020   1:38 AM         460800 zstd.dll
jmacey commented 2 years ago

The dll is not copied to my target directory when building a project against openimageio. It is created in the vcpkg tree.

Jon

On 13 Sep 2021, at 07:06, Jack·Boos·Yu @.***> wrote:



I confirmed that squish.dll was copied into tools/openimageio:

PS F:\vcpkg\packages\openimageio_x86-windows\tools\openimageio> ls

Directory: F:\vcpkg\packages\openimageio_x86-windows\tools\openimageio

Mode LastWriteTime Length Name


d----- 9/12/2021 10:56 PM plugins -a---- 7/19/2021 6:49 PM 94208 boost_filesystem-vc141-mt-x32-1_75.dll -a---- 7/19/2021 6:52 PM 66048 boost_thread-vc141-mt-x32-1_75.dll -a---- 10/19/2020 12:38 AM 135680 brotlicommon.dll -a---- 10/19/2020 12:38 AM 41984 brotlidec.dll -a---- 1/15/2021 1:05 AM 52736 bz2.dll -a---- 2/3/2021 11:18 PM 526848 freetype.dll -a---- 1/6/2021 1:00 AM 274432 Half-2_5.dll -a---- 3/12/2021 8:09 AM 832000 harfbuzz.dll -a---- 8/15/2021 8:15 PM 339968 heif.dll -a---- 9/12/2021 10:55 PM 93696 iconvert.exe -a---- 1/17/2021 6:50 PM 28398592 icudt67.dll -a---- 1/17/2021 6:50 PM 2095104 icuin67.dll -a---- 1/17/2021 6:50 PM 1377792 icuuc67.dll -a---- 9/12/2021 10:55 PM 41984 idiff.exe -a---- 1/6/2021 1:00 AM 181760 Iex-2_5.dll -a---- 9/12/2021 10:55 PM 53760 igrep.exe -a---- 9/12/2021 10:55 PM 119296 iinfo.exe -a---- 1/6/2021 1:00 AM 2668544 IlmImf-2_5.dll -a---- 1/6/2021 1:00 AM 35840 IlmThread-2_5.dll -a---- 1/6/2021 1:00 AM 84992 Imath-2_5.dll -a---- 9/12/2021 10:55 PM 238592 iv.exe -a---- 10/14/2020 10:54 PM 528896 jpeg62.dll -a---- 1/3/2021 9:56 PM 417792 libde265.dll -a---- 2/6/2021 6:42 PM 162304 libpng16.dll -a---- 5/31/2021 3:01 AM 2905600 libx265.dll -a---- 2/18/2021 1:49 AM 132608 lzma.dll -a---- 9/12/2021 10:55 PM 94208 maketx.exe -a---- 9/12/2021 10:55 PM 564736 oiiotool.exe -a---- 9/12/2021 10:55 PM 6078464 OpenImageIO.dll -a---- 9/12/2021 10:54 PM 581120 OpenImageIO_Util.dll -a---- 10/19/2020 1:02 AM 409088 pcre2-16.dll -a---- 9/12/2021 10:56 PM 9 qt.conf -a---- 8/15/2021 8:42 PM 4476416 Qt5Core.dll -a---- 8/15/2021 8:45 PM 5555712 Qt5Gui.dll -a---- 8/15/2021 8:47 PM 4606976 Qt5Widgets.dll -a---- 9/1/2021 11:05 PM 41472 squish.dll -a---- 2/18/2021 1:50 AM 380928 tiff.dll -a---- 10/14/2020 10:55 PM 73216 zlib1.dll -a---- 12/23/2020 1:38 AM 460800 zstd.dll

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://eur02.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fmicrosoft%2Fvcpkg%2Fpull%2F20092%23issuecomment-917869179&data=04%7C01%7Cjmacey%40bournemouth.ac.uk%7Cafef699a2a8b49cb270c08d9767c9308%7Cede29655d09742e4bbb5f38d427fbfb8%7C0%7C0%7C637671099674711497%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=%2F9%2FenLaroL15%2B%2BIvPNiSYYlwnqSt8rXC2Vp8qOgnh%2Bw%3D&reserved=0, or unsubscribehttps://eur02.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FAACK4G2TCBYRB5WUX7TRSHTUBWIE3ANCNFSM5DY3OAKQ&data=04%7C01%7Cjmacey%40bournemouth.ac.uk%7Cafef699a2a8b49cb270c08d9767c9308%7Cede29655d09742e4bbb5f38d427fbfb8%7C0%7C0%7C637671099674711497%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=y%2BjMgaapyEqknFIdXXwh68PtQh5VzicWa84UILDu8qA%3D&reserved=0. Triage notifications on the go with GitHub Mobile for iOShttps://eur02.safelinks.protection.outlook.com/?url=https%3A%2F%2Fapps.apple.com%2Fapp%2Fapple-store%2Fid1477376905%3Fct%3Dnotification-email%26mt%3D8%26pt%3D524675&data=04%7C01%7Cjmacey%40bournemouth.ac.uk%7Cafef699a2a8b49cb270c08d9767c9308%7Cede29655d09742e4bbb5f38d427fbfb8%7C0%7C0%7C637671099674721490%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=GHvOE1V8Xcs2Suykgh0k0V5S7KOOivEtzYO44cnbO68%3D&reserved=0 or Androidhttps://eur02.safelinks.protection.outlook.com/?url=https%3A%2F%2Fplay.google.com%2Fstore%2Fapps%2Fdetails%3Fid%3Dcom.github.android%26referrer%3Dutm_campaign%253Dnotification-email%2526utm_medium%253Demail%2526utm_source%253Dgithub&data=04%7C01%7Cjmacey%40bournemouth.ac.uk%7Cafef699a2a8b49cb270c08d9767c9308%7Cede29655d09742e4bbb5f38d427fbfb8%7C0%7C0%7C637671099674721490%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=XdijJ2%2B%2FuRz2hWGrzQIMvTwD2gHwm1OfK6hZh3HAe4I%3D&reserved=0.


BU is a Disability Confident Employer and has signed up to the Mindful Employer charter. Information about the accessibility of University buildings can be found on the BU AccessAble webpages. This email is intended only for the person to whom it is addressed and may contain confidential information. If you have received this email in error, please notify the sender and delete this email, which must not be copied, distributed or disclosed to any other person. Any views or opinions presented are solely those of the author and do not necessarily represent those of Bournemouth University or its subsidiary companies. Nor can any contract be formed on behalf of the University or its subsidiary companies via email.

JackBoosY commented 2 years ago

After reading the source code, oiio requires squish, but it wasn't exported in the oiio cmake config files. That cause the usuage issue. I think that's a oiio bug, @lgritz, could you please help comfirm this?

All squish related codes here: In src/dds.imageio/CMakeLists.txt:

if (Libsquish_FOUND)
    # External libsquish was found -- use it
    add_oiio_plugin (ddsinput.cpp
                     LINK_LIBRARIES Libsquish::Libsquish
                     )
...

In _src/cmake/add_oiioplugins.cmake:

macro (add_oiio_plugin)
    cmake_parse_arguments (_plugin "" "NAME" "SRC;INCLUDE_DIRS;LINK_LIBRARIES;DEFINITIONS" ${ARGN})
...
            set (format_plugin_libs ${format_plugin_libs} ${_plugin_LINK_LIBRARIES} PARENT_SCOPE)
...

In src/libOpenImageIO/CMakeLists.txt:

...
target_link_libraries (OpenImageIO
...
        PRIVATE
...
            ${format_plugin_libs} # Add all the target link libraries from the plugins
...
jmacey commented 2 years ago

It could be, I can’t remember squish being a dependancy in the older version.

On 13 Sep 2021, at 08:53, Jack·Boos·Yu @.**@.>> wrote:

After reading the source code, I think that's a oiio bug. In src/dds.imageio/CMakeLists.txt:

if (Libsquish_FOUND)

External libsquish was found -- use it

add_oiio_plugin (ddsinput.cpp
                 LINK_LIBRARIES Libsquish::Libsquish
                 )

...

In src/cmake/add_oiio_plugins.cmake:

macro (add_oiio_plugin) cmake_parse_arguments (_plugin "" "NAME" "SRC;INCLUDE_DIRS;LINK_LIBRARIES;DEFINITIONS" ${ARGN}) ... set (format_plugin_libs ${format_plugin_libs} ${_plugin_LINK_LIBRARIES} PARENT_SCOPE) ...

In src/libOpenImageIO/CMakeLists.txt:

... target_link_libraries (OpenImageIO ... PRIVATE ... ${format_plugin_libs} # Add all the target link libraries from the plugins ...

squish wasn't exported in the cmake config files but oiio requires it.

@lgritzhttps://eur02.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Flgritz&data=04%7C01%7Cjmacey%40bournemouth.ac.uk%7C3473a9a22e1949dec7f508d9768ba10a%7Cede29655d09742e4bbb5f38d427fbfb8%7C0%7C0%7C637671164334549418%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=tluKPL2Cmg4340ja4ZtPfQkk6QNHDfMZPk0fdZytgOA%3D&reserved=0 Can you please take a look?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://eur02.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fmicrosoft%2Fvcpkg%2Fpull%2F20092%23issuecomment-917933965&data=04%7C01%7Cjmacey%40bournemouth.ac.uk%7C3473a9a22e1949dec7f508d9768ba10a%7Cede29655d09742e4bbb5f38d427fbfb8%7C0%7C0%7C637671164334559412%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=DLBXVsYav53lxZNGTQCW0CJwE3Qjljb6RCeIYPMO6wU%3D&reserved=0, or unsubscribehttps://eur02.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FAACK4G4LQL2RYKQUCXK35QLUBWUY5ANCNFSM5DY3OAKQ&data=04%7C01%7Cjmacey%40bournemouth.ac.uk%7C3473a9a22e1949dec7f508d9768ba10a%7Cede29655d09742e4bbb5f38d427fbfb8%7C0%7C0%7C637671164334559412%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=V1ATvJolFdyN9SZHo0Eo0al1V9gMmufAwreLOIfHXaA%3D&reserved=0. Triage notifications on the go with GitHub Mobile for iOShttps://eur02.safelinks.protection.outlook.com/?url=https%3A%2F%2Fapps.apple.com%2Fapp%2Fapple-store%2Fid1477376905%3Fct%3Dnotification-email%26mt%3D8%26pt%3D524675&data=04%7C01%7Cjmacey%40bournemouth.ac.uk%7C3473a9a22e1949dec7f508d9768ba10a%7Cede29655d09742e4bbb5f38d427fbfb8%7C0%7C0%7C637671164334559412%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=uF%2FBOH8OqthvQ%2FZLcoRa4c%2F0wNiOzoOI3sgZWyT4x9E%3D&reserved=0 or Androidhttps://eur02.safelinks.protection.outlook.com/?url=https%3A%2F%2Fplay.google.com%2Fstore%2Fapps%2Fdetails%3Fid%3Dcom.github.android%26referrer%3Dutm_campaign%253Dnotification-email%2526utm_medium%253Demail%2526utm_source%253Dgithub&data=04%7C01%7Cjmacey%40bournemouth.ac.uk%7C3473a9a22e1949dec7f508d9768ba10a%7Cede29655d09742e4bbb5f38d427fbfb8%7C0%7C0%7C637671164334569405%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=Ecwc54yM6ZGn2c7Rx7%2F9DHzCepQ7G%2Fpc2hw%2FBPW1IuU%3D&reserved=0.


BU is a Disability Confident Employer and has signed up to the Mindful Employer charter. Information about the accessibility of University buildings can be found on the BU AccessAble webpages. This email is intended only for the person to whom it is addressed and may contain confidential information. If you have received this email in error, please notify the sender and delete this email, which must not be copied, distributed or disclosed to any other person. Any views or opinions presented are solely those of the author and do not necessarily represent those of Bournemouth University or its subsidiary companies. Nor can any contract be formed on behalf of the University or its subsidiary companies via email.

BillyONeal commented 2 years ago

It seems like this port is trying to vendor libsquish when it should depend on the libsquish port instead?

JackBoosY commented 2 years ago

@BillyONeal No, oiio just use the internal FindLibsquish.cmake and oiio depends on libsquish.

BillyONeal commented 2 years ago

@BillyONeal No, oiio just use the internal FindLibsquish.cmake and oiio depends on libsquish.

If it wants to make squish.dll (as reported by @jmacey ) that doesn't sound like an internal-only use. (I know this isn't the bug you're necessarily trying to fix in this PR but ideally we would de-vendor that dependency)

jmacey commented 2 years ago

The way I see it is that squish is a dependency as is boost OpenEXR etc so should be copied. For example if you build the core oiio you don’t get the libraw libraries but if you build with the raw option you do. I guess squish is now core because of one of the default compressed formats so needs to be copied.

Sent from my iPad

On 16 Sep 2021, at 05:25, BillyONeal @.***> wrote:



@BillyONealhttps://eur02.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2FBillyONeal&data=04%7C01%7Cjmacey%40bournemouth.ac.uk%7C76607dc9457f437828b508d978c9f19c%7Cede29655d09742e4bbb5f38d427fbfb8%7C0%7C0%7C637673630999304320%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=gcQBjrXoZCZcOkI6eZxBAdFa3rKbbmlFiukdeOOKFys%3D&reserved=0 No, oiio just use the internal FindLibsquish.cmake and oiio depends on libsquish.

If it wants to make squish.dll (as reported by @jmaceyhttps://eur02.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fjmacey&data=04%7C01%7Cjmacey%40bournemouth.ac.uk%7C76607dc9457f437828b508d978c9f19c%7Cede29655d09742e4bbb5f38d427fbfb8%7C0%7C0%7C637673630999304320%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=RiMLrmKB7WeF%2B9%2F%2FOWsF%2BbyyoeXVwvFBLEreHpGp%2BKc%3D&reserved=0 ) that doesn't sound like an internal-only use. (I know this isn't the bug you're necessarily trying to fix in this PR but ideally we would de-vendor that dependency)

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://eur02.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fmicrosoft%2Fvcpkg%2Fpull%2F20092%23issuecomment-920564872&data=04%7C01%7Cjmacey%40bournemouth.ac.uk%7C76607dc9457f437828b508d978c9f19c%7Cede29655d09742e4bbb5f38d427fbfb8%7C0%7C0%7C637673630999314315%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=JYMtQSSFsN042aX%2BqVhkYeIuoWfjKKk4kPLU1dgJbuY%3D&reserved=0, or unsubscribehttps://eur02.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FAACK4GZ4TFYCY6VMZHMXS33UCFWRTANCNFSM5DY3OAKQ&data=04%7C01%7Cjmacey%40bournemouth.ac.uk%7C76607dc9457f437828b508d978c9f19c%7Cede29655d09742e4bbb5f38d427fbfb8%7C0%7C0%7C637673630999314315%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=Zm13KVJvCZXOSnKySJHsgKeI6oCs5UDLCku5bZjrZXA%3D&reserved=0. Triage notifications on the go with GitHub Mobile for iOShttps://eur02.safelinks.protection.outlook.com/?url=https%3A%2F%2Fapps.apple.com%2Fapp%2Fapple-store%2Fid1477376905%3Fct%3Dnotification-email%26mt%3D8%26pt%3D524675&data=04%7C01%7Cjmacey%40bournemouth.ac.uk%7C76607dc9457f437828b508d978c9f19c%7Cede29655d09742e4bbb5f38d427fbfb8%7C0%7C0%7C637673630999324306%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=rsXojI2DttJRhq7ki%2FLrIHKY25AiZd6uh5%2Fd4jDhmfw%3D&reserved=0 or Androidhttps://eur02.safelinks.protection.outlook.com/?url=https%3A%2F%2Fplay.google.com%2Fstore%2Fapps%2Fdetails%3Fid%3Dcom.github.android%26referrer%3Dutm_campaign%253Dnotification-email%2526utm_medium%253Demail%2526utm_source%253Dgithub&data=04%7C01%7Cjmacey%40bournemouth.ac.uk%7C76607dc9457f437828b508d978c9f19c%7Cede29655d09742e4bbb5f38d427fbfb8%7C0%7C0%7C637673630999324306%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=mHYEYQVKGpoWJTYGOLLQvnSzpgLh3V2TxA2XqkwXllM%3D&reserved=0.


BU is a Disability Confident Employer and has signed up to the Mindful Employer charter. Information about the accessibility of University buildings can be found on the BU AccessAble webpages. This email is intended only for the person to whom it is addressed and may contain confidential information. If you have received this email in error, please notify the sender and delete this email, which must not be copied, distributed or disclosed to any other person. Any views or opinions presented are solely those of the author and do not necessarily represent those of Bournemouth University or its subsidiary companies. Nor can any contract be formed on behalf of the University or its subsidiary companies via email.

JackBoosY commented 2 years ago

@BillyONeal No, oiio just use the internal FindLibsquish.cmake and oiio depends on libsquish.

If it wants to make squish.dll (as reported by @jmacey ) that doesn't sound like an internal-only use. (I know this isn't the bug you're necessarily trying to fix in this PR but ideally we would de-vendor that dependency)

No, it won't, I confirmed that.

BillyONeal commented 2 years ago

@JackBoosY

I see, there was some confusion here. I was under the impression that I said "this vendors libsquish" and you said, effectively, "it vendors a custom version of libsquish that it only uses internally". But what you actually said was "it does depend on the libsquish port" and Bill was blind.

It still seems like the targets exported by OpenImageIoConfig.cmake should be finding libsquish on their own rather than forcing customers to add Libsquish::Libsquish ?

BillyONeal commented 2 years ago

To clarify for other reviewers, the situation is:

The usage text ends up being:

The package openimageio:x86-windows provides CMake targets:

find_package(Modules CONFIG REQUIRED) target_link_libraries(main PRIVATE OpenImageIO::OpenImageIO)

find_package(OpenImageIO CONFIG REQUIRED) target_link_libraries(main PRIVATE Libsquish::Libsquish OpenImageIO::OpenImageIO OpenImageIO::OpenImageIO_Util)

BillyONeal commented 2 years ago
The package openimageio:x86-windows provides CMake targets:

find_package(Modules CONFIG REQUIRED)
target_link_libraries(main PRIVATE OpenImageIO::OpenImageIO)

This does not work:

C:\Users\bion\Desktop>C:\Dev\vcpkg\vcpkg.exe list | findstr openimageio
openimageio:x86-windows                            2.3.7.2#2        A library for reading and writing images, and a ...

C:\Users\bion\Desktop>type CMakeLists.txt
cmake_minimum_required(VERSION 3.21)
project(demo CXX)

add_executable(main main.cpp)

find_package(Modules CONFIG REQUIRED)
target_link_libraries(main PRIVATE OpenImageIO::OpenImageIO)

C:\Users\bion\Desktop>type main.cpp
#include <stdio.h>

int main() {
    puts("Hello world.");
    return 0;
}

C:\Users\bion\Desktop>cmake -G Ninja -DCMAKE_BUILD_TYPE=Debug -DCMAKE_TOOLCHAIN_FILE=C:/Dev/vcpkg/scripts/buildsystems/vcpkg.cmake -S . -B out
-- The CXX compiler identification is MSVC 19.29.30133.0
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Check for working CXX compiler: C:/Program Files (x86)/Microsoft Visual Studio/2019/Enterprise/VC/Tools/MSVC/14.29.30133/bin/Hostx86/x86/cl.exe - skipped
-- Detecting CXX compile features
-- Detecting CXX compile features - done
CMake Error at C:/Dev/vcpkg/scripts/buildsystems/vcpkg.cmake:784 (_find_package):
  Could not find a package configuration file provided by "Modules" with any
  of the following names:

    ModulesConfig.cmake
    modules-config.cmake

  Add the installation prefix of "Modules" to CMAKE_PREFIX_PATH or set
  "Modules_DIR" to a directory containing one of the above files.  If
  "Modules" provides a separate development package or SDK, be sure it has
  been installed.
Call Stack (most recent call first):
  CMakeLists.txt:6 (find_package)

-- Configuring incomplete, errors occurred!
See also "C:/Users/bion/Desktop/out/CMakeFiles/CMakeOutput.log".

C:\Users\bion\Desktop>
BillyONeal commented 2 years ago

I merged this anyway because I observe that the problems I'm reporting here aren't problems caused by this PR.

JackBoosY commented 2 years ago

@BillyONeal That's because vcpkg wrongly analysis the installed cmake files, and not related to this PR.

lgritz commented 2 years ago

Hi, I'm the main author of OpenImageIO.

Libsquish is an optional dependency. If it's found externally by OIIO's FindLibsquish, that library will be used.

If Libsquish is NOT found externally, it will use embedded source code rather than the library.

In neither case does OIIO produce a libsquish library.

If it's against vcpkg's policy to for one package to embed the code of another package that's also in vcpkg, then all you need to do is (from vcpkg's perspective) make the vcpkg libsquish be a necessary package dependency for OpenImgeIO, in which case, the library will be used and the internally embedded version of libsquish will be ignored entirely.

Is there something I'm doing wrong that I can fix?

dg0yt commented 2 years ago

On wrong usage information, cf. https://github.com/microsoft/vcpkg/issues/20190. I

jmacey commented 2 years ago

Hi Larry,

This issue is quite new, I used OpenImage IO all of last academic year (as students were using home machines vcpkg made it so much easier to install), this issue has cropped up in the last 3 months or so. I'm guessing a flag is missing to tell it to use Squish rather than the internal one as you say.

The issue only really manifests on Windows as the DLL needs to be copied to the executable target directory where as in Mac and Linux they are linked statically by default.

lgritz commented 2 years ago

I apologize, as I do not use Windows myself, so sometimes things that break under Windows can go unnoticed (though our GitHub Actions CI does build and test on Windows, and seem to work) and I'm definitely not at all knowledgeable about how the DLLs need to be placed on Windows.

From what I understand, if vcpkg already builds libsquish and OIIO's FindLibsquish is able to find it (maybe setting $Libsquish_ROOT can help if it's not being found), that should all work, I think. Additionally, if it's not found, the libsquish.dll shouldn't come into play at all, since OIIO will use the fully embedded version.

In any case, if there's a change I can make on the OIIO side that will help this process, please let me know (or, better, submit a PR to OpenImageIO).

JackBoosY commented 2 years ago

@lgritz Another issue please see my comment.

lgritz commented 2 years ago

@JackBoosY this may just reflect my Windows ignorance, but in the case where the external libsquish is found, doesn't libOpenImageIO link against it? Doesn't that automatically pull it in when a downstream app links against libOpenImageIO?

Are you saying I need to additionally mention it somehow in the OpenImageIOConfig? Why is that not necessary for any of the other libraries we use?

JackBoosY commented 2 years ago

@lgritz According to the official document https://cmake.org/cmake/help/latest/command/target_link_libraries.html#id4 and https://cmake.org/cmake/help/latest/prop_tgt/INTERFACE_LINK_LIBRARIES.html, all dependencies that must be linked in use should be declared as PUBLIC to use export function to export them to INTERFACE_LINK_LIBRARIES.

jmacey commented 2 years ago

I can't see a new PR can you send a link please.


From: Jack·Boos·Yu @.> Sent: 10 September 2021 09:42 To: microsoft/vcpkg @.> Cc: Jon MacEy @.>; Mention @.> Subject: Re: [microsoft/vcpkg] [openimageio] Re-fix the usage (#20092)

@jmaceyhttps://eur02.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fjmacey&data=04%7C01%7Cjmacey%40bournemouth.ac.uk%7C1c3a0287de3b47223a8e08d97436f3ee%7Cede29655d09742e4bbb5f38d427fbfb8%7C0%7C0%7C637668601627536230%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=lMbrhF%2F3nhIiwHV62Mhdosa%2Fs53m5iS8SzZm0AL2V50%3D&reserved=0 Can you please test this PR?

Thanks.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://eur02.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fmicrosoft%2Fvcpkg%2Fpull%2F20092%23issuecomment-916737542&data=04%7C01%7Cjmacey%40bournemouth.ac.uk%7C1c3a0287de3b47223a8e08d97436f3ee%7Cede29655d09742e4bbb5f38d427fbfb8%7C0%7C0%7C637668601627536230%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=h3PnUpEoRIlmoZAU3r7UQ0bj%2BOc44Gsu719uq85T3AQ%3D&reserved=0, or unsubscribehttps://eur02.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FAACK4G6CSRJ6MKRZZ4N5FTDUBHAIBANCNFSM5DY3OAKQ&data=04%7C01%7Cjmacey%40bournemouth.ac.uk%7C1c3a0287de3b47223a8e08d97436f3ee%7Cede29655d09742e4bbb5f38d427fbfb8%7C0%7C0%7C637668601627546226%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=gfIj7oDfi1d6nBxf5dvOSVTibx2vXJbVAkjPaeSWh6Y%3D&reserved=0. Triage notifications on the go with GitHub Mobile for iOShttps://eur02.safelinks.protection.outlook.com/?url=https%3A%2F%2Fapps.apple.com%2Fapp%2Fapple-store%2Fid1477376905%3Fct%3Dnotification-email%26mt%3D8%26pt%3D524675&data=04%7C01%7Cjmacey%40bournemouth.ac.uk%7C1c3a0287de3b47223a8e08d97436f3ee%7Cede29655d09742e4bbb5f38d427fbfb8%7C0%7C0%7C637668601627546226%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=Llq68Bkv5N%2FmPPxaT9cl8BHMME02hfiFWgObpUDh%2FQo%3D&reserved=0 or Androidhttps://eur02.safelinks.protection.outlook.com/?url=https%3A%2F%2Fplay.google.com%2Fstore%2Fapps%2Fdetails%3Fid%3Dcom.github.android%26referrer%3Dutm_campaign%253Dnotification-email%2526utm_medium%253Demail%2526utm_source%253Dgithub&data=04%7C01%7Cjmacey%40bournemouth.ac.uk%7C1c3a0287de3b47223a8e08d97436f3ee%7Cede29655d09742e4bbb5f38d427fbfb8%7C0%7C0%7C637668601627556219%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=lbLvSqOYqnsT6j7nqpU4Zv%2BfaUeSkHpiATeyyVr3hH8%3D&reserved=0.


BU is a Disability Confident Employer and has signed up to the Mindful Employer charter. Information about the accessibility of University buildings can be found on the BU AccessAble webpages. This email is intended only for the person to whom it is addressed and may contain confidential information. If you have received this email in error, please notify the sender and delete this email, which must not be copied, distributed or disclosed to any other person. Any views or opinions presented are solely those of the author and do not necessarily represent those of Bournemouth University or its subsidiary companies. Nor can any contract be formed on behalf of the University or its subsidiary companies via email.

lgritz commented 2 years ago

@JackBoosY But it's not part of OIIO's public interface. OIIO internals call into libsquish, but downstream apps that use OIIO's APIs don't need to call libsquish APIs. Is that not the meaning of PRIVATE here? If I've misunderstood, then what would be an example of where PRIVATE would be the right designation?

BillyONeal commented 2 years ago

@lgritz Even if you only use it internally, it needs to be on your customers' link lines. If you make it PUBLIC, cmake will add those bits to customers' link lines transitively.

lgritz commented 2 years ago

@JackBoosY I modified OIIO's CI so that the Windows tests for sure use vcpkg's libsquish (it had previously just used the embedded version), and it seems to work. What's different?

@BillyONeal OK, I can accept that I've misunderstood these modifiers (though they sure have worked for me for a long time). But then, can you explain to me what is an example of when PRIVATE would be appropriate? What's it for, if I have to use PUBLIC for anything that needs to be available to downstream clients at link time?

Is this Windows specific? Because on Linux and OSX, when libOpenImageIO.so links to libsquish.so, a downstream program only needs to link against libOpenImageIO.so. I only need to mark as PUBLIC if the downstream app might directly call into libsquish (i.e., if libsquish types or APIs are part of OIIO's public API).

jmacey commented 2 years ago

For me this is a specific vcpkg / windows thing. If I build anything against olio the program builds but will not run as squish.dll is not copied into the target directory. If I copy the squish.dll into the directory I will work.

This is quite a new thing (last 3-4 months as I don’t update vcpkg that often), It first manifested with the spurious AND in the original error report which has now been fixed. Now I get the squish problem which is not a build / link error but a runtime one on Windows only.

Sorry can’t be specific as to when this error started to occur but I know for certain all was fine in my 2nd Semester so this was around Feb until June when the students submitted their work. Think this points to a bigger issue with vcpkg versions and ports but I guess that discussion is for another day.

Also building for source doesn’t cause to many issues either, however I can’t expect all UG students to do this hence using vcpkg for ease :-)

On 17 Sep 2021, at 17:27, Larry Gritz @.**@.>> wrote:

@BillyONealhttps://eur02.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2FBillyONeal&data=04%7C01%7Cjmacey%40bournemouth.ac.uk%7C893cc96beb314ee61e0508d979f811b7%7Cede29655d09742e4bbb5f38d427fbfb8%7C0%7C0%7C637674928614551188%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=2WSVVBK62oNsWuDqgSFHXyCbCFnSe6ERackrXfknnS8%3D&reserved=0 OK, I can accept that I've misunderstood these modifiers (though they sure have worked for me for a long time). But then, can you explain to me what is an example of when PRIVATE would be appropriate? What's it for, if I have to use PUBLIC for anything that needs to be available to downstream clients at link time?


BU is a Disability Confident Employer and has signed up to the Mindful Employer charter. Information about the accessibility of University buildings can be found on the BU AccessAble webpages. This email is intended only for the person to whom it is addressed and may contain confidential information. If you have received this email in error, please notify the sender and delete this email, which must not be copied, distributed or disclosed to any other person. Any views or opinions presented are solely those of the author and do not necessarily represent those of Bournemouth University or its subsidiary companies. Nor can any contract be formed on behalf of the University or its subsidiary companies via email.

BillyONeal commented 2 years ago

@BillyONeal OK, I can accept that I've misunderstood these modifiers (though they sure have worked for me for a long time). But then, can you explain to me what is an example of when PRIVATE would be appropriate? What's it for, if I have to use PUBLIC for anything that needs to be available to downstream clients at link time?

I'm not aware of great uses for PRIVATE libraries that work across platforms, but on Windows where each DLL is an island, if you know the result is a DLL, you don't transitively need customers to supply libraries on their link lines. I think there may be similar cases for *nix but I understand that less.

The more common example is to have private/internal headers that customers aren't supposed to have in their environment but that a project uses internally.

lgritz commented 2 years ago

On Unix/Linux, with dynamic libraries, if you have libA that depends on libB only for internals (doesn't expose B in A's public API), that dependency to B can be PRIVATE, and libA linking against libB is enough to ensure that when appC links against libA, it will automatically pull in libB. At least for shared libraries. For static, you may need to specify them, but my impression was that CMake handled the decisions about whether to add B explicitly to the linkage for A depending on whether it was static or dynamic.

Is that not possible in Windows?

So the general guidance is that I should pretty much always be using PUBLIC, regardless of whether a dependency is exposed through my APIs, in order to guarantee that the transitive link needs are met?

I probably have a lot of places to fix, then.

lgritz commented 2 years ago

Doing some more reading...

Generally, dynamic libraries encode their dependency information (what they were linked against when you linked the library itself). Static libraries do not. So what CMake is supposed to do is understand this, and for PRIVATE library dependencies, pass on the library interface if it's static but not dynamic.

If I understand correctly, marking everything as PUBLIC seems like overkill.

However, I can believe that what happens is that, if static libs are used for libsquish, although cmake will pass along the library dependency in our exported config, a downstream consumer still won't have the FindLibsquish.cmake, and so may not have a good way to find the library when it needs it for linking?

JackBoosY commented 2 years ago

@lgritz

F:\vcpkg\packages\openimageio_x86-windows\bin> dumpbin /DEPENDENTS .\OpenImageIO.dll
Microsoft (R) COFF/PE Dumper Version 14.16.27045.0
Copyright (C) Microsoft Corporation.  All rights reserved.

Dump of file .\OpenImageIO.dll

File Type: DLL

  Image has the following dependencies:

    OpenImageIO_Util.dll
    squish.dll
    heif.dll
    jpeg62.dll
    zlib1.dll
    libpng16.dll
    tiff.dll
    IlmImf-2_5.dll
    Imath-2_5.dll
    Half-2_5.dll
    Iex-2_5.dll
    boost_thread-vc141-mt-x32-1_75.dll
    KERNEL32.dll
    MSVCP140.dll
    WS2_32.dll
    VCRUNTIME140.dll
    api-ms-win-crt-runtime-l1-1-0.dll
    api-ms-win-crt-heap-l1-1-0.dll
    api-ms-win-crt-math-l1-1-0.dll
    api-ms-win-crt-stdio-l1-1-0.dll
    api-ms-win-crt-locale-l1-1-0.dll
    api-ms-win-crt-convert-l1-1-0.dll
    api-ms-win-crt-environment-l1-1-0.dll
    api-ms-win-crt-time-l1-1-0.dll
    api-ms-win-crt-string-l1-1-0.dll
    api-ms-win-crt-filesystem-l1-1-0.dll

  Summary

       4C000 .data
       8A000 .rdata
       2F000 .reloc
        1000 .rsrc
      4CA000 .text
        1000 _RDATA
lgritz commented 2 years ago

@JackBoosY so the PUBLIC/PRIVATE is ok the way it is, the problem was just that the debug OIIO build was selecting the release squish build?

That sounds like it's probably a bug in my FindLibsquish.cmake. Let me see if I can produce a fix for that.

lgritz commented 2 years ago

You know what? OIIO's CI tests Windows, but only release build. Adding a debug build to see if I can reproduce.

dg0yt commented 2 years ago

AFAIU multiple issues are discussed here.

  1. Windows, vcpkg issue: app-local deployment of squish.dll. Independent of exported cmake configuration. But possible related to build types/item 3, https://github.com/microsoft/vcpkg/issues/20233#issuecomment-921957884.
  2. All platforms, OIIO issue: Passing linking information for static linkage. Yes, this means PUBLIC in cmake, but Libs.private in pkg-config. And populating the link interface when consuming OIIO mea
  3. All platforms: OIIO/vcpkg/user integration: OIIO comes with a set of cmake find modules. These modules must be installed with OIIO, in order to populate the target link interface when consuming OIIO. The problem is that these modules are not prepared to deal with vcpkg debug+relase setup, in particular the use of different binary names (d debug suffix). For some dependencies, this is patched in vcpkg's port.
JackBoosY commented 2 years ago

@lgritz The result of dumpbin proves that if we want to use oiio.lib, you must add squish.lib to the link queue, which means that libsquish should be exported to the INTERFACE_LINK_LIBRARIES of openimageioTargets.cmake, which means that PUBLIC should be used instead of PRIVATE. Otherwise, we will meet the LNK2019 problem.

JackBoosY commented 2 years ago
  1. Windows, vcpkg issue: app-local deployment of squish.dll. Independent of exported cmake configuration. But possible related to build types/item 3, [OpenImageIO] [libsquish] Dependent squish library is not copied in debug mode #20233 (comment).

When users use libsquish through vcpkg, they should use Findlibsquish.cmake installed in share/OpenImageIO, which also solves this issue.

  1. All platforms, OIIO issue: Passing linking information for static linkage. Yes, this means PUBLIC in cmake, but Libs.private in pkg-config. And populating the link interface when consuming OIIO mea

Yes, agreed.

  1. All platforms: OIIO/vcpkg/user integration: OIIO comes with a set of cmake find modules. These modules must be installed with OIIO, in order to populate the target link interface when consuming OIIO. The problem is that these modules are not prepared to deal with vcpkg debug+relase setup, in particular the use of different binary names (d debug suffix). For some dependencies, this is patched in vcpkg's port.

As I said in 1, even if libsquish does not export its config.cmake/targets.cmake, we can use the installed Findlibsquish.cmake to find and use it.

dg0yt commented 2 years ago

As I said in 1, even if libsquish does not export its config.cmake/targets.cmake, we can use the installed Findlibsquish.cmake to find and use it.

Basically yes. But:

File list for libsquish, from microsoft.vcpkg.ci:

libsquish:/bin/squish.dll
libsquish:/debug/bin/squishd.dll
libsquish:/debug/lib/squishd.lib
libsquish:/include/squish.h
libsquish:/include/squish_export.h
libsquish:/lib/squish.lib
libsquish:/share/libsquish/copyright
libsquish:/share/libsquish/vcpkg_abi_info.txt
JackBoosY commented 2 years ago

@dg0yt Fine, I will make a PR to export the libsquish targets.

dg0yt commented 2 years ago

So what CMake is supposed to do is understand this, and for PRIVATE library dependencies, pass on the library interface if it's static but not dynamic.

If I understand correctly, marking everything as PUBLIC seems like overkill.

@lgritz After reading another PR, I would like to another insight: The effect of the PUBLIC and PRIVATE keywords is consistent over various target_... cmake commands. The purpose is less obvious for target_link_libraries (PRIVATE perhaps only useful for header-only libraries or file-less configuration sets) but it is quite obvious for target_compile_definitions:

if(BUILD_SHARED_LIBS)
    target_compile_options(lcms2 PRIVATE -DCMS_DLL_BUILD)
    target_compile_options(lcms2 PUBLIC -DCMS_DLL)
endif()
target_compile_options(lcms2 PRIVATE -DUNICODE -D_UNICODE)

CMake doesn't require any PUBLIC/PRIVATE/INTERFACE keywords for target_link_libraries, but everything is public then, and you must not mix the forms.

lgritz commented 2 years ago

I don't buy the "only header libraries are PRIVATE", that seems to contradict everything I read in Craig Scott's book, "Professional CMake," and many other sources.

From Professional CMake, 9th ed, p. 248,

CMake handles some special cases specific to linking static libraries. If a library A is listed as a PRIVATE dependency for a static library target B, then A will effectively be treated as a PUBLIC dependency as far as linking is concerned (and only for linking). This is because the private A library will still need to be added to the linker command line of anything linking to B in order for symbols from A to be found at link time. If B was a shared library, the private library A that it depends on would not need to be listed on the linker command line. This is all handled transparently by CMake, so the developer typically doesn’t need to concern themselves with the details beyond specifying the PUBLIC, PRIVATE and INTERFACE dependencies with target_link_libraries().

I believe that PRIVATE is the right designator to use here, because the squish library is used internally to OIIO, but is not part of the exported API. If squish is a dynamic library, there is no need to propagate this transitive dependency because it's handled by the linker; if squish is a static library, CMake already is supposed to know that (for this one case) it needs to treat it as public and propagate the dependency. I think we're using PUBLIC/PRIVATE correctly, but something else is going wrong for this one library (circumstantial evidence: if I was really using PRIVATE incorrectly, wouldn't ALL the libraries have the same failures?).

dg0yt commented 2 years ago

CMake handles some special cases specific to linking static libraries. If a library A is listed as a PRIVATE dependency for a static library target B, then A will effectively be treated as a PUBLIC dependency as far as linking is concerned (and only for linking).

Okay. I would prefer to see "special cases" in the CMake documentation, not in secondary literature. And CMake documentation just has the following:

Libraries and targets following PRIVATE are linked to, but are not made part of the link interface.

I guess the behaviour comes down to this code: https://github.com/Kitware/CMake/blob/6261210a671aa20d691b816df6a2237777995b06/Source/cmTargetLinkLibrariesCommand.cxx#L482-L499 which uses $<LINK_ONLY:...> when PRIVATE is used while touching a static library. It seems right ... but cmake documentation also states

Content of ... except when evaluated in a link interface while propagating Transitive Usage Requirements, in which case it is the empty string.

I am confused now :confused:

lgritz commented 2 years ago

"Linked to, but not part of the link interface" is terse, but has all the information. For shared libraries, A linking B means that an app linking A only needs to name A (which itself retains the reference to B). But for static libraries, this is not the case; an app using A needs to also link B explicitly, and CMake is supposed to track this.

Here's some primary documentation that implies this: https://cmake.org/cmake/help/v3.21/guide/tutorial/Selecting%20Static%20or%20Shared%20Libraries.html#mathfunctions-cmakelists-txt-add-library-static In this example, SqrtLibrary is a static library with a compiled component, and the MathFunctions library calls target_link_libraries(MathFunctions PRIVATE SqrtLibrary) so it's definitely not the case that PRIVATE is only for header-only libraries. They expect this example to work, presumably.

Also, to add the the confusion, I'm using PRIVATE for all the external libraries, yet there is only this one library that we are having trouble with. To me, that indicates that to really understand what's going on, we need to understand what is different about this library.

I have a few hypotheses, but not having a Windows machine and being a total vcpkg noob, I'm not sure which are plausible, but maybe you can recognize which have merit:

lgritz commented 2 years ago

Also, https://cmake.org/cmake/help/v3.21/prop_tgt/LINK_INTERFACE_LIBRARIES.html :

By default linking to a shared library target transitively links to targets with which the library itself was linked. For an executable with exports (see the ENABLE_EXPORTS target property) no default transitive link dependencies are used. This property replaces the default transitive link dependencies with an explicit list. When the target is linked into another target using the target_link_libraries() command, the libraries listed (and recursively their link interface libraries) will be provided to the other target also. If the list is empty then no transitive link dependencies will be incorporated when this target is linked into another target even if the default set is non-empty. This property is initialized by the value of the CMAKE_LINK_INTERFACE_LIBRARIES variable if it is set when a target is created. This property is ignored for STATIC libraries.

Can you tell if the vcpkg libsquish is generating shared, static, or both? I'm still gravitating toward suspecting that the problem here might be that the vcpkg libsquish build itself is maybe making a static library only that is not correctly set up to mix properly with the other dynamic libraries?

lgritz commented 2 years ago

To hammer a previous point with a concrete example: Why does PRIVATE link against gif library work, and against libtiff works, but PRIVATE link against squish does not? I think that when we can articulate the answer to that question, we'll know how to solve the problem for squish once and for all.

dg0yt commented 2 years ago

@lgritz wrote:

Why does PRIVATE link against gif library work, and against libtiff works, but PRIVATE link against squish does not?

Let's refer to "libsquish", to avoid confusion with the software named "Squish" which has an official Find module in CMake.

Depending on actual values are given is to target_link_libraries, the exported config is different for various libraries. I built openimageio[gif,webp]:x64-linux from vpckg master now (so with vcpkg patches). The INTERFACE_LINK_LIBRARIES for OpenImageIO::OpenImageIO has the following entries:

OpenImageIO::OpenImageIO_Util
$<$<TARGET_EXISTS:Imath::Imath>:Imath::Imath>
$<$<TARGET_EXISTS:Imath::Half>:Imath::Half>
$<$<TARGET_EXISTS:IlmBase::Imath>:IlmBase::Imath>
$<$<TARGET_EXISTS:IlmBase::Half>:IlmBase::Half>
IlmBase::IlmBaseConfig
IlmBase::Half
IlmBase::IexMath
IlmBase::IlmBaseConfig
IlmBase::IlmBaseConfig
IlmBase::IlmBaseConfig
IlmBase::Iex
Threads::Threads
-pthread
$<LINK_ONLY:$<$<TARGET_EXISTS:OpenEXR::OpenEXR>:OpenEXR::OpenEXR>>
$<LINK_ONLY:$<$<TARGET_EXISTS:OpenEXR::IlmImf>:OpenEXR::IlmImf>>
$<LINK_ONLY:$<$<TARGET_EXISTS:IlmBase::IlmThread>:IlmBase::IlmThread>>
$<LINK_ONLY:$<$<TARGET_EXISTS:IlmBase::Iex>:IlmBase::Iex>>
$<LINK_ONLY:OpenEXR::IlmImfConfig>
$<LINK_ONLY:OpenEXR::IlmImfConfig>
$<LINK_ONLY:IlmBase::Iex>
$<LINK_ONLY:IlmBase::Half>
$<LINK_ONLY:IlmBase::Imath>
$<LINK_ONLY:IlmBase::IlmThread>
$<LINK_ONLY:ZLIB::ZLIB>
$<LINK_ONLY:IlmBase::IlmBaseConfig>
$<LINK_ONLY:IlmBase::Half>
$<LINK_ONLY:IlmBase::IexMath>
$<LINK_ONLY:IlmBase::IlmBaseConfig>
$<LINK_ONLY:IlmBase::IlmBaseConfig>
$<LINK_ONLY:IlmBase::IlmBaseConfig>
$<LINK_ONLY:IlmBase::Iex>
$<LINK_ONLY:Threads::Threads>
$<LINK_ONLY:Libsquish::Libsquish>
${_IMPORT_PREFIX}/lib/libgif.a
$<LINK_ONLY:heif>
$<$<NOT:$<CONFIG:DEBUG>>:${_IMPORT_PREFIX}/lib/libjpeg.a>
$<$<CONFIG:DEBUG>:${_IMPORT_PREFIX}/debug/lib/libjpeg.a>
$<LINK_ONLY:OpenEXR::IlmImfConfig>
$<LINK_ONLY:OpenEXR::IlmImfConfig>
$<LINK_ONLY:IlmBase::Iex>
$<LINK_ONLY:IlmBase::Half>
$<LINK_ONLY:IlmBase::Imath>
$<LINK_ONLY:IlmBase::IlmThread>
$<LINK_ONLY:ZLIB::ZLIB>
$<LINK_ONLY:IlmBase::IlmBaseConfig>
$<LINK_ONLY:IlmBase::Half>
$<LINK_ONLY:IlmBase::IexMath>
$<LINK_ONLY:IlmBase::IlmBaseConfig>
$<LINK_ONLY:IlmBase::IlmBaseConfig>
$<LINK_ONLY:IlmBase::IlmBaseConfig>
$<LINK_ONLY:IlmBase::Iex>
$<LINK_ONLY:Threads::Threads>
$<$<NOT:$<CONFIG:DEBUG>>:${_IMPORT_PREFIX}/lib/libjpeg.a>
$<$<CONFIG:DEBUG>:${_IMPORT_PREFIX}/debug/lib/libjpeg.a>
$<$<NOT:$<CONFIG:DEBUG>>:${_IMPORT_PREFIX}/lib/libtiff.a>
$<$<CONFIG:DEBUG>:${_IMPORT_PREFIX}/debug/lib/libtiffd.a>
$<$<NOT:$<CONFIG:DEBUG>>:${_IMPORT_PREFIX}/lib/liblzma.a>
$<$<CONFIG:DEBUG>:${_IMPORT_PREFIX}/debug/lib/liblzmad.a>
$<$<NOT:$<CONFIG:DEBUG>>:${_IMPORT_PREFIX}/lib/libjpeg.a>
$<$<CONFIG:DEBUG>:${_IMPORT_PREFIX}/debug/lib/libjpeg.a>
$<$<NOT:$<CONFIG:DEBUG>>:${_IMPORT_PREFIX}/lib/libz.a>
$<$<CONFIG:DEBUG>:${_IMPORT_PREFIX}/debug/lib/libz.a>
$<LINK_ONLY:m>
$<$<NOT:$<CONFIG:DEBUG>>:${_IMPORT_PREFIX}/lib/libjpeg.a>
$<$<CONFIG:DEBUG>:${_IMPORT_PREFIX}/debug/lib/libjpeg.a>
$<LINK_ONLY:ZLIB::ZLIB>
$<LINK_ONLY:WebP::WebP>
$<LINK_ONLY:WebP::WebPDemux>
$<LINK_ONLY:ZLIB::ZLIB>
$<LINK_ONLY:$<$<TARGET_EXISTS:OpenColorIO::OpenColorIO>:OpenColorIO::OpenColorIO>>
$<LINK_ONLY:$<$<TARGET_EXISTS:OpenColorIO::OpenColorIOHeaders>:OpenColorIO::OpenColorIOHeaders>>
$<LINK_ONLY:$<$<BOOL:>:pugixml::pugixml>>
$<$<NOT:$<CONFIG:DEBUG>>:${_IMPORT_PREFIX}/lib/libbz2.a>
$<$<CONFIG:DEBUG>:${_IMPORT_PREFIX}/debug/lib/libbz2d.a>
$<LINK_ONLY:ZLIB::ZLIB>
${_IMPORT_PREFIX}/lib/libboost_filesystem.a
${_IMPORT_PREFIX}/lib/libboost_system.a
${_IMPORT_PREFIX}/lib/libboost_thread.a
$<LINK_ONLY:-lpthread>
${_IMPORT_PREFIX}/lib/libboost_chrono.a
${_IMPORT_PREFIX}/lib/libboost_date_time.a
${_IMPORT_PREFIX}/lib/libboost_atomic.a
$<LINK_ONLY:rt>
$<LINK_ONLY:dl>

So the form varies:

This illustrates the pros and cons of the variants: