Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=225751
Summary: Merge Review: file-roller Product: Fedora Extras Version: devel Platform: All OS/Version: Linux Status: NEW Severity: normal Priority: normal Component: Package Review AssignedTo: nobody@fedoraproject.org ReportedBy: nobody@fedoraproject.org QAContact: fedora-package-review@redhat.com CC: caillon@redhat.com
Fedora Merge Review: file-roller
http://cvs.fedora.redhat.com/viewcvs/devel/file-roller/ Initial Owner: caillon@redhat.com
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Merge Review: file-roller
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=225751
bdpepple@ameritech.net changed:
What |Removed |Added ---------------------------------------------------------------------------- AssignedTo|nobody@fedoraproject.org |bdpepple@ameritech.net Flag| |fedora-review?
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Merge Review: file-roller
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=225751
bdpepple@ameritech.net changed:
What |Removed |Added ---------------------------------------------------------------------------- AssignedTo|bdpepple@ameritech.net |caillon@redhat.com Flag|fedora-review? |fedora-review-
------- Additional Comments From bdpepple@ameritech.net 2007-02-03 13:07 EST ------- Good: * Package name conforms to the Fedora Naming Guidelines * Group Tag is from the official list * Buildroot has all required elements * All paths begin with macros * All necessary BuildRequires listed. * Package builds in Mock.
Must Fix: * Source URL in not canonical * Remove unnecessary Requires: Requires(post): desktop-file-utils Requires(postun): desktop-file-utils Refer to: http://fedoraproject.org/wiki/Packaging/ScriptletSnippets?action=show&re...
Minor: * Duplicate BuildRequires: glib2-devel (by pango-devel), pango-devel (by gtk2-devel), gtk2-devel (by libgnomeui-devel), libgnomeprint22-devel (by libgnomeprintui22-devel), autoconf (by libtool) * Could use -disable-static and not bother building static libs. * It looks like the Requires on GConf is unnecessary. * Is the conflicts on nautilus still necessary?
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Merge Review: file-roller
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=225751
------- Additional Comments From mclasen@redhat.com 2007-02-03 13:54 EST ------- FWIW, I disagree with the purging of "Duplicate BuildRequires" that seems to be proposed. If the configure script explicitly checks for glib2, it is a good idea to have an explicit BR for glib2-devel, even if pango-devel happens to pull it in already.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Merge Review: file-roller
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=225751
------- Additional Comments From caillon@redhat.com 2007-02-03 14:25 EST ------- I also disagree with canonicalizing the Source URL. It will need to be regularly updated to stay in sync, at least for GNOME URLs.
Sources are typically "automatically" updated with the expansion of the %{version} macro, for example. So, %{name}-%{version}.tar.bz2 for example. With GNOME, you see things such as: http://ftp.gnome.org/pub/gnome/sources/file-roller/ and you need to update the sourceball's parent directory as well.
So, 2.16.0 would have a URL of http://ftp.gnome.org/pub/gnome/sources/file-roller/2.16/%%7Bname%7D-%%7Bvers...
and 2.17.0 would have one of http://ftp.gnome.org/pub/gnome/sources/file-roller/2.17/%%7Bname%7D-%%7Bvers...
It's my impression we don't want to force people to do that.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Merge Review: file-roller
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=225751
------- Additional Comments From kevin@tummy.com 2007-02-03 14:39 EST ------- Note that you could do something like:
http://ftp.gnome.org/pub/gnome/sources/file-roller/%%7Bversion%7D/%%7Bname%7...
If that format always holds true.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Merge Review: file-roller
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=225751
------- Additional Comments From mclasen@redhat.com 2007-02-03 20:15 EST ------- nope, %{version} expsnds to 2.17.2, but the directory does not include the mico version number.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Merge Review: file-roller
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=225751
------- Additional Comments From bdpepple@ameritech.net 2007-02-04 09:52 EST ------- (In reply to comment #2)
FWIW, I disagree with the purging of "Duplicate BuildRequires" that seems to be proposed. If the configure script explicitly checks for glib2, it is a good idea to have an explicit BR for glib2-devel, even if pango-devel happens to pull it in already.
Agreed, the comments in the minor section of comment #1 aren't considered blockers for the packages approval. They are merely suggestion of things to look at.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Merge Review: file-roller
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=225751
caillon@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |CLOSED Resolution| |RAWHIDE
------- Additional Comments From caillon@redhat.com 2007-02-05 15:14 EST ------- I fixed the desktop-file-utils issue, but did not canonicalize the source because of the reasons I outlined.
--disable-static doesn't exactly disable building of all static objects, which I filed upstream: http://bugzilla.gnome.org/show_bug.cgi?id=404717
Left the rest alone.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Merge Review: file-roller
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=225751
roozbeh@farsiweb.info changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|CLOSED |ASSIGNED Keywords| |Reopened Resolution|RAWHIDE | Flag|fedora-review- |fedora-review?
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Merge Review: file-roller
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=225751
roozbeh@farsiweb.info changed:
What |Removed |Added ---------------------------------------------------------------------------- AssignedTo|caillon@redhat.com |bdpepple@ameritech.net
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Merge Review: file-roller
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=225751
toshio@tiki-lounge.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |toshio@tiki-lounge.com
------- Additional Comments From toshio@tiki-lounge.com 2007-02-08 00:18 EST ------- Canonicalizing the sources is a requirement: - MUST: The sources used to build the package must match the upstream source, as provided in the spec URL. Reviewers should use md5sum for this task.
The requirement predates the packaging committee but I belive I recall its purpose being to aid reviewers and possible automated QA scripts in verifying sources. Let me know if you consider this important enough for me to bring up at the next packaging committee meeting.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Merge Review: file-roller
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=225751
------- Additional Comments From caillon@redhat.com 2007-02-08 12:25 EST ------- Then please bring it up. The source URL can be derived by going to the website given in the URL field, and in some cases, notably such as with the mozilla sources, I get them many times before they are posted publicly (for security releases) so cannot say with 100% certainty what the actual Source URL will be at build time for my packages.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Merge Review: file-roller
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=225751
------- Additional Comments From toshio@tiki-lounge.com 2007-02-08 21:07 EST ------- I see two issues being expressed here:
1) Convenience. file-roller will need a one-character change in the spec approximately four times per year. 2) Factuality. When we are heavily involved with upstream (ex: Mozilla), there will be times when we have access to tarballs that aren't yet available in publicly available trees.
These are the reasons not to include the full URL in the source line. The reasons for having the full source URL are to help reviewers, qa people, and automated scripts find and verify the source.
I asked about this on fedora-packaging and got a limited response where #1 was seen as not having significant benefits for a change. #2 was seen as something that we need to make an exception for but needed more information. So if you're okay with it, I'd like to drop trying to change #1 and concentrate on #2.
For #2, we'd like to know if the source tarball eventually lands at a specific URL or if it's often not publically available at all. Is it a snapshot or something more formal? If the tarball eventually lands someplace, just that it wouldn't necessarily be present at the time we build, then we probably want to include the full URL where the tarball will eventually land with a note that the source may not be present there at this exact moment. If the source will never land one person suggested using a patch against the last released tarball. Another suggestion was to allow the mozilla source to go through with a spec file comment explaining why there is no upstream URL. A third suggestion was to treat it like a Red Hat created application which is only distributed in the srpm. We don't have a policy for that yet so what the proceedure will be for that is unknown. If that's the route you think we need to go, I'll start drafting something with Jeremy and let you know what it looks like.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Merge Review: file-roller
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=225751
------- Additional Comments From caillon@redhat.com 2007-02-08 21:26 EST ------- #1. There are other packages where this is more frequent and might be more than just a 1 character change, such as some sort of checksum in the path. The bottom line is that while there are patterns, they are impossible to script in every case and is an IMO unnecessary requirement.
#2. When I get it from upstream which is about 75% of the time, it will eventually land somewhere, but I don't always know what the URL will be. Sometimes I am forced to pull directly from CVS and this tarball will never be posted publicly. In the mozilla case, patching against the latest tarball is wrong. There are hundreds of patches between releases and to sort them out is silly. Shipping a megadiff is also wrong because it makes it harder to maintain, and I'd like to avoid becoming Debian.
#3 Additionally, forcing an http URI which the current guidelines mandate is not going to fly for packages which don't distribute tarballs, but are maintained in a git repository and have people check them out by tag.
I would like to see all these items addressed and furthermore, I'd like to see this changed from a required item to a recommended item. There is no real one-size-fits all here that I can tell. Additionally, It has no impact on the quality of the RPM or the testability. Aiding reviewers is nice, but does not impact the RPM itself, and they can always ask. As far as doing automated md5sum matching, it is also nice, but I personally have no issue verifying it myself, as I do for all source tarballs before I do anything with them (even locally).
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Merge Review: file-roller
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=225751
toshio@tiki-lounge.com changed:
What |Removed |Added ---------------------------------------------------------------------------- OtherBugsDependingO| |197974 nThis| |
------- Additional Comments From toshio@tiki-lounge.com 2007-02-08 21:55 EST ------- #1 Please give me some arguments as to why it's an unnecessary requirement. Without arguments, it's going to fail to pass the committee. With arguments it may or may not pass but at least it'll be considered.
#2 Given your relation with upstream on Mozilla we could use the snapshot method from #3 or the unwritten sources-only-in-srpm policy. Let me know which one would be preferable.
#3 I thought was taken care of in the rules for snapshots. However, I'm unable to find any snapshot guidelines on the wiki. Looks like it's been an undocumented guideline passed down on the mailing lists. Basically, you have to give the commands to generate the source tarball from the repo in a comment or provide a script which can generate the tarball. Snapshots also use a date in the release tag which you may or may not like.
I'll write up the snapshot guideline and propose it since this is something that we've been doing for a while. Let me know if I need to put my head together with jeremy and expedite the "sources only in srpm" policy as well.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Merge Review: file-roller
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=225751
------- Additional Comments From mclasen@redhat.com 2007-02-10 18:14 EST ------- Regardless wether it passes committee or not, there are multiple scenarios where there simply is not suitable source URL that can be put there.
I have no problem putting the full source url for gnome packages in the spec files, since I do so many package updates from gnome ftp that I can type the full source url for a gnome tarball without looking.
But what about packages which do not publish upstream releases in tarball form at all, or packages where the fedora source rpm is the preferred form of distribution because they are Red Hat inhouse projects ?
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Merge Review: file-roller
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=225751
------- Additional Comments From toshio@tiki-lounge.com 2007-02-10 19:05 EST ------- As explained in #13, there are de facto rules for pulling from revision control which need to be written down and voted on. And Jeremy Katz is working on the inhouse Red Hat "source rpm is canonical" policy. We need to write those two policies in such a way that Mozilla and similar are covered.
The way things have worked until now, the sources have been assumed to be available somehow (otherwise it's not open source.) If it's a tarball, provide a URL. If it's a snapshot from revision control, either a comment on how to pull that revision or a script to pull it and construct the tarball is necessary.
I'm not sure what Jeremy's plans are WRT srpm's being the canonical source but if the only public source is the rpm itself then the rules will have to reflect that.
The overarching reason is that sources need to be checked against upstream. One of RPM's design goals is to cleanly separate the upstream code (in the form of a tarball) from the vendor changes (in the form of patches). Including the information necessary to check this in the spec file helps reviewers to check that the tarball is actually based on upstream. In cases where we're upstream we should theoretically be able to apply other, better tests to show this: like tapping the developer on the shoulder and asking if he really released 0.2 yesterday with the following md5sum.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Merge Review: file-roller
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=225751
------- Additional Comments From pertusus@free.fr 2007-02-11 05:46 EST ------- In some rare case there it isn't possible to have an url, when there was an upstream project, but it has disappeared and one happens to find a tarball somewhere.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Merge Review: file-roller
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=225751
------- Additional Comments From mclasen@redhat.com 2007-02-11 08:19 EST ------- There is also the case of upstream projects which are hosted e.g. at sourceforge, where it is very hard to come up with a working url. The sourceforge download urls I have come up with had the filename as a parameter behind a '?', and rpmbuild does not grok that, it seems to assume that the filename part of the source url is always delimited by a '/'
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Merge Review: file-roller
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=225751
------- Additional Comments From ville.skytta@iki.fi 2007-02-11 08:38 EST ------- http://downloads.sourceforge.net/$project/$tarball-$version.tar.gz works in those cases.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Merge Review: file-roller
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=225751
------- Additional Comments From toshio@tiki-lounge.com 2007-02-12 12:22 EST ------- (In reply to comment #15)
In some rare case there it isn't possible to have an url, when there was an upstream project, but it has disappeared and one happens to find a tarball somewhere.
In those cases, I generally consider that we have become upstream per: http://www.fedoraproject.org/wiki/Extras/OrphanedPackages?highlight=%28upstr...
As such it will fall under the "We're upstream and source rpm is canonical" when it is created. Or we could use Debian or some other distro as the upstream and have a comment that explains how to extract the source tarball from their package.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Merge Review: file-roller
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=225751
toshio@tiki-lounge.com changed:
What |Removed |Added ---------------------------------------------------------------------------- OtherBugsDependingO|197974 | nThis| |
------- Additional Comments From toshio@tiki-lounge.com 2007-02-14 16:13 EST ------- I have a draft of the new guideline here:: http://www.fedoraproject.org/wiki/PackagingDrafts/SourceUrl
I'm sending it out to the fedora-packaging list for suggestions and comments today.
caillon: If you could talk a bit about the Mozilla case and pose any suggestions you might have about getting your Mozilla workflow to fit comfortably in one of those sections that would be beneficial.
Since it seems no one here is demanding that file-roller fits under this rule, I'm unblocking this review from FE-GUIDELINES with the expectation that the Soure0: line would contain the canonical URL. If that is in error, please feel free to let me know along with some suggestions for enhancing the draft.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Merge Review: file-roller
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=225751
------- Additional Comments From caillon@redhat.com 2007-02-14 16:47 EST ------- Looks good from the brief glance I took, but I strongly feel this whole thing should be a "good practises" recommendation and not a requirement. If you're trying to prevent against "bad" RPMs, well you're not going to do that if there are exceptions... Even for a good SRPM, someone could simply fork an open source project, not have a repo other than the SRPM, and distribute whatever code they want that way in extras, theoretically. This has no bearing on the actual packaging or quality of RPMs. It's only redeeming quality is that it might potentially help with automated verification of upstream sources, but that does not exist right now and that potential benefit should be enough to convince most packagers to do this. There's simply no reason to make it a hard requirement IMO other than because it's always been that way (which is no real reason).
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Merge Review: file-roller
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=225751
mcepl@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |NEEDINFO CC| |mcepl@redhat.com Flag| |needinfo?(bdpepple@ameritech | |.net)
------- Additional Comments From mcepl@redhat.com 2007-02-27 05:56 EST ------- Brian, so what is your conclusion about this bug (theoretical discussions about Source0 URLs standard aside)?
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Merge Review: file-roller
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=225751
bugzilla@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Severity|normal |medium Priority|normal |medium
------- Additional Comments From mclasen@redhat.com 2007-06-17 00:16 EST ------- Stalled review ?
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Merge Review: file-roller
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=225751
bdpepple@ameritech.net changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEEDINFO |ASSIGNED Flag|fedora-review?, |fedora-review+ |needinfo?(bdpepple@ameritech| |.net) |
------- Additional Comments From bdpepple@ameritech.net 2007-06-17 10:11 EST ------- Going over the spec file again, all my initial blockers have been fixed. Setting review to '+'.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Merge Review: file-roller
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=225751
bdpepple@ameritech.net changed:
What |Removed |Added ---------------------------------------------------------------------------- AssignedTo|bdpepple@ameritech.net |mclasen@redhat.com
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Merge Review: file-roller
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=225751
bugzilla@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Product|Fedora Extras |Fedora
mclasen@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |CLOSED Resolution| |CURRENTRELEASE
------- Additional Comments From mclasen@redhat.com 2007-08-10 21:28 EST ------- Review done.
package-review@lists.fedoraproject.org
-
Red Hat Bugzilla