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

Build failure: jiten #271600

Closed
IreneKnapp opened this issue Dec 2, 2023 · 9 comments · Fixed by #274349
Closed

Build failure: jiten #271600

IreneKnapp opened this issue Dec 2, 2023 · 9 comments · Fixed by #274349
Labels
0.kind: build failure A package fails to build

Comments

@IreneKnapp
Copy link
Contributor

Steps To Reproduce

Steps to reproduce the behavior:

  1. Check out nixos-23.11 and cd into it
  2. NIXPKGS_ALLOW_UNFREE nix-build -A jiten

Build log

https://gist.github.com/IreneKnapp/8c1284536cc214e5736a6b06e7f443d2

Additional context

This same version of jiten works fine on 23.05.

Notify maintainers

@obfusk

Metadata

Please run nix-shell -p nix-info --run "nix-info -m" and paste the result.

 - system: `"x86_64-linux"`
 - host os: `Linux 6.5.9, NixOS, 23.05 (Stoat), 23.05.20231027.1a3c95e`
 - multi-user?: `yes`
 - sandbox: `yes`
 - version: `nix-env (Nix) 2.13.6`
 - channels(root): `"nixos-22.11"`
 - nixpkgs: `/nix/var/nix/profiles/per-user/root/channels/nixos`

Note that that channel is unused because my system is flake-ified, which is why it's out of date. The host is nixos-23.05 because this came to light while upgrading, but the affected build is nixos-23.11 as described above.

Priorities

Add a 👍 reaction to issues you find important.

@IreneKnapp IreneKnapp added the 0.kind: build failure A package fails to build label Dec 2, 2023
@lf-
Copy link
Member

lf- commented Dec 2, 2023

AttributeError: 'Cookie' object has no attribute 'name'

That's truly baffling, but probably an upstream bug in jiten. I don't get why it's happening though, since the cookies in the jar should be the stdlib ones? which have the right attribute already? https://github.com/python/cpython/blob/3.12/Lib/http/cookiejar.py#L743-L780

@kirillrdy
Copy link
Member

i am running bisect now, it used to build, so something must have changed

@IreneKnapp
Copy link
Contributor Author

it was pointed out to me elsewhere that since jiten is non-free, Hydra doesn't run the tests for it; that's probably how this happened

I'm not a Python expert but the error message suggests to me that one of the libraries used by the test has changed its API

@lf-
Copy link
Member

lf- commented Dec 2, 2023

I'm not a Python expert but the error message suggests to me that one of the libraries used by the test has changed its API

the thing is, i think this is probably a stdlib Cookie, which makes it extremely confusing why it could possibly be missing that attribute

@Whovian9369
Copy link

Whovian9369 commented Dec 2, 2023

My personal hunch is that it likely has to do with Flask updating past v2.3.2, as the changelog entry for 2.3.2 mentions:

Set Vary: Cookie header when the session is accessed, modified, or refreshed.

Though I didn't expect it to succeed, I replicated the error with Flask 2.3.2. Going to see if I can get Flask 2.3.1 working, and tested with jiten as well.
Edit: Didn't get it working with 2.3.1 either and with seemingly the same error, huh.

@obfusk
Copy link
Member

obfusk commented Dec 14, 2023

Sorry! I haven't been able to work on Jiten for a while, so I missed this. The problem seems to be an incompatible change in test cookies in werkzeug 2.3.0; tracking this upstream in obfusk/jiten#159.

I'm not yet sure when I'll be able to release a new version of Jiten (and I'm not currently working on nixpkgs as I do not currently have a working nix install), but this should be fixed on master now; this patch seems to fix compatibility with newer flask & werkzeug: obfusk/jiten@476fa5d...2c553c5

it was pointed out to me elsewhere that since jiten is non-free, Hydra doesn't run the tests for it; that's probably how this happened

Only the pitch data files and mp3s are non-free (non-commercial use only). Maybe we can eventually add a second jiten-foss package or something -- simply not checking out the non-free submodule before building should work, but the current tests will likely fail, so I'd have to account for that first -- for users that don't want the non-free data files? That one could then be tested automatically, which should catch most errors in regular jiten too (though I'd have caught this in CI if not for the fact I wasn't able to work on Jiten for a year).

@IreneKnapp
Copy link
Contributor Author

oh hey, thanks so much for the investigation and the fix! I'm going to prepare a PR that cherry-picks the fix, so you don't have to worry about time pressure on an upstream release.

that's a good thought about the jiten-foss thing, and let me know if you'd like my help on the nix parts of it at any point.

@obfusk
Copy link
Member

obfusk commented Dec 15, 2023

Thanks for the PR! I'm hoping to have the tests pass w/o the non-free submodule present by the next release (tracking in obfusk/jiten#161). I'll let you know when there's a new release and/or I can use help with the nix packaging then :)

@IreneKnapp
Copy link
Contributor Author

looking forward to it! :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
0.kind: build failure A package fails to build
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants