Skip to content
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

[meson] Update meson to 0.58.1 #18393

Closed
wants to merge 5 commits into from

Conversation

Neumann-A
Copy link
Contributor

No description provided.

@Neumann-A Neumann-A marked this pull request as ready for review June 13, 2021 12:29
@JackBoosY JackBoosY added the category:tool-update The issue is with build tool or build script, which requires update or should be executed correctly label Jun 15, 2021
@JackBoosY
Copy link
Contributor

Does the simage regression related to meson changes?

@Neumann-A
Copy link
Contributor Author

Neumann-A commented Jun 15, 2021

@JackBoosY: looks more like a problem with the TIFF wrapper

CMake Error at /Users/vagrant/Data/installed/x64-osx/share/jpeg/vcpkg-cmake-wrapper.cmake:1 (_find_package):
  _find_package for module JPEG called with REQUIRED, but
  CMAKE_DISABLE_FIND_PACKAGE_JPEG is enabled.  A REQUIRED package cannot be
  disabled.
Call Stack (most recent call first):
  /Users/vagrant/Data/work/1/s/scripts/buildsystems/vcpkg.cmake:745 (include)
  /Users/vagrant/Data/installed/x64-osx/share/tiff/vcpkg-cmake-wrapper.cmake:17 (find_package)
  /Users/vagrant/Data/work/1/s/scripts/buildsystems/vcpkg.cmake:745 (include)
  CMakeLists.txt:222 (find_package)


CMake Error at /Users/vagrant/Data/work/1/s/scripts/buildsystems/vcpkg.cmake:791 (_find_package):
  _find_package for module ZLIB called with REQUIRED, but
  CMAKE_DISABLE_FIND_PACKAGE_ZLIB is enabled.  A REQUIRED package cannot be
  disabled.

To be exact it is always using find_package(<something> REQUIRED) which is not correct. It needs to check <PackageName>_FIND_REQUIRED and <PackageName>_FIND_QUIETLY to set those options for the dependent ports correctly. Maybe find_dependency works in the wrapper? (Probably not; probably requires an ARGS MATCHES like syntax)

cc @dg0yt because of git blame ;)

@JackBoosY
Copy link
Contributor

JackBoosY commented Jun 15, 2021

When building simage:x64-uwp that install tiff first:

     4>D:\buildtrees\simage\src\mage-1.8.0-17f988a113.clean\src\simage_tiff.c(88,18): error C4996: 'uint16': libtiff type deprecated; please use corresponding C99 stdint.h type 

@dg0yt
Copy link
Contributor

dg0yt commented Jun 15, 2021

looks more like a problem with the TIFF wrapper

  • JPEG is required iff port tiff is used with feature "jpeg".
  • ZLIB is required iff port tiff is used with feature "zip".
  • jpeg, lzma and zip are opt-out features of port tiff because tiff was built with them before I updated that port.

Port simage is configured with CMAKE_DISABLE_FIND_PACKAGE_JPEG and CMAKE_DISABLE_FIND_PACKAGE_ZLIB for OSX_OR_WINDOWS. I don't know why this is done, but this conflicts with tiff's transitive dependencies.

So how about opting out from tiff's features "jpeg" and "zip" in port simage?

@Neumann-A
Copy link
Contributor Author

@dg0yt

don't know why this is done, but this conflicts with tiff's transitive dependencies.

They are only required if and only if find_package(TIFF REQUIRED). If find_package(TIFF) is called the transitive dependencies should not be marked as REQUIRED. instead if the dependency is not satisfied TIFF_Found should be set to false.

@dg0yt
Copy link
Contributor

dg0yt commented Jun 15, 2021

When building simage:x64-uwp that install tiff first:

     4>D:\buildtrees\simage\src\mage-1.8.0-17f988a113.clean\src\simage_tiff.c(88,18): error C4996: 'uint16': libtiff type deprecated; please use corresponding C99 stdint.h type 

tiff 4.3.0 changed its type system. This was explicitly mentioned in #18187. With that PR, I patched a similar issue in port selene.

@dg0yt
Copy link
Contributor

dg0yt commented Jun 15, 2021

If find_package(TIFF) is called the transitive dependencies should not be marked as REQUIRED. instead if the dependency is not satisfied TIFF_Found should be set to false.

I see, it becomes clear with the simage example. Unfortunately it didn't come up during CI and review. I'm not sure if I can act quickly for tiff at the moment.

git grep REQUIRED ports/*/vcpkg-cmake-wrapper.cmake shows a few more hits which might need to be reviewed then.

@JackBoosY
Copy link
Contributor

Depends on #18473. Since this is a tool update PR, I need to make sure all tests pass.

@JackBoosY JackBoosY added the depends:different-pr This PR or Issue depends on a PR which has been filed label Jun 16, 2021
@dg0yt
Copy link
Contributor

dg0yt commented Jun 18, 2021

Does the simage regression related to meson changes?

Same as for gn update problem:
simage for windows triplets does not depend on tiff (and some other ports) so it must not use it. That's why it was not covered in CI for port tiff.

@BillyONeal
Copy link
Member

I poked @ras0219 @ras0219-msft about the "vcpkg-find-acquire-program" server intermittent failure that got triggered by tool update

@dg0yt
Copy link
Contributor

dg0yt commented Jun 24, 2021

looks more like a problem with the TIFF wrapper
...
cc @dg0yt because of git blame ;)

For osx, I came to the conclusion that the error is one level above the TIFF wrapper: #18631

@JackBoosY
Copy link
Contributor

looks more like a problem with the TIFF wrapper
...
cc @dg0yt because of git blame ;)

For osx, I came to the conclusion that the error is one level above the TIFF wrapper: #18631

Related: #18602.

@JackBoosY JackBoosY removed the depends:different-pr This PR or Issue depends on a PR which has been filed label Jul 2, 2021
@JackBoosY
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@JackBoosY
Copy link
Contributor

The gl3w:x86-windows regression will be fixed by another PR.

@JackBoosY JackBoosY added the depends:different-pr This PR or Issue depends on a PR which has been filed label Jul 5, 2021
@JackBoosY
Copy link
Contributor

/azp run

@JackBoosY JackBoosY removed the depends:different-pr This PR or Issue depends on a PR which has been filed label Jul 9, 2021
@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@JackBoosY JackBoosY added the info:reviewed Pull Request changes follow basic guidelines label Jul 14, 2021
@xavier2k6 xavier2k6 mentioned this pull request Jul 15, 2021
14 tasks
@PhoebeHui
Copy link
Contributor

The 'set(MESON_VERSION 0.58.0)' need to change the version to '0.58.1', since PR #18817 already covered all changes in this PR, @Neumann-A, do you mind to close this PR? or I can revert the changes in PR #18817.

@PhoebeHui PhoebeHui added requires:author-response and removed info:reviewed Pull Request changes follow basic guidelines labels Jul 16, 2021
@Neumann-A Neumann-A closed this Jul 16, 2021
@Neumann-A Neumann-A deleted the update_meson branch September 18, 2021 08:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:tool-update The issue is with build tool or build script, which requires update or should be executed correctly
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants