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

Fix failing nix build #56

Merged
merged 7 commits into from
Nov 3, 2024
Merged

Conversation

youwen5
Copy link
Contributor

@youwen5 youwen5 commented Nov 2, 2024

The new release fails to build when using (with both the project's own flake.nix, and in nixpkgs). This is preventing the update from being pushed into nixpkgs.

I fixed a separate issue in the flake.nix and tracked down the build failure to the failure of the test show_error_when_searching_manga. It passes normally with cargo test and cargo build but fails in the nix sandbox build environment for whatever reason. Increasing max_ticks to a much larger number seems to solve the issue but I am not familiar enough with the internals of the codebase and ratatui to know if this is a good permanent solution. Feel free to suggest a better fix for this.

Flake lock file updates:

• Updated input 'crane':
    'github:ipetkov/crane/7ce92819802bc583b7e82ebc08013a530f22209f' (2024-08-18)
  → 'github:ipetkov/crane/8658adcdad49b8f2c6cbf0cc3cb4b4db988f7638' (2024-11-01)
• Removed input 'crane/nixpkgs'
• Updated input 'flake-utils':
    'github:numtide/flake-utils/b1d9ab70662946ef0850d488da1c9019f3a9752a' (2024-03-11)
  → 'github:numtide/flake-utils/c1dfcf08411b08f6b8615f7d8971a2bfa81d5e8a' (2024-09-17)
• Updated input 'nixpkgs':
    'github:nixos/nixpkgs/8a3354191c0d7144db9756a74755672387b702ba' (2024-08-18)
  → 'github:nixos/nixpkgs/807e9154dcb16384b1b765ebe9cd2bba2ac287fd' (2024-10-29)
• Updated input 'rust-overlay':
    'github:oxalica/rust-overlay/45e98fbd62c32e5927e952d2833fa1ba4fb35a61' (2024-08-21)
  → 'github:oxalica/rust-overlay/1ff38ca26eb31858e4dfe7fe738b6b3ce5d74922' (2024-11-02)
@youwen5
Copy link
Contributor Author

youwen5 commented Nov 2, 2024

Closes #57

…ed.rs

simplified as it had so many conditions which were not the focus of the test
@josueBarretogit
Copy link
Owner

Hi thanks for helping solve this issue

Feel free to suggest a better fix for this.

I made the mistake of doing so much in one single test, so many things need to happen, I updated it can you check if it works? also is there a github action to emulate the environment this test failed in? I'm not very knowledgeable of nix

@youwen5
Copy link
Contributor Author

youwen5 commented Nov 2, 2024

Would you like me to set up an automatic Nix build/check action? It should require minimal maintenance after it is set up.

Since all Nix build environments are identical this action should be able to reproduce the test build failure.

@josueBarretogit
Copy link
Owner

Would you like me to set up an automatic Nix build/check action? It should require minimal maintenance after it is set up.

Since all Nix build environments are identical this action should be able to reproduce the test build failure.

yes to prevent this kind of stuff from happening again

ci: configure better options for nix check

ci: use checkout v4

ci: consolidate nix-check action into main action
@youwen5
Copy link
Contributor Author

youwen5 commented Nov 2, 2024

OK i tested the new changes and they seem to build properly with nix build. I need to check if it works in the nixpkgs package too, I think another test actually fails there due to the different Rust builder.

I added a job to the GitHub action that builds the Nix package as well.

@youwen5
Copy link
Contributor Author

youwen5 commented Nov 3, 2024

Just checked, it now builds in nixpkgs with no more failing tests so this looks good to merge.

@josueBarretogit josueBarretogit merged commit 131a520 into josueBarretogit:main Nov 3, 2024
6 checks passed
@josueBarretogit
Copy link
Owner

Many thanks hopefully stuff like this doesn´t happen again (it will most likely 💀)

@josueBarretogit
Copy link
Owner

It happened again

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

Successfully merging this pull request may close these issues.

2 participants