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

perlPackages.IOPty: upgrade to 1.20 #369330

Open
wants to merge 3 commits into
base: staging
Choose a base branch
from

Conversation

pks-t
Copy link
Contributor

@pks-t pks-t commented Dec 30, 2024

This version fixes some broken tests that started to happen in the unstable branch of nixpkgs.

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 25.05 Release Notes (or backporting 24.11 and 25.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@stigtsp
Copy link
Member

stigtsp commented Dec 30, 2024

Hm, just noticing that IOTty and IOPty points to the same tarball. Would you mind also updating this? IOPty should then be an alias to IOTty I think.

@pks-t pks-t force-pushed the pks-io-pty-upgrade branch from ee7e502 to 1aff608 Compare December 30, 2024 11:21
@pks-t
Copy link
Contributor Author

pks-t commented Dec 30, 2024

True. Adapted the PR accordingly.

@pks-t
Copy link
Contributor Author

pks-t commented Dec 30, 2024

Hm, doesn't seem to work as-is though. Let me have another look.

@stigtsp
Copy link
Member

stigtsp commented Dec 30, 2024

It also seems to trigger a mass rebuild, so would need to go into staging if that is the case

Edit: Hmm, I think 500-1000 is OK for master?

Drop the now-unneeded patch. It is not required anymore starting with
1735a78 (Make function checks more robust within shared libs,
2023-04-04), and with the patch a couple of tests fail.
These are both equivalent to one another, and we're about to convert the
IOPty package to become an alias of IOTty. It follows that the IOPty
package will only be available when `conf.allowAliases = true`.
The IOPty and IOTty packages refer to the same tarball. Make the former
an alias of the latter so that we don't have to maintain this package
twice.
@pks-t pks-t force-pushed the pks-io-pty-upgrade branch from 920f077 to 00074c0 Compare December 30, 2024 13:05
@pks-t pks-t changed the base branch from master to staging December 30, 2024 13:06
@pks-t
Copy link
Contributor Author

pks-t commented Dec 30, 2024

I've changed it to staging for now. Happy to revert back to master in case it's not needed after all.

@ofborg ofborg bot requested review from qbit, de11n, invokes-su and deftdawg December 30, 2024 21:45
@ofborg ofborg bot added 10.rebuild-darwin: 501+ 10.rebuild-linux: 501+ 10.rebuild-darwin-stdenv This PR causes stdenv to rebuild 10.rebuild-linux-stdenv This PR causes stdenv to rebuild labels Dec 30, 2024
@@ -70,7 +70,7 @@ perlPackages.buildPerlPackage rec {
FileLibMagic
HashMerge
HTTPMessage
IOPty
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure if this should be changed. The rex source points to IO::Pty: https://github.com/RexOps/Rex/blob/master/t/os_dependencies.t#L14

Copy link
Member

Choose a reason for hiding this comment

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

IO::Pty is included in the IO-Tty package so this seems fine?

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

Successfully merging this pull request may close these issues.

3 participants