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

i686-elf-grub 2.06, x86_64-elf-grub 2.06 (new formulae) #208658

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

rabarbra
Copy link

  • Have you followed the guidelines for contributing?
  • Have you ensured that your commits follow the commit style guide?
  • Have you checked that there aren't other open pull requests for the same formula update/change?
  • Have you built your formula locally with HOMEBREW_NO_INSTALL_FROM_API=1 brew install --build-from-source <formula>, where <formula> is the name of the formula you're submitting?
  • Is your test running fine brew test <formula>, where <formula> is the name of the formula you're submitting?
  • Does your build pass brew audit --strict <formula> (after doing HOMEBREW_NO_INSTALL_FROM_API=1 brew install --build-from-source <formula>)? If this is a new formula, does it pass brew audit --new <formula>?

@github-actions github-actions bot added the new formula PR adds a new formula to Homebrew/homebrew-core label Feb 23, 2025
Copy link
Contributor

Thanks for contributing to Homebrew! 🎉 It looks like you're having trouble with a CI failure. See our contribution guide for help. You may be most interested in the section on dealing with CI failures. You can find the CI logs in the Checks tab of your pull request.

@rabarbra
Copy link
Author

The CI pipeline is failing because of CI/Linux job.
The CI/Linux is failing because of this:

==> FAILED
Full audit --formula i686-elf-grub --online --new output
  i686-elf-grub
    * The homepage URL https://www.gnu.org/software/grub/ is not reachable
    * Binaries built for a non-native architecture were installed into i686-elf-grub's prefix.
      The offending files are:
        /home/linuxbrew/.linuxbrew/Cellar/i686-elf-grub/2.06/lib/i686-elf/grub/i386-pc/i686-elf-boot.image	(i386)
        /home/linuxbrew/.linuxbrew/Cellar/i686-elf-grub/2.06/lib/i686-elf/grub/i386-pc/i686-elf-boot_hybrid.image	(i386)
        /home/linuxbrew/.linuxbrew/Cellar/i686-elf-grub/2.06/lib/i686-elf/grub/i386-pc/i686-elf-cdboot.image	(i386)
        /home/linuxbrew/.linuxbrew/Cellar/i686-elf-grub/2.06/lib/i686-elf/grub/i386-pc/i686-elf-diskboot.image	(i386)
        /home/linuxbrew/.linuxbrew/Cellar/i686-elf-grub/2.06/lib/i686-elf/grub/i386-pc/i686-elf-kernel.exec	(i386)
        /home/linuxbrew/.linuxbrew/Cellar/i686-elf-grub/2.06/lib/i686-elf/grub/i386-pc/i686-elf-lnxboot.image	(i386)
        /home/linuxbrew/.linuxbrew/Cellar/i686-elf-grub/2.06/lib/i686-elf/grub/i386-pc/i686-elf-lzma_decompress.image	(i386)
        /home/linuxbrew/.linuxbrew/Cellar/i686-elf-grub/2.06/lib/i686-elf/grub/i386-pc/i686-elf-pxeboot.image	(i386)
        /home/linuxbrew/.linuxbrew/Cellar/i686-elf-grub/2.06/lib/i686-elf/grub/i386-pc/kernel.img	(i386)
  Error: 2 problems in 1 formula detected.

The CI pipeline is showing failures in the Linux job due to two issues:

  1. Homepage unreachable: This appears to be a false positive as https://www.gnu.org/software/grub/ is accessible and passes the check in the macOS jobs.

  2. Non-native architecture binaries: The audit is flagging i386 binaries in the package. However, this is expected and necessary behavior for this formula as it's a cross-compiler toolchain specifically designed to produce i386/i686 binaries. And brew audit is again passing macOS jobs, while prefix still contains non-native binaries.

Would appreciate guidance on how to best handle these CI checks.

@rabarbra rabarbra marked this pull request as ready for review February 23, 2025 10:59
@botantony
Copy link
Contributor

This is a common problem with gnu.org/nongnu.org homepages (#206757). Try to use savannah.gnu.org if it has a homepage for these programs

@botantony
Copy link
Contributor

botantony commented Feb 25, 2025

also can you please add at least one mirror for each formula? and why do you split one program into 2 formulae?

@rabarbra
Copy link
Author

rabarbra commented Feb 25, 2025

Thank you for quick reaction!

GRUB tools are used to create and work with bootable images for specific architectures.

Tools like grub-mkrescue, grub-mkimage, and grub-mkstandalone embed architecture-specific software during their compilation process. This is controlled through target-specific variables:

TARGET_OBJCOPY=#{target}-objcopy  
TARGET_STRIP=#{target}-strip
TARGET_NM=#{target}-nm
TARGET_RANLIB=#{target}-ranlib

I have these in my formulas.

There is no universal version of these tools that can produce bootable images for any architecture. Instead:

  • To create bootable images for i686 (32-bit) systems, you need i686-elf-grub-mkrescue compiled with tools for i686 architecture
  • To create bootable images for x86_64 (64-bit) systems, you need x86_64-elf-grub-mkrescue

When installing GRUB, you typically only install the tools for the specific architecture you're working with. This approach leverages the cross-compiler toolchains already available in Homebrew (i686-elf-binutils and x86_64-elf-binutils for example), allowing you to build boot images targeted at specific architectures regardless of your host system.

I will change homepage to https://savannah.gnu.org/projects/grub later today and add this mirror: https://mirrors.ocf.berkeley.edu/gnu/grub/grub-2.06.tar.xz.

But formulas will still not pass the Linux CI job because of brew audit detecting non-native binaries. Question is if we can omit this check. And why does it pass all the MacOs CIs, while there are still non-native binaries in formula's prefix. Why this test is different for MacOs and Linux?

@botantony
Copy link
Contributor

macOS CI is kinda strange (maybe because macOS uses Clang vs. Linux uses GCC) but I can say for sure that Homebrew would never ever accept non-native binaries

@rabarbra
Copy link
Author

But these binaries are not executables, they are images for specific architectures, used by GRUB tools to work with images and GRUB itself compiled for these architectures, needed to create images.

Problem is how brew audit defines if it is executable binary in Linux. It supposed to skip not executables (see this line: https://github.com/Homebrew/brew/blob/b4dbbf19a21352dd5ed0748e32e6c3b7f15e800c/Library/Homebrew/extend/os/linux/keg_relocate.rb#L76).

It happens like this in MacOs: ruby-macho gem just returns ":dunno" for these files (https://github.com/Homebrew/brew/blob/master/Library/Homebrew/os/mac/mach.rb).

But for Linux the test is following: https://github.com/Homebrew/brew/blob/b4dbbf19a21352dd5ed0748e32e6c3b7f15e800c/Library/Homebrew/os/linux/elf.rb#L91.
It only tests if it is elf file with ET_EXEC flag in ELF header's e_type field. But file like this is not necessary executable. GRUB's make install gives only read permissions for these files, not execute.

@gromgit
Copy link
Contributor

gromgit commented Feb 25, 2025

Try adding lib/i686-elf/** to audit_exceptions/mismatched_binary_allowlist.json.

Add GNU GRUB bootloader cross-compiler for i686-elf target. This formula
builds GRUB bootloader components specifically for i386/i686 architecture,
which is essential for OS development and bootloader customization.

Signed-off-by: Polina Simonenko <[email protected]>
Add GNU GRUB bootloader cross-compiler for x86_64-elf target. This formula
builds GRUB bootloader components specifically for x86_64 architecture,
which is essential for OS development and bootloader customization.

Signed-off-by: Polina Simonenko <[email protected]>
@github-actions github-actions bot added the automerge-skip `brew pr-automerge` will skip this pull request label Feb 25, 2025
sha256 "b79ea44af91b93d17cd3fe80bdae6ed43770678a9a5ae192ccea803ebb657ee1"
license "GPL-3.0-or-later"

depends_on "autoconf" => :build
Copy link
Member

Choose a reason for hiding this comment

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

Since the configure script is shipped, it should built without autoconf/automake

Copy link
Author

Choose a reason for hiding this comment

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

You are right, I'll remove these.


depends_on "autoconf" => :build
depends_on "automake" => :build
depends_on "gcc@11" => [:build, :test]
Copy link
Member

Choose a reason for hiding this comment

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

Does it really need GCC? And only one specific (outdated) version?

Copy link
Author

Choose a reason for hiding this comment

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

I added version 11 here for a reason, but I forgot why. I'll try to find this case again and take closer look, if this can be changed.

depends_on "automake" => :build
depends_on "gcc@11" => [:build, :test]
depends_on "help2man" => :build
depends_on "make" => :build
Copy link
Member

Choose a reason for hiding this comment

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

Does it only build with GNU make? It seems unlikely.

depends_on "texinfo" => :build
depends_on "x86_64-elf-binutils" => :build
depends_on "x86_64-elf-gcc" => [:build, :test]
depends_on "gettext"
Copy link
Member

Choose a reason for hiding this comment

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

Is this really a runtime dependence? Not build-only?

Copy link
Author

Choose a reason for hiding this comment

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

Yes, it is:

➜  ~ otool -L /opt/homebrew/bin/i686-elf-grub-file
/opt/homebrew/bin/i686-elf-grub-file:
        /opt/homebrew/opt/gettext/lib/libintl.8.dylib (compatibility version 13.0.0, current version 13.2.0)
        /System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation (compatibility version 150.0.0, current version 3208.0.0)
        /usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1351.0.0)
➜  ~ otool -L /opt/homebrew/bin/i686-elf-grub-glue-efi
/opt/homebrew/bin/i686-elf-grub-glue-efi:
        /opt/homebrew/opt/gettext/lib/libintl.8.dylib (compatibility version 13.0.0, current version 13.2.0)
        /System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation (compatibility version 150.0.0, current version 3208.0.0)
        /usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1351.0.0)
➜  ~ otool -L /opt/homebrew/bin/i686-elf-grub-mkimage
/opt/homebrew/bin/i686-elf-grub-mkimage:
        /opt/homebrew/opt/xz/lib/liblzma.5.dylib (compatibility version 12.0.0, current version 12.4.0)
        /opt/homebrew/opt/gettext/lib/libintl.8.dylib (compatibility version 13.0.0, current version 13.2.0)
        /System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation (compatibility version 150.0.0, current version 3208.0.0)
        /usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1351.0.0)

--libdir=#{lib}/#{target}
--with-platform=pc
--program-prefix=#{target}-
TARGET_CC=#{target}-gcc
Copy link
Member

Choose a reason for hiding this comment

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

I'm surprised because GNU tools are usually able to detect these target tools if the correct -build, -host and -target arguments (one or more of them, probably -target in this case) is provided.

@botantony
Copy link
Contributor

I still don't understand why we have to split this formula. Why not build 2 different targets in one formula?

@rabarbra
Copy link
Author

rabarbra commented Feb 26, 2025

I still don't understand why we have to split this formula. Why not build 2 different targets in one formula?

You can specify only single target for configure script, not multiple.
But my configuration for x86_64 is actually wrong: I forgot to change --wich-platform value to efi from pc. I'll also fix this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge-skip `brew pr-automerge` will skip this pull request new formula PR adds a new formula to Homebrew/homebrew-core
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants