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

Fix bootstrapping MSYS2 pacman (#11499) #12080

Merged
merged 3 commits into from
Jun 24, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion ports/ffmpeg/CONTROL
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
Source: ffmpeg
Version: 4.2-11
Version: 4.2-12
Build-Depends: zlib
Homepage: https://ffmpeg.org
Description: a library to decode, encode, transcode, mux, demux, stream, filter and play pretty much anything that humans and machines have created.
Expand Down
2 changes: 1 addition & 1 deletion ports/ffnvcodec/CONTROL
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
Source: ffnvcodec
Version: 9.1.23.1
Version: 9.1.23.1-1
Homepage: https://github.com/FFmpeg/nv-codec-headers
Description: FFmpeg version of Nvidia Codec SDK headers.
2 changes: 1 addition & 1 deletion ports/icu/CONTROL
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
Source: icu
Version: 67.1-1
Version: 67.1-2
Homepage: http://icu-project.org/apiref/icu4c/
Description: Mature and widely used Unicode and localization library.
Supports: !(arm|uwp)
2 changes: 1 addition & 1 deletion ports/libpq/CONTROL
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
Source: libpq
Version: 12.2-2
Version: 12.2-3
Build-Depends: libpq[bonjour] (osx)
Supports: !uwp
Homepage: https://www.postgresql.org/
Expand Down
2 changes: 1 addition & 1 deletion ports/libvpx/CONTROL
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
Source: libvpx
Version: 1.8.1-6
Version: 1.8.1-7
Homepage: https://github.com/webmproject/libvpx
Description: The reference software implementation for the video coding formats VP8 and VP9.
Supports: !(uwp&arm)
2 changes: 1 addition & 1 deletion ports/tensorflow-cc/CONTROL
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
Source: tensorflow-cc
Version: 1.14-2
Version: 1.14-3
Description: Library for computation using data flow graphs for scalable machine learning
Build-Depends: c-ares
Supports: !x86
12 changes: 12 additions & 0 deletions scripts/cmake/vcpkg_acquire_msys.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,18 @@ function(vcpkg_acquire_msys PATH_TO_ROOT_OUT)
COMMAND ${PATH_TO_ROOT}/usr/bin/bash.exe --noprofile --norc -c "PATH=/usr/bin;gpgconf --homedir /etc/pacman.d/gnupg --kill all"
WORKING_DIRECTORY ${TOOLPATH}
)
# we need to update pacman before anything else due to pacman transitioning
# to using zstd packages, and our pacman is too old to support those
_execute_process(
COMMAND ${PATH_TO_ROOT}/usr/bin/bash.exe --noprofile --norc -c "PATH=/usr/bin;pacman -Sy pacman --noconfirm"
WORKING_DIRECTORY ${TOOLPATH}
)
Comment on lines +101 to +106
Copy link
Contributor

@emptyVoid emptyVoid Jun 24, 2020

Choose a reason for hiding this comment

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

This part is not needed (see msys2/MSYS2-packages#1962 (comment)).

The same approach was first implemented in #11443 (merged the next day after #11499), and it stopped working the moment upstream changed compression scheme for some of pacman dependency packages to zstd, then we resorted to a workaround you can now see in #11443. The workaround was removed in #11810 as obsolete since upstream reverted core packages to xz.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

They're zstd again, at least when I tried earlier today.

Copy link
Contributor

@emptyVoid emptyVoid Jun 24, 2020

Choose a reason for hiding this comment

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

If you are referring to the errors seen in msys-pacman-*-err.log, those are for non-core packages.

At the moment vcpkg_acquire_msys silently ignores any errors in Acquiring MSYS2... region, as it does not use RESULT_VARIABLE of _execute_process function. Thus the core system upgrade fails, and pacman stays at some old version which is unable to install zstd packages.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If we don't need those packages why not just update pacman instead of all the packages?

Copy link
Contributor

Choose a reason for hiding this comment

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

We do not update all the packages, we only do the core system upgrade so that any subsequent package installs would succeed. Some of the packages vcpkg ports require depend on the core packages, and the core packages could not be upgraded in the same shell with non-core installs.

Copy link
Contributor Author

@endrift endrift Jun 24, 2020

Choose a reason for hiding this comment

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

There's a pacman -Syu in there that I didn't add, so yes you do update all the packages. I can try removing it and seeing if any additional tests fail, but I'm not sure I should touch that.

Copy link
Contributor

@emptyVoid emptyVoid Jun 24, 2020

Choose a reason for hiding this comment

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

Yes, I'm referring to pacman -Syu too -- it actually updates the package database and makes a core system upgrade only (see MSYS2 Installation step #5). To upgrade all the packages an additional pacman -Su command is necessary.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, yes, because of how MSYS2 works. I forgot about that. I'll try it without my first pacman -Sy pacman.

# dash relies on specific versions of the base packages, which prevents us
# from doing a proper update. However, we don't need it so we remove it
_execute_process(
COMMAND ${PATH_TO_ROOT}/usr/bin/bash.exe --noprofile --norc -c "PATH=/usr/bin;pacman -Rc dash --noconfirm"
WORKING_DIRECTORY ${TOOLPATH}
)
_execute_process(
COMMAND ${PATH_TO_ROOT}/usr/bin/bash.exe --noprofile --norc -c "PATH=/usr/bin;pacman -Syu --noconfirm"
WORKING_DIRECTORY ${TOOLPATH}
Expand Down