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

CI: add a step which uninstalls all potentially left over packages #87

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

lazka
Copy link
Member

@lazka lazka commented May 22, 2024

In case a previous run has been aborted mid-run, there might still be packages installed. Make sure we get back to the minimal state again.

In case a previous run has been aborted mid-run, there might still be
packages installed. Make sure we get back to the minimal state again.
@lazka
Copy link
Member Author

lazka commented May 22, 2024

@jeremyd2019 is this useful?

@jeremyd2019
Copy link
Member

I do install some extra packages on the arm runner setup too. base-devel git mingw-w64-clang-aarch64-toolchain procps-ng psmisc zip unzip vim etc-update. I don't actually see a good reason for git to be in there at the moment. I included toolchain in there with the idea that it would reduce the amount of package thrashing.

@lazka
Copy link
Member Author

lazka commented May 22, 2024

ah, right, now I remember. hm

@lazka
Copy link
Member Author

lazka commented May 22, 2024

Something like this?

@jeremyd2019
Copy link
Member

The incantation I usually use to remove left-over packages is pacman -Rnsc $(pacman -Qtdq), but I just noticed today this does not work right when there's a circular dependency (like there was with harfbuzz).

@jeremyd2019
Copy link
Member

I tried your script to get the TO_REMOVE list, and I see:

mingw-w64-clang-aarch64-bzip2
mingw-w64-clang-aarch64-clang-analyzer
mingw-w64-clang-aarch64-clang-tools-extra
mingw-w64-clang-aarch64-crt-git
mingw-w64-clang-aarch64-expat
mingw-w64-clang-aarch64-libc++
mingw-w64-clang-aarch64-libmangle-git
mingw-w64-clang-aarch64-libsystre
mingw-w64-clang-aarch64-libtre-git
mingw-w64-clang-aarch64-lldb
mingw-w64-clang-aarch64-llvm-openmp
mingw-w64-clang-aarch64-lua
mingw-w64-clang-aarch64-make
mingw-w64-clang-aarch64-mpdecimal
mingw-w64-clang-aarch64-ncurses
mingw-w64-clang-aarch64-ntldd
mingw-w64-clang-aarch64-openssl
mingw-w64-clang-aarch64-pkgconf
mingw-w64-clang-aarch64-python
mingw-w64-clang-aarch64-python-six
mingw-w64-clang-aarch64-readline
mingw-w64-clang-aarch64-sqlite3
mingw-w64-clang-aarch64-tcl
mingw-w64-clang-aarch64-termcap
mingw-w64-clang-aarch64-tk
mingw-w64-clang-aarch64-tools-git
mingw-w64-clang-aarch64-tzdata
mingw-w64-clang-aarch64-winpthreads-git
mingw-w64-clang-aarch64-winstorecompat-git
procps-ng

some of that is expected (since you used -clang instead of -toolchain), but notably the -*-git and -libc++ packages stand out, it appears that script doesn't handle provides.

@lazka
Copy link
Member Author

lazka commented May 22, 2024

hm, I guess I could expand the group in the array if wanted (pactree doesn't do groups which is why I went for clang only)

jeremyd2019 added a commit to jeremyd2019/winautoconfig that referenced this pull request May 25, 2024
MSYS2 Git is not normally needed.  GfW is already installed.

I'm not sure why I installed zip, but unzip can be avoided since I'm
already (ab)using the bsdtar that comes with Windows to extract one zip
file, may as well use it for that one too.

A potential future reduction of pakages would be to switch from
-toolchain to -cc (or -clang).

See also msys2/msys2-autobuild#87
@jeremyd2019
Copy link
Member

For the record, I removed git zip unzip from the packages I'll pre-install moving forward, and I am seriously considering switching from -toolchain to -cc to further reduce the set of packages (largely because lldb is part of toolchain, and lldb depends on python. There are probably other 'large' dependency branches in toolchain as well).

Regardless, what concerns me about this is that when I tested the scriptlet it thought it needed to uninstall -crt-git and friends. That suggests to me that it does not properly handle provides aliases (-clang depends on -crt, and -crt-git provides -crt). That's probably a showstopper for this change as-is

@jeremyd2019
Copy link
Member

jeremyd2019 commented May 26, 2024

What has been kicking around in my mind is what makepkg does for --syncdeps --rmdeps: it saves a list of all installed packages at the start (pacman -Qq), and then at the end it removes all packages that were not in that list (https://github.com/msys2/msys2-pacman/blob/761a878373f16db3c1825d1e8eea4c5167c36057/scripts/makepkg.sh.in#L322-L346). Unfortunately, if the runner fails in such a way that makepkg doesn't uninstall things, it seems unlikely that a subsequent step in the job would be able to clean things up either (ie, in a post step).

@lazka lazka marked this pull request as draft May 26, 2024 17:44
@lazka
Copy link
Member Author

lazka commented May 26, 2024

Regardless, what concerns me about this is that when I tested the scriptlet it thought it needed to uninstall -crt-git and friends. That suggests to me that it does not properly handle provides aliases (-clang depends on -crt, and -crt-git provides -crt). That's probably a showstopper for this change as-is

Indeed, seems like a pactree bug

# https://github.com/msys2/msys2-autobuild/pull/87#issuecomment-2125614088
KEEP+=($(pacman -Sgq mingw-w64-clang-aarch64-toolchain) "zip" "unzip" "vim" "etc-update" "psmisc")
pacman -S --noconfirm --needed "${KEEP[@]}"
TO_REMOVE=$(comm -23 <(pacman -Qq | sort) <(for tokeep in "${KEEP[@]}"; do pactree -ul "$tokeep" | sed 's/[<>=].*//'; done | sort -u))
Copy link
Member

@jeremyd2019 jeremyd2019 May 26, 2024

Choose a reason for hiding this comment

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

Suggested change
TO_REMOVE=$(comm -23 <(pacman -Qq | sort) <(for tokeep in "${KEEP[@]}"; do pactree -ul "$tokeep" | sed 's/[<>=].*//'; done | sort -u))
TO_REMOVE=$(comm -23 <(pacman -Qq | sort) <(pacman -Qq $(for tokeep in "${KEEP[@]}"; do pactree -ul "$tokeep" | sed 's/[<>=].*//'; done) | sort -u))

? pacman -Qq seems to resolve provides to the "real" package name.

Copy link
Member

Choose a reason for hiding this comment

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

Also I don't know if sort -u should be inside the $() subshell too, or just at the end. I originally had it just inside, and comm was then unhappy that its input was not sorted anymore.

# XXX: this depends on marix.packages just being base+base-devel
KEEP=("base" "base-devel")
# https://github.com/msys2/msys2-autobuild/pull/87#issuecomment-2125614088
KEEP+=($(pacman -Sgq mingw-w64-clang-aarch64-toolchain) "zip" "unzip" "vim" "etc-update" "psmisc")
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
KEEP+=($(pacman -Sgq mingw-w64-clang-aarch64-toolchain) "zip" "unzip" "vim" "etc-update" "psmisc")
KEEP+=($(pacman -Sgq mingw-w64-clang-aarch64-toolchain) "vim" "etc-update" "procps-ng" "psmisc")

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants