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.

Sign up for GitHub

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

Jump to bottom

zig 0.9.1 #95485

Closed
wants to merge 1 commit into from
Closed

zig 0.9.1 #95485

wants to merge 1 commit into from

Conversation

messense
Copy link
Contributor

Created with brew bump-formula-pr.

@BrewTestBot BrewTestBot added the bump-formula-pr PR was created using `brew bump-formula-pr` label Feb 20, 2022
@carlocab
Copy link
Member

This release breaks formulae that build using Zig. See: #95160

@carlocab carlocab added CI-build-dependents-from-source Pass --build-dependents-from-source to brew test-bot. help wanted Task(s) needing PRs from the community or maintainers CI-no-fail-fast Continue CI tests despite failing GitHub Actions matrix builds. labels Feb 20, 2022
@messense
Copy link
Contributor Author

Saw that, I'll try to fix it.

@BrewTestBot BrewTestBot added the automerge-skip `brew pr-automerge` will skip this pull request label Feb 20, 2022
@messense
Copy link
Contributor Author

Looks like the fastfec formulae is fixable by using curl from Homebrew, shall I open a new PR to do that?

@carlocab
Copy link
Member

That doesn't seem like the right fix, as this is probably a regression in Zig. Can you open an issue there?

@messense
Copy link
Contributor Author

upstream also says install curl from Homebrew so I doubt it's a zig bug.

@messense
Copy link
Contributor Author

Anyway, here is the issue report: ziglang/zig#10946

@carlocab
Copy link
Member

I'm certain this is a regression in Zig. From the just-built fastfec:

==> brew linkage --cached fastfec
System libraries:
  /usr/lib/libSystem.B.dylib
  /usr/lib/libcurl.4.dylib
  /usr/lib/libpcre.0.dylib

This means that it's using Homebrew curl and Homebrew pcre headers but linking with the system libraries. Mixing libraries and headers of different versions is a bad idea and a recipe for disaster.

The existing bottle has

❯ brew linkage fastfec
System libraries:
  /usr/lib/libSystem.B.dylib
  /usr/lib/libcurl.4.dylib
Homebrew libraries:
  /usr/local/opt/pcre/lib/libpcre.1.dylib (pcre)

but doesn't have a dependency on Homebrew curl, so doesn't use mismatched headers to build.

cho-m
cho-m reviewed Feb 21, 2022
View reviewed changes
Formula/zig.rb Show resolved Hide resolved
@cho-m
Copy link
Member

cho-m commented Feb 21, 2022 •
edited
Loading

Note that the issue occurs with existing zig 0.9.0 bottle. It appears to work on previous zig 0.8.1, so breakage may be changes between those releases.

With zig 0.8.1, fastfec build uses sysroot and SDK paths (snippets of command):
/opt/homebrew/Cellar/zig/0.8.1_1/bin/zig clang -fno-caret-diagnostics -target aarch64-unknown-macosx-gnu -nostdinc -fno-spell-checking -isystem /opt/homebrew/Cellar/zig/0.8.1_1/lib/zig/include -isystem /Library/Developer/CommandLineTools/SDKs/MacOSX12.sdk/usr/include -Xclang ........... -isysroot /Library/Developer/CommandLineTools/SDKs/MacOSX12.sdk -iwithsysroot /usr/include -iwithsysroot /usr/local/include -iframeworkwithsysroot /Library/Frameworks -iframeworkwithsysroot /System/Library/Frameworks ...........

With zig 0.9+, there is no sysroot or SDK paths
/opt/homebrew/Cellar/zig/0.9.0/bin/zig clang -fno-caret-diagnostics -target aarch64-unknown-macosx-gnu -nostdinc -fno-spell-checking -isystem /opt/homebrew/Cellar/zig/0.9.0/lib/zig/include -isystem /opt/homebrew/Cellar/zig/0.9.0/lib/zig/libc/include/aarch64-macos.12-gnu -isystem /opt/homebrew/Cellar/zig/0.9.0/lib/zig/libc/include/any-macos.12-any -isystem /opt/homebrew/Cellar/zig/0.9.0/lib/zig/libc/include/any-macos-any .......... -I /opt/homebrew/Cellar/pcre/8.45/include -isystem /usr/include /System/Library/Frameworks .........

@carlocab
Copy link
Member

Ah, yes, I did build this locally with 0.9 to test but didn't notice the build failure since I had Homebrew curl installed. But I did notice the failure to link with Homebrew pcre.

cho-m
cho-m reviewed Feb 21, 2022
View reviewed changes
Formula/fastfec.rb Outdated
args << if OS.linux?
Formula["curl"].opt_prefix
else
MacOS.sdk_path_if_needed
Copy link
Member

@cho-m cho-m Feb 21, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Probably can get it to build by appending /"usr" to SDK path since it is doing -isystem <input>/"include".

@carlocab
Copy link
Member

The fastfec failure is already pre-existing, so let's ignore it for now.

@carlocab carlocab removed the CI-build-dependents-from-source Pass --build-dependents-from-source to brew test-bot. label Feb 21, 2022
@BrewTestBot BrewTestBot removed the automerge-skip `brew pr-automerge` will skip this pull request label Feb 21, 2022
@carlocab carlocab mentioned this pull request Feb 21, 2022
Open
@carlocab carlocab mentioned this pull request Feb 21, 2022
Open
@BrewTestBot
Copy link
Member

🤖 A scheduled task has triggered a merge.

@messense messense deleted the bump-zig-0.9.1 branch February 25, 2022 11:26
@cho-m cho-m mentioned this pull request Feb 26, 2022
Closed
@github-actions github-actions bot added the outdated PR was locked due to age label Mar 28, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 28, 2022
@chenrui333 chenrui333 removed the help wanted Task(s) needing PRs from the community or maintainers label Jul 16, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@cho-m cho-m cho-m left review comments

@carlocab carlocab carlocab approved these changes

Assignees
No one assigned
Labels
bump-formula-pr PR was created using `brew bump-formula-pr` CI-no-fail-fast Continue CI tests despite failing GitHub Actions matrix builds. outdated PR was locked due to age
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@messense @carlocab @cho-m @BrewTestBot @chenrui333