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

Create universal builds on macos #612

Merged

Conversation

jeffhostetler
Copy link
Collaborator

Update config.mak.uname and GitHub workflow to create a DMG and PKG containing universal binaries rather than separate single-platform packages.

@jeffhostetler jeffhostetler self-assigned this Oct 24, 2023
@jeffhostetler
Copy link
Collaborator Author

The compile/link in the macos build is failing because libcurl is not fat. Homebrew does not ship universal binaries, but does ship skinny binaries for both platforms. We should look to add a step that takes them and lipo's them together before trying to compile git.

@dscho
Copy link
Member

dscho commented Nov 3, 2023

Update: I got things mostly working in https://github.com/microsoft/git/tree/jh/macos-universal, but there's now a problem with docbook (seems that the xmlcatalog command has not run correctly, that should have updated the mappings so that no web request is made for those xsl files): https://github.com/microsoft/git/actions/runs/6734747576/job/18306328885#step:5:778. Got "side-tracked" by -rc0 release work.

@dscho
Copy link
Member

dscho commented Nov 3, 2023

warning: failed to load external entity "http://docbook.sourceforge.net/release/xsl/current/html/docbook.xsl"
compilation error: file docbook.xsl line 3 element import
xsl:import : unable to load http://docbook.sourceforge.net/release/xsl/current/html/docbook.xsl

I've seen similar problems before (in Git for Windows) when the corresponding entry in /etc/xml/catalog was missing. These entries should be written by the xmlcatalog command, once the style sheets have been installed. I believe that they should be installed correctly and then this part should ensure that they are used. But maybe something goes wrong when xmlcatalog is run?

@dscho
Copy link
Member

dscho commented Nov 4, 2023

warning: failed to load external entity "http://docbook.sourceforge.net/release/xsl/current/html/docbook.xsl"
compilation error: file docbook.xsl line 3 element import
xsl:import : unable to load http://docbook.sourceforge.net/release/xsl/current/html/docbook.xsl

I've seen similar problems before (in Git for Windows) when the corresponding entry in /etc/xml/catalog was missing. These entries should be written by the xmlcatalog command, once the style sheets have been installed. I believe that they should be installed correctly and then this part should ensure that they are used. But maybe something goes wrong when xmlcatalog is run?

I fixed this via 757c884: it was picking up the wrong flavor of Homebrew (/usr/local/) because we hard-coded that preference, and when I explicitly installed the Intel flavor of Homebrew on an ARM runner, well, things broke.

@dscho
Copy link
Member

dscho commented Nov 4, 2023

Update: I got things to build, but the validation of the installer is failing because it cannot find the .pkg. Here is the corresponding run: https://github.com/microsoft/git/actions/runs/6754709732#artifacts

@dscho
Copy link
Member

dscho commented Nov 4, 2023

the validation of the installer is failing because it cannot find the .pkg

D'oh, it's still looking for *x86_64*.pkg, but the file name is git-2.42.0.vfs.0.0-universal-universal.pkg...

@dscho
Copy link
Member

dscho commented Nov 5, 2023

the validation of the installer is failing because it cannot find the .pkg

D'oh, it's still looking for *x86_64*.pkg, but the file name is git-2.42.0.vfs.0.0-universal-universal.pkg...

I managed to fix it (and while at it, extend the validation matrix to also test the result on macOS/ARM): https://github.com/microsoft/git/actions/runs/6759896633/job/18373034703

@dscho dscho changed the base branch from vfs-2.42.0 to tentative/vfs-2.43.0 November 5, 2023 09:02
jeffhostetler and others added 7 commits November 5, 2023 10:10
This commit puts `.dmg` on a diet by skipping dashed built-ins.

This uses the `SKIP_DASHED_BUILT_INS` on macOS to avoid even building
them. Otherwise, they would end up as non-hard-linked copies in the
`.dmg` file which makes that disk image rather unnecessarily large.

Signed-off-by: Jeff Hostetler <jeffhostetler@github.com>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
When building Git as a universal binary on macOS, the binary supports more than
one target architecture. This is a bit of a problem for the `HOST_CPU`
setting that is woefully unprepared for such a situation, as it wants to
show architecture hard-coded at build time.

In preparation for switching to universal builds, work around this by
special-casing `universal` and replacing it at run-time with the known
values `x86_64` or `arm64`.

Signed-off-by: Jeff Hostetler <jeffhostetler@github.com>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
This switches to universal builds, i.e. binaries that contain both
x86_64 and arm64 parts. That way, the same `.pkg`/`.dmg` can be used on
Intel as well as on ARM Macs.

To that end, we are installing the Intel flavor of Homebrew on a
macOS/ARM runner, install the dependencies both in the Intel and ARM
flavor of Homebrew, and then use `lipo` to concoct universal versions of
the dependencies. Since we do not ship those dependencies anyway, things
work magically in the installed setups as long as the dependencies are
present there, too.

Note that for this hack to work, we no longer want to override the
`PATH` by prefixing `/usr/local/bin/` because `/usr/local/` is the home
of Homebrew's Intel variant only, not of the ARM variant (which calls
`/opt/homebrew/` its home). And in any case, Homebrew's `bin/` directory
is in the `PATH` of the runners we use, no need to be explicit about it.

Signed-off-by: Jeff Hostetler <jeffhostetler@github.com>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
We configure the build using a `config.mak` file for better
debuggability, which means that we also need to copy the `config.mak`
file to the build directory within `payload/` so that the (re-)build
(`make ... payload`) works correctly.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Sometimes the Makefile process is too fast and the `hdiutil` command
fails with:

	hdiutil: create failed - Resource busy

A couple moments later that command would succeed, though. So let's just
try again after sleeping 5s.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Now that we're building universal binaries on macOS, let's verify them
on Intel _and_ on ARM runners.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Let's also verify that the correct CPU is reported in universal builds'
`git version --build-options`.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
@dscho dscho force-pushed the jeffhostetler/macos-universal-builds branch 2 times, most recently from 8ce9814 to eb881c1 Compare November 5, 2023 09:26
@dscho
Copy link
Member

dscho commented Nov 5, 2023

Okay, it works now (and I finally force-pushed a clean set of commits to this here PR). This workflow (which has only a couple of commits to reduce building to macOS) shows that it works, and that we're also nicely reducing the .dmg to a manageable size. @jeffhostetler could you please review?

@jeffhostetler
Copy link
Collaborator Author

Notes: testing the artifacts from #612 (comment)

  • the dmg and pkg are named "git-2.42.0.vfs.0.0-universal-universal.pkg" -- we should drop one of those words, but that can wait until later.
  • by dropping the duplicate dashed versions from the dmg/pkg, the size drops from 217MB to 47MB.

@jeffhostetler jeffhostetler merged commit dc61fa0 into tentative/vfs-2.43.0 Nov 6, 2023
79 checks passed
@jeffhostetler jeffhostetler deleted the jeffhostetler/macos-universal-builds branch November 6, 2023 16:12
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.

None yet

2 participants