homebrew-core icon indicating copy to clipboard operation
homebrew-core copied to clipboard

nesfab 1.6 (new formula)

Open vtbassmatt opened this issue 8 months ago • 18 comments

  • [x] Have you followed the guidelines for contributing?
  • [x] Have you ensured that your commits follow the commit style guide?
  • [x] Have you checked that there aren't other open pull requests for the same formula update/change?
  • [x] 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?
  • [x] Is your test running fine brew test <formula>, where <formula> is the name of the formula you're submitting?
  • [x] 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>?

Adds NESFab. NESFab is a programming language/compiler for creating NES games. It qualifies for inclusion in Homebrew because it's maintained (v1.6 came out within the last week), is known (689 GitHub stars), is used (there has been recent GitHub and Discord activity related to macOS builds), and has a homepage.

vtbassmatt avatar Mar 16 '25 20:03 vtbassmatt

Thanks for contributing to Homebrew! :tada: 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.

github-actions[bot] avatar Mar 16 '25 21:03 github-actions[bot]

Hi Homebrew maintainers. I'm feeling stuck after multiple rounds of searching, editing, and resubmitting. I feel like I'm missing something simple.

This project uses a C++20 feature (std::lexicographical_compare_three_way) which Clang apparently didn't fully support until libc++ 17. I searched discussions and found https://github.com/orgs/Homebrew/discussions/5977 which led to https://github.com/Homebrew/homebrew-core/pull/188332, which I aped in my formula:

# ...
  on_macos do
    depends_on "llvm" => :build if DevelopmentTools.clang_build_version <= 1599
  end
# ...
  fails_with :clang do
    build 1599
    cause "Missing std::lexicographical_compare_three_way"
  end

However, the build still fails on macOS 13. (It builds on macOS 15 because I happened to get a successful run there. The PR has always failed before trying macOS 14, so I'm not confident whether it succeeds or fails there.)

I see two paths forward:

  • Correct my use of depends_on / fails_with to ensure we get a version of libc++ that works; I need someone with more Clang expertise to point me in the right direction here.
  • Remove macOS 13 (and possibly 14?) from the set of supported platforms. Upstream wouldn't build locally (by default) on that/those platforms, so this might also be acceptable.

Thanks for any help you can offer!

vtbassmatt avatar Mar 17 '25 00:03 vtbassmatt

@vtbassmatt I think this error happens because it still uses system Clang. Set CXX to the full path to Homebrew's LLVM Clang if clang_build_version is less that 1600

botantony avatar Mar 17 '25 00:03 botantony

@vtbassmatt I think this error happens because it still uses system Clang. Set CXX to the full path to Homebrew's LLVM Clang if clang_build_version is less that 1600

Thank you! I see now that the PR I copied from was using CXX=#{ENV.cxx} and I was not. I've kicked another build to see if this does it.

vtbassmatt avatar Mar 17 '25 00:03 vtbassmatt

  Failed to execute /opt/homebrew/opt/llvm/bin/clang++ -w -Os -std=c++20 -pthread -Wno-unused-parameter -Wno-unknown-warning-option -Wno-narrowing -Wno-missing-field-initializers -Wno-missing-braces -Wno-unused-command-line-argument -fmax-errors=3 -ftemplate-depth=100 -pipe -Isrc -DVERSION="1.6" -DGIT_COMMIT="da18a43d-homebrew" -DNDEBUG -Wno-unused-variable -c -o obj/token.o src/token.cpp -isysroot/Library/Developer/CommandLineTools/SDKs/MacOSX13.sdk --sysroot=/Library/Developer/CommandLineTools/SDKs/MacOSX13.sdk -isystem/opt/homebrew/include -isystem/Library/Developer/CommandLineTools/SDKs/MacOSX13.sdk/System/Library/Frameworks/OpenGL.framework/Versions/Current/Headers -I/opt/homebrew/opt/icu4c@77/include -I/opt/homebrew/opt/llvm@16/include -fdebug-prefix-map=/private/tmp/nesfab-20250316-8709-5r20tf/nesfab-da18a43dc7b941cc4c56c949303aa37633fdc1b0=.
  
  Xcode and/or the CLT appear to be misconfigured. Try one or both of the following:
    xcodebuild -license
    sudo xcode-select --switch /path/to/Xcode.app

SMillerDev avatar Mar 17 '25 08:03 SMillerDev

  Failed to execute /opt/homebrew/opt/llvm/bin/clang++ -w -Os -std=c++20 -pthread -Wno-unused-parameter -Wno-unknown-warning-option -Wno-narrowing -Wno-missing-field-initializers -Wno-missing-braces -Wno-unused-command-line-argument -fmax-errors=3 -ftemplate-depth=100 -pipe -Isrc -DVERSION="1.6" -DGIT_COMMIT="da18a43d-homebrew" -DNDEBUG -Wno-unused-variable -c -o obj/token.o src/token.cpp -isysroot/Library/Developer/CommandLineTools/SDKs/MacOSX13.sdk --sysroot=/Library/Developer/CommandLineTools/SDKs/MacOSX13.sdk -isystem/opt/homebrew/include -isystem/Library/Developer/CommandLineTools/SDKs/MacOSX13.sdk/System/Library/Frameworks/OpenGL.framework/Versions/Current/Headers -I/opt/homebrew/opt/icu4c@77/include -I/opt/homebrew/opt/llvm@16/include -fdebug-prefix-map=/private/tmp/nesfab-20250316-8709-5r20tf/nesfab-da18a43dc7b941cc4c56c949303aa37633fdc1b0=.
  
  Xcode and/or the CLT appear to be misconfigured. Try one or both of the following:
    xcodebuild -license
    sudo xcode-select --switch /path/to/Xcode.app

Yep, a new hell 😁 because a different thing breaks when I use llvm@latest, so I’m trying to use llvm@16 to match what works on macOS 14 & 15. I had to go to bed so didn’t get a chance to troubleshoot this one last night.

vtbassmatt avatar Mar 17 '25 09:03 vtbassmatt

😭 OK, I'm crying uncle. I cannot seem to get this building on macOS 13.

  1. It builds with gcc, but then doesn't link because its dependency (Boost) was built with Clang
    • Possible solution: build Boost myself using GCC (and link statically). This is at odds with https://docs.brew.sh/Acceptable-Formulae#stuff-that-requires-vendored-versions-of-homebrew-formulae. The purity-vs-usefulness distinction offers some leeway to do this, but getting Boost building feels both heavyweight and yak-shave-y.
  2. It doesn't build with the macOS 13 default Clang because std::lexicographical_compare_three_way wasn't part of that libc++
    • No solution found: I don't think upstream will accept removing this call. It's part of the C++20 standard, and apparently offers significant runtime savings. Older Clang was not fully implementing the standard yet.
  3. It doesn't build with depends_on: 'llvm' (which is @19) because that libc++ removed char_traits<unsigned char>
    • Possible solution: I'm working with upstream to get this fixed. There may still be an onion-peeling exercise to see what else has dropped/changed between Xcode's LLVM on Sequoia and bleeding-edge LLVM.
  4. It doesn't build with depends_on: 'llvm@18' (or 17 or 16) because every compiler invocation fails with:
Xcode and/or the CLT appear to be misconfigured. Try one or both of the following:
  xcodebuild -license
  sudo xcode-select --switch /path/to/Xcode.app"

Number 4 perplexes me the most. It actually seems like the correct path forward, if someone can help me understand what I need to do in the Homebrew CI to get this fixed.

  • xcodebuild -license requires user-at-keyboard (I think? I couldn't figure out how to do it headless).
  • I don't know where to tellxcode-select to look for some different Xcode.
  • I tried adding the LDFLAGS suggested in https://formulae.brew.sh/formula/llvm@18, but that didn't fix anything.

Help, please 🙏

vtbassmatt avatar Mar 17 '25 12:03 vtbassmatt

@vtbassmatt instead of LDFLAGS try to use this:

ENV.prepend_path "HOMEBREW_LIBRARY_PATHS", Formula["llvm@18"].opt_lib/"c++"

botantony avatar Mar 17 '25 13:03 botantony

@vtbassmatt instead of LDFLAGS try to use this:

ENV.prepend_path "HOMEBREW_LIBRARY_PATHS", Formula["llvm@18"].opt_lib/"c++"

Thanks. Gave it a try, and it still failed in the same way. I've cribbed some stuff from the llvm@18 test stanza that I'm trying now:

      toolchain_path = "/Library/Developer/CommandLineTools"
      cpp_base = (MacOS.version >= :big_sur) ? MacOS::CLT.sdk_path : toolchain_path

      system "make", "GIT_COMMIT=#{git_sha}-homebrew", "CXX=#{ENV.cxx}",
             "-isysroot", MacOS::CLT.sdk_path,
             "-isystem", "#{cpp_base}/usr/include/c++/v1",
             "-isystem", "#{MacOS::CLT.sdk_path}/usr/include",
             "-isystem", "#{toolchain_path}/usr/include",
             "release"

though I'm not sure what Make will do with those -isys* lines.

vtbassmatt avatar Mar 17 '25 14:03 vtbassmatt

It doesn't build with the macOS 13 default Clang because std::lexicographical_compare_three_way https://github.com/llvm/llvm-project/issues/68591

Seems like the easiest way is declaring that it needs at least macOS 14 to build.

SMillerDev avatar Mar 17 '25 14:03 SMillerDev

It doesn't build with the macOS 13 default Clang because std::lexicographical_compare_three_way llvm/llvm-project#68591

Seems like the easiest way is declaring that it needs at least macOS 14 to build.

That's the conclusion I've arrived at as well 🤣. I'm going to try dropping Ventura before I pull any more hair out.

vtbassmatt avatar Mar 17 '25 14:03 vtbassmatt

Appreciate everyone's help getting here! Yup, it turned out that I just needed to say "so long" to Ventura / macOS 13. It builds on 14+.

vtbassmatt avatar Mar 17 '25 15:03 vtbassmatt

@SMillerDev sorry to bother you - do I need to do anything else to have this contribution accepted?

vtbassmatt avatar Mar 23 '25 19:03 vtbassmatt

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

github-actions[bot] avatar Apr 14 '25 13:04 github-actions[bot]

Not stale. Waiting until upstream does a point release that I can rebase to.

vtbassmatt avatar Apr 14 '25 14:04 vtbassmatt

upstream has made a 1.6_mac release, https://github.com/pubby/nesfab/releases/tag/v1.6_mac?

chenrui333 avatar May 30 '25 03:05 chenrui333

upstream has made a 1.6_mac release, https://github.com/pubby/nesfab/releases/tag/v1.6_mac?

Unfortunately it didn’t have everything needed to land a working Homebrew package.

Enough time has elapsed now that perhaps the maintainer can be persuaded to do another point release. I was waiting to see if anyone had more thoughts on https://github.com/Homebrew/homebrew-core/pull/211113#discussion_r2009923395 though.

vtbassmatt avatar May 30 '25 10:05 vtbassmatt

Enough time has elapsed now that perhaps the maintainer can be persuaded to do another point release.

yeah, probably, also better to move the installation process into the upstream as well (like one goal to install both flavors)

chenrui333 avatar May 30 '25 14:05 chenrui333

This is ready for Homebrew maintainer review. Thanks for your patience while I waited for upstream to release a new version with all the right fixes.

vtbassmatt avatar Aug 13 '25 19:08 vtbassmatt

Is this formula acceptable now? Thanks!

vtbassmatt avatar Sep 21 '25 18:09 vtbassmatt

There is a 1.8 release available. I would prefer to check if game.nes runs on a NES emulator (like we do with interpreters or compilers), but I guess it is not that simple. Just asserting it exists should be enough. In general it looks good to me, let's see what other maintainers think

botantony avatar Oct 04 '25 13:10 botantony

Needs to also be rebased as the bottles are out-of-date (will have broken linkage to Boost since 1.87.0 -> 1.88.0). Can be done at same time as updating formula to 1.8 version.

cho-m avatar Oct 04 '25 17:10 cho-m

Thanks for all the help trying to get this over the hump. Fighting random issues out of my control stopped being fun a long time ago, and that goes double for a platform I don't care about (Linux).

vtbassmatt avatar Oct 12 '25 15:10 vtbassmatt