-
Notifications
You must be signed in to change notification settings - Fork 605
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
git-extra: publish both with and without MINGW prefix #455
git-extra: publish both with and without MINGW prefix #455
Conversation
arch=('any') | ||
mingw_arch=('mingw32' 'mingw64' 'clangarm64') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is consistent with how the MSYS2 team works. For example: https://github.com/msys2/MINGW-packages/blob/c33af64b6b5ac989b2611da1aca6b5a6fb77c618/mingw-w64-curl/PKGBUILD#L11-L12
0ce99c8
to
e442fe2
Compare
pkgrel=1 | ||
pkgdesc="Git for Windows extra files" | ||
arch=('i686' 'x86_64') | ||
arch=('any') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the idea of publishing the packages concurrently.
However, we will need to publish the i686
and x86_64
variants of git-extra
, not the any
variant, because Git for Windows' tooling is unable to handle any package without mingw-w64-
prefix and without the CPU architecture in its suffix.
I also wondered whether there is a better way wherein the git-extra
package would be automatically uninstalled and the mingw-w64-git-extra
package would be installed. On a first read, https://wiki.archlinux.org/title/PKGBUILD#replaces would be the correct solution there. But I fear that won't work for us because we will provide mingw-w64-git-extra
both in the git-for-windows
and in the git-for-windows-mingw32
repository. And if there are two packages that replace the same package, and those two packages conflict with each other, I wonder whether Pacman does the right thing.
But then, we could exclude that package specifically from git-for-windows-mingw32
(and from git-for-windows-mingw64
) so that only one or the other is available, depending whether you run Pacman in git-sdk-32
or git-sdk-64
.
Having said all that, I think you're right and we can only do that once we have taken care of all of our tooling, for which we need a transitional period where we build both git-extra
and mingw-w64-git-extra
.
Back to the arch
problem: we cannot specify arch
conditional on the pkgname
, sadly, as it is a global.
So I fear that we will have to either put in a hack, or copy the PKGBUILD
file whole-sale. The hack would look somewhat like this:
if test -z "$BUILD_MINGW_W64_GIT_EXTRA"
then
pkgname=$_realname
arch=('i686' 'x86_64')
else
pkgname=$MINGW_PACKAGE_PREFIX-$_realname
arch=('any')
fi
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch! Fixed by simply overriding the arch
in package_git-extra()
🎉 I'm really impressed by the flexibility we have in those PKGBUILD
files.
The resulting package names look good:
git-extra-1.1.616.3f26b1b0c-1-x86_64.pkg.tar.zst
mingw-w64-x86_64-git-extra-1.1.616.3f26b1b0c-1-any.pkg.tar.zst
Until now, the git-extra package was published under that exact name. However, it's _actually_ a MINGW package which installs things both in e.g. /mingw64/bin and /etc/profile.d. This is problematic for GfW's upcoming arm64 support, which uses the x64 MSYS shell, so it'll need its own package. At the same time, packages for multiple architectures can't be installed side-by-side like other MINGW-packages, as this one writes to shared folders like /etc/profile.d as well. Therefore we mark them as conflicting each other. With this change, we'll publish the following packages side by side, so the repos will look like this: - git-for-windows/git-extra (old) - git-for-windows-mingw32/git-extra (old) - git-for-windows/mingw-w64-x86_64-git-extra (new) - git-for-windows-mingw32/mingw-w64-i686-git-extra (new) - git-for-windows-clangarm64/mingw-w64-aarch64-git-extra (new) Publishing side by side allows us to gradually update other parts of GfW to start using the new package names moving forward. Signed-off-by: Dennis Ameling <dennis@dennisameling.com>
Signed-off-by: Dennis Ameling <dennis@dennisameling.com>
3e5aebe
to
7197b4c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very nice! I confirmed (by downloading the PR build artifacts and inspecting them locally) that the packages are built correctly, i.e. their file name and their .PKGINFO
files contain the expected architecture. That's much cleaner than the hack I feared we need!
/deploy git-extra EDIT: I guess I should not have tried to avoid a regular expression in git-for-windows/gfw-helper-github-app@1cf6d04... |
…ring Apparently it is not, as I got this response for git-for-windows/build-extra#455 (comment): TypeError: Cannot read properties of undefined (reading 'startsWith') Let's just do the same as for the `/mention` command: use regular expressions. If we use only greedy expressions, we should not run into the same issues as caused the scary StackOverflow outage described in https://stackstatus.tumblr.com/post/147710624694/outage-postmortem-july-20-2016. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
/deploy git-extra Okay, I think I finally got it. Let's try this again, it was so much fun. |
Currently, we won't see any stack trace in the error output, e.g. while diagnosing the problem we encountered in git-for-windows/build-extra#455 (comment). Let's make that easier to pinpoint. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Still trying to figure out why git-for-windows/build-extra#455 (comment) failed... Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
/deploy git-extra The workflow run was started |
That one worked! (After kicking of a re-run after |
There we go! Thank you so much!!
|
@dennisameling should we use the On a totally unrelated topic: how about moving https://github.com/dennisameling/git-sdk-arm64/ to the |
That makes sense. I'll have a look at that tomorrow or on Friday.
Sounds good to me! I tried sending it over to the Update: I tried transferring it to your personal user but that also didn't work, as my repo is a fork of https://github.com/git-for-windows/git-sdk-64: Please feel free to take whatever action you deem necessary to port it over to the |
Unfortunately, I do not have full access, but I am rather convinced that I would have run into the same troubles. Instead, I re-forked |
Following up on this discussion.
Until now, the
git-extra
package was published under that exact name. However, it's actually a MINGW package which installs things both in e.g./mingw64/bin
and/etc/profile.d
. This is problematic for GfW's upcoming arm64 support, which uses the x64 MSYS shell, so it'll need its own package. At the same time, packages for multiple architectures can't be installed side-by-side like other MINGW-packages, as this one writes to shared folders like/etc/profile.d
as well. Therefore we mark them as conflicting each other.With this change, we'll publish the following packages side by side, so the repos will look like this:
git-for-windows/git-extra
(old)git-for-windows-mingw32/git-extra
(old)git-for-windows/mingw-w64-x86_64-git-extra
(new)git-for-windows-mingw32/mingw-w64-i686-git-extra
(new)git-for-windows-clangarm64/mingw-w64-aarch64-git-extra
(new)Publishing side by side allows us to gradually update other parts of GfW to start using the new package names moving forward.
Test results
After building with
MINGW_ARCH=mingw64 makepkg-mingw
, two files were created:git-extra-1.1.614.d551cf0cc-1-any.pkg.tar.zst
(legacy package:git-extra
)mingw-w64-x86_64-git-extra-1.1.614.d551cf0cc-1-any.pkg.tar.zst
(new package:mingw-w64-git-extra
)Assuming that
git-extra
is already installed (e.g. when working fromgit-sdk-64
), let's test these packages.git-extra
package still works as normal after the changes in this PR:mingw-w64-git-extra
correctly indicates that it conflicts withgit-extra
and Pacman suggests to remove the old package first:git-extra
, is indeed provided by the new package:git-extra
if we want? Yes, we can:conflict
logic:Suggested strategy
git-extra
instead ofmingw-w64-git-extra
git-extra
package