Bug 708765 - Review Request: frogr - Flickr Remote Organizer for GNOME
Summary: Review Request: frogr - Flickr Remote Organizer for GNOME
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Gianluca Sforna
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2011-05-29 12:33 UTC by Mario Blättermann
Modified: 2011-09-30 18:40 UTC (History)
11 users (show)

Fixed In Version: frogr-0.6.1-1.fc16
Clone Of:
Environment:
Last Closed: 2011-09-14 22:25:08 UTC
Type: ---
Embargoed:
giallu: fedora-review+
gwync: fedora-cvs+


Attachments (Terms of Use)
Patches to be applied over frogr 0.5 for packaging (8.68 KB, application/x-gzip)
2011-06-03 22:33 UTC, Mario Sanchez Prada
no flags Details
Additional patches that might be interesting to apply (1.72 KB, application/x-gzip)
2011-06-04 06:39 UTC, Mario Sanchez Prada
no flags Details


Links
System ID Private Priority Status Summary Last Updated
GNOME Bugzilla 656688 0 None None None 2019-01-21 11:40:48 UTC

Description Mario Blättermann 2011-05-29 12:33:43 UTC
Spec URL: http://mariobl.fedorapeople.org/Review/SPECS/frogr.spec
SRPM URL: http://mariobl.fedorapeople.org/Review/SRPMS/frogr-0.5-1.fc15.src.rpm
Description:
Frogr intends to be a complete GNOME application to remotely manage a flickr
account from the desktop. It uses flickcurl, from Dave Beckett to
communicate with the server through the publicly available flickr REST API.

The program doesn't provide all the expected features yet, seems it needs additional libraries which are not requested during the build process. Stay tuned, I try to find them out.

Comment 1 Mario Sanchez Prada 2011-05-30 17:51:43 UTC
As the main developer of frogr so far, please let me say thanks for you reporting this bug and pushing for getting frogr integrated in Fedora. Thanks!

Now, just a couple of comments that I hope you'll find useful:

