Bug 238248 - Review Request: ddccontrol - TFT monitor parameters control
Review Request: ddccontrol - TFT monitor parameters control
Status: CLOSED NOTABUG
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Nobody's working on this, feel free to take it
Fedora Package Reviews List
:
Depends On:
Blocks: FE-DEADREVIEW
  Show dependency treegraph
 
Reported: 2007-04-28 05:41 EDT by Aleksey Kontsevich
Modified: 2012-01-02 13:24 EST (History)
3 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2007-07-07 01:58:57 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:


Attachments (Terms of Use)
ddccontrol SRPM (622.11 KB, application/x-rpm)
2007-04-28 05:44 EDT, Aleksey Kontsevich
no flags Details

  None (edit)
Description Aleksey Kontsevich 2007-04-28 05:41:03 EDT
Propose to add ddccontrol to the repository for setup TTF monitors from DDC that have not buttons. This is an analogue of MagicTune for Samsung monitors. Could be used for others too.

Description: DDCcontrol is a program running on Linux, used to control monitor parameters,like brightness and contrast, by software, i.e. without using the OSD
and the buttons in front of the monitor.
Comment 1 Aleksey Kontsevich 2007-04-28 05:44:17 EDT
Created attachment 153693 [details]
ddccontrol SRPM

DDC Control Source RPM
Comment 2 Bernard Johnson 2007-04-29 14:14:21 EDT
Looks like you don't have a sponsor, so I'm adding the FE-NEEDSPONSOR blocker.
Comment 3 manuel wolfshant 2007-05-08 16:29:10 EDT
Just a few comments, after a quick glance. Note that I cannot sponsor you, so I
am not assigning the package to me.
- BuildRoot is not one of the values imposed by
http://fedoraproject.org/wiki/Packaging/Guidelines
- gcc-c++ is part of the default build env, so there is no need to BR it
- % make and %install should start by cleaning the buildroot
- you should use (if possible) parallel make for building; if SMP build is
non-functional, this should be mentioned in the spec
- mock build fails with:
checking pci/pci.h usability... yes
checking pci/pci.h presence... yes
checking for pci/pci.h... yes
checking for pci_alloc in -lpci... no
configure: error: PCI utils library not found, please install pci-utils.
error: Bad exit status from /var/tmp/rpm-tmp.22865 (%build)

I admit I am surprised by this error, since according to the root.log both
pciutils and pciutils-devel are installed.
- please make sure that you do not need gettext as a BR (you seem to build
translations and this is a BR for them) or add it if you do.
Comment 4 manuel wolfshant 2007-05-08 16:33:39 EDT
I apologize, I meant "%install and %clean should start by cleaning [...]", not
"%make and %install [...]". In other words, you need to add a new section
(%clean) and to add rm -fR %{buildroot} both to %clean and to %install
Comment 5 Kiyoshi Matsui 2007-05-09 05:26:08 EDT
I am not yet a package-maintainer.  This is my "pre-review" to exercise
reviewing.

rpmbuild builds rpms successfully from the source with ddccontrol.spec,
and rpm -i installs the generated binary rpms (libddccontrol, ddccontrol,
ddccontrol-applet) successfully.  mock, however, ends with errors.  This
is due to lack of gettext in BuildRequires.  Adding this to the spec
file, mock builds rpms successfully.

rpmlint says on ddccontrol-0.4.2-1.fc6.src.rpm:
        --------------------start here---------------------
W: ddccontrol non-standard-group System/Configuration/Hardware
The value of the Group tag in the package is not valid.  Valid groups are:
"Amusements/Games", "Amusements/Graphics", "Applications/Archiving",
"Applications/Communications", "Applications/Databases",
"Applications/Editors", "Applications/Emulators", "Applications/Engineering",
"Applications/File", "Applications/Internet", "Applications/Multimedia",
"Applications/Productivity", "Applications/Publishing", "Applications/System",
"Applications/Text", "Development/Debug", "Development/Debuggers",
"Development/Languages", "Development/Libraries", "Development/System",
"Development/Tools", "Documentation", "System Environment/Base", "System
Environment/Daemons", "System Environment/Kernel", "System
Environment/Libraries", "System Environment/Shells", "User
Interface/Desktops", "User Interface/X", "User Interface/X Hardware Support".

W: ddccontrol hardcoded-path-in-buildroot-tag /var/tmp/%{name}-buildroot
A path is hardcoded in your Buildroot tag. It should be replaced
by something like %{_tmppath}/%name-root.

E: ddccontrol no-cleaning-of-buildroot %install
You should clean $RPM_BUILD_ROOT in the %clean section and just after the
beginning of %install section. Use "rm -Rf $RPM_BUILD_ROOT".

E: ddccontrol no-cleaning-of-buildroot %clean
You should clean $RPM_BUILD_ROOT in the %clean section and just after the
beginning of %install section. Use "rm -Rf $RPM_BUILD_ROOT".

W: ddccontrol no-%clean-section
The spec file doesn't contain a %clean section to remove the files installed
by the %install section.
        --------------------end here------------------------

rpmlint says on the generated ddccontrol-0.4.2-1.fc6.i368.rpm:
(Hereafter, I remove the same message about grouping above.)
        --------------------start here---------------------
E: ddccontrol standard-dir-owned-by-package /usr/share/man/man1
This package owns a directory that is part of the standard hierarchy, which
can lead to default directory permissions or ownerships being changed to
something non-standard.

E: ddccontrol setuid-binary /usr/bin/ddcpci root 04711
The file is setuid, this may be dangerous, especially if this 
file is setuid root.

E: ddccontrol non-standard-executable-perm /usr/bin/ddcpci 04711
A standard executable should have permission set to 0755. If you get this
message, it means that you have a wrong executable permissions in some files
included in your package.

W: ddccontrol incoherent-version-in-changelog 0.4.2-alt2 0.4.2-1.fc6
The last entry in %changelog contains a version identifier that is not
coherent with the epoch:version-release tuple of the package.

W: ddccontrol unstripped-binary-or-object /usr/bin/ddccontrol
W: ddccontrol unstripped-binary-or-object /usr/bin/ddcpci
W: ddccontrol unstripped-binary-or-object /usr/bin/gddccontrol
W: ddccontrol one-line-command-in-%post #%update_menus
You should use %post -p <command> instead of using:

%post
<command>

It will avoid the fork of a shell interpreter to execute your command as
well as allows rpm to automatically mark the dependency on your command
for the excecution of the scriptlet.

W: ddccontrol one-line-command-in-%postun #%clean_menus
You should use %postun -p <command> instead of using:

%postun
<command>

It will avoid the fork of a shell interpreter to execute your command as
well as allows rpm to automatically mark the dependency on your command
for the excecution of the scriptlet.
        --------------------end here------------------------

rpmlint says on the generated ddccontrol-applet-0.4.2-1.fc6.i368.rpm:
        --------------------start here---------------------
W: ddccontrol-applet no-documentation
The package contains no documentation (README, doc, etc).
You have to include documentation files.

W: ddccontrol-applet unstripped-binary-or-object /usr/lib/ddccontrol/ddcc-applet
        --------------------end here------------------------

rpmlint says on the generated libddccontrol-0.4.2-1.fc6.i368.rpm:
        --------------------start here---------------------
W: libddccontrol no-documentation
The package contains no documentation (README, doc, etc).
You have to include documentation files.

W: libddccontrol unstripped-binary-or-object /usr/lib/libddccontrol.so.0.0.0
        --------------------end here------------------------

Refer to:
        http://fedoraproject.org/wiki/Packaging/Guidelines

Please rewrite the spec file first.
Comment 6 Aleksey Kontsevich 2007-05-15 16:03:19 EDT
(In reply to comment #5)
> Please rewrite the spec file first.
You said it to me? I think I can do it but I do not know when ;)
Comment 7 Kiyoshi Matsui 2007-05-16 04:58:51 EDT
(In reply to comment #6)
> (In reply to comment #5)
> > Please rewrite the spec file first.
> You said it to me? I think I can do it but I do not know when ;)

Hello Aleksey,

Though I am not yet a package-maintainer, I think that you would do
better to revise your package on several points, and the package will
get closer to the Fedora guidelines by the expected revisions.

This bugzilla still lacks an assignee.  However, you can revise the
package anytime.
Comment 8 Aleksey Kontsevich 2007-05-17 17:05:44 EDT
No changes had been made to its spec? Some CVS, SVN?
Comment 9 Kiyoshi Matsui 2007-05-18 11:04:11 EDT
(In reply to comment #8)
> No changes had been made to its spec? Some CVS, SVN?

Hello Aleksey,

What do you mean?

Anyway, a new Fedora package must be reviewed, must be revised to
conform to the Fedora packaging guidelines, and must be approved by an
assignee.  Then, it will be permitted to put in Fedora's cvs.

About this process, refer to
    http://fedoraproject.org/wiki/PackageMaintainers/Join

I recommend you first to revise the package, then upload it as an
attachement to this page as the current one.
Comment 10 Mamoru TASAKA 2007-05-25 13:54:52 EDT
What is the status of this bug?
Comment 11 Aleksey Kontsevich 2007-05-31 15:01:15 EDT
Status? Searching time to fix spec to conform to the Fedora packaging guidelines ;)
Comment 12 Mamoru TASAKA 2007-06-09 13:33:19 EDT
Again ping?
Comment 13 Mamoru TASAKA 2007-06-17 11:26:08 EDT
ping again?
Comment 14 Mamoru TASAKA 2007-06-27 12:08:34 EDT
This bug will be closed if no response is received from the
reporter by 7/7.
Comment 15 Mamoru TASAKA 2007-07-07 01:58:57 EDT
CLOSING.

If someone want to maintain this package on Fedora, please file
a new bug report, thank you!
Comment 16 Paul P Komkoff Jr 2008-08-01 20:01:20 EDT
This thread is relevant to my interests
(I will open new bug as soon as I will finish my RPMs)
Comment 17 Aleksey Kontsevich 2008-08-28 14:48:05 EDT
May be it is better to just reopen it? ;)

Note You need to log in before you can comment on or make changes to this bug.