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

haskellPackages.rio: skip a broken test on aarch64-darwin #179925

Merged

Conversation

andrew-d
Copy link
Contributor

@andrew-d andrew-d commented Jul 2, 2022

Description of changes

rio has tests that don't work on aarch64-darwin, and are essentially the same as the unliftio testst that were skipped in #135453. Let's skip these in the rio test suite as well. Along with #179924 this unbreaks git-annex on my M1 machine.

Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • 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/)
  • 22.11 Release Notes (or backporting 22.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
    • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
  • Fits CONTRIBUTING.md.

@ofborg ofborg bot added the 6.topic: darwin Running or building packages on Darwin label Jul 2, 2022
@andrew-d andrew-d requested a review from domenkozar July 2, 2022 16:43
@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux labels Jul 2, 2022
Comment on lines 284 to 293
# same as above, but called from rio; 'xit' skips a test
rio = overrideCabal (drv: {
preConfigure = ''
sed -i 's/\bit /xit /g' test/RIO/FileSpec.hs
'';
}) super.rio;
Copy link
Member

Choose a reason for hiding this comment

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

Could you report this upstream and put a link to the github issue here so we have some way of tracking when this is fixed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@cdepillabout - It's the same issue as above (fpco/unliftio#87); do you want me to leave the same comment here?

Copy link
Member

Choose a reason for hiding this comment

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

I'm not super familiar with the development of rio and unliftio, but it looks like they are developed in separate repos?

My guess is that posting an additional issue to the rio repo, so that we can track when the tests are fixed, would be helpful for us.

Although maybe you're trying to say that these failing rio tests would also be fixed as soon at the unliftio issue is fixed? Like, this isn't an issue directly with rio, but it is just an effect of unliftio being broken? In which case, could you update the issue so that the maintainers here in Nixpkgs would know that this override for rio could be removed as soon as the linked unliftio issue has been closed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@cdepillabout - Yes, sorry, should have been more clear. These failing rio tests will be fixed as soon as the unliftio tests are fixed; the issue is with unliftio, and rio just calls into the functions with an issue. I updated the comment here; is that better?

Copy link
Member

Choose a reason for hiding this comment

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

Great, very easy to understand now!

@cdepillabout
Copy link
Member

Please rebase on haskell-updates as well.

@andrew-d andrew-d force-pushed the andrew/rio-skip-broken-test branch from 6691839 to 4efb405 Compare July 2, 2022 18:02
@andrew-d andrew-d changed the base branch from master to haskell-updates July 2, 2022 18:02
@andrew-d
Copy link
Contributor Author

andrew-d commented Jul 2, 2022

@cdepillabout - Rebased and changed the merge branch; does this look right? This is my first Haskell package change in nixpkgs.

@andrew-d andrew-d requested a review from cdepillabout July 2, 2022 18:03
@andrew-d andrew-d force-pushed the andrew/rio-skip-broken-test branch from 4efb405 to f867d66 Compare July 3, 2022 19:30
@andrew-d andrew-d force-pushed the andrew/rio-skip-broken-test branch from f867d66 to 3f4d5f2 Compare July 3, 2022 19:47
@cdepillabout
Copy link
Member

Thanks a lot, this looks good! We always love to get these types of fixes!

@cdepillabout cdepillabout merged commit 2275bc9 into NixOS:haskell-updates Jul 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
6.topic: darwin Running or building packages on Darwin 6.topic: haskell 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants