[Bug 1089559] Review Request: tlp - Advanced power management tool for Linux

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



https://bugzilla.redhat.com/show_bug.cgi?id=1089559



--- Comment #4 from Jeremy Newton <alexjnewt@xxxxxxxxx> ---
(In reply to Christian Dersch from comment #3)
> Hi Jeremy!
> 
> There are some issues with the package:
> 
> 1.) Your package doesn't build in mock (please use it :), because the macros
> %{_presetdir} and %{_unitdir} are unknown. The macro definition is part of
> the systemd package, which should be a BuildRequires. Please add it!

Ah my bad, I'll fix this issue right away.. I usually check with mock, I was
negligent this time ;)

> 2.) I'm not the author of the file 50-tlp.preset, maybe you got this
> information because I created the very first rpm for TLP (some years ago,
> for openSUSE, no systemd) ;) Other rpms were derived from this work.

Indeed, you are correct. It seems Andreas Roederer wrote this. I'll give him
the credit.

> 3.) The sense of the preset file is enabling tlp automatically after
> installation? If yes: Please remove it, services should be enabled manually.
> Confirm https://fedoraproject.org/wiki/Packaging:Systemd#Why_don.27t_we....

I was under the impression that %systemd_post, %systemd_preun and
%systemd_postun_with_restart were required for systemd services period... See
here: https://fedoraproject.org/wiki/Packaging:ScriptletSnippets#Systemd

> 3.) Please order Requires (and BuildRequires) alphabetically

Will do

> About your questions:
> 
> 1.) About TLP_NO_PMUTILS=1 I have to check later. I don't know the answer
> right now. 

Alright thanks! I'll redirect the question to the code maintainer/developer.

> 2.) CONFIG_ACPI_PROC_EVENT is not set
> http://pkgs.fedoraproject.org/cgit/kernel.git/tree/config-x86-generic#n89

Oh okay, so my assumption was correct; as from what I know,
CONFIG_ACPI_PROC_EVENT is defaulted to N as of 2.6.23... if it even exists in
the current kernel.

> 3.) There is a macro for the udev rules, this is also part of the systemd
> package ;) It is called %{_udevrulesdir}, you can confirm the file
> /usr/lib/rpm/macros.d/macros.systemd for these macros

Ahhh the more you know. Thanks for the tip!


I do have another question/concern though. I know my package spec gives a
hardcoded-library-path in rpmlint... I only do this because building it x86-64
causes a build error, due to %{_libdir} being /usr/lib64, not /usr/lib, which
is necessary for this package. Granted, I'm told it SHOULD be patched to go
into %{_libexecdir} instead. Do you have any thoughts on this?

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
_______________________________________________
package-review mailing list
package-review@xxxxxxxxxxxxxxxxxxxxxxx
https://admin.fedoraproject.org/mailman/listinfo/package-review





[Index of Archives]     [Fedora Legacy]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]