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

tere: 1.5.0 -> 1.5.1-unstable-2024-04-01 #298527

Merged
merged 1 commit into from
Jun 5, 2024
Merged

Conversation

ProducerMatt
Copy link
Member

@ProducerMatt ProducerMatt commented Mar 24, 2024

Description of changes

Updated to the latest version, as well as the latest dev version which contains fixes for 4 failing cargo tests. More details about the failing tests here

pbsds edit: fixes #298489

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.05 Release Notes (or backporting 23.05 and 23.11 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.

@ProducerMatt
Copy link
Member Author

Will fix #298489

pkgs/tools/misc/tere/default.nix Outdated Show resolved Hide resolved
pkgs/tools/misc/tere/default.nix Outdated Show resolved Hide resolved
pkgs/tools/misc/tere/default.nix Outdated Show resolved Hide resolved
@ProducerMatt ProducerMatt changed the title tere: 1.5.0 -> 1.5.1 tere: 1.5.0 -> 1.5.1-unstable-2024-04-01 Apr 7, 2024
@ProducerMatt
Copy link
Member Author

ProducerMatt commented Apr 7, 2024

Updated to latest dev version which fixes the issues with the test. Also corrected issues mentioned by @mrene, thank you.

@mrene
Copy link
Contributor

mrene commented Apr 7, 2024

Changes look good!

There is still a test failure under aarch64-darwin, it feels specific to nix's build environment so it could be skipped.

Test log

     Running tests/cli.rs (target/aarch64-apple-darwin/release/deps/cli-49276a274f19a142)

running 4 tests
test tests::first_run_prompt_accept ... FAILED
test tests::output_on_exit_without_cd ... ok
test tests::first_run_prompt_cancel ... ok
test tests::basic_run ... ok

failures:

---- tests::first_run_prompt_accept stdout ----
thread 'tests::first_run_prompt_accept' panicked at tests/cli.rs:132:5:
assertion `left == right` failed
  left: "Error: Io(Os { code: 30, kind: ReadOnlyFilesystem, message: \"Read-only file system\" })\r\n"
 right: "/private/tmp/nix-build-tere-1.5.1-unstable-2024-04-01.drv-0/.tmpOtGxew\r\n"
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace


failures:
    tests::first_run_prompt_accept

test result: FAILED. 3 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.34s

error: test failed, to rerun pass `--test cli`

@arduano
Copy link
Contributor

arduano commented Apr 8, 2024

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

1 package built:
  • tere

@timon-schelling
Copy link
Contributor

checkFlags = [ "--skip=first_run_prompt_accept" ];

Plus a comment on why the test should be skipped should be enough to resolve this right?

@ProducerMatt
Copy link
Member Author

@mgunyho any ideas about this failing test? I figure I'll disable it since NixOS + aarch64 is such a niche, but wanted to ping you and see #298527 (comment)

@mgunyho
Copy link

mgunyho commented Apr 13, 2024

Hm, that test creates a temporary folder and then tries to place a file in it, apparently the filesystem is read-only and that's why it's failing. It's weird that creating the temp folder succeeds though. But yeah, it can be skipped with --skip first_run_prompt_accept as mentioned. I suggest skipping that test only for this platform if possible, just to be sure.

@timon-schelling
Copy link
Contributor

@ProducerMatt Anything new?

@ProducerMatt
Copy link
Member Author

@timon-schelling My bad, I dropped the ball. Here's the version with the commented test. If this builds fine on ARM Darwin then we can merge away.

@timon-schelling
Copy link
Contributor

@ProducerMatt I think u need to use stdenv.targetPlatform.system instead of system and accept stdenv as a parameter of the pkg function.

@ProducerMatt
Copy link
Member Author

@timon-schelling done!

@timon-schelling
Copy link
Contributor

Properly not getting merged before 24.05?

@timon-schelling
Copy link
Contributor

@mrene

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-ready-for-review/3032/3921

@pbsds
Copy link
Member

pbsds commented Jun 1, 2024

fixes https://hydra.nixos.org/build/260562089 and https://hydra.nixos.org/build/260494209, but https://hydra.nixos.org/build/260624373 and https://hydra.nixos.org/build/260446230 still fails with new errors

I think disabling first_run_prompt_accept on x86_64-darwin also makes sense, consider stdenv.isDarwin.

I don't understand why aarch64-linux is looking for what to me seems like a cross linker:

error: linker `aarch64-linux-gnu-gcc` not found

@ProducerMatt
Copy link
Member Author

I've pushed the suggested changes to GitHub but they haven't showed up yet. Hopefully it's not another of my long string of Git mistakes. 😬

@ProducerMatt
Copy link
Member Author

ProducerMatt commented Jun 3, 2024

It was, in fact, another of my long string of Git mistakes. 🙃

@pbsds
Copy link
Member

pbsds commented Jun 4, 2024

It seems tere doesn't receive frequent attention in terms of PRs. Please mark aarch64-linux as meta.broken

@ProducerMatt
Copy link
Member Author

Implemented this script described by @jyooru in #145726 (comment)

Let's see if it fixes aarch64-linux

@pbsds pbsds merged commit 2ae2d53 into NixOS:master Jun 5, 2024
24 checks passed
@ProducerMatt
Copy link
Member Author

@pbsds thank you for the feedback and the merge approval! Hope you have a good day! 🙂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment