Bug 954108 - Review Request: gimp-gap - The GIMP Animation Package
Summary: Review Request: gimp-gap - The GIMP Animation Package
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Mario Blättermann
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: DESIGN-SW
TreeView+ depends on / blocked
 
Reported: 2013-04-20 20:01 UTC by Pavel Alexeev
Modified: 2013-12-21 08:03 UTC (History)
6 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2013-12-21 08:03:38 UTC
Type: ---
Embargoed:
mario.blaettermann: fedora-review+
gwync: fedora-cvs+


Attachments (Terms of Use)

Description Pavel Alexeev 2013-04-20 20:01:57 UTC
Spec URL: https://github.com/Hubbitus/Fedora-packaging/blob/master/SPECS/gimp-gap.spec
SRPM URL: http://hubbitus.info/rpm/Fedora18/gimp-gap/gimp-gap-2.7.0-2.GITe75bd46.src.rpm
Description: The GIMP-GAP (GIMP Animation Package) is a collection of Plug-Ins to
extend GIMP 2.6 with capabilities to edit and create animations as
sequences of single frames.

Fedora Account System Username: hubbitus

Comment 1 Vasiliy Glazov 2013-04-26 05:23:57 UTC
Change spec URL to direct https://raw.github.com/Hubbitus/Fedora-packaging/master/SPECS/gimp-gap.spec

Comment 2 Antonio T. (sagitter) 2013-04-26 17:36:21 UTC
Hi Pavel.

gimp-gap's project seems idle since several months ...

Is it just for GIMP 2.6 ? Or even for its following releases ?

If it works with Gimp 2.8 (currently available in Fedora) too, '%description' in the .spec file is deceptive:

>The GIMP-GAP (GIMP Animation Package) is a collection of Plug-Ins to
>extend GIMP 2.6 with capabilities to edit and create animations as
>sequences of single frames.

Comment 3 Pavel Alexeev 2013-04-27 16:29:13 UTC
It is primary build for Gimp 1.8 on Fedora. And its tested by some peoples from my repository http://hubbitus.info/rpm/Fedora18/gimp-gap/

And I had deleted version mention from description to avoid confusing.

Comment 4 Pavel Alexeev 2013-04-27 16:29:45 UTC
Sorry, for gimp 2.8 off course, not 1.8.

Comment 5 Antonio T. (sagitter) 2013-04-28 19:07:39 UTC
- 'libjpeg-turbo-devel' is missing among BuildRequires 

- Your package doesn't build in mock on rawhide, it needs 'automake17'.

Also, 

- there are some warnings during configure tasks:

>configure.in:34: installing './config.guess'
>configure.in:34: installing './config.sub'
>configure.in:30: installing './install-sh'
>configure.in:30: installing './missing'
>Makefile.am: installing './INSTALL'
>Makefile.am: installing './COPYING' using GNU General Public License v3 file
>Makefile.am:     Consider adding the COPYING file to the version control system
>Makefile.am:     for your code, to avoid questions about which license your >project uses
>gap/Makefile.am: installing './depcomp'
>Symlinking file mkinstalldirs
>Symlinking file po/Makefile.in.in
>
>Please add the files
>  codeset.m4 gettext.m4 glibc21.m4 iconv.m4 isc-posix.m4 lcmessage.m4
>  progtest.m4
>from the /usr/share/aclocal directory to your autoconf macro directory
>or directly to your aclocal.m4 file.
>You will also need config.guess and config.sub, which you can get from
>ftp://ftp.gnu.org/pub/gnu/config/.
>
>checking for a BSD-compatible install... /usr/bin/install -c
>...

These latest two points must be taken in consideration in my opinion, especially for next rebuildings. You should re-configure locally makefile instead to use autogen.sh that runs --configure-- command already before %configure macro during RPM building.   

- %{_datadir}/locale/*/*/*

It is not necessary when you already use "%files -f gimp20-gap.lang"

Comment 6 Pavel Alexeev 2013-04-29 12:11:47 UTC
(In reply to comment #5)
> These latest two points must be taken in consideration in my opinion,
> especially for next rebuildings. You should re-configure locally makefile
> instead to use autogen.sh that runs --configure-- command already before
> %configure macro during RPM building.   
I think using upstream script is good idea to do not introduce well known bugs again.
As I rerun configure after call autogen.sh it should not be problem I think?

All other issues addressed, thank you.

Scratch build: http://koji.fedoraproject.org/koji/taskinfo?taskID=5312048
Spec changes: https://github.com/Hubbitus/Fedora-packaging/commit/c62ab84ea7f39496c75b95dc8bb80fbce48827cd
Srpm: http://hubbitus.info/rpm/Fedora18/gimp-gap/gimp-gap-2.7.0-3.GITe75bd46.src.rpm

Comment 7 Vasiliy Glazov 2013-04-29 12:34:01 UTC
1. In Release: add dist tag. Like 
Release:        2%{?dist}
2. Use parallel make:
make %{?_smp_mflags}
3. Update srpm package with the same spec file as in Spec URL.

Comment 8 Antonio T. (sagitter) 2013-04-29 16:58:40 UTC
(In reply to comment #6)
> (In reply to comment #5)
> > These latest two points must be taken in consideration in my opinion,
> > especially for next rebuildings. You should re-configure locally makefile
> > instead to use autogen.sh that runs --configure-- command already before
> > %configure macro during RPM building.   
> I think using upstream script is good idea to do not introduce well known
> bugs again.

Well, it's the shortest way but

https://bugzilla.redhat.com/buglist.cgi?quicksearch=autoconf

:)

> As I rerun configure after call autogen.sh it should not be problem I think?

In primis, it prolongs building time.

Comment 9 Pavel Alexeev 2013-04-29 18:33:03 UTC
(In reply to comment #7)
> 1. In Release: add dist tag. Like 
> Release:        2%{?dist}
Thanks. Done.
> 2. Use parallel make:
> make %{?_smp_mflags}
It break build. Spec file already stated it in comment.
> 3. Update srpm package with the same spec file as in Spec URL.
It is.

(In reply to comment #8)
> (In reply to comment #6)
> > (In reply to comment #5)
> Well, it's the shortest way but
> 
> https://bugzilla.redhat.com/buglist.cgi?quicksearch=autoconf
> :)
Sorry, but I don't understand what you want said. What will really different if I copypaste most of that's command in spec file instead of call it in intendes script?

> > As I rerun configure after call autogen.sh it should not be problem I think?
> In primis, it prolongs building time.
On my mind it have no worth for optimization but it have some rationale though.
I pass arguments to autogen.sh to configure it in single step.

Spec changes: https://github.com/Hubbitus/Fedora-packaging/commit/39d43b4a65ea980f121abab4d2c1254ca55d756d
Srpm: http://hubbitus.info/rpm/Fedora18/gimp-gap/gimp-gap-2.7.0-4.GITe75bd46.fc18.src.rpm

Comment 10 Antonio T. (sagitter) 2013-04-29 21:04:00 UTC
(In reply to comment #9)
> (In reply to comment #7)
> > 1. In Release: add dist tag. Like 
> > Release:        2%{?dist}
> Thanks. Done.
> > 2. Use parallel make:
> > make %{?_smp_mflags}
> It break build. Spec file already stated it in comment.
> > 3. Update srpm package with the same spec file as in Spec URL.
> It is.
> 
> (In reply to comment #8)
> > (In reply to comment #6)
> > > (In reply to comment #5)
> > Well, it's the shortest way but
> > 
> > https://bugzilla.redhat.com/buglist.cgi?quicksearch=autoconf
> > :)
> Sorry, but I don't understand what you want said. What will really different
> if I copypaste most of that's command in spec file instead of call it in
> intendes script?

Although it is an old page, this wiki summarizes the issue: http://fedoraproject.org/wiki/PackagingDrafts/AutoConf

Comment 11 Vasiliy Glazov 2013-04-30 06:44:36 UTC
Remove xvidcore from dependency. It available only in rpmfusion repository.

Comment 12 Pavel Alexeev 2013-04-30 15:38:53 UTC
@Antonio gimp-gap does not contain .configure script as it from git and not release, so I need generate it.

@Vasiliy, thanks. Done.

Spec changes: https://github.com/Hubbitus/Fedora-packaging/commit/f1c0077fbb6dc2da5feee6251c81f511313e8f1e
Srpm: http://hubbitus.info/rpm/Fedora18/gimp-gap/gimp-gap-2.7.0-5.GITe75bd46.fc18.src.rpm

Comment 13 Antonio T. (sagitter) 2013-04-30 18:35:29 UTC
(In reply to comment #12)
> @Antonio gimp-gap does not contain .configure script as it from git and not
> release, so I need generate it.
> 

You may work with upstream in order to release gimp-gap with a preconfigured script (like most upstreams do) otherwise by managing scripts locally, ensuring yourself they have been generated fine especially after a new Autotools release; integrating all changes as patches or additional files.

Comment 14 Michael Schwendt 2013-05-01 10:35:26 UTC
* I strongly recommend running "autoreconf -f" in the gimp-gap directory and fixing all the errors and warnings and submitting the fixes upstream.

* "sh autogen.sh" on F19 (perhaps not only there) runs into problems detecting "automake >= 1.7". It fails despite F19 coming with Automake 1.13.

Comment 15 Pavel Alexeev 2013-05-01 17:23:42 UTC
(In reply to comment #13)
> (In reply to comment #12)
> > @Antonio gimp-gap does not contain .configure script as it from git and not
> > release, so I need generate it.
> > 
> You may work with upstream in order to release gimp-gap with a preconfigured
> script (like most upstreams do)
As I known it is common practice to distribute in tarball configure scripts only for released versions.

> otherwise by managing scripts locally,
> ensuring yourself they have been generated fine especially after a new
> Autotools release;
autogen.sh already prefer most new available version. It especially for F19 and rawhide i need patch it. Please see in spec patch and comment on upstream bugreport about it.

> integrating all changes as patches or additional files.
Changes in what? It is because I prefer do not run separate command instead of upstream autor generation script.

(In reply to comment #14)
> * I strongly recommend running "autoreconf -f" in the gimp-gap directory and
> fixing all the errors and warnings and submitting the fixes upstream.
"autoreconf -f" silent on gimp-gap directory after run autogen.sh (before it fails and require run automake, autoheader and so on).

> * "sh autogen.sh" on F19 (perhaps not only there) runs into problems
> detecting "automake >= 1.7". It fails despite F19 coming with Automake 1.13.
Yes, it why patch provided. See spec and comment before.

Comment 16 Michael Schwendt 2013-05-01 18:15:28 UTC
> "autoreconf -f" silent on gimp-gap directory after run autogen.sh (before it
> fails and require run automake, autoheader and so on).

Please read "man autoreconf", so you know what it does.

A first run of "sh autogen.sh" prints the same stuff, because it also runs the individual autotools. Those are issues that should get fixed. Especially the "deprecated" stuff may lead to problems sometime in the future, and then you will be unable to create the configure script.

There is no need to use the autogen.sh script anyway, which also executes the "configure" script, because you should be able to run "autoreconf" instead (or autoreconf -f). And you can use %configure appropriately, too.

Comment 17 Pavel Alexeev 2013-05-01 19:34:02 UTC
(In reply to comment #16)
> A first run of "sh autogen.sh" prints the same stuff, because it also runs
> the individual autotools. Those are issues that should get fixed. Especially
> the "deprecated" stuff may lead to problems sometime in the future, and then
> you will be unable to create the configure script.
I think we may focusing on fixing such issues when it happened. "Sometime in future" when it really will lead to problems and will unable to generate configure script, is not?

> There is no need to use the autogen.sh script anyway, which also executes
> the "configure" script, because you should be able to run "autoreconf"
> instead (or autoreconf -f). And you can use %configure appropriately, too.
I sure what I may omit upstream author work and run command from autogen.sh directly from spec. But for what? I don't see any advantages to do that. Run configure script in any case is appropriate and desired as I call make then.

Comment 18 Michael Schwendt 2013-05-01 22:09:09 UTC
Perhaps you can be convinced with this spec file patch then:

--- gimp-gap.spec.orig	2013-04-30 11:48:37.000000000 +0200
+++ gimp-gap.spec	2013-05-02 00:06:56.846396039 +0200
@@ -40,7 +40,6 @@
 %prep
 %setup -q -n %{name}
 
-%patch0 -p1 -b .automake-1.12
 %patch1 -p1 -b .unbundle
 
 # Bundled libs (list from SUSE)
@@ -50,7 +49,8 @@
 
 
 %build
-./autogen.sh --disable-libavformat --libdir=%{_libdir} CFLAGS="${CFLAGS:-%optflags}" LDFLAGS="${LDFLAGS:-%__global_ldflags}"
+autoreconf -i -f
+%configure --disable-libavformat
 
 # Parralel build terminated with error
 make LIBS="$LIBS -lm"

Comment 19 Pavel Alexeev 2013-05-02 16:58:07 UTC
What it is change really? Only omit some runtime versions requirement checks (I agree it may be some excessive as it listed in spec-file too, but it may be changed in future releases, so it good to have it from upstream too)?

I'm not expert really and can't say if "autoreconf -i -f" do exactly the same things like call auto*-stuff, glib-gettextize, intltoolize with same arguments. If you think it is not and have issues, please be more specific on problems in autogen.sh script - we then need to report them upstream [1] instead of make workarounds.

1 https://fedoraproject.org/wiki/Staying_close_to_upstream_projects

Comment 20 Michael Schwendt 2013-05-02 17:40:18 UTC
> What it is change really?

See "rpm --eval %configure". Using %configure does a bit more than plain "./configure". For example, it adds and corrects some parameters, such as localedir, which is wrong with your spec file: DLOCALEDIR=\""/usr/local/share/locale"\" 

If you insist on using autogen.sh, at least patch it to not run "configure" automatically, so you can use %configure in the spec file. ;-)

Comment 21 Pavel Alexeev 2013-05-02 18:51:12 UTC
As stated before on my mind it is not problem run it twice, but if you insist:

Srpm: http://hubbitus.info/rpm/Fedora18/gimp-gap/gimp-gap-2.7.0-6.GITe75bd46.fc18.src.rpm
Spec changes: https://github.com/Hubbitus/Fedora-packaging/commit/f744fc6d00e9c77dd673bb05d92f3ba66d0c163b

Comment 22 Michael Schwendt 2013-05-02 19:00:15 UTC
Well, since configure scripts cache some values (some things automatically, some as implemented by the author), it can be problematic to re-run it with heavily changed options.

Btw, gimp-gap-2.7.0-5.GITe75bd46.fc18.src.rpm did _not_ run %configure at all and therefore built with wrong parameters (comment 20). So, it's a good thing to fix that, and I hope you agree with that.

Comment 23 Pavel Alexeev 2013-05-02 19:07:35 UTC
(In reply to comment #22)
> Btw, gimp-gap-2.7.0-5.GITe75bd46.fc18.src.rpm did _not_ run %configure at
> all and therefore built with wrong parameters (comment 20). So, it's a good
> thing to fix that, and I hope you agree with that.
Yes, it was changed early. See comment 9.

Comment 24 Christopher Meng 2013-08-30 06:53:26 UTC
Status?

Comment 25 Pavel Alexeev 2013-08-31 16:07:12 UTC
Ready for review. Feel free to take.

Comment 26 Mario Blättermann 2013-11-01 12:01:53 UTC
Scratch build fails:
http://koji.fedoraproject.org/koji/taskinfo?taskID=6122242

From build.log:

Package gimp-2.0 was not found in the pkg-config search path.
Perhaps you should add the directory containing `gimp-2.0.pc'
to the PKG_CONFIG_PATH environment variable
No package 'gimp-2.0' found
Package gimp-2.0 was not found in the pkg-config search path.
Perhaps you should add the directory containing `gimp-2.0.pc'
to the PKG_CONFIG_PATH environment variable
No package 'gimp-2.0' found


"gimp-devel >= 2.6.0" seems to be insufficient here. Maybe "pkgconfig(gimp-2.0)" works...?

Moreover, "sed" is part of the minimum build environment and not to be added as build requirement:
https://fedoraproject.org/wiki/HOWTOFindMissingBuildRequires#Exceptions

Comment 28 Mario Blättermann 2013-11-04 11:37:07 UTC
rpmlint output, omitting the wrong FSF address errors:

gimp-gap.i686: W: file-not-utf8 /usr/share/doc/gimp-gap-2.7.0/ChangeLog
gimp-gap.src: W: invalid-url Source0: gimp-gap-e75bd46.tar.xz
gimp-gap.x86_64: E: incorrect-fsf-address /usr/share/gimp/2.0/scripts/gap-dup-continue.scm
gimp-gap.x86_64: W: file-not-utf8 /usr/share/doc/gimp-gap-2.7.0/ChangeLog
gimp-gap-debuginfo.i686: W: spurious-executable-perm /usr/src/debug/gimp-gap/gap/gap_mov_exec.c
gimp-gap-debuginfo.i686: W: spurious-executable-perm /usr/src/debug/gimp-gap/gap/gap_story_att_trans_dlg.c
gimp-gap-debuginfo.i686: W: spurious-executable-perm /usr/src/debug/gimp-gap/gap/gap_story_undo_types.h
gimp-gap-debuginfo.i686: W: spurious-executable-perm /usr/src/debug/gimp-gap/gap/gap_story_undo.h
gimp-gap-debuginfo.i686: W: spurious-executable-perm /usr/src/debug/gimp-gap/gap/gap_story_undo.c
gimp-gap-debuginfo.i686: W: spurious-executable-perm /usr/src/debug/gimp-gap/gap/gap_mov_xml_par.c
gimp-gap-debuginfo.i686: W: spurious-executable-perm /usr/src/debug/gimp-gap/gap/gap_story_main.h
gimp-gap-debuginfo.x86_64: W: spurious-executable-perm /usr/src/debug/gimp-gap/gap/gap_mov_exec.c
gimp-gap-debuginfo.x86_64: W: spurious-executable-perm /usr/src/debug/gimp-gap/gap/gap_mov_dialog.c
gimp-gap-debuginfo.x86_64: W: spurious-executable-perm /usr/src/debug/gimp-gap/gap/gap_story_att_trans_dlg.c
gimp-gap-debuginfo.x86_64: W: spurious-executable-perm /usr/src/debug/gimp-gap/gap/gap_story_undo_types.h
gimp-gap-debuginfo.x86_64: W: spurious-executable-perm /usr/src/debug/gimp-gap/gap/gap_story_undo.h
gimp-gap-debuginfo.x86_64: W: spurious-executable-perm /usr/src/debug/gimp-gap/gap/gap_story_undo.c
gimp-gap-debuginfo.x86_64: W: spurious-executable-perm /usr/src/debug/gimp-gap/gap/gap_mov_xml_par.c
gimp-gap.spec: W: invalid-url Source0: gimp-gap-e75bd46.tar.xz
6 packages and 1 specfiles checked; 286 errors, 25 warnings.


There are a few source files where the executable bit is set. Please remove them. Besides that, convert the Changelog file to UTF-8. For the wrong FSF addresses contact upstream.

Comment 29 Pavel Alexeev 2013-11-04 16:06:41 UTC
Incorrect FSF address issue filled: https://bugzilla.gnome.org/show_bug.cgi?id=711402

There still present rpmlint warning on debuginfo subpackage:
gimp-gap-debuginfo.x86_64: E: non-standard-dir-perm /usr/src/debug/gimp-gap/gap/iter_ALT/gen 0775L
despite my change directory permissions in %prep.
But I think it is not serious issue to dig why it happened.

File permissions and encoding addressed.

Changes: https://github.com/Hubbitus/Fedora-packaging/commit/2fd7121a7a2a3b84789859b752fb3a6cb2034cd2
Spec: https://raw.github.com/Hubbitus/Fedora-packaging/2fd7121a7a2a3b84789859b752fb3a6cb2034cd2/SPECS/gimp-gap.spec
Scratch build: http://koji.fedoraproject.org/koji/taskinfo?taskID=6136258
Srpm: http://hubbitus.info/rpm/Fedora19/gimp-gap/gimp-gap-2.7.0-8.GITe75bd46.fc19.src.rpm

Comment 30 Mario Blättermann 2013-11-04 18:15:05 UTC
(In reply to Pavel Alexeev (aka Pahan-Hubbitus) from comment #29)
> Incorrect FSF address issue filled:
> https://bugzilla.gnome.org/show_bug.cgi?id=711402
>
OK.
> There still present rpmlint warning on debuginfo subpackage:
> gimp-gap-debuginfo.x86_64: E: non-standard-dir-perm
> /usr/src/debug/gimp-gap/gap/iter_ALT/gen 0775L
> despite my change directory permissions in %prep.
> But I think it is not serious issue to dig why it happened.
I can see the invalid-url warnings only. OK, here we go:

---------------------------------
key:

[+] OK
[.] OK, not applicable
[X] needs work
---------------------------------

[+] MUST: rpmlint must be run on the source rpm and all binary rpms the build produces. The output should be posted in the review.
[+] MUST: The package must be named according to the Package Naming Guidelines.
[+] MUST: The spec file name must match the base package %{name}, in the format %{name}.spec unless your package has an exemption.
[+] MUST: The package must meet the Packaging Guidelines.
[+] MUST: The package must be licensed with a Fedora approved license and meet the Licensing Guidelines.
[+] MUST: The License field in the package spec file must match the actual license.
    GPLv2+
[.] MUST: If (and only if) 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 must be included in %doc.
[+] MUST: The spec file must be written in American English.
[+] MUST: The spec file for the package MUST be legible.
[+] MUST: The sources used to build the package must match the upstream source, as provided in the spec URL. Reviewers should use sha256sum for this task as it is used by the sources file once imported into git. If no upstream URL can be specified for this package, please see the Source URL Guidelines for how to deal with this.
    $ sha256sum *
    20bfa0084f6a681169b7471639e33a04c7c3cefbcd553076c59d53d3f5a0a5ea  gimp-gap-115d5d9.tar.xz.orig
    31f076cac22e2f8ade28e7203af9d4a94256d1b94702f2f73eac4909a7948495  gimp-gap-e75bd46.tar.xz

Of course, the checksums differ due to different commit numbers. Actually you have to tweak the script so that it uses a certain commit. Don't bother, I trust you and don't recognize it as a review blocker :)

[+] MUST: The package MUST successfully compile and build into binary rpms on at least one primary architecture.
[.] MUST: If the package does not successfully compile, build or work on an architecture, then those architectures should be listed in the spec in ExcludeArch. Each architecture listed in ExcludeArch MUST have a bug filed in bugzilla, describing the reason that the package does not compile/build/work on that architecture. The bug number MUST be placed in a comment, next to the corresponding ExcludeArch line.
[+] MUST: All build dependencies must be listed in BuildRequires, except for any that are listed in the exceptions section of the Packaging Guidelines ; inclusion of those as BuildRequires is optional. Apply common sense.
[+] MUST: The spec file MUST handle locales properly. This is done by using the %find_lang macro. Using %{_datadir}/locale/* is strictly forbidden.
[.] MUST: Every binary RPM package (or subpackage) which stores shared library files (not just symlinks) in any of the dynamic linker's default paths, must call ldconfig in %post and %postun.
[.] MUST: Packages must NOT bundle copies of system libraries.
[.] MUST: If the package is designed to be relocatable, the packager must state this fact in the request for review, along with the rationalization for relocation of that specific package. Without this, use of Prefix: /usr is considered a blocker.
[+] MUST: A package must own all directories that it creates. If it does not create a directory that it uses, then it should require a package which does create that directory.
[+] MUST: A Fedora package must not list a file more than once in the spec file's %files listings. (Notable exception: license texts in specific situations)
[+] MUST: Permissions on files must be set properly. Executables should be set with executable permissions, for example.
[+] MUST: Each package must consistently use macros.
[+] MUST: The package must contain code, or permissable content.
[.] MUST: Large documentation files must go in a -doc subpackage. (The definition of large is left up to the packager's best judgement, but is not restricted to size. Large can refer to either size or quantity).
[+] MUST: If a package includes something as %doc, it must 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: Static libraries must be in a -static package.
[.] MUST: Development files must be in a -devel package.
[.] MUST: In the vast majority of cases, devel packages must require the base package using a fully versioned dependency: Requires: %{name}%{?_isa} = %{version}-%{release}
[.] MUST: Packages must NOT contain any .la libtool archives, these must be removed in the spec if they are built.
[.] MUST: Packages containing GUI applications must include a %{name}.desktop file, and that file must be properly installed with desktop-file-install in the %install section. If you feel that your packaged GUI application does not need a .desktop file, you must put a comment in the spec file with your explanation.
[+] MUST: Packages must not own files or directories already owned by other packages. The rule of thumb here is that the first package to be installed should own the files or directories that other packages may rely upon. This means, for example, that no package in Fedora should ever share ownership with any of the files or directories owned by the filesystem or man package. If you feel that you have a good reason to own a file or directory that another package owns, then please present that at package review time. 
[+] MUST: All filenames in rpm packages must be valid UTF-8.


[.] SHOULD: If the source package does not include license text(s) as a separate file from upstream, the packager SHOULD query upstream to include it.
[.] SHOULD: The description and summary sections in the package spec file should contain translations for supported Non-English languages, if available.
[+] SHOULD: The reviewer should test that the package builds in mock.
    See Koji build above (which uses Mock anyway).
[+] SHOULD: The package should compile and build into binary rpms on all supported architectures.
[.] SHOULD: The reviewer should test that the package functions as described. A package should not segfault instead of running, for example.
[.] SHOULD: If scriptlets are used, those scriptlets must be sane. This is vague, and left up to the reviewers judgement to determine sanity.
[.] SHOULD: Usually, subpackages other than devel should require the base package using a fully versioned dependency.
[.] SHOULD: The placement of pkgconfig(.pc) files depends on their usecase, and this is usually for development purposes, so should be placed in a -devel pkg. A reasonable exception is that the main pkg itself is a devel tool not installed in a user runtime, e.g. gcc or gdb.
[.] SHOULD: If the package has file dependencies outside of /etc, /bin, /sbin, /usr/bin, or /usr/sbin consider requiring the package which provides the file instead of the file itself.
[.] SHOULD: your package should contain man pages for binaries/scripts. If it doesn't, work with upstream to add them where they make sense.


----------------

PACKAGE APPROVED

----------------

Comment 31 Pavel Alexeev 2013-11-04 20:40:46 UTC
Mario thank you very much for the review.

Would you like I also review some of you packages?

New Package SCM Request
=======================
Package Name: gimp-gap
Short Description: The GIMP Animation Package
Owners: hubbitus
Branches: F-19 F-20 EL-6
InitialCC:

Comment 32 Gwyn Ciesla 2013-11-05 01:19:18 UTC
Git done (by process-git-requests).

Comment 33 Fedora Update System 2013-11-05 15:05:56 UTC
gimp-gap-2.7.0-8.GITe75bd46.el6 has been submitted as an update for Fedora EPEL 6.
https://admin.fedoraproject.org/updates/gimp-gap-2.7.0-8.GITe75bd46.el6

Comment 34 Fedora Update System 2013-11-05 15:16:19 UTC
gimp-gap-2.7.0-8.GITe75bd46.fc19 has been submitted as an update for Fedora 19.
https://admin.fedoraproject.org/updates/gimp-gap-2.7.0-8.GITe75bd46.fc19

Comment 35 Fedora Update System 2013-11-05 15:43:09 UTC
gimp-gap-2.7.0-8.GITe75bd46.fc20 has been submitted as an update for Fedora 20.
https://admin.fedoraproject.org/updates/gimp-gap-2.7.0-8.GITe75bd46.fc20

Comment 36 Fedora Update System 2013-11-10 07:15:55 UTC
gimp-gap-2.7.0-8.GITe75bd46.fc20 has been pushed to the Fedora 20 stable repository.  If problems still persist, please make note of it in this bug report.

Comment 37 Fedora Update System 2013-11-14 03:34:41 UTC
gimp-gap-2.7.0-8.GITe75bd46.fc19 has been pushed to the Fedora 19 stable repository.  If problems still persist, please make note of it in this bug report.

Comment 38 Mario Blättermann 2013-11-17 15:17:49 UTC
Except the el6 one, all the other packages are marked as stable, so this ticket is ON_QA.

Comment 39 Fedora Update System 2013-11-23 02:37:54 UTC
gimp-gap-2.7.0-8.GITe75bd46.el6 has been pushed to the Fedora EPEL 6 stable repository.  If problems still persist, please make note of it in this bug report.


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