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

[vcpkg-acquire-msys] Improvement #11810

Merged
merged 4 commits into from
Jun 11, 2020

Conversation

emptyVoid
Copy link
Contributor

Describe the pull request

Drops a workaround introduced in #11443 as per a member of MSYS2 core packages are now once again use the original compression scheme supported by older versions of package manager (see msys2/MSYS2-packages#1962 (comment)).

Drops killing of gpg-agent.exe as GnuPG daemons are now being dealt with earlier in the script.

Removes updating of the package manager's database on package installs as the update could require a core system upgrade before some of the packages could be installed. Additionally this resolves a potential dependency conflict by ensuring all the MSYS2 dependent ports would get built against the same versions of MSYS2 packages.

@JackBoosY

Copy link
Contributor

@JackBoosY JackBoosY left a comment

Choose a reason for hiding this comment

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

Could you update x264 version info to test this PR? See documentation.

@emptyVoid
Copy link
Contributor Author

@JackBoosY, are you sure about x264? Although its logs will show if this PR works, it won't get built due to #11765.

There are a few other ports using vcpkg_configure_make which in turn utilizes vcpkg_acquire_msys on Windows: libb2, libcrafter, libmagic, libmesh, libwandio.

@JackBoosY
Copy link
Contributor

@emptyVoid Therefore, you can select a port to test this PR.

@julianxhokaxhiu
Copy link
Contributor

As you are on this already, is it possible to add an AppVeyor integration? At the current status, in order to use the existing msys2 installation I have to patch exactly this file at this line https://github.com/julianxhokaxhiu/ffmpegCI/blob/master/patch/scripts/cmake/vcpkg_acquire_msys.cmake#L39

If this could be configurable, it would be great. Or some kind of autodetection would be fine too.

Thank you in advance!

@emptyVoid
Copy link
Contributor Author

Configurability of the MSYS2 installation definitely sounds beneficial for the user.

@julianxhokaxhiu, could you, please, create a feature request issue explaining your suggestion to may be discuss and gather some insights on the matter (I do have some ideas though I'm not sure if they are good to go).

@emptyVoid
Copy link
Contributor Author

At the moment vcpkg_configure_make is broken for all the dependent packages:
#11765 - x264
#11832 - libb2, libmagic, libwandio
#11835 - libmesh
And libcrafter could not be built due to dependency: Target 'Windows' not supported by libpcap!

We could either wait for vcpkg_configure_make to be fixed, or could bump version in some other MSYS2 dependent port. @JackBoosY, what's your take on this?

@Neumann-A
Copy link
Contributor

we have a hen egg problem:

-- Downloading https://sourceforge.net/projects/msys2/files/REPOS/MSYS2/x86_64/libzstd-1.4.4-2-x86_64.pkg.tar.xz/download...
-- Downloading https://sourceforge.net/projects/msys2/files/REPOS/MSYS2/x86_64/zstd-1.4.4-2-x86_64.pkg.tar.xz/download...
CMake Error at scripts/cmake/vcpkg_download_distfile.cmake:99 (message):


  File does not have expected hash:

          File path: [ D:/vcpkg_common/downloads/temp/zstd-1.4.4-2-x86_64.pkg.tar.xz ]
      Expected hash: [ 2be7e243d4e600d092aa6a630d24cfc536a6c06a4fa8e0909b0364569d2f938e24f220de1f52edbc36adc7c69ca23a2a730675f2da82c1530d3d91136089d3e2 ]
        Actual hash: [ 6d3ab3e9181b49a4afb747fe05d3a4965a170e37d68cb7cacafc07a79b9d22360609d26bc9fecad9a6f6d0ea30e743aa06dcef9c7d8a886028ad9ec6ca9f0558 ]

so I'll be merging this one into #11836

@emptyVoid
Copy link
Contributor Author

That's most likely just a download issue -- stumbled on them a few times today, had to restart installs to make them go.

@julianxhokaxhiu
Copy link
Contributor

@emptyVoid Issue created and linked to this PR :)

@JackBoosY
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@JackBoosY JackBoosY added the category:vcpkg-bug The issue is with the vcpkg system (including helper scripts in `scripts/cmake/`) label Jun 10, 2020
Copy link
Contributor

@JackBoosY JackBoosY left a comment

Choose a reason for hiding this comment

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

Can you trigger the ffmpeg test by setting the ffmpeg version from 4.2-9 to 4.2-10?

@JackBoosY JackBoosY added the info:reviewed Pull Request changes follow basic guidelines label Jun 11, 2020
@dan-shaw dan-shaw merged commit a5e28c4 into microsoft:master Jun 11, 2020
@dan-shaw
Copy link
Contributor

Thanks for the PR!

@emptyVoid emptyVoid deleted the fix-vcpkg-acquire-msys branch June 11, 2020 10:28
@JackBoosY JackBoosY added category:tool-update The issue is with build tool or build script, which requires update or should be executed correctly and removed category:vcpkg-bug The issue is with the vcpkg system (including helper scripts in `scripts/cmake/`) labels Jun 12, 2020
JangBoo pushed a commit to JangBoo/vcpkg that referenced this pull request Jun 18, 2020
* [vcpkg-acquire-msys] Remove obsolete workaround.

* [vcpkg-acquire-msys] Don't refresh package database on package install.

* [vcpkg-acquire-msys] Drop no longer needed taskkill.

* Trigger CI rebuild to test vcpkg_acquire_msys.
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 info:reviewed Pull Request changes follow basic guidelines
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[ffmpeg] build failure
5 participants