(In reply to comment #0)
> Spec URL: http://mariobl.fedorapeople.org/Review/SPECS/frogr.spec
> SRPM URL: http://mariobl.fedorapeople.org/Review/SRPMS/frogr-0.5-1.fc15.src.rpm
> Description:
> Frogr intends to be a complete GNOME application to remotely manage a flickr
> account from the desktop. It uses flickcurl, from Dave Beckett to
> communicate with the server through the publicly available flickr REST API.

This description is not correct. Since frogr 0.3 flickcurl is not being used anymore, as we used our own libsoup-based library called flicksoup, which is not being released separately yet (its development happens inside the frogr tree at the moment). Nowadays, the "official" description that's used everywhere is something like this:

"Frogr is a small application for the GNOME desktop that allows users
to manage their accounts in the Flickr image hosting website. It
supports all the basic tasks, including uploading pictures, adding
descriptions, setting tags and managing sets."


> The program doesn't provide all the expected features yet, seems it needs
> additional libraries which are not requested during the build process. Stay
> tuned, I try to find them out.

I think you'll find useful the list I use to generate "homegrown" fedora packages while frogr is not shipped right from the distro:

 BuildRequires:	gtk3-devel > 3.0, libsoup-devel > 2.24, libxml2-devel > 2.6.8, libexif-devel > 0.6.14
 Requires: gtk3 > 3.0, libsoup > 2.24, libxml2 > 2.6.8, libexif > 0.6.14, gvfs, desktop-file-utils

Also, you can take a look to the .spec file I used to build those "homegrown" packages for frogr 0.5:
http://git.gnome.org/browse/frogr/tree/frogr.spec?id=003b3101cedb6e129eff60af4e4b6976afb3ab4c


Hope this helps!

Thanks,
Mario

Comment 2 Mario Blättermann 2011-05-30 18:09:16 UTC
(In reply to comment #1)
> As the main developer of frogr so far, please let me say thanks for you
> reporting this bug and pushing for getting frogr integrated in Fedora. Thanks!
> 
> Now, just a couple of comments that I hope you'll find useful:
> 
> (In reply to comment #0)
> > Spec URL: http://mariobl.fedorapeople.org/Review/SPECS/frogr.spec
> > SRPM URL: http://mariobl.fedorapeople.org/Review/SRPMS/frogr-0.5-1.fc15.src.rpm
> > Description:
> > Frogr intends to be a complete GNOME application to remotely manage a flickr
> > account from the desktop. It uses flickcurl, from Dave Beckett to
> > communicate with the server through the publicly available flickr REST API.
> 
> This description is not correct. Since frogr 0.3 flickcurl is not being used
> anymore, as we used our own libsoup-based library called flicksoup, which is
> not being released separately yet (its development happens inside the frogr
> tree at the moment). Nowadays, the "official" description that's used
> everywhere is something like this:
> 
> "Frogr is a small application for the GNOME desktop that allows users
> to manage their accounts in the Flickr image hosting website. It
> supports all the basic tasks, including uploading pictures, adding
> descriptions, setting tags and managing sets."

The description from my spec file is picked up from Frogr's website:

http://live.gnome.org/Frogr

Or, actually, what I recognize as Frogr's website. Could be there is another website which isn't known to me. If I'm right, someone should update this.

Regarding your hints, I will have a look at the dependencies tomorrow. Many thanks for your help!

Comment 3 Mario Sanchez Prada 2011-05-30 18:21:25 UTC
(In reply to comment #2)
> [...]
> > "Frogr is a small application for the GNOME desktop that allows users
> > to manage their accounts in the Flickr image hosting website. It
> > supports all the basic tasks, including uploading pictures, adding
> > descriptions, setting tags and managing sets."
> 
> The description from my spec file is picked up from Frogr's website:
> 
> http://live.gnome.org/Frogr
> 
> Or, actually, what I recognize as Frogr's website. Could be there is another
> website which isn't known to me. If I'm right, someone should update this.

Hmm.. I think you're doing something wrong, since what is stated in http://live.gnome.org/Frogr, in the "Overview" section, is the following:

"""
Frogr is a small application for the GNOME desktop that allows users to manage their accounts in the Flickr image hosting website. It supports all the basic Flickr features, including uploading pictures, adding descriptions, setting tags and managing sets and groups pools.

This application is written in C and uses flicksoup, a libsoup-based library to communicate with the server through the publicly available flickr REST API.

This project is Free Software and published under the terms of the GNU Public License v3."
"""

As you can see, the first paragraph is basically what I pasted you before, with the simple addition of "... and group pools" (which would be a nice addition to Fedora's description, if you ask me).

There's an old and deprecated site at Google Code (http://code.google.com/p/frogr) but no mention to flickcurl is being made there nowadays either... you must be getting that description from somewhere else. If you find it and it's my fault please let me know and I'll change it.


> Regarding your hints, I will have a look at the dependencies tomorrow. Many
> thanks for your help!

Glad to see you found it useful.

Thanks!

Comment 4 Mario Blättermann 2011-05-30 18:49:08 UTC
(In reply to comment #3)
> Hmm.. I think you're doing something wrong, since what is stated in
> http://live.gnome.org/Frogr, in the "Overview" section, is the following:
> 
> """
> Frogr is a small application for the GNOME desktop that allows users to manage
> their accounts in the Flickr image hosting website. It supports all the basic
> Flickr features, including uploading pictures, adding descriptions, setting
> tags and managing sets and groups pools.
> 
> This application is written in C and uses flicksoup, a libsoup-based library to
> communicate with the server through the publicly available flickr REST API.
> 
> This project is Free Software and published under the terms of the GNU Public
> License v3."
> """
> 
> As you can see, the first paragraph is basically what I pasted you before, with
> the simple addition of "... and group pools" (which would be a nice addition to
> Fedora's description, if you ask me).
> 
Yes, you are right. Now I know it again: I had googled for "frogr fedora" and got the spec file. Don't know the source, but I've downloaded it and began to edit it to match the new upstream version. In any case, thanks for your hints.

I was surprised about the spec file in Git, which shows that you was working continuously on the Fedora package. And never thought about to publish it...? OK, no problem, once we have a reviewer and all the mistakes related to the package are fixed, Frogr will become part of the official Fedora package pool.

Comment 5 Mario Sanchez Prada 2011-05-30 19:40:38 UTC
(In reply to comment #4)
> [...]
> > As you can see, the first paragraph is basically what I pasted you before, with
> > the simple addition of "... and group pools" (which would be a nice addition to
> > Fedora's description, if you ask me).
> > 
> Yes, you are right. Now I know it again: I had googled for "frogr fedora" and
> got the spec file. Don't know the source, but I've downloaded it and began to
> edit it to match the new upstream version. In any case, thanks for your hints.

Ah, ok. So you started from an old version of the .spec file I was using. That explains everything :-)

> I was surprised about the spec file in Git, which shows that you was working
> continuously on the Fedora package. And never thought about to publish it...?

:-)

Yes, I was using the .spec file a lot, at first just to generate .rpm files when I made a release, but very often lately since I switched from Ubuntu to Fedora, to generate pacakges that I could easily install in my system to dog food frogr.

However, I never thought about publishing it myself because I had no clue about the .spec file being correct or not. For me it was a "just works" file that easily generated .rpm files when I needed it, without needed to worry too much about being a proper spec file, which would probably need to follow some guidelines/rules/whatever that I, as an unexperienced Fedora user, had no idea about.

Actually the initial version of that .spec file was not written by me and I just cared about updating it whenever needed to keep generating rpms when I needed it.

Anyway, I agree with you that it's weird to have that .spec file in there and not having pushed for integrating frogr in Fedora before, but as I said in a comment to my last post about frogr, the simple reason for that is that I lacked the self-confidence and the knowledge on the processes in the Fedora community for doing that, so I just went for the "dear lazyweb" approach of asking the world to help me with packaging issues :-)

> OK, no problem, once we have a reviewer and all the mistakes related to the
> package are fixed, Frogr will become part of the official Fedora package pool.

Great! And glad again to see that frogr is reaching fedora in one way or another :-)

Comment 6 Mario Sanchez Prada 2011-05-31 13:15:10 UTC
Breaking News:

As a result of a conversation in my blog [1] Christophe Fergeau, who obviously knows a lot more than me when it comes to Fedora packaging :-), has been doing some follow-up work on that matter and have updated the frogr.spec file in frogr git repository, which I think you should consider to avoid duplicating efforts:

  http://git.gnome.org/browse/frogr/tree/frogr.spec

I already told him about this being tracked in this bugzilla, so all the people interested can be in the same page.

Thanks Christophe!

[1] http://blogs.igalia.com/mario/2011/05/27/frogr-0-5-released

Comment 7 Mario Blättermann 2011-05-31 18:56:08 UTC
Many thanks for all the hints and improvements! I've now crowded the old spec file and built a new one based on the one in Git.

Spec URL: http://mariobl.fedorapeople.org/Review/SPECS/frogr.spec
SRPM URL: http://mariobl.fedorapeople.org/Review/SRPMS/frogr-0.5-1.fc15.src.rpm
Description:
Frogr is a small application for the GNOME desktop that allows users
to manage their accounts in the Flickr image hosting website. It
supports all the basic tasks, including uploading pictures, adding
descriptions, setting tags and managing sets.

However, I'm unsure if we need such a changelog. It reflects all the changes due to the software itself, although our package changelog is actually for changes to the package. The packaging guidelines say the following [1]:

"Changelog entries should provide a brief summary of the changes done to the package between releases, including noting updating to a new version, adding a patch, fixing other spec sections, note bugs fixed, and CVE's if any. They must never simply contain an entire copy of the source CHANGELOG entries. The intent is to give the user a hint as to what changed in a package update without overwhelming them with the technical details. Links to upstream changelogs can be entered for those who want additional information."

Means, we can drop all the changelog entries which also be found in the upstream changelog anyway.

[1] http://fedoraproject.org/wiki/PackagingGuidelines#Changelogs

Comment 8 Mario Blättermann 2011-05-31 18:57:44 UTC
Sorry, just pushed the wrong srpm link. The right one is:

SRPM URL: http://mariobl.fedorapeople.org/Review/SRPMS/frogr-0.5-2.fc15.src.rpm

Comment 9 Mario Blättermann 2011-05-31 19:20:39 UTC
Just tested Frogr 0.5. It starts with an empty window, although Frogr is already authorized to Flickr correctly. I can choose local pictures, tag them, change the description and upload them. But once the pictures have been uploaded to Flickr, the window becomes empty again. Is this behavior intended to be so, or should I actually be able to browse through the pictures at flickr.com?

Comment 10 Mario Sanchez Prada 2011-05-31 19:43:46 UTC
(In reply to comment #7)
> [...]
> However, I'm unsure if we need such a changelog. It reflects all the changes
> due to the software itself, although our package changelog is actually for
> changes to the package. The packaging guidelines say the following [1]:
> 
> [...]
> 
> Means, we can drop all the changelog entries which also be found in the
> upstream changelog anyway.
> 
> [1] http://fedoraproject.org/wiki/PackagingGuidelines#Changelogs

I agree with you. Let's drop all the entries (that are in NEWS file anyway) and keep there just the changes related to the package, as it's already being done in Debian (I mean, in the package shipped with debian distros, not in my particular version of the debian/changelog file)

Comment 11 Mario Sanchez Prada 2011-05-31 19:49:03 UTC
(In reply to comment #9)
> Just tested Frogr 0.5. It starts with an empty window, although Frogr is
> already authorized to Flickr correctly. I can choose local pictures, tag them,
> change the description and upload them. But once the pictures have been
> uploaded to Flickr, the window becomes empty again. Is this behavior intended
> to be so, or should I actually be able to browse through the pictures at
> flickr.com?

What you described is exactly the expected behavior :-)

Even though the original idea behind frogr is to have a full featured flickr manager, the truth is that at the moment it is just a flickr uploader, so that's why you don't see anything as soon as you start frogr, and the only feedback you have about being logged in is the string in the status bar saying "Connected as username".

Also, the behavior of getting the window empty again after uploading is also the expected behaviour, since every picture is removed after a successful upload, both to give feedback to the user (as in "one picture less to be uploaded") and to leave the application in a handy state if something fails in the middle, so resuming the process would be just a matter of clicking upload again.

So, short answer: yes, it's the expected behaviour, since it's just an uploader at the moment, despite of the word "organizer" in the app name.

Thanks

Comment 12 Mario Sanchez Prada 2011-05-31 19:52:16 UTC
(In reply to comment #7)
> Many thanks for all the hints and improvements! I've now crowded the old spec
> file and built a new one based on the one in Git.
> 
> Spec URL: http://mariobl.fedorapeople.org/Review/SPECS/frogr.spec
[...]

In the following section:

   %files -f %{name}.lang
   %defattr(-,root,root,-)
   %{_bindir}/%{name}
   %{_datadir}/%{name}/*
   %{_datadir}/pixmaps/%{name}.xpm
   %{_datadir}/icons/hicolor/*/apps/%{name}.png
   %{_datadir}/icons/hicolor/*/apps/%{name}.svg
   %{_datadir}/applications/%{name}.desktop
   %{_mandir}/man1/frogr.1.*
   %doc README NEWS COPYING AUTHORS THANKS TODO MAINTAINERS TRANSLATORS


Don't you need to list there something like %{_datadir}/gnome/help/%{name}/*?

I'm talking from my vast ignorance on the topic, just wondering because when I was playing yesterday with the .spec file, I needed to do it so the help files got installed through the rpm package.

Comment 13 Christophe Fergeau 2011-05-31 19:52:47 UTC
Fwiw, 
$ rpmlint frogr-0.5-2.fc15.src.rpm 
frogr.src: W: spelling-error Summary(en_US) Flickr -> Flick, Flicker, Flicks
frogr.src: I: enchant-dictionary-not-found de
frogr.src:4: W: mixed-use-of-spaces-and-tabs (spaces: line 4, tab: line 1)
1 packages and 0 specfiles checked; 0 errors, 2 warnings.

The 1st warning is a false alarm, I'm not sure what the 2nd one means, but it's related to the german descriptions that are in there I guess, the last one should be fixed :)

Comment 14 Christophe Fergeau 2011-05-31 19:54:23 UTC
(In reply to comment #12)
> Don't you need to list there something like %{_datadir}/gnome/help/%{name}/*?

Yes you do, but only when you're using git :) The help files were added a few days ago so are not part of the 0.5 release. The rpm build fails when there are files that were installed by "make install" but are not listed in %files, so missing files get noticed.

Comment 15 Mario Blättermann 2011-05-31 20:12:14 UTC
(In reply to comment #14)
> (In reply to comment #12)
> > Don't you need to list there something like %{_datadir}/gnome/help/%{name}/*?
> 
> Yes you do, but only when you're using git :) The help files were added a few
> days ago so are not part of the 0.5 release. The rpm build fails when there are
> files that were installed by "make install" but are not listed in %files, so
> missing files get noticed.

Right. Currently, we speak about v0.5, which doesn't ship the help files yet. Given the current state of the manual, you shouldn't think about a new release. We should concentrate the effort now to package the 0.5 release for F15 and Rawhide.

Moreover, I think about to provide a GTK+2 based version for F14. Is Frogr still fully compatible to GTK+2, or are there any constraints?

Comment 16 Mario Sanchez Prada 2011-05-31 20:34:20 UTC
(In reply to comment #14)
> (In reply to comment #12)
> > Don't you need to list there something like %{_datadir}/gnome/help/%{name}/*?
> 
> Yes you do, but only when you're using git :) The help files were added a few
> days ago so are not part of the 0.5 release. The rpm build fails when there are
> files that were installed by "make install" but are not listed in %files, so
> missing files get noticed.

Ah! Yes, you're right, sorry for the noise. I forgot for a moment that we were talking about 0.5 :-)

PS: Nice to see anyway that I was not wrong with my guessing :-)

Comment 17 Mario Sanchez Prada 2011-05-31 20:44:13 UTC
(In reply to comment #15)
> [...]
> Right. Currently, we speak about v0.5, which doesn't ship the help files yet.
> Given the current state of the manual, you shouldn't think about a new release.
> We should concentrate the effort now to package the 0.5 release for F15 and
> Rawhide.

Yes, forget about it (see my previous comment)

> Moreover, I think about to provide a GTK+2 based version for F14. Is Frogr
> still fully compatible to GTK+2, or are there any constraints?

Yes, it's fully compatible (no constraints at all), actually the packages I provide in my PPA for Ubuntu Lucid and Maverick are compiled with gtk2, while in Ubuntu Natty the packe is compiled with gtk3.

The configure.ac is written so frogr will compile this way:

  * If gtk3 devel files are present, frogr will build with gtk3 (default).

  * If no gtk3 devel files are present, but gtk2 devel files are, frogr will build with gtk2 (default fallback when gtk3 is not present).

  * If both gtk2 and gtk3 devel files installed frogr will build with gtk3.

  * If both gtk2 and gtk3 devel files installed, but you still want to force
using gtk2, you can pass --with-gtk=2.0 to the configure script.

So as you can see, gtk3 is the default but frogr will build with gtk2 perfectly anyway, either by not having gtk3 dev files present or because you explicitly say it so. About restrictions there are none of them since the code already uses some #ifdef ... #endif regions to take into account the differences that must be taken into account in frogr when it comes to using gtk2 or gtk3

Comment 18 Mario Blättermann 2011-06-01 20:22:57 UTC
Updated files:

Spec URL: http://mariobl.fedorapeople.org/Review/SPECS/frogr.spec
SRPM URL: http://mariobl.fedorapeople.org/Review/SRPMS/frogr-0.5-3.fc15.src.rpm

I've removed most of the stuff in %changelog, according to the packaging guidelines. Furthermore, I've fixed the tabs/spaces in the header to make rpmlint happy.

Once the package is approved and created a Git repo, I will also request a branch for F14 to build a GTK2 based package.

Comment 19 Martin Gieseking 2011-06-01 21:14:24 UTC
Hi Mario, here are some comments on the latest spec:

- building the package with mock fails because of missing BR: intltool

- You should drop the flags "-Wall -g0 -O2" from CFLAGS in file configure.ac 
  as they overwrite Fedora's %optflags and lead to debuginfo packages without 
  sources.

- drop the trailing asterisk from %{_datadir}/%{name}/* for proper directory 
  ownership

I cleared the fedora-review flag as it should be set by the reviewer and you can't review your own packages. ;)


$ rpmlint *.rpm
frogr.src: W: spelling-error Summary(en_US) Flickr -> Flick, Flicker, Flicks
frogr.src: W: spelling-error Summary(de) Flickr -> Flicker, Flicke, Flickt
frogr.x86_64: W: spelling-error Summary(en_US) Flickr -> Flick, Flicker, Flicks
frogr.x86_64: W: spelling-error Summary(de) Flickr -> Flicker, Flicke, Flickt
frogr-debuginfo.x86_64: E: debuginfo-without-sources
3 packages and 0 specfiles checked; 1 errors, 4 warnings.

Comment 20 Mario Blättermann 2011-06-01 21:27:20 UTC
(In reply to comment #19)

> I cleared the fedora-review flag as it should be set by the reviewer and you
> can't review your own packages. ;)
> 
Sorry, actually I want to have a look at the possible flags and I set this flag probably by accidence... Regarding your other issues, I'll have a look at them tomorrow.

Comment 21 Mario Blättermann 2011-06-01 21:44:18 UTC
(In reply to comment #19)
> - You should drop the flags "-Wall -g0 -O2" from CFLAGS in file configure.ac 
>   as they overwrite Fedora's %optflags and lead to debuginfo packages without 
>   sources.

Just tested it with the following patch:

--- configure.ac	2011-06-01 23:28:51.084852332 +0200
+++ configure.ac	2011-06-01 23:30:08.294665301 +0200
@@ -140,7 +140,7 @@
    AC_DEFINE([DEBUG_ENABLED], [1], [Debug enabled])
    CFLAGS="$CFLAGS -DG_ENABLE_DEBUG -DG_ERRORCHECK_MUTEXES -Wall -Werror -g3 -O0"
 else
-   CFLAGS="$CFLAGS -DG_DISABLE_ASSERT -DG_DISABLE_CHECKS -DG_DISABLE_CAST_CHECKS -Wall -g0 -O2"
+   CFLAGS="$CFLAGS -DG_DISABLE_ASSERT -DG_DISABLE_CHECKS -DG_DISABLE_CAST_CHECKS "
 fi
 
 # Substitute


But compiling fails with the following error:

+ make -j3
CDPATH="${ZSH_VERSION+.}:" && cd . && /bin/sh /home/mariobl/rpmbuild/BUILD/frogr-0.5/missing --run aclocal-1.11 -I m4
aclocal-1.11: couldn't open directory `m4': No such file or directory
make: *** [aclocal.m4] Error 1

Is that due to my change?

Comment 22 Christophe Fergeau 2011-06-01 21:50:52 UTC
It's likely, make detects configure.ac is newer than configure and tries to regenrate it. You can either also patch configure in a similar way to get rid of the extra flags, or rerun autoreconf -fi before %configure. NB: I haven't checked in fedora guidelines if there's a recommended way of doing this.

Comment 23 Susi Lehtola 2011-06-01 22:15:47 UTC
You might want to try if
 make CFLAGS="$RPM_OPT_FLAGS" %{?_smp_mflags}
solves the problem with the optimization flags. (This in case sources only in C, if you have C++ then you also have to define CXXFLAGS.)

Comment 24 Martin Gieseking 2011-06-02 07:07:15 UTC
CFLAGS also defines some preprocessor constants (-DG_DISABLE_ASSERT -DG_DISABLE_CHECKS -DG_DISABLE_CAST_CHECKS) that probably have to be added when overwriting CFLAGS in the make statement. I'm not sure if they are actually required, though.

Mario Sanchez Prada, since it's a bad idea to hard-code -g0 and -O2 in configure.ac, I suggest to remove them in the next release.


Also, the source headers contain a wrong FSF address. It's recommended to use the wording mentioned in COPYING:
    You should have received a copy of the GNU General Public License
    along with this program.  If not, see <http://www.gnu.org/licenses/>.


frogr seems to use the bundled library flicksoup (src/flicksoup) which is available at http://gitorious.org/flicksoup. This is not allowed in Fedora. The library must be packaged separately.
https://fedoraproject.org/wiki/Packaging:No_Bundled_Libraries

Comment 25 Martin Gieseking 2011-06-02 07:21:33 UTC
Here are some further information on how to treat the bundled library:
https://fedoraproject.org/wiki/Packaging:Treatment_Of_Bundled_Libraries

Comment 26 Mario Sanchez Prada 2011-06-02 09:20:16 UTC
(In reply to comment #24)
> CFLAGS also defines some preprocessor constants (-DG_DISABLE_ASSERT
> -DG_DISABLE_CHECKS -DG_DISABLE_CAST_CHECKS) that probably have to be added when
> overwriting CFLAGS in the make statement. I'm not sure if they are actually
> required, though.
> 
> Mario Sanchez Prada, since it's a bad idea to hard-code -g0 and -O2 in
> configure.ac, I suggest to remove them in the next release.

I did it because it was handy for me, but in the end what I do need for development are the flags when debug mode is enabled. I actually don't care much about those others in release mode, so I agree and have already removed them for the next release:

http://git.gnome.org/browse/frogr/commit/?id=bb99f7443acd6d247e7494f4eced8d3635ec2c30

> Also, the source headers contain a wrong FSF address. It's recommended to use
> the wording mentioned in COPYING:
>     You should have received a copy of the GNU General Public License
>     along with this program.  If not, see <http://www.gnu.org/licenses/>.

Fixed in master:
http://git.gnome.org/browse/frogr/commit/?id=33573b2f8b9d8b261dbcba688a9d422e5a3f8a1f

> frogr seems to use the bundled library flicksoup (src/flicksoup) which is
> available at http://gitorious.org/flicksoup. This is not allowed in Fedora.
> The library must be packaged separately.
> https://fedoraproject.org/wiki/Packaging:No_Bundled_Libraries

Hmmm... we have an issue here, because that library is not mature yet to be published independently, and without it frogr won't work, so we need to find a solution for it if we want to ship frogr in Fedora.

But first, let me tell you why this situation of having frogr bundling a, in theory, separate library:

I started developing flicksoup (yes, I also hold the authorship and copyright over it) back in 2009 when frogr was still using flickcurl to communicate with flickr, hoping that at some point I could release flicksoup and then just link frogr against it. However, mainly due to my permanent lack of (contigous) time for these tasks, flicksoup ended up being stuck at some point, as well as frogr, because of a kind of an egg-chicken problem: frogr didn't evolve because flicksoup was not ready yet, and flicksoup didn't evolve because there was no specific application (frogr) using it that drove the development of that library in some way (because, due to my lack of time, building a 100% Flickr API complete library was completely out of scope).

So, at late 2010 I decided to just move the sources of the flicksoup library inside frogr so I could actually use it inside of that app to replace flickcurl, via static linking... and that actually worked pretty well, since I was able not only to replace flickcurl in a frogr that only uploaded pictures by then (no sets, no groups, no cancelling operations, no error messages...) in the way I always wanted to: through a GNOME-like library.

And so far, the situation was that I haven't had much time to think about releasing flicksoup so I kept on developing changes for it just inside frogr, and dumping every now and then those changes into its gitorious repo, just in case someone wanted to use what I had in there, which is possible to build and install as a shared library. And whenever I thought about that, I always came up to the conclusion that the library was not mature enough (if I released it now, I couldn't promise I wouldn't break API/ABI in the next releases), so I decided to wait and keep using it this way...

And now we have a situation because of this so I, as the author and copyright holder of both frogr and flicksoup, propose the following solution, if possible:

  A. Stop saying all around (frogr web site, package descriptions, internal files in frogr...) that Frogr "uses the flicksoup library... blablabla", and stop treating that as if it were a separate and independently maintained library (which it is not, it's just a dump of what I develop inside frogr).

  B. Use the same license for files inside src/flicksoup than for the rest of frogr, that is, the GNU GPLv3 license.

  C. If necessary, I could even temporarily remove the flicksoup repository in gitorious to strenght the idea of flicksoup not being a separately maintained library (yet). When ready, if that ever happens (hope so!), I would do the required changes by then: release flicksoup as a separate library, remove the flicksoup-related files from frogr, and make frogr depend on flicksoup.

  D. ... anything else? <-- put your additional suggestion here

IMHO, this makes sense seems the truth is that, as I said, nos the flicksoup repository is just a dump for the changes I do in those files inside frogr, and it's actually kind of redundant thing, so it would even make sense to drop it even if this issue haven't had arised because of packaging frogr for Fedora.

What do you think?

Comment 27 Christophe Fergeau 2011-06-02 09:54:13 UTC
I would keep src/flicksoup files as LGPLv3 if your long term plan/hope is to ship it as a separate library. If you get external patches to src/flicksoup, you don't want to be stuck with GPLv3 licensing the day you want to make it an external library.
Apart from that, your plan makes sense to me. Another option might be to install flicksoup as a .so as part of flickr "make install", and to also install .h files. This way, the library can be put in separate lib/-devel subpackages. However, this is not a really good solution, since I'm under the impression that if someone tries to use it, things will break every time frogr gets upgraded.
I don't know if this is OK wrt fedora guidelines, but at least your plan makes sense to me.

Comment 28 Mario Sanchez Prada 2011-06-02 10:05:06 UTC
(In reply to comment #27)
> I would keep src/flicksoup files as LGPLv3 if your long term plan/hope is to
> ship it as a separate library. If you get external patches to src/flicksoup,
> you don't want to be stuck with GPLv3 licensing the day you want to make it an
> external library.

Yeah, I for sure would prefer not to touch the license, just putting options over the table, not consider to go ahead with any of them yet.

> Apart from that, your plan makes sense to me. Another option might be to
> install flicksoup as a .so as part of flickr "make install", and to also
> install .h files. This way, the library can be put in separate lib/-devel
> subpackages. However, this is not a really good solution, since I'm under the
> impression that if someone tries to use it, things will break every time frogr
> gets upgraded.

Already though of that (actually at the beginning frogr used flicksoup in that way), but if I'm shipping an application and not a library I think it's better to build flicksoup files inside of it as a static object, as it's being done now.

> I don't know if this is OK wrt fedora guidelines, but at least your plan 
> makes sense to me.

Glad to hear that. Hopefully only (A) will be needed (I do not like much (B) and (C)) :-)

Comment 29 Martin Gieseking 2011-06-02 14:28:36 UTC
I ask on IRC how to handle this issue and got some helpful information from Rex Dieter. First of all, it's not possible to bundle the library "silently", i.e.  FPC must grant an exception from the no-bundled-libraries policy. Mario (B.), you should ask for such an exception as described here:
https://fedoraproject.org/wiki/Packaging:No_Bundled_Libraries#Exceptions 

I think you could also package a Git snapshot of flicksoup and use it instead of the bundled library. In this case, all changes to the bundled library should of course be pushed to the flicksoup repo so that both source pools are in sync and future updates work smoothly. If the FPC doesn't grant an exception, this is probably the only alternative. 


(In reply to comment #26)
> if I released it
> now, I couldn't promise I wouldn't break API/ABI in the next releases

That shouldn't be too much of a problem as long as you bump the soname and soversion accordingly.

Comment 30 Mario Sanchez Prada 2011-06-02 15:17:19 UTC
(In reply to comment #29)
> [...]
> I think you could also package a Git snapshot of flicksoup and use it instead
> of the bundled library. In this case, all changes to the bundled library
> should of course be pushed to the flicksoup repo so that both source pools 
> are in sync and future updates work smoothly. 

Hmmm... the more I think about it the more I regret having started with flicksoup as a separate thing in the first place. I should have started everything together and split it later on, then we wouldn't have this problem now... :/

Currently flicksoup files inside of frogr are not on sync with those in gitorious (frogr's files are more recent) and they probably will change a little bit more during the development of frogr 0.6, specially in those functions related to proxy support.

> If the FPC doesn't grant an exception, this is probably the only alternative. 

If the Fedora Packaging Comitee granted that exception that would be awesome. As I said, I'm open to (temporarily at least) removing the git repository of flicksoup in gitorious, and I'm also open, if bundling a static library in frogr is a problem (as I understand from your words), to merge even more those files in src/fliksoup with frogr, so they would be compiled just through the same set of files in the SOURCES variable, in src/Makefile.am.

That way, there wouldn't be anymore a libflicksoup.a frogr would link against, just the source files as they are, avoiding the problems with Fedora packaging, if I got it right.

In the future, if I (or anyone else) manage to get the right timespan to make flicksoup an independent package out of frogr, we would just remove those files and set the dependency.

But all this, when it comes to releasing frogr 0.5 in fedora, would mean that such an exception should be granted. Or well... if such an exception was not granted we could also release frogr 0.5.1 with those modifications in place, in case you agree is a good idea

If this idea (remove the libflicksoup.a from frogr) was feasible, I'd say is the best way to go. And if keeping the flicksoup files inside of frogr with the LGPLv3 license was not a problem for Fedora, then it would be perfect.
 
> (In reply to comment #26)
> > if I released it
> > now, I couldn't promise I wouldn't break API/ABI in the next releases
> 
> That shouldn't be too much of a problem as long as you bump the soname and
> soversion accordingly.

Yeah, I know, but the problem is other... more related with me feeling confident and comfortable enough to releasing  the library separately. And because one reason of another the truth is that I still do not feel like it...

Btw, just in case you wonder, do not worry about me perhaps disliking the idea of dropping the flicksoup repo from gitorious... actually that wouldn't be that bad for me, since it would free me from the burden of having to keep two copies of the same file set on sync, and I could always re-open it in the future when I feel confident enough to propose flicksoup as an independent package :-).

Comment 31 Mario Sanchez Prada 2011-06-02 21:12:55 UTC
(In reply to comment #30)
> [...]
> > If the FPC doesn't grant an exception, this is probably the only alternative. 
> 
> If the Fedora Packaging Comitee granted that exception that would be awesome.
> As I said, I'm open to (temporarily at least) removing the git repository of
> flicksoup in gitorious, and I'm also open, if bundling a static library in
> frogr is a problem (as I understand from your words), to merge even more those
> files in src/fliksoup with frogr, so they would be compiled just through the
> same set of files in the SOURCES variable, in src/Makefile.am.
> 
> That way, there wouldn't be anymore a libflicksoup.a frogr would link against,
> just the source files as they are, avoiding the problems with Fedora packaging,
> if I got it right.

I did this change upstream in frogr, you can check it out here:
http://git.gnome.org/browse/frogr/commit/?id=29baa0f75d8247c8769375c1a1e616c6cc81ddb5

Now there's no more a libflicksoup.la file, as all the fsp-*.[ch] files are compiled as part of the frogr binary. Can somebody point out if in this situation frogr would be more suitable to be packaged for Fedora? 

Also still not removed the flicksoup repo from gitorious, waiting for some feedback from this thread on the topic, although I'm now more in the mood of removing it than of keeping it :-)

> [...]
> But all this, when it comes to releasing frogr 0.5 in fedora, would mean that
> such an exception should be granted. Or well... if such an exception was not
> granted we could also release frogr 0.5.1 with those modifications in place, in
> case you agree is a good idea

As I mentioned here, if releasing frogr 0.5.1 with these latest modifications in place help in any way to Fedora packagers, I'll gladly do it so. Just ask.

> If this idea (remove the libflicksoup.a from frogr) was feasible, I'd say is
> the best way to go. And if keeping the flicksoup files inside of frogr with the
> LGPLv3 license was not a problem for Fedora, then it would be perfect.

At the moment I kept the files in src/flicksoup/ as LGPLv3. Hope there's no issue with that.

Last but not least, I've changed the wording about flicksoup inside the files that were mentioning it inside frogr, to stop telling that is a "bundled library", which is not yet.

Hope these changes help moving towards our goal :-)

Comment 32 Christophe Fergeau 2011-06-03 11:58:45 UTC
I'm not sure I'd have made this build system change to frogr :) There are plenty of projects building various helper libraries during the build, and then linking everything together to generate the final binary. This is the most convenient way of having separate directories when doing recursive builds with automake, which was the norm until recently.

What I'd do would be to remove libflickrsoup from gitorious and to insist that for now libflickrsoup is just an internal library private to frogr, which it is. The day you decide to make libflickrsoup an actual library that you want to support, you should be able to generate a git repository with full history using git filter-branch.

Comment 33 Mario Sanchez Prada 2011-06-03 12:47:51 UTC
(In reply to comment #32)
> I'm not sure I'd have made this build system change to frogr :) There are
> plenty of projects building various helper libraries during the build, and then
> linking everything together to generate the final binary. This is the most
> convenient way of having separate directories when doing recursive builds with
> automake, which was the norm until recently.

Well, I'm not an expert on this matters but as frogr is a small application and not sure that this makes a big difference in terms of building it. However, if that helps Fedora packagers to be happier, then I'll be happier too, knowing that in the future the door to make flicksoup an independent library is still open.

Anyway, reverting that commit is also a possibility in case we wanted to do it so. For the time being, and to avoid too much ping-pong with this, I'll leave it the way it is now: no libflicksoup.a file.

> What I'd do would be to remove libflickrsoup from gitorious and to insist that
> for now libflickrsoup is just an internal library private to frogr, which it
> is. The day you decide to make libflickrsoup an actual library that you want to
> support, you should be able to generate a git repository with full history
> using git filter-branch.

Done, and also changed the description in live.gnome.org/Frogr not to mention it.

Hope now the FPC believes me when I say flicksoup is not an independently maintained library yet :-)

Comment 34 Martin Gieseking 2011-06-03 14:22:36 UTC
I agree with Christophe that it shouldn't be a problem to build private static libraries and link them to the binary. As long as they are an exclusive part of the project this should be fine. 

In order to emphasize flicksoup is an integral part of frogr and not intended to be used in third-party projects, I suggest to unify the source headers and release everything under GPLv3. Having differently licensed files might require some explanation (and an additional COPYING file with the LGPL license text in src/flicksoup ;-)). You can change the license back to LGPLv3 -- or whatever license you prefer -- once flicksoup is ready to become a separate project.

Comment 35 Christophe Fergeau 2011-06-03 14:42:04 UTC
(In reply to comment #34)
>
> In order to emphasize flicksoup is an integral part of frogr and not intended
> to be used in third-party projects, I suggest to unify the source headers and
> release everything under GPLv3. Having differently licensed files might require
> some explanation (and an additional COPYING file with the LGPL license text in
> src/flicksoup ;-)). You can change the license back to LGPLv3 -- or whatever
> license you prefer -- once flicksoup is ready to become a separate project.

Here I disagree, LGPLv3 licensed code can be used as if it was licensed under the GPLv3, so no compatibility issues as far as I know, and if external contributions are made to this code, relicensing from GPLv3 to LGPLv3 will be annoying (ie need to contribute everyone to get their agreement). So I don't really see a compelling reason to do the relicensing.

Comment 36 Mario Sanchez Prada 2011-06-03 15:07:21 UTC
(In reply to comment #34)
> I agree with Christophe that it shouldn't be a problem to build private static
> libraries and link them to the binary. As long as they are an exclusive part of
> the project this should be fine. 

Ok, it's obvious then (as per your comment and Christophe's) that I misunderstood you in the first comments: I though having a .a file was actually an issue. I'll probably change it at some point back to how it was before. Now that flicksoup is no longer published independently, and that I've updated references in wording inside frogr (and in live.gnome.org) that shouldn't be an issue I guess.

> In order to emphasize flicksoup is an integral part of frogr and not intended
> to be used in third-party projects, I suggest to unify the source headers and
> release everything under GPLv3. Having differently licensed files might require
> some explanation (and an additional COPYING file with the LGPL license text in
> src/flicksoup ;-)). You can change the license back to LGPLv3 -- or whatever
> license you prefer -- once flicksoup is ready to become a separate project.

If putting a COPYING file inside src/flicksoup is good enough I'd go for this approach, instead of changing the license of flicksoup files to GPLv3 now.

Btw, not sure at this point whether it would be still needed that I release 0.5.1 to ease life of Fedora packagers. Perhaps it's already clear enough that flicksoup is not maintained separately :-), otherwise just ping me, give me some time (cause I did this just in spare time), and I'll try to do it asap.

Thanks!

Comment 37 Mario Sanchez Prada 2011-06-03 15:32:15 UTC
(In reply to comment #36)
> [...]
> If putting a COPYING file inside src/flicksoup is good enough I'd go for this
> approach, instead of changing the license of flicksoup files to GPLv3 now.
 
http://git.gnome.org/browse/frogr/commit/?id=56616a73f0816ca4d22cedf5225671e76e655a06

:-)

Comment 38 Martin Gieseking 2011-06-03 15:38:30 UTC
(In reply to comment #35)
> Here I disagree, LGPLv3 licensed code can be used as if it was licensed under
> the GPLv3, so no compatibility issues as far as I know, and if external
> contributions are made to this code, relicensing from GPLv3 to LGPLv3 will be
> annoying (ie need to contribute everyone to get their agreement). So I don't
> really see a compelling reason to do the relicensing.


Of course it's possible to keep the separate license but then you have to explain why an integral part of frogr not intended to be used externally needs its own license. If everything goes into an application licensed under GPLv3, why do you need LGPLv3 code in a separate directory? This just looks like a project bundled with the tarball. But that's only my humble opinion. Let's FPC have a look at this. Mario B. would you ask for a bundled library exception?



(In reply to comment #36)
> Ok, it's obvious then (as per your comment and Christophe's) that I
> misunderstood you in the first comments: I though having a .a file was actually an issue

No, sorry for the confusion. Linking object files directly or putting them in a static library and link the .a file doesn't make a big difference. The question is: Do we have a library that could be used outside the project (and is promoted a s such) or is it just a module exclusively related to the main project.

Comment 39 Mario Sanchez Prada 2011-06-03 17:26:22 UTC
(In reply to comment #38)
> (In reply to comment #35)
> > Here I disagree, LGPLv3 licensed code can be used as if it was licensed under
> > the GPLv3, so no compatibility issues as far as I know, and if external
> > contributions are made to this code, relicensing from GPLv3 to LGPLv3 will be
> > annoying (ie need to contribute everyone to get their agreement). So I don't
> > really see a compelling reason to do the relicensing.
> 
> 
> Of course it's possible to keep the separate license but then you have to
> explain why an integral part of frogr not intended to be used externally needs
> its own license. If everything goes into an application licensed under GPLv3,
> why do you need LGPLv3 code in a separate directory? This just looks like a
> project bundled with the tarball. But that's only my humble opinion. Let's FPC
> have a look at this. Mario B. would you ask for a bundled library exception?

Yeah, I understand. That's why I added a "Note: <explanation>" in the README file, because the truth is that I hope at some poing to have some time to devote to make it a library (or that someone else step in and do it :-)). Thinking that way, it makes a lot of sense to me to license those files as LGPL.
 
> (In reply to comment #36)
> > Ok, it's obvious then (as per your comment and Christophe's) that I
> > misunderstood you in the first comments: I though having a .a file was actually an issue
> 
> No, sorry for the confusion. Linking object files directly or putting them in a
> static library and link the .a file doesn't make a big difference.

Ok, got it. I'll probably get it back to the original situation at some point then.

> The question is: Do we have a library that could be used outside the project (and is
> promoted a s such) or is it just a module exclusively related to the main
> project.

After recent events, I'd say now the answer is simple: no, we do not have such a library that could be used outside the project yet.

What I'll probably do in the future, unless someone else do it first, is to wait till frogr being stable and feature complete enough so I can forget about
releasing new versions for some time, then focusing on starting my "evil plan":

  1. Take the flicksoup files out of frogr and release them as an independent library.
  2. Try to convince people out there to package that library, so I can further safely make frogr depend on it for future releases, (at least in those distros already packaging frogr).
  3. Make the next release of frogr depend on that library, removing all the files under src/flicksoup
  4. Release new version frogr depending on flicksoup, finally.

How does it sound? 

Keep in mind that my "evil plan" could take months to materialize, perhaps when my second child is born (late August 2011) it could be a good moment to think about it. Why? well, if you think that I started frogr right when my first son was born, you could understand it would make sense to me to do something similar with flicksoup when the second one comes to life... kind of my "personal way of welcoming my sons to the world of FS", right since their most early days" :-)

Comment 40 Mario Blättermann 2011-06-03 18:12:45 UTC
(In reply to comment #39)
> > The question is: Do we have a library that could be used outside the project (and is
> > promoted a s such) or is it just a module exclusively related to the main
> > project.
> 
> After recent events, I'd say now the answer is simple: no, we do not have such
> a library that could be used outside the project yet.
> 
In any case, I would prefer a proper way for the time being. As the Gitorious repo has been crowded, it is no longer needed to ask for a "bundled library" exception, because there's no library anymore outside there.

What does it mean for the technical side of the package? My idea is to use the recent Git commits as patches to make the debug file creation work. Then we could go ahead with Frogr itself. Once a day we have a separate library, then it is time to roll up the process again. For now, we should concentrate the effort to v0.5, even with patches, of course.

Comment 41 Martin Gieseking 2011-06-03 18:45:27 UTC
(In reply to comment #40)
> In any case, I would prefer a proper way for the time being. As the Gitorious
> repo has been crowded, it is no longer needed to ask for a "bundled library"
> exception, because there's no library anymore outside there.

Yes, you should ask for an exception to ensure that the library hasn't to be packaged separately. It's still a bundled library with its own licensing.

Comment 42 Christophe Fergeau 2011-06-03 19:06:54 UTC
(In reply to comment #41)
> Yes, you should ask for an exception to ensure that the library hasn't to be
> packaged separately. It's still a bundled library with its own licensing.

These are some C files that are licensed differently because they are generic enough to be useful outside of this project, I'm not sure where you are seeing a library here.

Comment 43 Martin Gieseking 2011-06-03 19:39:41 UTC
(In reply to comment #42)
> These are some C files that are licensed differently because they are generic
> enough to be useful outside of this project, I'm not sure where you are seeing
> a library here.

Yes, generic files in a separate folder with the name of a former library. There are still many websites with references to flicksoup as a library. You even find it packaged separately on a cygwin site. I don't want to argue here. I just suggest to ensure that we provide a package that conforms the guidelines and that the latest changes are sufficient. So where's the problem to simply ask the FPC people? But if you think it's not necessary, then please continue.

Comment 44 Christophe Fergeau 2011-06-03 19:56:34 UTC
I'm fine with asking the FPC people, it just sounds like unnecessary red tape to me. The reason for the "no bundled library" rule is to make sure every library user uses the same version installed as a shared library since it makes security fixes and bug fixes much easier. Given that upstream doesn't want flicksoup to be used/packaged as a separate library and that frogr is the only user, my position is that it's not a bundled library, just an internal detail of frogr build system.

But yeah, let's ask FPC if it's the way to go.

Comment 45 Mario Sanchez Prada 2011-06-03 21:09:04 UTC
(In reply to comment #40)
> [...]
> What does it mean for the technical side of the package? My idea is to use the
> recent Git commits as patches to make the debug file creation work. Then we
> could go ahead with Frogr itself. Once a day we have a separate library, then
> it is time to roll up the process again. For now, we should concentrate the
> effort to v0.5, even with patches, of course.

I'm ok with that, but if it's ok to you, I'd like to help you by providing those specific patches to you in an organized way, after having squashed all those little commits I've been pushing lately, so handling them while packaging would not be a mess.

That way I would provide Fedora packagers a very easy way to package frogr in a Fedora compliant way.

I propose this becase after so many (small) changes since the 0.5 release, I'm afraid just picking patches and applying them on the fly while packaging could be not that easy for someone not familiar with the project (that is me), 

What do you think?

Comment 46 Mario Sanchez Prada 2011-06-03 21:38:51 UTC
(In reply to comment #39)
> (In reply to comment #38)
[...]
> > No, sorry for the confusion. Linking object files directly or putting them in a
> > static library and link the .a file doesn't make a big difference.
> 
> Ok, got it. I'll probably get it back to the original situation at some point
> then.

Done, back to the origin (and sorry for the ping pong)

http://git.gnome.org/browse/frogr/commit/?id=b7b668cb303f3cc4b87276b14fc983bdc8a7f8ec

Comment 47 Mario Sanchez Prada 2011-06-03 22:33:51 UTC
Created attachment 502924 [details]
Patches to be applied over frogr 0.5 for packaging

(In reply to comment #45)
> [...]
> I'm ok with that, but if it's ok to you, I'd like to help you by providing
> those specific patches to you in an organized way, after having squashed all
> those little commits I've been pushing lately, so handling them while packaging
> would not be a mess.
> 
> That way I would provide Fedora packagers a very easy way to package frogr in a
> Fedora compliant way.

I'll be away of a laptop during all the weekend and I don't know if someone would like to do some work in the meanwhile about this... so now attaching a tar.gz file with all the patches that would need to be applied to frogr 0.5 release according this conversation, and taking into account that flicksoup files are being built again as an static file (see comment #46).

At the end the relevant patches are less and simpler than what I thought at first:

  0001-Do-not-set-Wall-g0-and-02-flags-in-release-builds.patch
  0002-Updated-FSF-address-in-source-headers.patch
  0003-Added-Lesser-to-source-headers-where-missing.patch
  0004-Changed-wording-in-the-license-about-flicksoup.patch
  0005-Changed-wording-about-flicksoup-in-README-file.patch
  0006-Added-LGPLv3-COPYING-file-under-src-flicksoup-and-up.patch

Hope this helps to packagers this time. For the next release it wouldn't be needed as all of them are already integrated upstream in master.

Have a nice weekend, and lots of thanks for the incredible feedback so far.

Comment 48 Mario Sanchez Prada 2011-06-04 06:39:57 UTC
Created attachment 502955 [details]
Additional patches that might be interesting to apply

You (fedora packagers) might also find interesting applying two additional patches that also are being applied specifically in the debian package:

  0001-Fix-segfault-when-no-valid-uris-are-passed-via-comma.patch
  0002-Updated-manpage-to-use-uri-instead-of-path.patch

Those are not strictly needed, but good to have.

The first one is about fixing a segfault in the _very unlikely_ case the user runs frogr from command line with wrong arguments (currently frogr only supports URIs to files as arguments, so even frogr --help will segfault. Yes, sad but true). The command line arguments are not intented to be used directly by the user but we need that frogr supports that in order to properly act as a handler for picture mimetypes, so you can "right click" on a picture in nautilus/eog/... and "Open with frogr...".

The second one is about updating the manpage, which should say "uri" instead of "path" after the following patch was applied for frogr 0.5:

http://git.gnome.org/browse/frogr/commit/?id=42196942bc467e7e97e69e5b06cddea3e8f2a6f7

About debian packages already bundling these two patches, here you have the mail were frogr 0.5-1 was accepted, where you can see them:

http://lists.debian.org/debian-devel-changes/2011/05/msg02377.html

Now it's for real... have a nice weekend!

Comment 49 Mario Sanchez Prada 2011-06-09 11:02:30 UTC
Just wondering... is there any place where I can follow the status of this request to the FPC? I found this site, but I'm not sure that's the place to follow it: https://fedorahosted.org/fpc/report/1

If there's anything else I can do from my side to help, just let me know.

Thanks!

Comment 50 Gianluca Sforna 2011-06-29 08:03:39 UTC
I can't find any FPC request on their trac instance.

Anyway, if the library is internal to the project I don't really see any issue for approving the package. For me, "internal" means it's not intended for external use, so for instance there are no header files installed by "make install" that would be used by a 3rd party user.

Comment 51 Mario Sanchez Prada 2011-06-29 09:41:59 UTC
(In reply to comment #50)
> I can't find any FPC request on their trac instance.
> 
> Anyway, if the library is internal to the project I don't really see any issue
> for approving the package. For me, "internal" means it's not intended for
> external use, so for instance there are no header files installed by "make
> install" that would be used by a 3rd party user.

Exactly. No header files at all will be installed in the system when you install frogr, they will remain internal to the project and will only be used when compiling frogr.

Thanks for the feedback

Comment 52 Martin Gieseking 2011-06-29 17:50:33 UTC
Since the reviewing process seems to have stalled, I asked Toshio Kuratomi (a member of the FPC) how to continue with the package. He agreed to go on without requesting a bundled library exception. The library may also be linked statically as long as no third-party project uses it. Once someone submits a package in the future that depends on flicksoup (despite upstream's decision to keep it private), we have to revisit Frogr too, and split out flicksoup to a separate (sub-)package.

So, sorry for having been a bit overcautious. If there's anybody who wants to take the review, he/she can continue without further hassle. :)

Comment 53 Gianluca Sforna 2011-06-30 08:14:16 UTC
Ok, taking the review, will probably work on this in the evening

Comment 54 Gianluca Sforna 2011-07-01 11:05:55 UTC
Few fixes to do:

1. Missing a BR for intltool
2. debuginfo package is missing sources because of the "-g0" CFLAG hardcoded in configure.ac; upstream should just omit it and things will not change for manual builds, while allowing our "-g" override. I used a trivial patch on configure to achieve this
3. rplint claims there's another incorrect FSF address in /usr/src/debug/frogr-0.5/src/flicksoup/fsp-session.c
4. there is a trailing ',' on the gtk3-devel BR line. You may want to remove it, and possibly also the "> version" thing, as F15 has the correct versions.

Comment 55 Mario Blättermann 2011-07-03 15:09:23 UTC
(In reply to comment #54)
> Few fixes to do:
> 
> 1. Missing a BR for intltool
Added.
> 2. debuginfo package is missing sources because of the "-g0" CFLAG hardcoded in
> configure.ac; upstream should just omit it and things will not change for
> manual builds, while allowing our "-g" override. I used a trivial patch on
> configure to achieve this
> 3. rplint claims there's another incorrect FSF address in
> /usr/src/debug/frogr-0.5/src/flicksoup/fsp-session.c
> 4. there is a trailing ',' on the gtk3-devel BR line. You may want to remove
> it, and possibly also the "> version" thing, as F15 has the correct versions.
Dropped the version numbers.

I've applied some patches from comment #47. No I get the following error:

+ make -j3
CDPATH="${ZSH_VERSION+.}:" && cd . && /bin/sh /home/mariobl/rpmbuild/BUILD/frogr-0.5/missing --run aclocal-1.11 -I m4
aclocal-1.11: couldn't open directory `m4': No such file or directory
make: *** [aclocal.m4] Error 1

Files:
Spec URL: http://mariobl.fedorapeople.org/Review/SPECS/frogr.spec
SRPM URL: http://mariobl.fedorapeople.org/Review/SRPMS/frogr-0.5-4.fc15.src.rpm

Comment 56 Christophe Fergeau 2011-07-04 08:31:27 UTC
If you patch configure.ac or Makefile.am, you should add an autoreconf -fi before %configure in %prep

Comment 57 Mario Blättermann 2011-07-04 18:49:15 UTC
(In reply to comment #56)
> If you patch configure.ac or Makefile.am, you should add an autoreconf -fi
> before %configure in %prep

Doesn't work, same error message. Due to the fact that all the patches are taken from Git, I should either use the Git content directly as source, or I should wait until frogr-0.6 has been released.

Comment 58 Mario Sanchez Prada 2011-07-04 19:20:43 UTC
I think in debian they used the workaround of ensuring that the m4 directory existed prior to that. Not sure if that could be applied here as well. 

Also, if you have some specific suggestion for integrating upstream I'll be glad to push it if that helps :)

Comment 59 Mario Sanchez Prada 2011-07-19 10:41:00 UTC
(In reply to comment #57)
> (In reply to comment #56)
> > If you patch configure.ac or Makefile.am, you should add an autoreconf -fi
> > before %configure in %prep
> 
> Doesn't work, same error message. Due to the fact that all the patches are
> taken from Git, I should either use the Git content directly as source, or I
> should wait until frogr-0.6 has been released.

JFTR, Berto (debian package maintainer for frogr) just told me now that it's a problem with the timestamp of some files. Using the published tarball should fix the issue, but that's not possible since some patches must be applied anyway so I agree waiting for frogr 0.6 is the best way to go. 

After all, the new release of frogr should happen anytime soon (August?), so no need to rush things, IMHO

Comment 60 Christophe Fergeau 2011-07-19 11:59:22 UTC
(In reply to comment #59)

> 
> After all, the new release of frogr should happen anytime soon (August?), so no
> need to rush things, IMHO

If you want to make sure the package is available in F16, make sure you'll still be on schedule even if you wait ( http://fedoraproject.org/wiki/Schedule ).
I won't comment on when at the latest the package has to ready to be available in f16 since I'm not familiar enough with fedora release process to know :)

Comment 61 Mario Sanchez Prada 2011-08-13 09:08:25 UTC
Frogr 0.6 has been released today. Feel free to package it for Fedora and/or ask me for whatever you might need from me to help with the task.

 - The announcement (sent also to gnome-announce-list, still pending on moderation): http://mail.gnome.org/archives/frogr-list/2011-August/msg00001.html

 - The tarballs:
http://download.gnome.org/sources/frogr/0.6/frogr-0.6.tar.xz  (2.08M)
  sha256sum: 1934b4ec738af0a27f334d753496e97659ba4e9317ad17e03d6f578152784d04

http://download.gnome.org/sources/frogr/0.6/frogr-0.6.tar.bz2 (2.16M)
  sha256sum: ca2b3ff4c1d20555815b29d97b0c404a06a6d37b680218bda9c1f8e70699857c

 - The frogr.spec file I'm currently using to manually build FC15 64 bit packages (just in case it helps): http://git.gnome.org/browse/frogr/tree/frogr.spec

Thanks!

Comment 62 Rahul Sundaram 2011-08-16 12:57:17 UTC
Mario Blättermann,  Are you going to bump up the spec to the newer version for review?

Comment 63 Alberto Garcia 2011-08-16 13:09:30 UTC
(In reply to comment #58)
> I think in debian they used the workaround of ensuring that the m4
> directory existed prior to that. Not sure if that could be applied
> here as well.

If you change configure.ac, 'make' will try to regenerate aclocal.m4
among others, and that will fail if there's no m4 directory.

That happens in general if configure.ac is newer than aclocal.m4, so
you might also run into that problem if the files are unmodified but
their timestamps are wrong (e.g. if you are not using the original
upstream tarball).

So, summarizing: if you need to regenerate aclocal.m4, make sure that
the m4 directory exists.

I'm not using any workaround in Debian since I'm not patching
configure.ac or any other autotools file.

Comment 64 Gianluca Sforna 2011-08-16 13:49:40 UTC
For the records, I tried to rebuild in mock the latest version and it seems ok, no workarounds needed

Comment 65 Alberto Garcia 2011-08-16 13:52:13 UTC
(In reply to comment #38)
> Of course it's possible to keep the separate license but then you
> have to explain why an integral part of frogr not intended to be
> used externally needs its own license.

My 2 cents:

My understanding of all this "No Bundled Libraries" rule is that in
general you want to avoid packaging a piece of software that includes
an external library in its source tree, for all the reasons explained
in the Fedora wiki page.

That looks like a sensible rule to me, but it's certainly not what
happens here. Apart from having two different licenses, none of the
problems detailed in the aforementioned wiki page apply to
Frogr/Flicksoup.

Flicksoup is not an external library, it's not being released
separately, there are no official tarballs and doesn't have a separate
upstream repository. It's not bundled with Frogr: it's an internal
part of it, much like mpglib was an internal part of mpg123 for a
decade or so.

I don't think that the fact that those files are released under a
different license makes any difference in this case.

Comment 66 Rahul Sundaram 2011-08-16 15:54:05 UTC
Alberto Garcia is correct.  All the patches in the current package is just not needed at all

Comment 67 Mario Blättermann 2011-08-16 18:40:58 UTC
(In reply to comment #66)
> Alberto Garcia is correct.  All the patches in the current package is just not
> needed at all

Yes, of course I've dropped all the patches because they came from the Git anyway. Here are the new files (with some delay, don't hit me):

Spec URL: http://mariobl.fedorapeople.org/Review/SPECS/frogr.spec
SRPM URL: http://mariobl.fedorapeople.org/Review/SRPMS/frogr-0.6-1.fc15.src.rpm

Koji scratch build:
http://koji.fedoraproject.org/koji/taskinfo?taskID=3276949

Comment 68 Mario Blättermann 2011-08-16 19:04:18 UTC
Small problems with 0.6:

The info panel shows "0.6~unreleased". Just a cosmetic issue, but moreover, the help files are partially in English, although I had tweaked the German translation before v0.6 was released. Perhaps we get a 0.6.1 release.

Comment 69 Mario Sanchez Prada 2011-08-16 21:32:48 UTC
(In reply to comment #68)
> Small problems with 0.6:
> 
> The info panel shows "0.6~unreleased". Just a cosmetic issue, but moreover, the
> help files are partially in English, although I had tweaked the German
> translation before v0.6 was released. Perhaps we get a 0.6.1 release.

This is very strange. I already commented in the bug you reported in GNOME's bugzilla, about it. Please see my comments in there:

https://bugzilla.gnome.org/show_bug.cgi?id=656688#c1

I'm afraid you'r using the wrong source code, but can't figure out why...

Comment 70 Mario Blättermann 2011-08-18 10:19:13 UTC
Sorry for the noise, the bug was in my own system, see https://bugzilla.gnome.org/show_bug.cgi?id=656688#c2. All is OK now, please go ahead with reviewing.

Comment 71 Mario Sanchez Prada 2011-08-18 11:03:30 UTC
(In reply to comment #70)
> Sorry for the noise, the bug was in my own system, see
> https://bugzilla.gnome.org/show_bug.cgi?id=656688#c2. All is OK now, please go
> ahead with reviewing.

That's great. However, and in order to perpetuate the bad luck I'm having with this tiny app :(, I think I will need to rollout a 0.6.1 bugfix release anyway, since I found a HUGE memory leak that is causing frogr to waste quite a lot of memory whenever the user opens the details dialog (a pretty common operation).

So, I'll probably release 0.6.1 soon, including that patch and some others less important (but important anyway) bugs detected after the release. In summary, the patches I'd be applying, unless some other sever bugs show up, would be:

  - fix a HUGE memory leak loading pixbufs in details window
  - more mem leaks fixed by Christophe Fergeau
  - a segfault in systems using GTK < 2.24
  - some problems in the Mac version (can't authorize)

Not sure whether this impacts or not packaging frogr for fedora, but just telling you in case you prefer to wait for the new bugfixing release.

Thanks and sorry for the mess,
Mario

Comment 72 Mario Blättermann 2011-08-18 12:18:01 UTC
(In reply to comment #71)
> So, I'll probably release 0.6.1 soon, including that patch and some others less
> important (but important anyway) bugs detected after the release. In summary,
> the patches I'd be applying, unless some other sever bugs show up, would be:
> 
>   - fix a HUGE memory leak loading pixbufs in details window
>   - more mem leaks fixed by Christophe Fergeau
>   - a segfault in systems using GTK < 2.24
>   - some problems in the Mac version (can't authorize)
> 
> Not sure whether this impacts or not packaging frogr for fedora, but just
> telling you in case you prefer to wait for the new bugfixing release.

If »soon« means within a week or two, I would wait with packaging. The "new package" policy in Fedora allows me to add a package during the whole distribution forming process. However, the package could remain in the testing repository somewhat longer, but it would be available anyway. In my mind, it is better to have as less patches as possible. Let's wait for 0.6.1.

Comment 73 Mario Sanchez Prada 2011-08-18 12:44:31 UTC
(In reply to comment #72)
> [...]
> If »soon« means within a week or two, I would wait with packaging. The "new
> package" policy in Fedora allows me to add a package during the whole
> distribution forming process. However, the package could remain in the testing
> repository somewhat longer, but it would be available anyway. In my mind, it is
> better to have as less patches as possible. Let's wait for 0.6.1.

In this specific case, I'm afraid soon means "really soon", like in "this week", which was my original idea when I wrote that comment, but even perhaps during today or tomorrow, if I manage to get some time.

Since it's a bugfixing release, and the number of patches to include is fairly small, not much additional testing will be needed, nor the usual additional steps that use to take more time when doing a normal release, so let's be optimist :-)

Thanks for understanding,
Mario

Comment 74 Gianluca Sforna 2011-08-18 15:20:23 UTC
Once in the repositories, the package can be updated a number of times during the lifetime of each Fedora release so no, the memory leak is not a problem for the review. I'll try to finalize it as soon as possible.

Comment 75 Mario Sanchez Prada 2011-08-18 15:44:53 UTC
As you wish, but as I said, I think it's very likely that I release the 0.6.1 during today or tomorrow (I found some kind beta testers who will help me trying it :-)), so keep that in mind in case you finally prefer to wait a day more or so.

Comment 76 Mario Sanchez Prada 2011-08-19 14:40:43 UTC
Here comes the 0.6.1 release:

 - The announcement (sent also to gnome-announce-list, still pending on
moderation):
http://mail.gnome.org/archives/frogr-list/2011-August/msg00002.html

 - The tarballs:
http://download.gnome.org/sources/frogr/0.6/frogr-0.6.1.tar.xz  (2.08M)
  sha256sum:
06d9ec7447495bb3d1572f7ad39b6f25bd4f40d35c03f73ea4b62cbb204972e1

http://download.gnome.org/sources/frogr/0.6/frogr-0.6.1.tar.bz2 (2.16M)
  sha256sum:
2ef99647d07f2c5236cb5101a130712f312d33aad14cfc651aa198f8b67e0d2b

Hope this time is for real! :-)

Thanks

Comment 77 Mario Blättermann 2011-08-20 07:14:19 UTC
(In reply to comment #76)
> Here comes the 0.6.1 release:
> 
And here comes the new package:

Spec URL: http://mariobl.fedorapeople.org/Review/SPECS/frogr.spec
SRPM URL: http://mariobl.fedorapeople.org/Review/SRPMS/frogr-0.6.1-1.fc15.src.rpm

Comment 78 Gianluca Sforna 2011-08-25 15:08:31 UTC
Ok, here is the review:

- package named according to package naming guidelines
- spec filename matches %{name}
- package licensed with open source compatible license (GPLv3)
- license matches actual license
- license file included in %doc
- spec written in American english
- spec legible
- source matches upstream e6a86c71599561f41cfd7d71f56dcd9f  frogr-0.6.1.tar.xz
- successfully builds in mock for F15 x86_64
- macro usage is consistent
- package contains code
- no large documentation
- correctly installs a .desktop file

Just a couple comments.

The Source0 URL is not good for the 0.6.1 release (check it out running "spectool -g frogr.spec"), you'll need to hardcode 0.6 instead of %{version} to fix it.

Strictly speaking, since you install docs in /usr/share/gnome/help you should either own that directory or require the packaging owning it (yelp), but I found a thread in fedora-packaging where it seems it's ok to leave it this way.

Ideally, frogr could be improved to print the error returned by the GNOME API when yelp is not available.

So, this package is APPROVED, but please be sure to fix the Source0 URL before importing it.

Comment 79 Martin Gieseking 2011-08-25 15:24:15 UTC
A quick note: 
%{_datadir}/%{name} is still unowned (see comment #19).

Comment 80 Mario Blättermann 2011-08-25 18:01:19 UTC
Many thanks for your review!

(In reply to comment #78)
> The Source0 URL is not good for the 0.6.1 release (check it out running
> "spectool -g frogr.spec"), you'll need to hardcode 0.6 instead of %{version} to
> fix it.
>
Yes, has been fixed. First I thought it would be fine to define a macro such as "%global baseversion 0.6", but I don't expect a lot of bugfix releases from the 0.6 branch.
> Strictly speaking, since you install docs in /usr/share/gnome/help you should
> either own that directory or require the packaging owning it (yelp), but I
> found a thread in fedora-packaging where it seems it's ok to leave it this way.
> 
> Ideally, frogr could be improved to print the error returned by the GNOME API
> when yelp is not available.
>
A task for Mario Sanchez Prada (CC'ed anyway). But as far as I know, there should already be an error message if Yelp hasn't been found.
> So, this package is APPROVED, but please be sure to fix the Source0 URL before
> importing it.

I will import this spec:
Spec URL: http://mariobl.fedorapeople.org/Review/SPECS/frogr.spec

(In reply to comment #79)
> A quick note: 
> %{_datadir}/%{name} is still unowned (see comment #19).

I've removed the asterisk from the appropriate line in %files.

Comment 81 Mario Blättermann 2011-08-25 18:05:32 UTC
New Package SCM Request
=======================
Package Name: frogr
Short Description: Flickr Remote Organizer for GNOME
Owners: mariobl
Branches: f15 f16
InitialCC:

Comment 82 Gwyn Ciesla 2011-08-25 20:11:02 UTC
Subject is capitalized, request is not, please make both the desired name. 
Thanks!

Comment 83 Mario Blättermann 2011-08-25 20:32:49 UTC
(In reply to comment #82)
> Subject is capitalized, request is not, please make both the desired name.

Thanks for the hint. Subject is now not capitalized, as usual, because the tarball is not capitalized, too.

Comment 84 Gwyn Ciesla 2011-08-26 09:50:22 UTC
Git done (by process-git-requests).

Comment 85 Fedora Update System 2011-08-27 19:36:54 UTC
frogr-0.6.1-1.fc16 has been submitted as an update for Fedora 16.
https://admin.fedoraproject.org/updates/frogr-0.6.1-1.fc16

Comment 86 Fedora Update System 2011-08-27 19:37:04 UTC
frogr-0.6.1-1.fc15 has been submitted as an update for Fedora 15.
https://admin.fedoraproject.org/updates/frogr-0.6.1-1.fc15

Comment 87 Fedora Update System 2011-08-28 05:41:45 UTC
frogr-0.6.1-1.fc16 has been pushed to the Fedora 16 testing repository.

Comment 88 Fedora Update System 2011-09-14 22:24:57 UTC
frogr-0.6.1-1.fc15 has been pushed to the Fedora 15 stable repository.

Comment 89 Fedora Update System 2011-09-30 18:40:39 UTC
frogr-0.6.1-1.fc16 has been pushed to the Fedora 16 stable repository.


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