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

swi-prolog-devel: Updated to version 9.1.17 #20918

Merged
merged 1 commit into from
Oct 17, 2023

Conversation

JanWielemaker
Copy link
Contributor

Description

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

macOS 13.4.1 22F770820d arm64
Xcode 14.3.1 14E300c

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?

@reneeotten reneeotten merged commit fa78046 into macports:master Oct 17, 2023
3 checks passed
@reneeotten
Copy link
Contributor

@JanWielemaker I noticed that the build fails on macOS < 10.10 (see for example, this log).
A quick glance shows that the build adds -std=gnu11 suggesting that it requires a compiler that understands the 2011 standard - correct? However you do not explicitly request a compiler that supports this in the Portfile, that can be done with compiler.c_standard and compiler.cxx_standard (see here).

Perhaps doing so would allow the package to build for older systems as well? Or is there another reason from the linked log file that would prohibit this anyway?

@JanWielemaker
Copy link
Contributor Author

Thanks for the comments.

A quick glance shows that the build adds -std=gnu11 suggesting that it requires a compiler that understands the 2011 standard - correct?

Yes. That is not version 9.1.17 though, but for quite some time.

Perhaps doing so would allow the package to build for older systems as well?

No. C11 is really required. The code base notably uses the C11 Generic construct. This made the code simpler and more robust. Note that the C++ binding requires C++-17 (which we could ditch if not available) and the (new) Python binding requires thead local storage. If there are strong reasons not to demand TLS we could of course replace that by pthread primitives as we also do for the core (we use native TLS when available and pthread otherwise)

P.s. Ideally we'd use GCC (the real one) and CMAKE_BUILD_TYPE=PGO. That gives 30%-40% performance improvement on Intel. This build is still unstable for Apple Silicon and the gain is smaller. At least, last time I tried.

Could you make these changes to the Portfile?

@reneeotten
Copy link
Contributor

okay, so from a quick read we should request a compiler that supports: C11, CXX17, and thread-local storage for it to build.

There are Portfile directives for all of those, which will pick a MacPorts provided Clang that supports those, installs that first and then compiles the port. I can make these changes and see if that fixes the build on older OSes.

Regarding GCC, that is certainly possible and there are Portfiles that provide these compilers as variants. You could take a look at those and/or read through the Wiki link I posted - hopefully most of this is explained there. However, that is more work than I am willing to out in given that I'm not even using the package ;)

@JanWielemaker
Copy link
Contributor Author

There are Portfile directives for all of those, which will pick a MacPorts provided Clang that supports those, installs that first and then compiles the port. I can make these changes and see if that fixes the build on older OSes.

Thanks!

Regarding GCC

Thanks for the pointers. Might have a look at that later. It seems much less a problem on Apple Silicon, so just waiting long enough also helps 😄

@reneeotten
Copy link
Contributor

@JanWielemaker I added the things discussed here to the swi-prolog-devel Portfile in 328db99 ; that certainly helped as the port now builds on all systems down to 10.6, with the exception of ppc.

@reneeotten
Copy link
Contributor

If applicable, we ("royal we", meaning you) could add the same to the swi-prolog Portfile with the next update

@JanWielemaker
Copy link
Contributor Author

Thanks! I guess the next swi-prolog update will be for the next stable cycle (9.2.0). Can take a while.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

3 participants