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

py-graph-tool: Update to version 2.57 #19570

Merged
merged 3 commits into from
Jul 30, 2023
Merged

Conversation

essandess
Copy link
Contributor

Description

py-graph-tool: Update to version 2.57
* Add 311, Remove 37
* Use boost181
* Bugfix for unary_function C++17 removal

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?

@macportsbot
Copy link

Notifying maintainers:
@Veence for port cgal5.

python/py-graph-tool/Portfile Outdated Show resolved Hide resolved
python/py-graph-tool/Portfile Outdated Show resolved Hide resolved
@@ -115,6 +133,7 @@ if {${name} ne ${subport}} {
destroot.cmd make
destroot.destdir \
DESTDIR=${destroot}
destroot.target install
Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't install the default value for destroot.target?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's overwritten by PG python, so this is necessary here.

Copy link
Member

Choose a reason for hiding this comment

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

It's overwritten by PG python, so this is necessary here.

Well, would it be more accurate to say that this is necessary when python.pep517 is enabled? Which is the default for 3.11 and later.

I'd need to dig into the upstream project a bit, before providing any official recommendations. But if you're working around that by setting destroot.target, etc, then perhaps it might be better to simply set python.pep517 to no. (Which might allow you to eliminate at least a few overrides you're setting.)

However, I'll let our more Python-savvy folks chime in with their thoughts and recommendations.

CC: @jmroot
CC: @reneeotten

Copy link
Member

Choose a reason for hiding this comment

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

I'd need to dig into the upstream project a bit, before providing any official recommendations. But if you're working around that by setting destroot.target, etc, then perhaps it might be better to simply set python.pep517 to no. (Which might allow you to eliminate at least a few overrides you're setting.)

That being said, I'm not overly concerned, so no need to hold up merging this.

@essandess
Copy link
Contributor Author

essandess commented Jul 23, 2023

Note: The CI and buildbots will all fail on this port for py10-graph-tool and so forth because of the way the boost ports are written: boost181, e.g., is always installed with variant +python311. There is no way to install the necessary boost variants in the Portfile.

cc: @mascguy

@mascguy
Copy link
Member

mascguy commented Jul 23, 2023

Note: The CI and buildbots will all fail on this port for py10-graph-tool and so forth because of the way the boost ports are written: boost181, e.g., is always installed with variant +python311. There is no way to install the necessary boost variants in the Portfile.

cc: @mascguy

I'll try to take a look at a subport approach for 181 soon, as this is definitely a pain...

@essandess
Copy link
Contributor Author

@mascguy May we please merge this PR and get a working graph-tool into the field for arm64 machines?

The boost Portfile fixes can be added later.

@mascguy
Copy link
Member

mascguy commented Jul 30, 2023

Presently building the 3.11 subport locally on 10.15 [with trace mode], to see whether all is well. It's taking far longer than expected though.

There are also an inordinate number of warnings, enough so to utterly pollute the build log. I may suppress some of them before merging, as they make it difficult to follow the compilation itself...

@mascguy
Copy link
Member

mascguy commented Jul 30, 2023

There are also an inordinate number of warnings, enough so to utterly pollute the build log. I may suppress some of them before merging, as they make it difficult to follow the compilation itself.

Which is probably due to the project Makefile setting -Wall and -Wextra. (With no immediately-apparent way to disable that without patching, based on a quick cursory review of the various AutoTools scripts.)

@mascguy mascguy force-pushed the py-graph-tool branch 2 times, most recently from 974bb0e to 56da46a Compare July 30, 2023 15:15
@mascguy
Copy link
Member

mascguy commented Jul 30, 2023

p.s. You inadvertently included an update to py-absl, which you also have an open PR for. So I dropped that commit from this branch.

@mascguy
Copy link
Member

mascguy commented Jul 30, 2023

Building a single subport takes a very long time, at least on my older MacPro. So I've only validated py311-graph-tool, but that looks good [with trace mode].

Given that there don't appear to be any dependents on this port, it might be worth dropping subports for 3.9 and prior.

@mascguy mascguy merged commit b492f2f into macports:master Jul 30, 2023
@essandess essandess deleted the py-graph-tool branch July 30, 2023 16:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
5 participants