-
-
Notifications
You must be signed in to change notification settings - Fork 14.1k
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
gvmd: init at 23.4.0 #303758
base: master
Are you sure you want to change the base?
gvmd: init at 23.4.0 #303758
Conversation
referencing #81418 |
8a762c8
to
f845c22
Compare
pkgs/by-name/gv/gvmd/package.nix
Outdated
|
||
buildPhase = '' | ||
substituteInPlace $src/src/sql_pg.c --replace "#include postgresql/libpq-fe.h" "#include ${postgresql}/include/libpq-fe.h" | ||
mkdir -p $out/{bin,etc,sbin,lib,usr/{share,include},run/gvmd/gvmd.pid} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
share
and include
are expected in $out
, not $out/usr
. Is there a particular reason not to use this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no need for the $out/usr will rely on the cmakeflags destination.
#305096 needed for gvmd |
ced0f63
to
9bc5d03
Compare
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/4075 |
''; | ||
|
||
meta = with lib; { | ||
description = "The central management service between security scanners and the user clients"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
description = "The central management service between security scanners and the user clients"; | |
description = "Central management service between security scanners and the user clients"; |
The policy has changed https://github.com/NixOS/nixpkgs/tree/master/pkgs#meta-attributes
pkgs/by-name/gv/gvmd/package.nix
Outdated
postInstall = '' | ||
mkdir -p \ | ||
$out/var/lib/gvm/gvmd/gvm-checking \ | ||
$out/run/gvmd/gvmd.pid |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These two should not be needed. If there is a reason, write in the comment above postInstall
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what will be the best way to approach this error. reason for the postintsall.
f1da724
to
6c10d5b
Compare
In between build time Flags are what's left. |
@Tochiaha Please, ask someone else to review this PR. The inputs I could offer, I already did. And I don't really understand how this application works. And how things are arranged. So my judgement here is not a good one. |
for reference it needs some other GVM package and Pgsql from this https://greenbone.github.io/docs/latest/22.4/source-build/index.html#id115 line down shows the setup. also when "gvmd --create-user=admin --password=''" is used it logs error regading roles and user for DB pgsql in $out/var/log/gvm/gvmd.log and run time. creating user and role with Pgsql and executing "gvmd --create-user=admin --password=''" results in this ouput "User created" which shows build time and runtime successful for gvmd. Achitecture for gvm can be seen here for better understanding https://greenbone.github.io/docs/latest/architecture.html#id1 |
As recommended @fabaff can help he done other required packages of GVM. |
I really don't understand what you are doing.
This makes no sense to me. Because the nix store is not writable. How/why are you saving logs there? My guess is, you are packaging a service which needs a NixOS module. I've already tried to explain to you this. Continuously I see red flags in what you are doing. And you don't seem to understand what I tell you. There is a distinction between what is to be done at build time and what is to be done at runtime. And a package is not part of the runtime. Any runtime consideration should be sorted at a NixOS module. The package can only set a default value. It's static. So far you have not answered how you are executing the application. How is this being run at runtime? Why you continuously add runtime concerns to a package? I don't have context of this application. I think you need feedback from someone that knows best. Which is not me. Still. I highly recommend you to add other reviews. I think, you should seek help. Particularly from someone that understand what you are doing. |
|
Most of these parameters shouldn't go to the nix store. The package is just a default value for these parameters. And it should be a standard system parameter, no $out (which is nix store). Then, there are the runtime concerns I told you. Which is separate of the package. |
-DDATADIR=$out/share \ | ||
-DGVM_SYSCONF_DIR=$out/etc/gvm \ | ||
-DGVM_DATA_DIR=$out/share/gvm \ | ||
-DGVM_STATE_DIR=$out/var/lib/gvm \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
State in the nix store?
-DGVMD_MAN_DIR=$out/share \ | ||
-DDATADIR=$out/share \ | ||
-DGVM_SYSCONF_DIR=$out/etc/gvm \ | ||
-DGVM_DATA_DIR=$out/share/gvm \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Which kind of data is this?
-DINCLUDEDIR=$out/include \ | ||
-DGMP_DIR=$out/share \ | ||
-DGVMD_MAN_DIR=$out/share \ | ||
-DDATADIR=$out/share \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
-DDATADIR=$out/share \ | |
-DDATADIR=$out/share \ |
Which kind of data is this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
without it result to https://termbin.com/bdhv
-DBINDIR=$out/bin \ | ||
-DSBINDIR=$out/sbin \ | ||
-DLIBDIR=$out/lib \ | ||
-DLOCALSTATEDIR=$out/var \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
-DLOCALSTATEDIR=$out/var \ | |
-DLOCALSTATEDIR=$out/var \ |
State in the nix store?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
without it result to https://termbin.com/bdhv
-DGVM_SYSCONF_DIR=$out/etc/gvm \ | ||
-DGVM_DATA_DIR=$out/share/gvm \ | ||
-DGVM_STATE_DIR=$out/var/lib/gvm \ | ||
-DGVMD_STATE_DIR=$out/var/lib/gvm/gvmd \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
-DGVMD_STATE_DIR=$out/var/lib/gvm/gvmd \ | |
-DGVMD_STATE_DIR=$out/var/lib/gvm/gvmd \ |
State in the nix store?
-DGVM_STATE_DIR=$out/var/lib/gvm \ | ||
-DGVMD_STATE_DIR=$out/var/lib/gvm/gvmd \ | ||
-DGVM_LIB_INSTALL_DIR=$out/lib \ | ||
-DGVMD_RUN_DIR=$out/run/gvmd \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
-DGVMD_RUN_DIR=$out/run/gvmd \ | |
-DGVMD_RUN_DIR=$out/run/gvmd \ |
Is it a pid
? If it is it shouldn't be in the nix store.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is this?
i can still do NixOS service module but consider this https://github.com/NixOS/nixpkgs/blob/nixos-unstable/pkgs/tools/security/ospd-openvas/default.nix#L45 @mweinelt any help here? |
Not interested in this package. Did only contribute a drive-by fix. |
Thank you. |
The NixOS module may be optional, but offering invalid paths as default paths for package was the main issue. Note that, at this point in time, we are seeking to clarify the arguments purposes and values. I have asked you several questions (for each argument). Try to answer them. We need to clarify if these arguments are correct. Once that is done, I don't have objections with this PR. |
Working on it. |
|
||
preFixup = '' | ||
substituteInPlace $out/lib/systemd/system/gvmd.service \ | ||
--replace-fail "/run/ospd/ospd-openvas.sock" "${ospd-openvas}/run/ospd/ospd-openvas.sock" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A sock
shouldn't exist in the nix store... That wouldn't be write-able.
Co-authored-by: Wolfgang Walther <[email protected]>
Release: https://github.com/greenbone/gvmd/releases/tag/v23.4.0
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.