[Bug 230324] Review Request: avrdude -Software for programming Atmel AVR Microcontroller

[bugzilla at redhat.com]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

Summary: Review Request: avrdude -Software for programming Atmel AVR Microcontroller


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





------- Additional Comments From j.w.r.degoede at hhs.nl  2007-02-28 08:06 EST -------
(In reply to comment #3)
> (In reply to comment #1)
> * The package builds against libusb if it finds it => Non deterministic builds
> 
> If you want to force building against libusb:
> => BuildRequires: libusb-devel
> 

You're right I missed that in the ./configure output, I say we _want_ usb
support, so that BuildRequires: libusb-devel should be added.

Trond, you didn't do that in your new version, so can you fix that please.

> * What also makes me wonder is the sources shipping a *.info but the package not
> installing it ?!?
> 

It is, good catch. I see that there also is no %doc, see below.

> 
> (In reply to comment #2)
> > * Can you explain a bit about how the config file is not supposed to 
> > be modified by end-users? 
> I doubt Trond's statement. It's a system-wide configuration file, being
> generated by the configure script, not a sample.
> IMO, if it's a sample then it must not be located under /etc but should be
> placed elsewhere (e.g. %doc)
> 
> For the moment I'd recommend to use
> %configure ... --sysconfdir=%{_sysconfdir}/avrdude
> And to mark it %config(noreplace)
> 
> i.e. to treat it as a system-wide config file.
> 

Agreed, but why put it in a seperate subdir of /etc, do we expect avrdude to
have multiple config files, is there a guideline for this I'm not aware of. I
think this is deviating from upstream without a good reason. Making things like
the man and info page be out of sync with whats installed. Ralf can you explain
this?

New MUST FIX list:
==================
* add BuildRequires: libusb-devel
* install info page, to %install add
  install -p -m 644 doc/avrdude.info $RPM_BUILD_ROOT%{_infodir}
  and add it to %files and add the nescesarry scriptlets, see:
http://fedoraproject.org/wiki/Packaging/ScriptletSnippets#head-47896da5fb2662d75deefeb9ba75145a398515db
* add a %doc to %files, may I suggest:
  %doc AUTHORS COPYING ChangeLog* NEWS README doc/TODO


Trond maybe its a good idea to wait with a new version until Ralf answers my
question about the avrdude.conf location.



(In reply to comment #4)
> (In reply to comment #2) 
> > * Missing BuildRequires: ncurses-devel readline-devel. Note that ncurses-devel
> >   is not really needed as readline-devel already Requires it.
> 
> I did not have any problems when building the package in mock, but I added the
> requirements anyway.
>  

Sometimes a package can build just fine without some BuildRequires, but then is
missing some functionality, thats the case here. Thats why you should always
take a good look at ./configure's output. I thought I did, but I missed the
libusb usage.


-- 
Configure bugmail: https://bugzilla.redhat.com/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug, or are watching the QA contact.