Skip to content

python312Packages.ofxhome: remove nose and fix tests#327239

Merged
emilazy merged 1 commit intoNixOS:masterfrom
pyrox0:denose/ofxhome
Jul 28, 2024
Merged

python312Packages.ofxhome: remove nose and fix tests#327239
emilazy merged 1 commit intoNixOS:masterfrom
pyrox0:denose/ofxhome

Conversation

@pyrox0
Copy link
Member

@pyrox0 pyrox0 commented Jul 15, 2024

Description of changes

Part of #326513

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/)
  • 24.11 Release Notes (or backporting 23.11 and 24.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.

@github-actions github-actions bot added the 6.topic: python Python is a high-level, general-purpose programming language. label Jul 15, 2024
@ofborg ofborg bot added 8.has: package (new) This PR adds a new package 11.by: package-maintainer This PR was created by a maintainer of all the package it changes. 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. labels Jul 15, 2024
Copy link
Member

@emilazy emilazy left a comment

Choose a reason for hiding this comment

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

This thing just doesn’t work at all. It’s a client for a site that has been dead for 7 years.

I really want to say we should just put this one out of its misery, but I guess it has one connection to a package we might conceivably want to work in Nixpkgs through ofxclient and ledger-autosync. There’s a report here: egh/ledger-autosync#141.

I see a few options here:

  1. Send a PR to ledger-autosync moving it to the more modern ofxtools; it looks like there isn’t that much code or API surface being exercised, although it would be a compatibility‐breaking change because they pass a configuration file directly to ofxclient. This would be firmly supererogatory FOSS ecosystem behaviour and I don’t at all expect you to actually do it.

  2. Patch ofxhome out of ofxclient, send a probably‐futile upstream PR to the same maintainer’s other dormant project. But that might be gnarly and it’s possible they’re quite coupled.

  3. Keep this zombie package alive just for its reverse dependencies.

  4. Remove ledger-autosync entirely (but apparently even in its current state it does still work with downloaded OFX files).

Do you have any inclinations here? I don’t have any desire to block doing 3 as this is clearly an improvement on the status quo, but this kind of rot does make me wish we could take the opportunity to apply a little pressure on the ecosystem here. Don’t burn yourself out sending patches to random upstreams though :) It seems like ledger-autosync still has users and still works, so it’s probably best to just keep this building for now and hope that someone who actually cares migrates ledger-autosync to ofxtools.

Copy link
Member

@emilazy emilazy left a comment

Choose a reason for hiding this comment

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

Result of nixpkgs-review pr 327239 run on x86_64-linux 1

8 packages built:
  • python311Packages.ofxclient
  • python311Packages.ofxclient.dist
  • python311Packages.ofxhome
  • python311Packages.ofxhome.dist
  • python312Packages.ofxclient
  • python312Packages.ofxclient.dist
  • python312Packages.ofxhome
  • python312Packages.ofxhome.dist

Merging this for now; hopefully downstreams will catch up in time. Thanks!

@emilazy emilazy merged commit 99e195c into NixOS:master Jul 28, 2024
@pyrox0 pyrox0 deleted the denose/ofxhome branch August 4, 2024 23:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

6.topic: python Python is a high-level, general-purpose programming language. 8.has: package (new) This PR adds a new package 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 11.by: package-maintainer This PR was created by a maintainer of all the package it changes.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants