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

glog: patch /usr/bin/true in tests #378903

Merged
merged 1 commit into from
Feb 5, 2025

Conversation

Pleune
Copy link
Contributor

@Pleune Pleune commented Feb 2, 2025

Low priority because current checkPhase does not fail. But perhaps in the future this could cause a failure.

Tests appear to use /usr/bin/true as a fake mailer, which causes an error in the check phase. This does not cause the tests to fail, and this is why the replace is a warn and not fail. Patching it nonetheless.

In fact, I only saw the error because that test failed when other constraints on my system caused the logging tests to fail, and print more verbose output.

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.

pkgs/by-name/gl/glog/package.nix Outdated Show resolved Hide resolved
pkgs/by-name/gl/glog/package.nix Outdated Show resolved Hide resolved
pkgs/by-name/gl/glog/package.nix Outdated Show resolved Hide resolved
@tobim
Copy link
Contributor

tobim commented Feb 2, 2025

This PR should target staging instead.

@SuperSandro2000 SuperSandro2000 changed the title glog: patch /usr/bin/true in tests (low priority) glog: patch /usr/bin/true in tests Feb 2, 2025
@SuperSandro2000
Copy link
Member

This PR rebuilds a lot of packages which means we must target staging. Please follow the contributing guide to not potentially ping a lot of people.

Tests appear to use `/usr/bin/true` as a fake mailer, which causes an
error in the check phase. This does not cause the tests to fail, and
this is why the replace is a warn and not fail. Patching it nonetheless.
@Pleune Pleune force-pushed the glog-usr-bin-true branch from 4d2a801 to 4f680c2 Compare February 3, 2025 17:54
@Pleune Pleune changed the base branch from master to staging February 3, 2025 17:55
@github-actions github-actions bot added 6.topic: python 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: changelog 6.topic: emacs Text editor 6.topic: rust 6.topic: vim 6.topic: cinnamon Desktop environment 6.topic: vscode 6.topic: continuous integration Affects continuous integration (CI) in Nixpkgs, including Ofborg and GitHub Actions and removed 6.topic: python 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: changelog 6.topic: emacs Text editor 6.topic: rust 6.topic: vim 6.topic: cinnamon Desktop environment 6.topic: vscode labels Feb 3, 2025
@github-actions github-actions bot removed the 6.topic: continuous integration Affects continuous integration (CI) in Nixpkgs, including Ofborg and GitHub Actions label Feb 3, 2025
@ofborg ofborg bot added 2.status: merge conflict This PR has merge conflicts with the target branch and removed 2.status: merge conflict This PR has merge conflicts with the target branch labels Feb 3, 2025
@Pleune Pleune requested a review from tobim February 3, 2025 17:58
@Pleune
Copy link
Contributor Author

Pleune commented Feb 3, 2025

Thanks both for the informative feedback. I have rebased onto staging and confirmed a good build on my system. I think I have done this in a way to avoid extra pings, so if that is not the case let me know.

@wegank wegank added the 12.approvals: 1 This PR was reviewed and approved by one reputable person label Feb 4, 2025
@SuperSandro2000 SuperSandro2000 merged commit 6334286 into NixOS:staging Feb 5, 2025
31 of 33 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants