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

pacman-helper.sh: add quick_remove mode #597

Merged
merged 4 commits into from
Feb 20, 2025

Conversation

jeremyd2019
Copy link
Contributor

@jeremyd2019 jeremyd2019 commented Feb 19, 2025

add an action parameter to the prior quick_add function, and use if checks to see if we are adding or removing. Provide wrapper functions for quick_add and quick_remove.

For git-for-windows/git-sdk-32#39 (comment)

@jeremyd2019 jeremyd2019 force-pushed the jeremyd2019-quick-remove branch from 1fa46a0 to 6f7eb41 Compare February 19, 2025 00:16
@jeremyd2019 jeremyd2019 force-pushed the jeremyd2019-quick-remove branch from 6f7eb41 to f60bfeb Compare February 19, 2025 01:10
@jeremyd2019 jeremyd2019 marked this pull request as ready for review February 19, 2025 01:12
@jeremyd2019 jeremyd2019 force-pushed the jeremyd2019-quick-remove branch from f60bfeb to c0979f9 Compare February 19, 2025 01:22
@dscho
Copy link
Member

dscho commented Feb 19, 2025

Hey @jeremyd2019 good stuff!

I worked on this a bit more (and this time I was able to push the PR branch! Yay!). In particular, I wanted to make this a bit more flexible, so that would could unshadow, say, p7zip from both the i686 and the x86_64 repository in one go.

The main idea is to skip the <architecture> argument and use either full package filenames or partial package names that include the architecture. For example, removing p7zip could now be accomplished like this:

$ pacman-helper.sh quick_remove p7zip-i686 p7zip-x86_64

To keep things simpler, I want to avoid near-duplicating the for loop, and I also wanted to avoid the if/else/fi pattern in favor of storing quick_add or quick_remove in the action variable and then calling the respective function via $action <argument>....

What do you think of that new version? If you're fine with it, I'll auto-squash the fixup!/amend! and merge.

dscho added a commit to jeremyd2019/git-for-windows-automation that referenced this pull request Feb 19, 2025
I adjusted the `pacman-helper.sh quick_remove` functionality in
git-for-windows/build-extra#597 (commits)
so that the architecture would be specified via the package names or
filenames directly, making it possible to remove packages for multiple
architectures in one go.

This requires a change in the `remove-packages-from-repo` workflow
which I hereby make.

Signed-off-by: Johannes Schindelin <[email protected]>
@jeremyd2019
Copy link
Contributor Author

OK.

I don't necessarily want to hold up this PR, because it isn't really related to the changes here, but some of these patterns would seem to cause issues for packages with numbers in their names. Specifically, I think the next update of msys2-runtime will git rm msys2-runtime-3.3 files as well. Something to think about...

@jeremyd2019
Copy link
Contributor Author

jeremyd2019 commented Feb 19, 2025

repo-add does have -R|--remove option, to remove the old file when adding a new version of a package. I don't know if that would work for you, and doesn't help the repo-remove case.

If you want to get deep into it, you can see repo-add does this to figure out what to remove:

find_pkgentry() {
        local pkgname=$1
        local pkgentry

        for pkgentry in "$tmpdir/db/$pkgname"*; do
                name=${pkgentry##*/}
                if [[ ${name%-*-*} = "$pkgname" ]]; then
                        echo $pkgentry
                        return 0
                fi
        done
        return 1
}

                        if (( RMEXISTING )); then
                                local oldfilename="$(sed -n '/^%FILENAME%$/ {n;p;q;}' "$pkgentry/desc")"
                                local oldfile="$(dirname "$1")/$oldfilename"
                        fi

        if (( RMEXISTING )); then
                msg2 "$(gettext "Removing old package file '%s'")" "$oldfilename"
                rm -f ${oldfile} ${oldfile}.sig
        fi

jeremyd2019 and others added 4 commits February 20, 2025 11:16
Quite a long time ago, Git for Windows rebuilt a couple of i686
packages that were no longer serviced by the MSYS2 project, for
example `cyrus-sasl`.

In the meantime, the MSYS2 project did rebuild even newer versions of
these packages, so we no longer need to maintain our own versions.

This requires some automation to remove packages from Git for Windows'
Pacman package database. This commit refactors the `quick_add` function
into a base function with an additional `action` parameter, adds the
small `quick_add` shim and a new `quick_remove` shim, too.

This functionality will be used in a new workflow added in
git-for-windows/git-for-windows-automation#114
to remove above-mentioned i686 packages.

Note that for now, `quick_remove` requires package _filenames_ to be
specified as command-line arguments, not package names. For example,

  pacman-helper.sh quick_remove cyrus-sasl-2.1.28-2-any.pkg.tar.xz

This is in line with `quick_add`'s command-line arguments, and hence
makes the implementation slightly simpler (because the diff is
smaller). Since `repo-remove` cannot accept filenames, the
implementation of the wrapper function `repo_remove` turns the
filenames into package names.

A subsequent commit will add support for accepting package names, too,
for convenience.

Co-authored-by: Johannes Schindelin <[email protected]>
Signed-off-by: Jeremy Drake <[email protected]>
Signed-off-by: Johannes Schindelin <[email protected]>
Instead of filenames, it is much more natural to specify the package
name. One caveat is that the package name must also contain the CPU
architecture in that case, but it can leave out the version number.

For example, instead of

  pacman-helper.sh quick_remove cyrus-sasl-2.1.28-2-i686.pkg.tar.xz

one can now write

  pacman-helper.sh quick_remove cyrus-sasl-i686

To make this safer with regards to package names that end in
`-<digit>[...]` (like `msys2-runtime-3.3`), either the full version has
to be provided, including pkgrel, or the version has to be omitted.

Co-authored-by: Jeremy Drake <[email protected]>
Signed-off-by: Johannes Schindelin <[email protected]>
Signed-off-by: Johannes Schindelin <[email protected]>
An important part of the `quick_*` functions is to remove the archives
that are no longer needed. For example, when adding a new version with
`quick_add`, the package archive of the superseded version (if any) is
removed.

Example: When deploying OpenSSH v9.9p2, `pacman-helper.sh quick_add
`openssh-9.9p2-1-x86_64.pkg.tar.xz`, was called, which removed
`openssh-9.9p1-2-x86_64.pkg.tar.xz` because that was no longer needed,
and likewise the corresponding `.sig` file.

As pointed out by Jeremy Drake, the shell pattern that is used to
perform this job, however, is too fragile: it does not account for
package names that have an `-<digit>` infix, e.g. `msys2-runtime-3.3`.

That is, when upgrading `msys2-runtime` via `quick_add`, it would not
only remove the superseded `msys2-runtime` package, but also the
`msys2-runtime-3.3` and the `msys2-runtime-3.3-devel` archives as well!

Lets' be more careful about this.

Sadly, there is no way to do this with non-greedy patterns: wildcard
patterns only support greedy matches. That is, `-[0-9]*` will happily
match anything via that `*`, including `-<digit>`, i.e. the maximal
suffix, not the minimal suffix.

So let's use "exclude patterns" (a Git-specific feature) to exclude
any false positives.

This is still slightly fragile, as it will prevent `quick_*` from
working correctly when passing, say, _both_ `msys2-runtime-3.3` _and_
`msys2-runtime`: exclude patterns are not positional, and therefore
this command would _not_ match `msys2-runtime-3.3`:

  git ls-files -- \
    'msys2-runtime-[0-9]*' \
    ':(exclude)msys2-runtime-[0-9]*-*-*-*' \
    'msys2-runtime-3.3-[0-9]*' \
    ':(exclude)msys2-runtime-3.3-[0-9]*-*-*-*'

The first `:(exclude)` pattern would prevent the subsequent pattern from
matching `msys2-runtime-3.3-<digit>`.

Nevertheless, this scenario is unlikely, as the principal use case of
`quick_add` is the `/deploy` slash command in Git for Windows'
MINGW-packages and MSYS2-packages PullRequests. And there, we only
deploy a single package at a time, not two.

And even in case we should run afoul of this fragility in a
(semi-manual) `quick_remove` invocation, the fix is easy: manually
remove the files that haven't been removed by mistake. This is all about
the tip of the `x86_64`, `aarch64` and `i686` branches of
https://github.com/git-for-windows/pacman-repo, after all, and such a
manual fix is but a trivial PullRequest away. Contrary to the problem
addressed by this commit, this fragility would not affect the operation
of the Pacman repository, either, because it would serve too many files,
not too few.

Helped-by: Jeremy Drake <[email protected]>
Signed-off-by: Johannes Schindelin <[email protected]>
@dscho dscho force-pushed the jeremyd2019-quick-remove branch from bbb1809 to 0ef4d2a Compare February 20, 2025 10:31
@dscho
Copy link
Member

dscho commented Feb 20, 2025

[...] some of these patterns would seem to cause issues for packages with numbers in their names. Specifically, I think the next update of msys2-runtime will git rm msys2-runtime-3.3 files as well. Something to think about...

@jeremyd2019 Very valid point! I added 0ef4d2a to address that.

I verified this by doing a dry-run of quick_adding a (stale, but doesn't matter as this was only for testing) msys2-runtime archive:

$ PACMANDRYRUN=1 ./pacman-helper.sh quick_add /var/cache/pacman/pkg/msys2-runtime-3.5.5-3-x86_64.pkg.tar.xz
[...]
Leaving temporary directory /tmp/tmp.6yspBjDZPQ/ for inspection

$ cd /tmp/tmp.6yspBjDZPQ/x86_64

$ git show --raw
commit 09472db379e348f218dc43894b9ce553c617becc (HEAD -> x86_64)
Author: Johannes Schindelin <[email protected]>
Date:   Thu Feb 20 11:25:20 2025 +0100

    Update 1 package(s)
    
    msys2-runtime -> 3.5.5-3
    
    Signed-off-by: Johannes Schindelin <[email protected]>

:100644 100644 e807a8f f786a67 M        git-for-windows-x86_64.db
:100644 100644 e807a8f f786a67 M        git-for-windows-x86_64.db.tar.xz
:100644 100644 2f4fa87 a268e91 M        git-for-windows-x86_64.files
:100644 100644 2f4fa87 a268e91 M        git-for-windows-x86_64.files.tar.xz
:000000 100644 0000000 a5482f3 A        msys2-runtime-3.5.5-3-x86_64.pkg.tar.xz
:000000 100644 0000000 55f4c8d A        msys2-runtime-3.5.5-3-x86_64.pkg.tar.xz.sig
:100644 000000 49574cf 0000000 D        msys2-runtime-3.5.7-1-x86_64.pkg.tar.xz
:100644 000000 18416eb 0000000 D        msys2-runtime-3.5.7-1-x86_64.pkg.tar.xz.sig

$ diff -u \
  <(git show HEAD^:git-for-windows-x86_64.db | tar tJvf - | sort -k 6) \
  <(git show HEAD:git-for-windows-x86_64.db | tar tJvf - | sort -k 6)
--- /dev/fd/63  2025-02-20 11:29:12.000000000 +0100
+++ /dev/fd/62  2025-02-20 11:29:12.000000000 +0100
@@ -128,8 +128,8 @@
 -rw-r--r-- root/root      1525 2023-05-09 23:00 msys2-runtime-3.3-3.3.6-1/desc
 drwxr-xr-x root/root         0 2023-05-09 23:00 msys2-runtime-3.3-devel-3.3.6-1/
 -rw-r--r-- root/root      1576 2023-05-09 23:00 msys2-runtime-3.3-devel-3.3.6-1/desc
-drwxr-xr-x root/root         0 2025-01-30 12:28 msys2-runtime-3.5.7-1/
--rw-r--r-- root/root       711 2025-01-30 12:28 msys2-runtime-3.5.7-1/desc
+drwxr-xr-x root/root         0 2025-02-20 11:25 msys2-runtime-3.5.5-3/
+-rw-r--r-- root/root      1521 2025-02-20 11:25 msys2-runtime-3.5.5-3/desc
 drwxr-xr-x root/root         0 2025-01-30 12:28 msys2-runtime-devel-3.5.7-1/
 -rw-r--r-- root/root       760 2025-01-30 12:28 msys2-runtime-devel-3.5.7-1/desc
 drwxr-xr-x root/root         0 2025-02-18 13:13 openssh-9.9p2-1/

I also verified that the quick_remove operation does what it is supposed to do:

$ PACMANDRYRUN=1 ./pacman-helper.sh quick_remove cyrus-sasl-2.1.28-2-i686
[...]
Leaving temporary directory /tmp/tmp.BxRTsyW8g0/ for inspection

$ cd /tmp/tmp.BxRTsyW8g0/i686/

$ git show --raw
commit acb31edf6d6e097ed0435551cf9f5c8bbb5d02fb (HEAD -> i686)
Author: Johannes Schindelin <[email protected]>
Date:   Thu Feb 20 11:29:48 2025 +0100

    Remove 1 package(s)

    cyrus-sasl

    Signed-off-by: Johannes Schindelin <[email protected]>

:100644 000000 ce16dbd 0000000 D        cyrus-sasl-2.1.28-2-i686.pkg.tar.xz
:100644 000000 b797413 0000000 D        cyrus-sasl-2.1.28-2-i686.pkg.tar.xz.sig
:100644 100644 90ce06d 48b9dbe M        git-for-windows-i686.db
:100644 100644 90ce06d 48b9dbe M        git-for-windows-i686.db.tar.xz
:100644 100644 89a39de 65eb381 M        git-for-windows-i686.files
:100644 100644 89a39de 65eb381 M        git-for-windows-i686.files.tar.xz

$ diff -u \
  <(git show HEAD^:git-for-windows-i686.db | tar tJvf - | sort -k 6) \
  <(git show HEAD:git-for-windows-i686.db
 | tar tJvf - | sort -k 6)
--- /dev/fd/63  2025-02-20 11:30:44.000000000 +0100
+++ /dev/fd/62  2025-02-20 11:30:44.000000000 +0100
@@ -6,8 +6,6 @@
 -rw-r--r-- root/root      1381 2023-01-26 23:03 ca-certificates-20211016-2/desc
 drwxr-xr-x root/root         0 2025-02-17 21:15 curl-8.12.1-2/
 -rw-r--r-- root/root       604 2025-02-17 21:15 curl-8.12.1-2/desc
-drwxr-xr-x root/root         0 2023-06-23 14:16 cyrus-sasl-2.1.28-2/
--rw-r--r-- root/root      1368 2023-06-23 14:16 cyrus-sasl-2.1.28-2/desc
 drwxr-xr-x root/root         0 2023-02-21 11:17 docbook-xsl-ns-1.79.2-2/
 -rw-r--r-- root/root      1308 2023-02-21 11:17 docbook-xsl-ns-1.79.2-2/desc
 drwxr-xr-x root/root         0 2024-06-14 11:31 fido2-tools-1.15.0-1/

So: all good to go no!

@dscho dscho merged commit ee3d397 into git-for-windows:main Feb 20, 2025
6 checks passed
dscho added a commit to jeremyd2019/git-for-windows-automation that referenced this pull request Feb 20, 2025
Git for Windows relies on MSYS2 for a lot of packages, but also provides
some packages of its own. Sometimes, Git for Windows even overrides
(or "shadows") MSYS2's packages. And sometimes Git for Windows needs to
stop overriding such packages.

This new GitHub workflow allows for stopping to override MSYS2 packages
by calling the shiny new `pacman-helper.sh quick_remove` functionality
added in git-for-windows/build-extra#597.

This will be used to clean up Git for Windows' Pacman repository after
the most recent batch of MSYS2 updates to its i686 repository, see
git-for-windows/git-sdk-32#39 (comment)
for full details.

Co-authored-by: Johannes Schindelin <[email protected]>
Signed-off-by: Jeremy Drake <[email protected]>
Signed-off-by: Johannes Schindelin <[email protected]>
@jeremyd2019 jeremyd2019 deleted the jeremyd2019-quick-remove branch February 20, 2025 18:06
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