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

git-artifacts: add ARM64 artifacts #3017

Merged
merged 1 commit into from
Feb 13, 2021

Conversation

dennisameling
Copy link

@dennisameling dennisameling commented Feb 4, 2021

Adds the ARM64 artifacts to the git-artifacts GH Actions workflow 🚀

Full run: https://github.com/dennisameling/git/actions/runs/537829979

One thing I noticed is that the ARM64 artifacts are significantly larger than the others. There might be some things we can enable on the MSVC-size to have it generate smaller binaries, but I'm not familiar enough with that toolchain to know how to do that.

@dennisameling dennisameling force-pushed the add-arm64-artifacts branch 3 times, most recently from 1fffb92 to 6bb1f51 Compare February 4, 2021 21:12
Copy link
Member

@dscho dscho left a comment

Choose a reason for hiding this comment

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

I left a couple suggestions how we could potentially fix this, but I think we first need that build-extra PR.

.github/workflows/git-artifacts.yml Outdated Show resolved Hide resolved
.github/workflows/git-artifacts.yml Outdated Show resolved Hide resolved
.github/workflows/git-artifacts.yml Outdated Show resolved Hide resolved
.github/workflows/git-artifacts.yml Outdated Show resolved Hide resolved
.github/workflows/git-artifacts.yml Outdated Show resolved Hide resolved
.github/workflows/git-artifacts.yml Outdated Show resolved Hide resolved
@dennisameling dennisameling force-pushed the add-arm64-artifacts branch 3 times, most recently from 5da7f80 to 0f8e66c Compare February 6, 2021 21:52
@dennisameling dennisameling requested a review from dscho February 6, 2021 22:24
@dennisameling
Copy link
Author

dennisameling commented Feb 6, 2021

@dscho applied your suggestions - hope we're good to go now! 😊

Successful run: https://github.com/dennisameling/git/actions/runs/543959657
Demo release (manual): https://github.com/dennisameling/git/releases/tag/2.30.0-beta3

@dennisameling
Copy link
Author

@dscho fixed - build-arm64 is now also using git-sdk-64-build-installers. Last time I checked, I forgot to set MSGFMT_EXE to the correct folder 😅

-DMSGFMT_EXE=`pwd`/git-sdk-64-build-installers/mingw64/bin/msgfmt.exe

@dscho
Copy link
Member

dscho commented Feb 7, 2021

Hmm. If we built with NO_GETTEXT (or the CMake equivalent), we would not even need msgfmt.exe. Meaning: we could totally save us the trouble to download any version of the Git for Windows SDK.

@dennisameling
Copy link
Author

dennisameling commented Feb 7, 2021

Like this? Looks like that works as well: https://github.com/dennisameling/git/runs/1849261988?check_suite_focus=true

Don't we need to do this in the other, already existing workflow then (vs-build)?

@dscho
Copy link
Member

dscho commented Feb 8, 2021

Like this?

Yes, please!

Don't we need to do this in the other, already existing workflow then (vs-build)?

Very good point! We even need it in git/git, not only in git-for-windows/git.

@dennisameling
Copy link
Author

Alright, just squashed the commits. Think we're good to go here, @dscho!

Actions run: https://github.com/dennisameling/git/actions/runs/550766092

Very good point! We even need it in git/git, not only in git-for-windows/git.

Will do this later today 👍🏼

@dennisameling
Copy link
Author

dennisameling commented Feb 11, 2021

Will do this later today 👍🏼

Done in #3040, sorry for the delay!

@@ -388,7 +448,8 @@ jobs:
run: |
& .\git-sdk-${{matrix.arch.bitness}}-build-installers\usr\bin\bash.exe -lc @"
set -x
/usr/src/build-extra/please.sh make_installers_from_mingw_w64_git --version=`$(cat pkg-${{matrix.arch.name}}/ver) -o artifacts --${{matrix.artifact.name}} --pkg=pkg-${{matrix.arch.name}}/mingw-w64-${{matrix.arch.name}}-git-[0-9]*.tar.xz --pkg=pkg-${{matrix.arch.name}}/mingw-w64-${{matrix.arch.name}}-git-doc-html-[0-9]*.tar.xz &&
[[ \"${{matrix.arch.arm64}}\" = true ]] && ARM64=\"--include-arm64-artifacts=\"`$PWD/arm64\"\" || ARM64=
Copy link
Member

Choose a reason for hiding this comment

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

I think those quotes should not be escaped, right?

Copy link
Member

Choose a reason for hiding this comment

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

Oh, forget what I said, this is inside a PowerShell step. But that means that there needs to be extra quoting around the path, like so:

Suggested change
[[ \"${{matrix.arch.arm64}}\" = true ]] && ARM64=\"--include-arm64-artifacts=\"`$PWD/arm64\"\" || ARM64=
[[ \"${{matrix.arch.arm64}}\" = true ]] && ARM64=\"--include-arm64-artifacts=\\\"`$PWD/arm64\\\"\" || ARM64=

Copy link
Author

Choose a reason for hiding this comment

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

Interesting. It worked without the quotes, but with the quotes it breaks the installer build: https://github.com/dennisameling/git/runs/1881018607?check_suite_focus=true

Looks like we need to tweak the release.sh script in build-extra/installer a bit. Will have a look tomorrow.

Copy link
Member

@dscho dscho left a comment

Choose a reason for hiding this comment

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

It looks really neat now, just the one quoting fix (which does not matter that much in practice right now because $PWD expands to a path without spaces).

Adds ARM64 artifacts to the git-artifacts GitHub Action workflow.

Signed-off-by: Dennis Ameling <dennis@dennisameling.com>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
@dscho dscho force-pushed the add-arm64-artifacts branch from d013398 to ab950ae Compare February 12, 2021 13:15
@dscho dscho merged commit a1dbd69 into git-for-windows:main Feb 13, 2021
@dennisameling dennisameling deleted the add-arm64-artifacts branch February 14, 2021 14:20
dscho added a commit that referenced this pull request Feb 15, 2021
dscho added a commit that referenced this pull request Feb 15, 2021
dscho added a commit that referenced this pull request Feb 15, 2021
dscho added a commit that referenced this pull request Feb 15, 2021
git-for-windows-ci pushed a commit that referenced this pull request Aug 2, 2021
dscho added a commit to dscho/git that referenced this pull request Aug 3, 2021
…artifacts

git-artifacts: add ARM64 artifacts
git-for-windows-ci pushed a commit that referenced this pull request Aug 3, 2021
dscho added a commit that referenced this pull request Aug 3, 2021
dscho added a commit that referenced this pull request Aug 3, 2021
dscho added a commit that referenced this pull request Aug 3, 2021
git-for-windows-ci pushed a commit that referenced this pull request Aug 4, 2021
git-for-windows-ci pushed a commit that referenced this pull request Aug 4, 2021
dscho added a commit that referenced this pull request Aug 4, 2021
dscho added a commit that referenced this pull request Aug 4, 2021
git-for-windows-ci pushed a commit that referenced this pull request Aug 4, 2021
git-for-windows-ci pushed a commit that referenced this pull request Aug 4, 2021
dscho added a commit that referenced this pull request Aug 6, 2021
git-for-windows-ci pushed a commit that referenced this pull request Aug 6, 2021
git-for-windows-ci pushed a commit that referenced this pull request Aug 6, 2021
dscho added a commit that referenced this pull request Aug 11, 2021
dscho added a commit that referenced this pull request Aug 13, 2021
dscho added a commit that referenced this pull request Aug 18, 2021
dscho added a commit that referenced this pull request Aug 23, 2021
git-for-windows-ci pushed a commit that referenced this pull request Aug 24, 2021
dscho added a commit that referenced this pull request Aug 31, 2021
git-for-windows-ci pushed a commit that referenced this pull request Sep 8, 2021
dscho added a commit that referenced this pull request Sep 13, 2021
dscho added a commit that referenced this pull request Oct 4, 2021
git-for-windows-ci pushed a commit that referenced this pull request Oct 12, 2021
git-for-windows-ci pushed a commit that referenced this pull request Oct 12, 2021
dscho added a commit that referenced this pull request Oct 13, 2021
git-for-windows-ci pushed a commit that referenced this pull request Oct 13, 2021
git-for-windows-ci pushed a commit that referenced this pull request Oct 13, 2021
git-for-windows-ci pushed a commit that referenced this pull request Oct 13, 2021
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