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

phosg: support tests; phosg, resource_dasm: some fixes for older systems and PPC #18103

Merged
merged 4 commits into from
Aug 18, 2023

Conversation

barracuda156
Copy link
Contributor

Description

Currently the port is broken on Catalina and downwards: https://ports.macports.org/port/phosg/details
This PR fixes the build on 10.6.8; it may fix it on some other systems too.
It also enables testing.

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

macOS 10.6.8 Server
Xcode 3.2.6

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?

P. S. Several tests fail in Rosetta:

--->  Testing phosg
Executing:  cd "/opt/local/var/macports/build/_opt_PPCRosettaPorts_devel_phosg/phosg/work/build" && ctest test 
Test project /opt/local/var/macports/build/_opt_PPCRosettaPorts_devel_phosg/phosg/work/build
      Start  1: EncodingTest
 1/12 Test  #1: EncodingTest .....................   Passed    0.03 sec
      Start  2: FilesystemTest
 2/12 Test  #2: FilesystemTest ...................   Passed    0.03 sec
      Start  3: HashTest
 3/12 Test  #3: HashTest .........................Subprocess aborted***Exception:   0.06 sec
      Start  4: ImageTest
 4/12 Test  #4: ImageTest ........................Subprocess aborted***Exception:   0.05 sec
      Start  5: JSONTest
 5/12 Test  #5: JSONTest .........................   Passed    0.04 sec
      Start  6: KDTreeTest
 6/12 Test  #6: KDTreeTest .......................   Passed    0.04 sec
      Start  7: LRUSetTest
 7/12 Test  #7: LRUSetTest .......................   Passed    0.03 sec
      Start  8: LRUMapTest
 8/12 Test  #8: LRUMapTest .......................   Passed    0.03 sec
      Start  9: ProcessTest
 9/12 Test  #9: ProcessTest ......................Subprocess aborted***Exception:   0.27 sec
      Start 10: StringsTest
10/12 Test #10: StringsTest ......................Subprocess aborted***Exception:   0.04 sec
      Start 11: TimeTest
11/12 Test #11: TimeTest .........................   Passed    1.03 sec
      Start 12: UnitTestTest
12/12 Test #12: UnitTestTest .....................   Passed    0.04 sec

67% tests passed, 4 tests failed out of 12

Total Test time (real) =   1.77 sec

The following tests FAILED:
	  3 - HashTest (Subprocess aborted)
	  4 - ImageTest (Subprocess aborted)
	  9 - ProcessTest (Subprocess aborted)
	 10 - StringsTest (Subprocess aborted)
Errors while running CTest

@macportsbot
Copy link

Notifying maintainers:
@fracai for port phosg.

@barracuda156
Copy link
Contributor Author

Added simple fixes for resource_dasm. Builds fine in Rosetta:

svacchanda$ port -v installed resource_dasm
The following ports are currently installed:
  resource_dasm @2023.03.15_0 (active) requested_variants='' platform='darwin 10' archs='ppc' date='2023-03-28T06:10:18+0700'

@barracuda156 barracuda156 changed the title phosg: support tests; some fixes for older systems and PPC phosg: support tests; phosg, resource_dasm: some fixes for older systems and PPC Mar 27, 2023
@@ -10,6 +10,7 @@ github.tarball_from archive

# blacklisting to select C++20 capable compilers
compiler.blacklist-append {clang < 1300}
compiler.cxx_standard 2020
Copy link
Member

Choose a reason for hiding this comment

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

@barracuda156 your dedication to fixing ports on older systems is insane! Thank you for working on this 👍

Are you running bleeding-edge MacPorts on your machine? I think compiler.cxx_standard 2020 just sets the 2017 standard on stable MacPorts since support was only added 3 days ago (macports/macports-base@f23dca5).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@harens Okay, but setting 2017 would make no difference now, but will have to be changed anyway later to 2020? Or do I miss something?
Alternatively we need to blacklist several versions of GCC, because otherwise they get picked, and build fails.

Copy link
Member

Choose a reason for hiding this comment

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

Alternatively we need to blacklist several versions of GCC, because otherwise they get picked, and build fails.

I'm thinking more along those lines. Keep the cxx 20 but some other compilers might need to be blacklisted. From the mentioned commit:

-----------------------------------------------------------------------
#| C++ Standard |   Clang   |  Xcode Clang   |   Xcode   |     GCC     |
#|---------------------------------------------------------------------|
#| 2017 (C++17) |    5.0    |  1000.11.45.2  |   10.0    |      7      |
#| 2020 (C++20) |    14     |  1400.0.29.102 |   14.0    |     11      |

@barracuda156 barracuda156 marked this pull request as draft March 28, 2023 16:39
@barracuda156
Copy link
Contributor Author

Changed to draft, since upstream added fixes, which have to be added here.

@fracai
Copy link
Contributor

fracai commented Mar 31, 2023

I can take a look at the upstream changes tomorrow.
Thanks for your work on fixing up the Portfile for older systems.

@barracuda156
Copy link
Contributor Author

I can take a look at the upstream changes tomorrow.

@fracai That would be awesome, thank you.

Thanks for your work on fixing up the Portfile for older systems.

We will also probably need some extra fixes for Clang builds on some older x86 OSs, judging by a quick look at buildbots’ logs.

@fracai
Copy link
Contributor

fracai commented Apr 1, 2023

The quick changes to bring in the upstream changes is just:

diff --git a/devel/phosg/Portfile b/devel/phosg/Portfile
index 29ae324fa5d..4abeb3d4132 100644
--- a/devel/phosg/Portfile
+++ b/devel/phosg/Portfile
@@ -5,14 +5,14 @@ PortGroup               cmake 1.1
 PortGroup               github 1.0
 PortGroup               compiler_blacklist_versions 1.0
 
-github.setup            fuzziqersoftware phosg e1bd000
+github.setup            fuzziqersoftware phosg fe69599
 github.tarball_from     archive
 
 # blacklisting to select C++20 capable compilers
 compiler.blacklist-append {clang < 1300}
 compiler.cxx_standard   2020
 
-version                 2023.03.04
+version                 2023.03.28
 revision                0
 categories              devel
 maintainers             {alum.wpi.edu:arno+macports @fracai} openmaintainer
@@ -21,9 +21,9 @@ license                 MIT
 description             Phosg is a basic C++ wrapper library around some common C routines.
 long_description        {*}${description}
 
-checksums               rmd160  526fb52a0eadbb3e33da6e2711c4c43afca04eda \
-                        sha256  802170acf4b8bae5383183c9361812d5309cb3e79d703e365b96feffe5d776cd \
-                        size    147998
+checksums               rmd160  f1ebcbb288ce104b54f67849e49f3296972b0469 \
+                        sha256  32ea845e7adc7f25c3493efbb46cb19c3b0cd13ecf9621e10a87f3e2ad2b1247 \
+                        size    148005
 
 set pyver               3.11
 set python_version      [string index ${pyver} 0][string range ${pyver} 2 end]
diff --git a/devel/resource_dasm/Portfile b/devel/resource_dasm/Portfile
index 690ade5b8cd..b6d0e2c9203 100644
--- a/devel/resource_dasm/Portfile
+++ b/devel/resource_dasm/Portfile
@@ -9,14 +9,14 @@ PortGroup               legacysupport 1.1
 # MAP_ANONYMOUS
 legacysupport.newest_darwin_requires_legacy 14
 
-github.setup            fuzziqersoftware resource_dasm 46ad32e
+github.setup            fuzziqersoftware resource_dasm 636c3d4
 github.tarball_from     archive
 
 # blacklisting to select C++20 capable compilers
 compiler.blacklist-append {clang < 1300}
 compiler.cxx_standard   2020
 
-version                 2023.03.15
+version                 2023.03.28
 revision                0
 categories              devel
 maintainers             {alum.wpi.edu:arno+macports @fracai} openmaintainer
@@ -25,9 +25,9 @@ license                 MIT
 description             Tools for reverse-engineering classic Mac OS applications and games.
 long_description        {*}${description}
 
