-
-
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
preload: init at 0.6.4 #141583
preload: init at 0.6.4 #141583
Conversation
Please follow https://github.com/NixOS/nixpkgs/blob/master/CONTRIBUTING.md the exeprefix and mapprefix will most likely need some changing in the module
|
Thanks, I'll be working on it |
BTW there is a guideline about naming PRs and commits You can rename a commit using If you are unsure about if the PR should be reviewed now or its work in progress you can mark it as draft. Things can take a lot of time until merge, I already sent PRs that took over a month until merge. |
ce49bc7
to
e949570
Compare
nixpkgs-fmt can be used to format the files
https://github.com/NixOS/nixpkgs/blob/master/CONTRIBUTING.md |
all-packages.nix shouldn't be formatted with nixpkgs-fmt |
i would recommend that you do |
Yes, already reverted. |
A elegant way to do this is to generate the conf using the ini generator
[1].
[1]
https://github.com/NixOS/nixpkgs/blob/master/pkgs/pkgs-lib/formats.nix#L77
…On Sat, Nov 6, 2021, 19:37 Artturi ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In nixos/modules/programs/preload.nix
<#141583 (comment)>:
> + configFile = pkgs.writeText "preload.conf"
+ ''
+ [model]
+ cycle = ${cfg.cycle}
+ usecorrelation = ${cfg.usecorrelation}
+ minsize = ${cfg.minsize}
+ memtotal = ${cfg.memtotal}
+ memfree = ${cfg.memfree}
+ memcached = ${cfg.memcached}
+
+ [system]
+ doscan = ${cfg.doscan}
+ dopredict = ${cfg.dopredict}
+ autosave = ${cfg.autosave}
+ mapprefix = ${cfg.mapprefix}
+ exeprefix = ${cfg.exeprefix}
+ processes = ${cfg.processes}
+ sortstrategy = ${cfg.sortstrategy}
+ '';
Please use rfc42 style options. then we wont have to have so many separate
options.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#141583 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ADXXO6GZWVDTTS2V5JF72ZLUKWU2DANCNFSM5F6H3D4Q>
.
|
I think it's a matter of waiting, as @lucasew mentioned:
But I'm not sure |
i will do some improvements and tests and force push the changes straight to your branch so remember to pull the changes correctly |
Considering the preload program hasn't received an update since 2013 i hope its not one of the most important packages for any distro Someone needs to test this on a user system instead of a vm https://nixos.wiki/wiki/Nixpkgs/Reviewing_changes#Modules |
I'm not sure exactly how AFAIU, "shrinkwrapped" binaries will make the dynamic linker startup complexity O(n) instead of O(n^2). (And per-library lookup O(1) instead of O(n).) |
@Artturin if you have a better alternative (for improving the startup of apps and is an active project) tell me, and we could happily dismiss this and use the alternative. |
I just installed it on a system of mine, seems to work just fine. What tests are needed? |
mapprefix = "/run/current-system/sw/share;/run/current-system/sw/lib;!/"; | ||
exeprefix = "!/run/current-system/sw/sbin/;/run/current-system/sw/bin;!/"; |
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.
@GustavoPeredo are you sure it works fine? the things that i am most concerned about are these 2
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.
They are following expected behavior, but it would be more useful if the nix store was added to these paths.
There is now a problem with how the module writes the configuration file: It only adds modified values to the file (excludes defaults). Does that mean the modules file has to be rewritten with each value as it's own mkOption? Like so:
|
I think we just need to add |
doCheck = false; | ||
|
||
meta = with lib; { | ||
homepage = "http://sourceforge.net/projects/preload"; |
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.
homepage = "http://sourceforge.net/projects/preload"; | |
homepage = "https://sourceforge.net/projects/preload/"; |
|
||
postPatch = '' | ||
# "--logdir=/var/log/preload" failed with unknown option | ||
substituteInPlace configure.ac \ | ||
--replace "logdir='\''${localstatedir}/log'" "logdir='\''${localstatedir}/log/preload'" | ||
''; |
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.
postPatch = '' | |
# "--logdir=/var/log/preload" failed with unknown option | |
substituteInPlace configure.ac \ | |
--replace "logdir='\''${localstatedir}/log'" "logdir='\''${localstatedir}/log/preload'" | |
''; |
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.
Were these formatting changes tested? I want to commit them, but had no time to test.
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 but I am highly confident that they just work. Nix does not care about the ordering but it is very good practice to use somewhat standard ordering.
src = fetchurl { | ||
url = "mirror://sourceforge/${pname}/${pname}-${version}.tar.gz"; | ||
sha256 = "d0a558e83cb29a51d9d96736ef39f4b4e55e43a589ad1aec594a048ca22f816b"; | ||
}; |
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.
}; | |
}; | |
postPatch = '' | |
# "--logdir=/var/log/preload" failed with unknown option | |
substituteInPlace configure.ac \ | |
--replace "logdir='\''${localstatedir}/log'" "logdir='\''${localstatedir}/log/preload'" | |
''; |
version = "0.6.4"; | ||
|
||
src = fetchurl { | ||
url = "mirror://sourceforge/${pname}/${pname}-${version}.tar.gz"; |
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.
url = "mirror://sourceforge/${pname}/${pname}-${version}.tar.gz"; | |
url = "mirror://sourceforge/preload/preload-${version}.tar.gz"; |
{ | ||
lib, | ||
stdenv, | ||
fetchurl, | ||
glib, | ||
pkg-config, | ||
help2man, | ||
autoreconfHook, | ||
}: | ||
stdenv.mkDerivation rec { |
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.
{ | |
lib, | |
stdenv, | |
fetchurl, | |
glib, | |
pkg-config, | |
help2man, | |
autoreconfHook, | |
}: | |
stdenv.mkDerivation rec { | |
{ lib | |
, stdenv | |
, fetchurl | |
, glib | |
, pkg-config | |
, help2man | |
, autoreconfHook | |
}: | |
stdenv.mkDerivation rec { |
{ | ||
config, | ||
lib, | ||
pkgs, | ||
... | ||
}: | ||
with lib; let | ||
cfg = config.programs.preload; | ||
settingsFormat = pkgs.formats.ini {}; | ||
configFile = settingsFormat.generate "preload.conf" cfg.settings; | ||
in { |
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.
{ | |
config, | |
lib, | |
pkgs, | |
... | |
}: | |
with lib; let | |
cfg = config.programs.preload; | |
settingsFormat = pkgs.formats.ini {}; | |
configFile = settingsFormat.generate "preload.conf" cfg.settings; | |
in { | |
{ config | |
, lib | |
, pkgs | |
, ... | |
}: | |
with lib; | |
let | |
cfg = config.programs.preload; | |
settingsFormat = pkgs.formats.ini {}; | |
configFile = settingsFormat.generate "preload.conf" cfg.settings; | |
in { |
type = types.submodule { | ||
freeformType = settingsFormat.type; | ||
}; | ||
apply = recursiveUpdate default; |
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.
default
can't be referenced here. Just moving it to the let block works.
Also settings.default
should be {}
to allow just using the default config.
Closing as dead and with conflicts. |
Motivation for this change
I'm loving nixos! The only problem is that I've noticed some apps take longer to load, that's why I'm packaging
preload
, so programs can load faster.NOTES:
1- This is my first package... I'm trying to learn here, so feel free to criticize as much as it is needed!
2- There are some, simingly, impossible to solve problems:
a) Upstream: The preload package can't create /var/preload/* and /var/log/preload.log, paths and files must be created/touched first (manually)
b) Indexing: I think indexing the entire nixstore would be too much for
preload
, it would be cool to be able to be able to get the paths of certain packages at build time, but I don't think this is possible. I'm currently indexing my home folder for my flatpaks, seem to be working well.c) Systemd:
preload
comes with a for systemd, should I package it as a module as well then? Similar to3- Where can I find more testing documentation? I'm having trouble finding resources
Things done
sandbox = true
set innix.conf
? (See Nix manual)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
./result/bin/
)