Skip to content

Conversation

@Shawn8901
Copy link
Contributor

@Shawn8901 Shawn8901 commented Jul 7, 2025

also migrate to new repository they have created

see release notes

https://github.com/VictoriaMetrics/VictoriaLogs/releases/tag/v1.25.0

i combined update and repo switch in one commit, as the new repo does not contain old(er) releases, and the old one no newer, thus a inbetween commit would render in a not buildable commit for vlogs.

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/)
  • Nixpkgs 25.11 Release Notes (or backporting 25.05 Nixpkgs Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
  • NixOS 25.11 Release Notes (or backporting 25.05 NixOS Release notes)
    • (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, pkgs/README.md, maintainers/README.md and other contributing documentation in corresponding paths.

Add a 👍 reaction to pull requests you find important.

@acid-bong
Copy link
Contributor

acid-bong commented Jul 7, 2025

Patching must be changed as well:

Running phase: patchPhase
substitute(): ERROR: file 'lib/storage/storage_test.go' does not exist

And, even tho all those tests were moved to lib/logstorage, they were heavily rewritten and don't have those time.After() calls at all

@nixpkgs-ci nixpkgs-ci bot added 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-darwin: 1 This PR causes 1 package to rebuild on Darwin. 10.rebuild-linux: 1 This PR causes 1 package to rebuild on Linux. labels Jul 7, 2025
@Shawn8901
Copy link
Contributor Author

yeah i was interrupted in between my testing (noticed that my vl test branch is broken) and got distracted by that.
I was creating the PR in draft pretty early to avoid other people doing double work.

@Shawn8901
Copy link
Contributor Author

Shawn8901 commented Jul 7, 2025

So, i guess the version is now okay as far as i can tell.
Tested locally against a setup with journald-upload proxied via vlagent (to allow basic authentication) [i am considering upstreaming those changes also, but basically a copy how vmagent works].

Be aware of VictoriaMetrics/VictoriaLogs#409 which results in victoria-logs -version not showing a version and journal having a version dummy.

edit:
The -version issue is solved by the latest push, its now showing a proper version

@Shawn8901 Shawn8901 marked this pull request as ready for review July 7, 2025 21:32
@nix-owners nix-owners bot requested a review from NyCodeGHG July 7, 2025 21:33
also migrate to new repository
ldflags = [
"-s"
"-w"
"-X github.com/VictoriaMetrics/VictoriaMetrics/lib/buildinfo.Version=${finalAttrs.version}"
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

@Shawn8901 Shawn8901 Jul 8, 2025

Choose a reason for hiding this comment

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

Please see the linked issue in my previous comment (#423260 (comment)), and this PR. VictoriaMetrics/VictoriaLogs#419 and
VictoriaMetrics/VictoriaLogs@7db3660

On the force push, that I did in the morning you see that that was explicitly the value before (which is grabbed from the linked Makefile), but that was not correct, thus resulting in VL not being able to report it's version.

I don't see that we should merge a version with a broken version report, even tho it's wrongly published on 1.25.0 by upstream.

Tho if you strongly ask for that I can ofc force push again the ref with the broken version report

Copy link
Member

Choose a reason for hiding this comment

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

Imo we should leave it working with the "wrong" repo url and change it in the next release

Copy link
Contributor

Choose a reason for hiding this comment

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

Tho if you strongly ask for that

I don't. If it's reported wrong, a fix/workaround is fine

Copy link
Contributor Author

@Shawn8901 Shawn8901 Jul 8, 2025

Choose a reason for hiding this comment

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

Imo we should leave it working with the "wrong" repo url and change it in the next release

The repo url is not wrong according upstream.
If I understand the upstream issue correct all software from Victoria Metrics (organization) is importing the buildInfo from the victoriametrics repo, thus needing the VM repo as into in -X argument.
changing it results in not working versions n info.

As upstream did change the -X argument and not add a new buildInfo that seems to be at least intended rn.

I might be still wrong in what I understand from the written text in the issue/or, but it's at least how .

But let's see if/when upstream changes that.

@NyCodeGHG
Copy link
Member

NyCodeGHG commented Jul 8, 2025

Successfully built and ran tests on x86_64-linux

@nixpkgs-ci nixpkgs-ci bot added 12.approvals: 1 This PR was reviewed and approved by one person. 12.approved-by: package-maintainer This PR was reviewed and approved by a maintainer listed in any of the changed packages. labels Jul 8, 2025
@sinavir
Copy link

sinavir commented Jul 8, 2025

Hello,

nice work,

Do you think you could add the vlagent binary to the package ? maybe we could do something similar to what is done with vmagent for victoriametrics ?

@sinavir
Copy link

sinavir commented Jul 8, 2025

Hello,

nice work,

Do you think you could add the vlagent binary to the package ? maybe we could do something similar to what is done with vmagent for victoriametrics ?

Ok you said the same thing earlier. I guess it will be done in a followup !

@GaetanLepage
Copy link
Contributor

nixpkgs-review result

Generated using nixpkgs-review-gha

Command: nixpkgs-review pr 423260

Logs: https://github.com/GaetanLepage/nixpkgs-review-gha/actions/runs/16253731501


x86_64-linux

✅ 1 package built:
  • victorialogs

aarch64-linux

✅ 1 package built:
  • victorialogs

x86_64-darwin (sandbox = true)

✅ 1 package built:
  • victorialogs

aarch64-darwin (sandbox = true)

✅ 1 package built:
  • victorialogs

@Shawn8901 Shawn8901 mentioned this pull request Jul 13, 2025
13 tasks
@GaetanLepage GaetanLepage merged commit df0503c into NixOS:master Jul 13, 2025
28 of 29 checks passed
@Shawn8901 Shawn8901 deleted the update-vlogs branch July 14, 2025 05:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-darwin: 1 This PR causes 1 package to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 10.rebuild-linux: 1 This PR causes 1 package to rebuild on Linux. 12.approvals: 1 This PR was reviewed and approved by one person. 12.approved-by: package-maintainer This PR was reviewed and approved by a maintainer listed in any of the changed packages.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants