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

Fix mac-fsevents-pm to build and run on macOS beyond X #1182

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

dhomeier
Copy link
Contributor

@dhomeier dhomeier commented Nov 9, 2024

Adding Perl 5.34.1 version; package did not even build to the Makefile creation step on 15.1 – it is unclear how it would ever have built on recent macOS since MacVersion.pm has been explicitly testing for $major == 10.
Not sure if the @arch combinations here will work for everything from 11.0-15.0, so should be checked on some other systems.

I tested for arm64 and x86_64 on Sequoia, both with one test failure, which probably points to a real issue:

/usr/bin/make test || exit 2
"/usr/bin/arch" -arm64 perl5.34 -MExtUtils::Command::MM -e 'cp_nonempty' -- FSEvents.bs /opt/sw2/src/fink.build/mac-fsevents-pm5341-0.14-1/Mac-FSEvents-0.14/blib/arch/auto/Mac/FSEvents/FSEvents.bs 644
PERL_DL_NONLAZY=1 "/usr/bin/arch" -arm64 perl5.34 "-MExtUtils::Command::MM" "-MTest::Harness" "-e" "undef *Test::Harness::Switches; test_harness(0, 'blib/lib', '/opt/sw2/src/fink.build/mac-fsevents-pm5341-0.14-1/Mac-FSEvents-0.14/blib/arch')" t/*.t
t/01use.t .................. ok   
t/02constructor.t .......... ok   
t/03event.t ................ ok   
t/04flags.t ................ 
    #   Failed test 'one event, because it's coalesced'
    #   at t/04flags.t line 66.
    #          got: '2'
    #     expected: '1'
    # Looks like you failed 1 test of 2.
t/04flags.t ................ 1/? 
#   Failed test 'none'
#   at t/04flags.t line 68.
t/04flags.t ................ 4/? # Looks like you failed 1 test of 4.
t/04flags.t ................ Dubious, test returned 1 (wstat 256, 0x100)
Failed 1/4 subtests 
t/05fork.t ................. ok   
t/06receive-all-changes.t .. ok   

Test Summary Report
-------------------
t/04flags.t              (Wstat: 256 Tests: 4 Failed: 1)
  Failed test:  1
  Non-zero exit status: 1
Files=6, Tests=14, 16 wallclock secs ( 0.04 usr  0.05 sys +  1.16 cusr  1.39 csys =  2.64 CPU)
Result: FAIL
Failed 1/6 test programs. 1/14 subtests failed.
make: *** [test_dynamic] Error 255

i.e.

    my @events = fetch_events($fs, $fh);
    is scalar(@events), 1, "one event, because it's coalesced";
    like $events[0]->path, qr/^\Q$tmp_abs/;

is returning 2 events where only 1 should be found.
Out of my perl depth to investigate this further.

As a check I also built on 10.14, which is passing all tests both before and with this patch.

@dhomeier dhomeier added help wanted arm64 Build issues and fixes for Apple Silicon labels Nov 9, 2024
@nieder
Copy link
Member

nieder commented Nov 9, 2024

From the upstream changelog:

0.22  2023-10-09 22:48:07 JST
        - Fix memory leak in "watch" method (#4)
        - Set PROTOTYPES: DISABLE (#5)
 
0.21  2023-07-13 19:28:40 JST
        - Cosmetic changes (#2)
 
0.20  2023-07-13 03:28:32 JST
        - Support recent macOS (#1)
        - Support M1/M2 chip (#1)
        - Now Mac-FSEvents is maintained at https://github.com/skaji/Mac-FSEvents

Please try updating to the latest and try again.

And this looks like the test failure in 04flags.t: skaji/Mac-FSEvents#7

@dhomeier
Copy link
Contributor Author

dhomeier commented Nov 9, 2024

I did not find any newer versions on CPAN, and the Github repo does not have any releases – do you know of any location to download 0.22 as source?

@nieder
Copy link
Member

nieder commented Nov 9, 2024

It's there on CPAN. It looks like it just changed maintainer, so our URL has to be edited to the new owner:
https://cpan.metacpan.org/authors/id/S/SK/SKAJI/Mac-FSEvents-0.22.tar.gz

-Source: mirror:cpan:authors/id/R/RH/RHOELZ/Mac-FSEvents-%v.tar.gz
+Source: mirror:cpan:authors/id/S/SK/SKAJI/Mac-FSEvents-%v.tar.gz

@dmacks
Copy link
Member

dmacks commented Nov 9, 2024

CPAN-module bundles are also reachable by bundle name (rather than author-id), which do not vary when the author or maintainer changes. See for example,
https://www.cpan.org/modules/by-module/Mac/
Would it be worthwhile doing a fink-wide update to *.info:Source: to use those sources?

@dhomeier
Copy link
Contributor Author

dhomeier commented Nov 9, 2024

And this looks like the test failure in 04flags.t: skaji/Mac-FSEvents#7

Interesting, actually it's the inverse now: the test has changed the expected output to 2 in 0.22, but is still returning 1 on macOS 10.13 – under 15.1 it's now passing both on arm64 and x86_64. Will try to test on 10.14 as well when I get the chance.

@dhomeier
Copy link
Contributor Author

dhomeier commented Nov 9, 2024

https://www.cpan.org/modules/by-module/Mac/
Would it be worthwhile doing a fink-wide update to *.info:Source: to use those sources?

They are on the same CPAN mirrors, so that would certainly be more robust. But would also need to identify the categories (which are in Mac, in SQL or wherever) for 1000-some packages!

@dmacks
Copy link
Member

dmacks commented Nov 9, 2024

https://www.cpan.org/modules/by-module/Mac/
Would it be worthwhile doing a fink-wide update to *.info:Source: to use those sources?

They are on the same CPAN mirrors, so that would certainly be more robust. But would also need to identify the categories (which are in Mac, in SQL or wherever) for 1000-some packages!

Foo-Bar-Whatever.tar.gz are in by-module/Foo

@dhomeier
Copy link
Contributor Author

dhomeier commented Nov 10, 2024

And this looks like the test failure in 04flags.t: skaji/Mac-FSEvents#7

Interesting, actually it's the inverse now: the test has changed the expected output to 2 in 0.22, but is still returning 1 on macOS 10.13 – under 15.1 it's now passing both on arm64 and x86_64. Will try to test on 10.14 as well when I get the chance.

Indeed on 10.14.6 that test still returns 1 with (system) perl 5.18.4, 5.30.3 and 5.34.1, making it fail, so really seems to reflect just the way the fs behave on the different OS versions rather than a "correct" or "incorrect" result. So should the version be patched to expect either the old or the new result depending on system version, and if so, up to what version?
Has the old package's test been confirmed to pass after adding the 5303 variant in ae2ec8d, which would likely have been 11.x?

@nieder
Copy link
Member

nieder commented Nov 11, 2024

For me on 13.6 (x86_64),
0.14
-pm5184: fails building with can't parse version string
-pm5282: fails building with can't parse version string
-pm5303: fails building with can't parse version string

0.22
-pm5184: builds and passes tests
-pm5303: builds and passes tests
-pm5341: builds and passes tests

As to how it was able to build previously, I added pm5282-pm5303 on a 10.14.6 machine, so it knew about that OS, even in the old v0.14.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
arm64 Build issues and fixes for Apple Silicon help wanted
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants