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

rio: 0.1.17 -> 0.2.3 #359621

Merged
merged 1 commit into from
Jan 18, 2025
Merged

Conversation

soanvig
Copy link
Contributor

@soanvig soanvig commented Nov 27, 2024

https://github.com/raphamorim/rio/releases/tag/v0.2.0
https://github.com/raphamorim/rio/releases/tag/v0.2.1
https://github.com/raphamorim/rio/releases/tag/v0.2.2
https://github.com/raphamorim/rio/releases/tag/v0.2.3

Extracted note from 0.2.0:

Breaking: Minimum MacOS version went from El Captain to Big Sur on ARM64 and Catalina on Intel x86.

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.

@TornaxO7
Copy link
Contributor

TornaxO7 commented Nov 28, 2024

On linux my system it builds, and tests pass, however for whatever reason the builder exists with 129 status code, and there is no other error in the logs...

This was already raised: https://discourse.nixos.org/t/getting-build-error-from-nix-build-although-theres-no-error/54561/2 by @TornaxO7

peepoYes

Here's what I've found (yet):

  1. It starts to fail when it reached this test:
rio> test sugarloaf/src/font/loader/mod.rs - font::loader::Database::with_face_data (line 397) ... ignored
  1. During the execution of this test, my memory consumption increased non-stop. I assume that the process got killed then afterwards which returned to the exit code 129.

Would be awesome if someone else has got an idea how to fix that because damn... this error is pretty annoying
cat_rage

and the fix would also fix the nix-flake in the repo of rio!

@soanvig
Copy link
Contributor Author

soanvig commented Nov 28, 2024

Regarding comment on the forums, I noticed that indeed their nix build failed with this error, but on master new "Nix build" CI work properly. I tried using that commit with no success.

Note, that CI in this PR succeeded - false positive or something is wrong with our machines?

@TornaxO7
Copy link
Contributor

Note, that CI in this PR succeeded - false positive or something is wrong with our machines?

Wait, you're right.

monkaW

Huh? How?

@soanvig
Copy link
Contributor Author

soanvig commented Dec 2, 2024

Latest nixos packages, using unstable, still failing on nix-build -k -A rio

@NovaViper
Copy link
Contributor

NovaViper commented Dec 8, 2024

Just tried it with my flake and also experienced the same error 129 failure. Though unlike @TornaxO7 noted, I didn't have steadily increasing memory usage, it stayed consistent

rio> test context::grid::test::test_single_context_respecting_margin_and_no_quad_creation ... ok
error: builder for '/nix/store/rgj7x2d45zy5hf44nm3rkga1byhrri15-rio-0.2.2.drv' failed with exit code 129;
       last 25 log lines:
       > test event_loop_proxy_send ... ok
       > test window_builder_sync ... ok
       > test window_sync ... ok
       >
       > test result: ok. 4 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s
       >
       >      Running unittests src/main.rs (target/x86_64-unknown-linux-gnu/release/deps/rio-a84fdce5a2408e08)
       >
       > running 47 tests
       > test bindings::tests::binding_matches_different_action ... ok
       > test bindings::tests::binding_matches_identical_mode ... ok
       > test bindings::tests::binding_matches_mode_notmode ... ok
       > test bindings::tests::binding_matches_itself ... ok
       > test bindings::tests::binding_matches_modes ... ok
       > test bindings::tests::binding_matches_notmodes ... ok
       > test bindings::tests::binding_matches_partial_intersection ... ok
       > test bindings::tests::binding_mismatches_notmode ... ok
       > test bindings::tests::binding_mismatches_unrelated ... ok
       > test bindings::tests::binding_trigger_modes ... ok
       > test bindings::tests::binding_trigger_notmodes ... ok
       > test bindings::tests::binding_with_mode_matches_empty_mode ... ok
       > test bindings::tests::binding_without_mode_matches_any_mode ... ok
       > test bindings::tests::bindings_overwrite ... ok
       > test bindings::tests::mods_binding_requires_strict_match ... ok
       > test context::grid::test::test_single_context_respecting_margin_and_no_quad_creation ... ok
       For full logs, run 'nix log /nix/store/rgj7x2d45zy5hf44nm3rkga1byhrri15-rio-0.2.2.drv'.
