openobserve: 0.7.2 -> 0.8.1

[risicle]

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 in all-packages.nix despite being by-name.

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:
  • NixOS test(s) (look inside nixos/tests)
  • and/or package tests
  • or, for functions and "core" functionality, tests in lib/tests or pkgs/test
  • made sure NixOS tests are linked to the relevant packages
• 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.

[happysalada]

it's uncanny, I was just about to do an update myself.
https://github.com/NixOS/nixpkgs/pull/289526/files
in general this looks good except for one problem that I ran into
I needed this in env for the ui part
NODE_OPTIONS="--max-old-space-size=8192";
so right after line 41.

[risicle]

Ah.. that problem went away for me when I built on a bigger machine, but if that does the trick all the better.

[happysalada]

@risicle the pkgs/by-name/check is failing , are we still good to merge ?

[risicle]

I think so, I don't think it's clever enough to detect this situation (though I thought they were going to fix that 😕)

[camelpunch]

@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?

[camelpunch]

You can reproduce this on master nixpkgs with pkgs/test/nixpkgs-check-by-name/scripts/run-local.sh master:

$ 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

[adisbladis]

Because this broke the by-name check I reverted it in #289655.

[happysalada]

Alright here is what I suggest for the next step for openobserve.
0.8.2-rc.1 has just been released, it's probably going to be released in a couple of days. I'm going to wait for that (I'm being lazy I know), and then push an update to my PR.
I'm wondering if we can just fix the apple_sdk_11_0 in the package.nix file for now. I've pushed that change to my branch and will test that when 0.8.2 comes about.

[happysalada]

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.

[infinisil]

I think so, I don't think it's clever enough to detect this situation (though I thought they were going to fix that 😕)

All unintentional pkgs/by-name false positives have been fixed by now. The failure is here because all packages defined via pkgs/by-name must use the standard callPackage, that's always been the case.

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