-
-
Notifications
You must be signed in to change notification settings - Fork 14.3k
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
openobserve: 0.7.2 -> 0.8.1 #289522
openobserve: 0.7.2 -> 0.8.1 #289522
Conversation
it's uncanny, I was just about to do an update myself. |
Ah.. that problem went away for me when I built on a bigger machine, but if that does the trick all the better. |
2a2d854
to
8979c17
Compare
@risicle the pkgs/by-name/check is failing , are we still good to merge ? |
I think so, I don't think it's clever enough to detect this situation (though I thought they were going to fix that 😕) |
@happysalada @risicle I'm getting the nixpkgs-check-by-name failure on an unrelated PR. Is it worth looking into what changed in this one? |
You can reproduce this on $ pkgs/test/nixpkgs-check-by-name/scripts/run-local.sh master
Using HEAD commit 573cead3dd6f4ca10ed1bc7dc3589bc85e150ab5
Creating Git worktree for the HEAD commit in /run/user/1000/tmp.fQkHak1Hbk/merged.. Done
Fetching base branch master to compare against.. 573cead3dd6f4ca10ed1bc7dc3589bc85e150ab5
Creating Git worktree for the base branch in /run/user/1000/tmp.fQkHak1Hbk/base.. Done
Merging base branch into the HEAD commit in /run/user/1000/tmp.fQkHak1Hbk/merged.. 573cead3dd6f4ca10ed1bc7dc3589bc85e150ab5
Reading pinned nixpkgs-check-by-name revision from pinned-tool.json.. d934204a0f8d9198e1e4515dd6fec76a139c87f0
Creating Git worktree for the nixpkgs-check-by-name revision in /run/user/1000/tmp.fQkHak1Hbk/tool-nixpkgs.. Done
Building/fetching nixpkgs-check-by-name..
/nix/store/5fjdmbiziyp47gfc9kmfgvxdlzd6bba1-nixpkgs-check-by-name
Running nixpkgs-check-by-name..
pkgs.openobserve: This attribute is manually defined (most likely in pkgs/top-level/all-packages.nix), which is only allowed if the definition is of the form `pkgs.callPackage pkgs/by-name/op/openobserve/package.nix { ... }` with a non-empty second argument.
Validation failed, see above errors
Cleaning up.. Done |
Because this broke the by-name check I reverted it in #289655. |
Alright here is what I suggest for the next step for openobserve. |
After another look, the vulnerability affects anything prior to 0.8.0, so 0.8.1 should be fine to merge. I'm going to test again my PR on x86_64 darwin and will merge if it passes. |
All unintentional I fully admit that the error messages aren't great for this case, and they should be more actionable, and also indicate whether merging would break master. I'll work on this next week, see NixOS/nixpkgs-vet#6 for more updates. I'll open a new PR to do this update that won't cause problems in a bit. Edit: #289751 |
This reverts commit ac637ef. The original PR (NixOS#289522) broke the `pkgs/by-name` check on master, so it was reverted in NixOS#289655. This reapplies the original commits and makes sure that the `pkgs/by-name` check works by moving it out of `pkgs/by-name`.
Description of changes
https://nvd.nist.gov/vuln/detail/CVE-2024-25106
Also needed to be switched to using
darwin.apple_sdk_11_0
to continue building on darwin x86_64 (tested on macos 12), which means it needs an entry inall-packages.nix
despite being by-name.Things done
nix.conf
? (See Nix manual)sandbox = relaxed
sandbox = true
nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD"
. Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/
)Add a 👍 reaction to pull requests you find important.