Bug 524346 - Review Request: avra - ATmel AVR 8-bit RISC microcontroller assembler
Summary: Review Request: avra - ATmel AVR 8-bit RISC microcontroller assembler
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
low
medium
Target Milestone: ---
Assignee: Chitlesh GOORAH
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2009-09-19 06:22 UTC by Alex Musolino
Modified: 2010-08-24 23:50 UTC (History)
4 users (show)

Fixed In Version: 1.2.3-4.fc11
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2009-10-03 18:58:38 UTC
chitlesh: fedora-review+
kevin: fedora-cvs+


Attachments (Terms of Use)

Description Alex Musolino 2009-09-19 06:22:17 UTC
Spec URL: http://silxnet.org/~alex/fedora/avra/avra.spec
SRPM URL: http://silxnet.org/~alex/fedora/avra/avra-1.2.3-1.fc10.src.rpm
Description: Avra is an assembler for Atmel's AVR 8-bit RISC microcontollers.
It is mostly compatible with Atmel's own assembler, but provides new features
such as better macro support and additional preprocessor directives.

This is my second Fedora package. I'm still looking for sponsorship.

Comment 1 Martin Gieseking 2009-09-19 09:29:04 UTC
Looks fine to me. Just a couple of notes:

- you should preserve the timestamps of the converted files:
    dos2unix -k AUTHORS
    iconv -f ISO-8859-1 -t UTF-8 README -o README.new
    touch -r README README.new
    mv README.new README

- remove INSTALL from %doc

- NEWS doesn't contain any useful information either. You can drop it too.

Comment 2 Alex Musolino 2009-09-19 09:47:22 UTC
Made changes as per above.

Spec URL: http://silxnet.org/~alex/fedora/avra/avra.spec
SRPM URL: http://silxnet.org/~alex/fedora/avra/avra-1.2.3-1.fc10.src.rpm

Comment 3 Chitlesh GOORAH 2009-09-19 10:16:16 UTC
I'll do the review.

If we get it in time for F-12, I'll add avra to the FEL-12 Livedvd.

Comment 4 Chitlesh GOORAH 2009-09-19 11:05:23 UTC
would you also package http://sourceforge.net/projects/side4linux/ ?

Comment 5 Chitlesh GOORAH 2009-09-19 12:11:17 UTC
#001: Add Examples to %doc

#002: Extract AVR000.zip and add the *.inc to /usr/share/avra-%{version}

#003: remove "avra" binary in %prep before starting any processes

Comment 6 Alex Musolino 2009-09-19 16:35:23 UTC
Changes made as per above.

Spec URL: http://silxnet.org/~alex/fedora/avra/avra.spec
SRPM URL: http://silxnet.org/~alex/fedora/avra/avra-1.2.3-2.fc10.src.rpm

As for side4linux, I guess I can package it as well. However, it's not something I plan to use myself, so I'm not sure if I would be the best person to maintain it.

Comment 7 Chitlesh GOORAH 2009-09-22 22:06:01 UTC
Alex, could you please make an unofficial review of this small package ?

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

I just want to see if you understood the basic RPM packaging. However your package looks good, I just need some time to verify the examples included.

I'm looking forward to add avra on the FEL 12 Livedvd.

Comment 8 Chitlesh GOORAH 2009-09-24 14:58:56 UTC
#001: the following should be in the %prep section

aclocal
autoconf
automake -a


# Fix up encoding of documentation
dos2unix -k AUTHORS
iconv -f ISO-8859-1 -t UTF-8 README -o README.new
touch -r README README.new
mv README.new README

# Unpack include files
unzip AVR000.zip -d include

# Fix permissions and encoding of examples
chmod 644 Example/*
dos2unix -k Example/*



#002: %doc
For example, do the following
%doc Example
instead
%doc Example/*

This will preserve the examples in a separate directory and not mixing it with other files

#003: use %{_datadir} macro instead of /usr/share

#004: Directory ownership
in %files,
add
%dir %{_datadir/%{name}-%{version}

#004: Preserve timestamps during make install process by adding 
INSTALL="%{_bindir}/install -p" to your make install

Also add "-p" to  your
install -m 0644 *.inc .....


#005: Add a small on %description stating that fedora also ships some deviced definition *.inc files in %{_datadir/%{name}-%{version}.

It will be helpful to the user

Comment 9 Alex Musolino 2009-09-25 12:22:26 UTC
All requested changes made (as per above).

Spec URL: http://silxnet.org/~alex/fedora/avra/avra.spec
SRPM URL: http://silxnet.org/~alex/fedora/avra/avra-1.2.3-3.fc10.src.rpm

Comment 10 Chitlesh GOORAH 2009-09-26 12:15:32 UTC
Can you please upload the srpm to your FASUSERNAME.fedorapeople.org space please ?
I'm having timeouts from your current host space.

Comment 11 Alex Musolino 2009-09-27 05:18:47 UTC
(In reply to comment #10)
> Can you please upload the srpm to your FASUSERNAME.fedorapeople.org space
> please ?
> I'm having timeouts from your current host space.  

Done - I forgot that I had access to that service now.

Spec URL: http://musolinoa.fedorapeople.org/avra/avra.spec
SRPM URL: http://musolinoa.fedorapeople.org/avra/avra-1.2.3-3.fc10.src.rpm

Comment 12 Chitlesh GOORAH 2009-09-28 19:42:13 UTC
(In reply to comment #8)

> #004: Preserve timestamps during make install process by adding 
> INSTALL="%{_bindir}/install -p" to your make install

It should rather be :
make INSTALL="%{_bindir}/install -p" install DESTDIR=$RPM_BUILD_ROOT

Please correct it, i'll complete the review as soon as possible.

Comment 14 Chitlesh GOORAH 2009-09-29 09:07:28 UTC
- MUST: The package is named according to the Package Naming Guidelines.
- MUST: The spec file name matches the base package %{name}
- MUST: The package meets the Packaging Guidelines.
- MUST: The package is licensed (GPLv2+) with an open-source compatible license
and meet other legal requirements as defined in the legal section of Packaging
Guidelines.
- MUST: The License field in the package spec file matches the actual license.
- MUST: the source package includes the text of the license(s) in its own file,
then that file, containing the text of the license(s) for the package is
included in %doc.
- MUST: the package does not contain any duplicate files in the %files
- MUST: the package owns all directories that it creates.
- MUST: The spec file must be written in American English.
- MUST: The spec file for the package is be legible. 
- MUST: The sources used to build the package must matches the upstream source,
as provided in the spec URL.
- MUST: The package successfully compiles and builds into binary rpms on at
least i586.
- MUST: All build dependencies is listed in BuildRequires.
- MUST: The spec file handles locales properly.: No locales in this package
- MUST: the package is not designed to be relocatable
- MUST: Permissions on files are set properly.
- MUST: The package has a %clean section, which contains rm -rf %{buildroot}
(or $RPM_BUILD_ROOT).
- MUST: The package consistently uses macros, as described in the macros
section of Packaging Guidelines.
- MUST: The package contains code, or permissible content. This is described in
detail in the code vs. content section of Packaging Guidelines.
- MUST: There are no Large documentation files
- MUST: %doc does not affect the runtime of the application. To summarize: If
it is in %doc, the program must run properly if it is not present.
- MUST: There are no Header files or static libraries 
- MUST: The package does not contain library files with a suffix 
- MUST: Package does NOT contain any .la libtool archives
- MUST: Package does not own files or directories already owned by other
packages. 

SHOULD Items:

 - SHOULD: The source package doesn't include license text(s) as COPYING
 - SHOULD: mock builds succcessfully in i586.
 - SHOULD: The reviewer tested that the package functions as described. A
package should not segfault instead of running, for example.
 - SHOULD:  Those scriptlets used are sane. 


Approved

Comment 15 Chitlesh GOORAH 2009-09-29 09:08:11 UTC
Let me know when you have built it for F-12, I'll add avra to the FEL Livedvd.

Comment 16 Alex Musolino 2009-09-29 11:31:49 UTC
New Package CVS Request
=======================
Package Name: avra
Short Description: ATmel AVR 8-bit RISC microcontroller assembler
Owners: musolinoa
Branches: F-10 F-11 F-12
InitialCC:

Comment 17 Kevin Fenzi 2009-09-29 20:16:50 UTC
cvs done.

Comment 18 Alex Musolino 2009-10-01 07:09:10 UTC
(In reply to comment #15)
> Let me know when you have built it for F-12, I'll add avra to the FEL Livedvd.  

Built successfully for F-10, F-11, F-12.

Comment 19 Chitlesh GOORAH 2009-10-01 13:08:18 UTC
Push avra and picprog to fedora stable repositories
https://admin.fedoraproject.org/updates/
don't forget to add the bugzilla number of both package reviews to your package submission. Bodhi will automatically close these bugzilla reports.

Comment 20 Fedora Update System 2009-10-01 15:13:49 UTC
avra-1.2.3-4.fc10 has been submitted as an update for Fedora 10.
http://admin.fedoraproject.org/updates/avra-1.2.3-4.fc10

Comment 21 Fedora Update System 2009-10-01 15:17:27 UTC
avra-1.2.3-4.fc11 has been submitted as an update for Fedora 11.
http://admin.fedoraproject.org/updates/avra-1.2.3-4.fc11

Comment 22 Fedora Update System 2009-10-03 18:58:32 UTC
avra-1.2.3-4.fc10 has been pushed to the Fedora 10 stable repository.  If problems still persist, please make note of it in this bug report.

Comment 23 Fedora Update System 2009-10-03 19:04:12 UTC
avra-1.2.3-4.fc11 has been pushed to the Fedora 11 stable repository.  If problems still persist, please make note of it in this bug report.

Comment 24 Alex Musolino 2010-08-24 14:06:04 UTC
Package Change Request
======================
Package Name: avra
New Branches: el6
Owners: musolinoa

Comment 25 Kevin Fenzi 2010-08-24 23:50:03 UTC
Git done (by process-git-requests).


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