-checksums               rmd160  190a5eb9a174d8eafdc17ca736f730e2d12862ec \
-                        sha256  db5e42d9ef9df9d4d40f42fd7c1b67cc2f5108a06e30ae983fd6e40046ad22e0 \
-                        size    457269
+checksums               rmd160  281b53d58e8af12e050474864428fa968fe29b13 \
+                        sha256  a929f63f583683cd2cecc840df1c7a8735f44a210bfea14c90f270b655eae48c \
+                        size    457689
 
 set pyver               3.11
 set python_version      [string index ${pyver} 0][string range ${pyver} 2 end]

Then I get a lot of unqualified call to 'std::move'. After qualifying all of those, there were a few other errors, that I unfortunately don't have a record of and I need to redo the std::move qualifying to get back to that step. I'll continue working on this. The unqualified stuff feels like I should report it upstream, though I'm not sure why that didn't show up previously. I think I need to take a closer look at the Portfile changes.

@fracai
Copy link
Contributor

fracai commented Apr 1, 2023

After stashing the phosg and resource_dasm upstream changes, I still get the "unqualified std" errors. So something changed somewhere else. Is anyone aware of something like a change to cmake or clang that introduces -Wunqualified-std-cast-call by default? I'm trying to see what else might have changed that's introducing the build failure, where it built fine earlier in the week.

@barracuda156
Copy link
Contributor Author

The quick changes to bring in the upstream changes is just

@fracai Yeah, I was just waiting for extra fixes, since what we have does not work properly yet.

After stashing the phosg and resource_dasm upstream changes, I still get the "unqualified std" errors. So something changed somewhere else. Is anyone aware of something like a change to cmake or clang that introduces -Wunqualified-std-cast-call by default? I'm trying to see what else might have changed that's introducing the build failure, where it built fine earlier in the week.

Nothing changed IMO, they were always failing, see: https://build.macports.org/builders/ports-10.15_x86_64-builder/builds/134711/steps/install-port/logs/stdio

@fracai
Copy link
Contributor

fracai commented Apr 1, 2023

Oh, waiting for additional upstream changes? As in changes coming from upstream? Or should I take a look at what needs to be changed?

What's weird is that they weren't always failing on my system. So I was confused why they were now. Though I just realized when I was testing, I reverted the commit hash updates, but not your patchfile changes, so that's "what changed". Basically I was testing partial changes and not realizing it.

So, to be clear on the current state, there're additional upstream changes coming that should help here.

@barracuda156
Copy link
Contributor Author

barracuda156 commented Apr 1, 2023

Oh, waiting for additional upstream changes? As in changes coming from upstream? Or should I take a look at what needs to be changed?

Maybe you could join in: fuzziqersoftware/phosg#24

What's weird is that they weren't always failing on my system.

Buildbots were failing from Catalina down. I cannot verify that locally, but we have logs.

So I was confused why they were now. Though I just realized when I was testing, I reverted the commit hash updates, but not your patchfile changes, so that's "what changed". Basically I was testing partial changes and not realizing it.

I have set my patches to be applied on systems which were already failing, so at least in theory nothing additional should have failed. In the worst case nothing gets fully fixed. (gcc build on 10.6.8 is fixed, but buildbots do not run that.)

So, to be clear on the current state, there're additional upstream changes coming that should help here.

Well, I hope so :)
And I cannot debug and fix Clang builds anyway.

@barracuda156 barracuda156 marked this pull request as ready for review May 25, 2023 22:14
@barracuda156
Copy link
Contributor Author

Perhaps won’t hurt merging as is?

@barracuda156 barracuda156 mentioned this pull request Aug 13, 2023
12 tasks
@reneeotten
Copy link
Contributor

are tour proposed patched accepted by upstream? Any indication on whether/when they'll release a new version. Ideally we don't want to carry all these patches forward..

@barracuda156
Copy link
Contributor Author

@reneeotten Well, it has been submitted back then: fuzziqersoftware/phosg#23

@reneeotten reneeotten merged commit 0d5fca4 into macports:master Aug 18, 2023
@barracuda156 barracuda156 deleted the phosg branch August 18, 2023 01:54
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.

5 participants