Skip to content

mosml: update to latest upstream, make -dynlibs its subport, close a bunch of old tickets #19629

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
Merged
merged 4 commits into from
Aug 15, 2023
Merged

mosml: update to latest upstream, make -dynlibs its subport, close a bunch of old tickets #19629

merged 4 commits into from
Aug 15, 2023

Conversation

barracuda156
Copy link
Contributor

Closes: https://trac.macports.org/ticket/39662
Closes: https://trac.macports.org/ticket/58503
Fixes: https://trac.macports.org/ticket/39665
Fixes: https://trac.macports.org/ticket/39664

Well, hopefully it works with Clangs.

Description

Type(s)
  • bugfix
  • enhancement
  • security fix
Tested on

macOS 10.6
Xcode 3.2

Verification

Have you

  • followed our Commit Message Guidelines?
  • squashed and minimized your commits?
  • checked that there aren't other open pull requests for the same change?
  • referenced existing tickets on Trac with full URL?
  • checked your Portfile with port lint --nitpick?
  • tried existing tests with sudo port test?
  • tried a full install with sudo port -vst install?
  • tested basic functionality of all binary files?
  • checked that the Portfile's most important variants haven't been broken?

@barracuda156 barracuda156 marked this pull request as draft July 26, 2023 04:40
@barracuda156
Copy link
Contributor Author

Sorry, I forgot to rename patches :)

@barracuda156 barracuda156 force-pushed the mosml branch 3 times, most recently from 1aab8a2 to 15f70b3 Compare July 26, 2023 08:50
@barracuda156 barracuda156 marked this pull request as ready for review July 26, 2023 09:00
@barracuda156
Copy link
Contributor Author

Took forever to fix this for new compilers, but now it works both with GCC 12 and Clang.

kencu
kencu reviewed Jul 26, 2023
View reviewed changes
@barracuda156
Copy link
Contributor Author

@kencu @ryandesign How about we drop just one lib which has that bug which Ryan discovered earlier and a fix for which appears unsatisfactory? They are independent, and even upstream disables installation of some. (This is all in -dynlibs, not the compiler itself.)

@barracuda156
Copy link
Contributor Author

barracuda156 commented Jul 27, 2023 •
edited
Loading

@kencu @ryandesign Dropped mysql plugin, related patch and dependency. Please review.

@barracuda156 barracuda156 marked this pull request as draft July 27, 2023 11:26
@barracuda156 barracuda156 marked this pull request as ready for review July 27, 2023 11:55
@barracuda156
Copy link
Contributor Author

@mascguy @ryandesign Could someone please review this and, hopefully, merge?

@barracuda156
Copy link
Contributor Author

@mascguy @ryandesign Any corrections are welcome, or otherwise we could perhaps merge it already.

@barracuda156 barracuda156 mentioned this pull request Aug 13, 2023
Closed
12 tasks
reneeotten
reneeotten requested changes Aug 13, 2023
View reviewed changes
Copy link
Contributor

@reneeotten reneeotten left a comment

Choose a reason for hiding this comment

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

Did upstream comment on the patches you propose; did they agree on them so that we don't need to carry all of this forward?

@barracuda156
Copy link
Contributor Author

barracuda156 commented Aug 13, 2023 •
edited
Loading

Did upstream comment on the patches you propose; did they agree on them so that we don't need to carry all of this forward?

I have no control over the upstream. Header fixes were submitted in kfl/mosml#72
Issue with Clang builds (due to which I had to disable a module) is reported here: kfl/mosml#71

@mascguy
Copy link
Member

mascguy commented Aug 15, 2023

Sergey, I pushed some minor changes, mostly to correct the dependencies: While the subport needs those related to graphics, the base port only needs gmp. Also, in terms of those graphics dependencies, libpng and zlib are also needed. (Confirmed by both upstream's docs, as well as by checking the shared libs via otool -L.)

Let me know if you're fine with the changes. If so, I'll go ahead and merge.

Oh, and would you prefer to maintain the separate commits? Or would you like to squash this down to one? (I'm fine with either, so it's up to you.)

mascguy
mascguy approved these changes Aug 15, 2023
View reviewed changes
Copy link
Member

@mascguy mascguy left a comment

Choose a reason for hiding this comment

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

We're good to merge, pending final approval from Sergey.

@mascguy
Copy link
Member

mascguy commented Aug 15, 2023

Just for completeness, I tested building both mosml and mosml-dynlibs on 10.6 and 10.7. And both look good.

Great work Sergey!

@barracuda156
Copy link
Contributor Author

@mascguy Thank you very much for helping here! Let’s merge!
(As for commits, up to your preference, either is fine.)

@mascguy mascguy merged commit 825dcb7 into macports:master Aug 15, 2023
@barracuda156 barracuda156 deleted the mosml branch August 15, 2023 20:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@reneeotten reneeotten reneeotten requested changes

@mascguy mascguy mascguy approved these changes

+1 more reviewer

@kencu kencu kencu left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

maintainer: none type: bugfix type: enhancement type: update

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@barracuda156 @mascguy @reneeotten @kencu @macportsbot