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

abcde: switch to Perl 5.34 #13720

Merged
merged 1 commit into from
Jan 21, 2022
Merged

abcde: switch to Perl 5.34 #13720

merged 1 commit into from
Jan 21, 2022

Conversation

chrstphrchvz
Copy link
Contributor

Description

Tested on

macOS 10.15.7
Perl 5.34.0

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?

@macportsbot
Copy link

Notifying maintainers:
@0x6772 for port abcde.

@macportsbot macportsbot added maintainer maintainer: open Affects an openmaintainer port labels Jan 20, 2022
@0x6772
Copy link

0x6772 commented Jan 20, 2022

I confess I'm relatively inexperienced with this port maintainer process. Shouldn't we be lint/building against macos-12 too?

@chrstphrchvz
Copy link
Contributor Author

Shouldn't we be lint/building against macos-12 too?

Ideally, yes, but the CI provided by GitHub Actions does not yet publicly offer macOS 12: actions/runner-images#3649

I’ve not personally checked this port on macOS 12, but it currently has no known failures according to https://ports.macports.org/port/abcde/builds/ so I expect it to work after switching it to Perl 5.34 including on macOS 12.

@0x6772
Copy link

0x6772 commented Jan 20, 2022

Ideally, yes, but the CI provided by GitHub Actions does not yet publicly offer macOS 12: actions/virtual-environments#3649

Gotcha.

I’ve not personally checked this port on macOS 12, but it currently has no known failures according to https://ports.macports.org/port/abcde/builds/ so I expect it to work after switching it to Perl 5.34 including on macOS 12.

I'm largely ignorant here. Have you found that "build successful" is a sufficient test case for "abcde actually works to rip CDs and encode audio files" in the past?

That is, the "build" is trivial for abcde (if the ports it depends on build, so will it), but I don't think that actually validates that the software will actually recognize a CD in a device, read from it successfully, and executes the various encoding libraries with the correct flags for the current versions of those libraries.

Maybe I'm overthinking it?

@0x6772
Copy link

0x6772 commented Jan 21, 2022

Btw, around a few other things, I'm in the process of doing that "actually works" test on macOS 11.6.2 (Darwin Kernel Version 20.6.0), which I think should be plenty sufficient, I just didn't have an alt-root environment where I'd already pulled the various dependent ports up to the Perl 5.34 bits to test with, so that's taking a few minutes.

Copy link

@0x6772 0x6772 left a comment

Choose a reason for hiding this comment

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

Changes make sense to me, and I was able to rip a CD successfully after upgrading the several dependent ports.

@mojca mojca merged commit 0971dc2 into macports:master Jan 21, 2022
@ryandesign
Copy link
Contributor

switch to Perl 5.34

Why? Ports should use perl 5.28 (to match the perl5 portgroup) unless there's a good reason not to.

https://github.com/macports/macports-ports/blob/master/_resources/port1.0/group/perl5-1.0.tcl#L17

@jmroot
Copy link
Member

jmroot commented Jan 21, 2022

switch to Perl 5.34

Why? Ports should use perl 5.28 (to match the perl5 portgroup) unless there's a good reason not to.

https://github.com/macports/macports-ports/blob/master/_resources/port1.0/group/perl5-1.0.tcl#L17

Though that default should change, since 5.28 is no longer getting security fixes.

@chrstphrchvz chrstphrchvz deleted the patch-1 branch January 21, 2022 11:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintainer: open Affects an openmaintainer port maintainer
Development

Successfully merging this pull request may close these issues.

6 participants