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

mono 6.12.0.199 #166109

Merged
merged 2 commits into from
Apr 16, 2024
Merged

mono 6.12.0.199 #166109

merged 2 commits into from
Apr 16, 2024
+25 −228

Conversation

fel1x-developer
Copy link
Contributor

@fel1x-developer fel1x-developer commented Mar 14, 2024 •
edited

  • 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>?

@fel1x-developer
Copy link
Contributor Author

Closes #165618

@github-actions github-actions bot added python Python use is a significant feature of the PR or issue macos-only Formula depends on macOS labels Mar 14, 2024
@fel1x-developer fel1x-developer mentioned this pull request Mar 14, 2024
Closed
6 tasks
@fel1x-developer fel1x-developer changed the title mono 6.12.0.206 mono 6.12.0.199 Mar 14, 2024
carlocab
carlocab previously requested changes Mar 14, 2024
View reviewed changes
Copy link
Member

@carlocab carlocab left a comment

Choose a reason for hiding this comment

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

Thanks for the PR, but I don't think that restricting this to Intel-only is a good idea.

Formula/m/mono.rb Outdated
depends_on "cmake" => :build
depends_on "pkg-config" => :build

depends_on arch: :x86_64
Copy link
Member

Choose a reason for hiding this comment

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

I don't think we want to do this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Are we sure that mono will run on apple silicon macs in this way? Seems like they still have problem with running mono on arm. mono/#21092.

Copy link
Member

Choose a reason for hiding this comment

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

If we don't have a working mono, it will not be shipped on ARM. But this declares an explicit dependency on x86, which is false.

Copy link
Member

@cho-m cho-m Mar 15, 2024 •
edited

Choose a reason for hiding this comment

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

We were originally able to build mono on ARM due to Rosetta on bare metal machines. At that time, I also had to disable msbuild since I could not get the current version to build.

Currently, fsharp won't build on our runners and no idea if support is planned by mono.

One last alternative would be to also disable fsharp from both Linux and ARM. I think that just leaves behind the mono runtime. The main problem building fsharp is that it needs to bootstrap off of .NET 3 (way too old for both Apple Silicon support and OpenSSL 3 support).

Copy link
Member

Choose a reason for hiding this comment

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

The main problem building fsharp is that it needs to bootstrap off of .NET 3 (way too old for both Apple Silicon support and OpenSSL 3 support).

any issue tracker for it?

maybe let's disable fsharp? at least is better than nothing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

any issue tracker for it?

The development of mono is not very active anymore, and it seems that they are not interested in bumping the F# version for now.

maybe let's disable fsharp? at least is better than nothing.

I'll try this.

@fel1x-developer
Copy link
Contributor Author

I removed fsharp and msbuild (build dependency for fsharp), and the build suceeds on both platforms.

On both platforms, autoreconf is necessary to the -flat_namespace issue on mono (mono/mono#21257)

@chenrui333 chenrui333 added the ready to merge PR can be merged once CI is green label Apr 7, 2024
@chenrui333
Copy link
Member

LGTM

@fel1x-developer
Copy link
Contributor Author

Any updates?

SMillerDev
SMillerDev reviewed Apr 11, 2024
View reviewed changes
Copy link
Member

@SMillerDev SMillerDev left a comment

Choose a reason for hiding this comment

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

So what useful parts do we actually have left for users if there is no more: Linux, msbuild or fsharp?

Formula/m/mono.rb Outdated Show resolved Hide resolved
@fel1x-developer
Copy link
Contributor Author

fel1x-developer commented Apr 11, 2024 •
edited

So what useful parts do we actually have left for users if there is no more: Linux, msbuild or fsharp?

We can bring linux build back since openssl@1.1 for linux was required for fsharp.
The only change is the removal of fsharp. Msbuild is a dependency for building fsharp.

@github-actions github-actions bot removed the macos-only Formula depends on macOS label Apr 11, 2024
@SMillerDev SMillerDev requested a review from a team April 15, 2024 09:00
@MikeMcQuaid
Copy link
Member

Thanks @fel1x-developer!

@Homebrew/core if this builds and gets us onto a newer version: it's worth it. Let's merge and iterate.

@fel1x-developer
Copy link
Contributor Author

I tried 6.12.206, but it reports LineUnavailableException(see #158402). We can merge this for now and I'll open a new PR for a newer version.

@github-actions GitHub Actions
Copy link
Contributor

:shipit: @p-linnane has requested bottles to be published to this PR.

@github-actions github-actions bot added the CI-published-bottle-commits The commits for the built bottles have been pushed to the PR branch. label Apr 16, 2024
@BrewTestBot BrewTestBot added this pull request to the merge queue Apr 16, 2024
Merged via the queue into Homebrew:master with commit 8d3b35c Apr 16, 2024
14 checks passed
@fel1x-developer fel1x-developer deleted the mono branch April 16, 2024 17:24
@github-actions github-actions bot added the outdated PR was locked due to age label May 17, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 17, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@MikeMcQuaid MikeMcQuaid MikeMcQuaid approved these changes

@SMillerDev SMillerDev SMillerDev approved these changes

@github-actions github-actions[bot] github-actions approved these changes

@p-linnane p-linnane p-linnane approved these changes

@cho-m cho-m Awaiting requested review from cho-m

@carlocab carlocab Awaiting requested review from carlocab

@chenrui333 chenrui333 Awaiting requested review from chenrui333

Assignees
No one assigned
Labels
CI-published-bottle-commits The commits for the built bottles have been pushed to the PR branch. outdated PR was locked due to age python Python use is a significant feature of the PR or issue ready to merge PR can be merged once CI is green
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
8 participants
@fel1x-developer @chenrui333 @MikeMcQuaid @SMillerDev @cho-m @carlocab @p-linnane @BrewTestBot