error: 1 dependencies of derivation '/nix/store/3qbkhb7xpl9b8rjszlrjc4v24v4qhbi2-home-manager-path.drv' failed to build
error: 1 dependencies of derivation '/nix/store/q5lpfpr8snml2jlq8dfbx1d9hvqi775f-home-manager-generation.drv' failed to build
error: 1 dependencies of derivation '/nix/store/a4vb54ydh5ghb6ph03qrjmlfh35hfh26-user-environment.drv' failed to build
error: 1 dependencies of derivation '/nix/store/rz6yfmz1dp9977f5sk56l78mpfkdh6fq-etc.drv' failed to build
error: 1 dependencies of derivation '/nix/store/fddagg7jp7r4601h0si4mpkvq49ws6rm-nixos-system-yoganova-25.05.20241205.d0797a0.drv' failed to build
┏━ 1 Errors:
┃ error: builder for '/nix/store/rgj7x2d45zy5hf44nm3rkga1byhrri15-rio-0.2.2.drv' failed with exit code…
┃        last 25 log lines:
┃        > test event_loop_proxy_send ... ok
┃        > test window_builder_sync ... ok
┃        > test window_sync ... ok
┃        >
┃        > test result: ok. 4 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.0…
┃        >
┃        >      Running unittests src/main.rs (target/x86_64-unknown-linux-gnu/release/deps/rio-a84fdc…
┃        >
┃        > running 47 tests
┃        > test bindings::tests::binding_matches_different_action ... ok
┃        > test bindings::tests::binding_matches_identical_mode ... ok
┃        > test bindings::tests::binding_matches_mode_notmode ... ok
┃        > test bindings::tests::binding_matches_itself ... ok
┃        > test bindings::tests::binding_matches_modes ... ok
┃        > test bindings::tests::binding_matches_notmodes ... ok
┃        > test bindings::tests::binding_matches_partial_intersection ... ok
┃        > test bindings::tests::binding_mismatches_notmode ... ok
┃        > test bindings::tests::binding_mismatches_unrelated ... ok
┃        > test bindings::tests::binding_trigger_modes ... ok
┃        > test bindings::tests::binding_trigger_notmodes ... ok
┃        > test bindings::tests::binding_with_mode_matches_empty_mode ... ok
┃        > test bindings::tests::binding_without_mode_matches_any_mode ... ok
┃        > test bindings::tests::bindings_overwrite ... ok
┃        > test bindings::tests::mods_binding_requires_strict_match ... ok
┃        > test context::grid::test::test_single_context_respecting_margin_and_no_quad_creation ... ok
┃        For full logs, run 'nix log /nix/store/rgj7x2d45zy5hf44nm3rkga1byhrri15-rio-0.2.2.drv'.
┣━ Dependency Graph:
┃                ┌─ ⏸ hm_fontconfigconf.d10hmfonts.conf waiting for 1 ⚠
┃             ┌─ ⏸ home-manager-files
┃          ┌─ ⏸ home-manager-generation
┃       ┌─ ⏸ unit-home-manager-novaviper.service
┃    ┌─ ⏸ system-units
┃    │     ┌─ ⚠ rio-0.2.2 failed with exit code 129 after ⏱ 12m14s in checkPhase
┃    │  ┌─ ⏸ home-manager-path
┃    ├─ ⏸ user-environment
┃ ┌─ ⏸ etc
┃ ⏸ nixos-system-yoganova-25.05.20241205.d0797a0
┣━━━ Builds
┗━ ∑ ⏵ 0 │ ✔ 0 │ ⏸ 9 │ ⚠ Exited after 1 build failures at 00:08:03 after 13m18s
Error:
   0: Command exited with status Exited(1)

Location:
   src/commands.rs:151

Build log
build-fail.txt

Nix info:

❯ nix-info -m
 - system: `"x86_64-linux"`
 - host os: `Linux 6.12.1, NixOS, 25.05 (Warbler), 25.05.20241205.d0797a0`
 - multi-user?: `yes`
 - sandbox: `yes`
 - version: `nix-env (Nix) 2.25.2`
 - nixpkgs: `/nix/store/nr5nl3zwzl02x3rnikjbry3s5xy7bm1d-source`

raphamorim added a commit to raphamorim/rio that referenced this pull request Dec 18, 2024
@raphamorim
Copy link

I re-enabled the checks on Rio term, I will try to do a bisect of split PR that broke this pipeline.

@raphamorim
Copy link

Ok, my guess is that after the split PR got merged, I introduced a test that's flaky.

The grid tests have a lot of unsafe calls of libc and if it breaks will not be logged properly because it's ub, and the test will just close.

My guess is that ci have a pointer alignment different than our machines, I will try to figure this out.

@TornaxO7 TornaxO7 mentioned this pull request Dec 19, 2024
10 tasks
@jcdickinson
Copy link
Contributor

I got it building on my machine/PR by disabling a subset of the tests, it seems the context::grid tests are somehow to blame. It's the one linked above.

@raphamorim
Copy link

@jcdickinson disable the grid tests fixed for you? Will give a try but looks everything is connected to the resize fn not working properly in the sandbox

@jcdickinson
Copy link
Contributor

@jcdickinson disable the grid tests fixed for you? Will give a try but looks everything is connected to the resize fn not working properly in the sandbox

Yeah, I ran the tests without parallelism plus --nocapture and it consistently crashed on those. Disabling them resulted in a complete build.

@raphamorim
Copy link

aha, ok. yea i imagine there's something off in the created context on nix "sandbox" i need to investigate how to normalize it

@raphamorim
Copy link

Ok, took a while sorry about that. Fixed this issue here -> raphamorim/rio#853

@soanvig
Copy link
Contributor Author

soanvig commented Dec 22, 2024

Tested with 67e64aeb4216eb4656386ef7cedef5456f0d42ff commit (rev hash: sha256-tZEBk4v9AJQaLVOeWVAgYfHP1m6NS3hIXxXOyNvvlM0=, cargo hash: sha256-HOkaPsHvRGGBDlnVRvMKCJLOrOQKDlaowLveL35JAK0=). Now there is no error.

That being said, let's wait for rio 0.2.3 to include that change, and it will be done

@soanvig soanvig changed the title (rio): 0.1.17 -> 0.2.2 (rio): 0.1.17 -> 0.2.3 Jan 14, 2025
@soanvig
Copy link
Contributor Author

soanvig commented Jan 14, 2025

@TornaxO7 @oluceps @otavio
Build works locally with 0.2.3. Please test it by yourself. PR is updated to 0.2.3

Copy link
Member

@SuperSandro2000 SuperSandro2000 left a comment

Choose a reason for hiding this comment

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

Why are there () around the package name?


src = fetchFromGitHub {
owner = "raphamorim";
repo = "rio";
rev = "v${version}";
hash = "sha256-10E7tIuix0BGKFbADLhcReRC01FXV/dBivJjfSe/X/c=";
rev = "v0.2.3";
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
rev = "v0.2.3";
rev = "v${version}";

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@SuperSandro2000 ah, good spot. I also merged all commits into one without parens around name

@soanvig soanvig force-pushed the update-rio-to-0.2.2 branch from 0f7d026 to b664df5 Compare January 14, 2025 15:31
@soanvig soanvig changed the title (rio): 0.1.17 -> 0.2.3 rio: 0.1.17 -> 0.2.3 Jan 14, 2025
@wegank wegank added the 12.approvals: 2 This PR was reviewed and approved by two reputable people label Jan 14, 2025
@soanvig
Copy link
Contributor Author

soanvig commented Jan 15, 2025

Rio 0.2.3 requires Rust 1.84 and that's why the build fails: https://logs.ofborg.org/?key=nixos/nixpkgs.359621&attempt_id=5b3f6784-0ab6-4564-87e2-1c459244839f

@SuperSandro2000
Copy link
Member

How about you downgrade to the latest version that works with our rust version? Otherwise we need to wait a bit for 1.84 to be available.

@raphamorim
Copy link

v0.2.4 released with MSRV back to 1.80.1 https://github.com/raphamorim/rio/releases/tag/v0.2.4

@soanvig
Copy link
Contributor Author

soanvig commented Jan 18, 2025

How about you downgrade to the latest version that works with our rust version? Otherwise we need to wait a bit for 1.84 to be available.

@SuperSandro2000 I cannot. Older versions of Rio 2.x had a bug leading to builds not passing.

@raphamorim splendid. Testing right now

@soanvig soanvig force-pushed the update-rio-to-0.2.2 branch from b664df5 to 178ee76 Compare January 18, 2025 00:41
@soanvig
Copy link
Contributor Author

soanvig commented Jan 18, 2025

@raphamorim works locally, now let's see how nixpks pipelines feel

@SuperSandro2000 SuperSandro2000 merged commit 26b33b8 into NixOS:master Jan 18, 2025
25 of 27 checks passed
@soanvig soanvig deleted the update-rio-to-0.2.2 branch January 18, 2025 03:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10 12.approvals: 2 This PR was reviewed and approved by two reputable people 12. first-time contribution This PR is the author's first one; please be gentle!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants