| Summary: | [EPEL] - Review Request -- thrift 0.6.1 | ||||||
|---|---|---|---|---|---|---|---|
| Product: | [Fedora] Fedora EPEL | Reporter: | Nelson Marques <nmo.marques> | ||||
| Component: | Package Review | Assignee: | Tom "spot" Callaway <tcallawa> | ||||
| Status: | CLOSED WONTFIX | QA Contact: | Fedora Extras Quality Assurance <extras-qa> | ||||
| Severity: | unspecified | Docs Contact: | |||||
| Priority: | unspecified | ||||||
| Version: | el5 | CC: | fedora-package-review, jeff, tcallawa | ||||
| Target Milestone: | --- | ||||||
| Target Release: | --- | ||||||
| Hardware: | Unspecified | ||||||
| OS: | Unspecified | ||||||
| Whiteboard: | |||||||
| Fixed In Version: | Doc Type: | Bug Fix | |||||
| Doc Text: | Story Points: | --- | |||||
| Clone Of: | Environment: | ||||||
| Last Closed: | 2012-07-25 11:43:19 UTC | Type: | --- | ||||
| Regression: | --- | Mount Type: | --- | ||||
| Documentation: | --- | CRM: | |||||
| Verified Versions: | Category: | --- | |||||
| oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |||||
| Cloudforms Team: | --- | Target Upstream Version: | |||||
| Attachments: |
|
||||||
|
Description
Nelson Marques
2011-11-29 13:06:32 UTC
I have updated the .spec file and SRPM, here's the new link for the SRPM (spec remains the same). SRPM: http://nmarques.fedorapeople.org/packages/thrift/thrift-0.6.1-1.el5.src.rpm Okay. There are quite a few things to work on here:
* Please start your release counter at 1, not 0 (unless it is some sort of pre-release package as documented in the guidelines)
* There is no need to have this section at all:
# -- package information
%define _name thrift
%define _version 0.6.1
%define _release 1
%define _group Development/Libraries
# -- filesystem information
%define _prefix /usr
# -- debug package
Just populate the Name:, Version:, Release:, Group: fields, as those will then automatically define %{name}, %{version}, %{release}, %{group}, respectively.
You also should never need to override _prefix.
* The BuildRoot is not one of the approved choices, see:
https://fedoraproject.org/wiki/EPEL/GuidelinesAndPolicies#Distribution_specific_guidelines
* Don't use 0%{?rhel_version} for conditionals. Use 0%{?rhel} (as described in https://fedoraproject.org/wiki/Packaging:DistTag)
* Explicit Requires should be for foo = %{version}-%{release}, not just foo = %{version}.
* The Group tag would not be identical for all subpackages. Not that it matters very much, to be honest, that tag is not required in RHEL 6 or newer, but if you're going to do it, please be more accurate.
* Also, the %description would not be identical for all subpackages. Please make them correct and applicable for each subpackage entry.
* Please don't double-up config options to configure on a single line. It makes it less obvious what is happening there, and more complicated to conditionalize or comment out in the future.
* The %{?jobs:-j%{jobs}} syntax isn't valid, replace it with %{?_smp_mflags}.
* %defattr(-,root,root) should be %defattr(-,root,root,-)
* For EL-5 or older, packages containing pkgconfig(.pc) files must Requires: pkgconfig (for directory ownership and usability).
* I'm not sure why you have split the glib library into a subpackage. Is this to minimize dependencies on the main binary?
* The LICENSE file must be included as %doc as part of every possible install transaction. This means that if a package with LICENSE is pulled in as a dependency of another subpackage, it does not need to be present, but in the case of items like the glib, perl, python subpackages, if they can be installed without the -libs subpackage, then they need to have LICENSE as %doc.
* You use both $RPM_BUILD_ROOT and %{buildroot}. Pick one and use it consistently throughout. I recommend %{buildroot}.
* Is it really necessary to use %{__make} and %{__cp} instead of simply "make" or "cp"?
* You still have FIXME - PHP REQUIRES
* Does make check not work for older releases than RHEL 7?
* Is there a reason this package isn't planned for Fedora?
Tom, Updated as you suggested and bumped release to 3. Regarding Fedora: I don't mind maintaining it for Fedora as long as it's the same version. There's a 0.7.0 available already for quite some time, but I've had a few reports that there's a few glitches. So could we keep 0.6.1 on Fedora until I have confirmation that some of the glitches were fixed ? NM I don't see why not, although, you should be prepared for users to file bugs that you are not at the latest version. :)
* To support Fedora, you'll want to make sure that all the conditionals are valid for Fedora (where %{rhel} is NULL) (and probably also enable the functionality that Fedora has but RHEL does not, like csharp, erlang, haskell, java, ruby).
* I think the php subpackage needs a copy of LICENSE too.
* You will also want to add the appropriate php requires, see:
https://fedoraproject.org/wiki/Packaging:EPEL#PHP_ABI_Check_Handling
* This conditional is weird:
%if %{_arch} == x86_64
Requires: perl(Math::BigInt)
%endif
Just Requires: perl(Math::BigInt) and be done with it. :)
* make check disappeared, I assume it doesn't work well?
Tom, The most important thing is to serve well and not enter in potential maintenance nightmares. So here's a question: - Can we work with two different branches? One for Fedora with the latest stable release (0.7.0) and one for RHEL with 0.6.1 which has less glitches. Still on the Fedora realm... here's a few things: - C# shouldn't be an issue. Your packaging is supreme. - haskell, sure. - Ruby - I'm not sure if we should provide the gem, as many people seem to prefer to have the gem built in separate, I've seen this a few times (no clue why). - Java should be ok, at least while RHEL doesn't provide the basic dependency versioning for ant, Fedora does. Now regarding perl(Math:BigInt), I followed what the upstream documentation suggested. I will introduce this change as well. About %check section, yes, I've removed it. I'm going to toy a bit more with this and submit hopefully still today a new spec. PS: As redundant information, the section I removed with the defines was mainly used by "Tirpitz", a small script I'm working to take a generic spec and provide a specific spec according to SUSE/Fedora/RHEL packaging guidelines. It's still a lot to do, and it's a part of my python learning process. We can live without it :). Extra question:
How well accepted would be something like:
%if 0%{?rhel}
Source0: foobar-1.0.0.tar.gz
%else
Source0: foobar-2.0.0.tar.gz
%endif
Would this be ok considering we would build 0.7.0 for Fedora and 0.6.1 for RHEL ?
NM
(In reply to comment #6) > Extra question: > > How well accepted would be something like: > > %if 0%{?rhel} > Source0: foobar-1.0.0.tar.gz > %else > Source0: foobar-2.0.0.tar.gz > %endif > > > Would this be ok considering we would build 0.7.0 for Fedora and 0.6.1 for RHEL > ? I wouldn't do it. The spec will get disgusting, and the changelog won't be sane anymore. I'd just do a 0.7.0 build for Fedora and a 0.6.1 build for EPEL. Show me both specs and I'll sanity check them individually. Tom, Regarding the Fedora stuff here's the deal: 1. C# - upstream doesn't provide a strong name, if I try to install the assembly with gacutil, it fails. will contact upstream regarding this. 2. JAVA - build is broken against java-openjdk. Either way, which java should link against? 3. Erlang - Done. 4. Ruby - Should we really provide the ruby gem? Most people I know prefer to build the gems themselves. Please advice. NM I'm not sure why people would prefer to build the ruby gem themselves, but even if they did, we should still provide it in a package for those who do not. As to C#, and Java, obviously, if they don't work, you should file bugs upstream and not enable them at this time. I think as far as which java, I would say openjdk6 for everything except rawhide, which should use openjdk7. (In reply to comment #9) > I'm not sure why people would prefer to build the ruby gem themselves, but even > if they did, we should still provide it in a package for those who do not. Ok, this is already done. > As to C#, and Java, obviously, if they don't work, you should file bugs > upstream and not enable them at this time. I think as far as which java, I > would say openjdk6 for everything except rawhide, which should use openjdk7. While I'm certain C# issue needs to be fixed by upstream, I'm not sure about the current JAVA issue which needs a bit more of attention and investigation. I'll update the request tomorrow once I test properly the JAVA build and investigate the C# history on JIRA. I've used the same spec for Fedora and RHEL and so far everything seems to be working and it's not really that confusing (at least for me). Created attachment 544692 [details]
Build with 0.8.0 on Fedora
Nelson, thanks for taking this on. I attached a patch to the spec file that has the changes that I made to get Thrift 0.8.0 to compile on Fedora. I'm just getting started with Thrift/Cassandra so I figured I'd start with the latest available. Hi Jeffrey, I will try to finish this submission this weekend and if possible a few others I have on the queue (lcm, darwin streaming server and a few perl modules). I've seen Tom also closed two reviews that will help enable a package I have for Fedora, so there's quite a few hours I need to fix all of this. Right now struggling to recover my system as RHEL update from 6.1 to 6.2 has some nefast effects on my laptop. NM Tom, Fedora builds are way too problematic and have way too much stuff to fix... I can't work them on work time because the build requires internet connection for erlang and java modules, it's dumb enough not to read my proxies, so I will either need a fix or direct connection to the internet. Furthermore, if redhat-rpm-config is a BuildRequires, the build just fails on the tests. Thrift 0.8.0 does seem to require some work to build properly. I have no interest in delaying Thrift 0.6.1 in EPEL because of this Fedora build. Anyway we can keep moving with the EPEL build and neglect Fedora submission for some time so I contact upstream and deal with this properly ? NM Nelson, I have no problem with you focusing on EPEL here, perhaps Jeff is interested in maintaining the Fedora side of things? That would be an ideal situation, as I could review both sides at one ticket, and let you move forward independently on the EPEL side. It is concerning that redhat-rpm-config causes the build to fail, as koji will have it present in the Build Environment even if it is not specified as an explicit BuildRequires, so I think you may need to fix that now, if it affects 0.6.1. Tom, Regarding 0.8.0 there's a few itches, some are actually new problems: * Parallel builds are broken; * Unit tests seem fail if redhat-rpm-config is around (needs a bit more of debugging, and most likely some fixes); Regarding the modules: * Java builds fine and installs fine (against java-1.7.0-openjdk); * C# has a open defect[1] on Thrift JIRA since 2009, opened by a Fedora contributor, but no upstream solution was provided since then; * Ruby packaging[2] is a nightmare, I was currently trying to fix this. I would rather keep myself focused on EPEL and not on Fedora which demands far more effort. [1] - https://issues.apache.org/jira/browse/THRIFT-509 [2] - http://fedoraproject.org/w/index.php?title=Archive:PackagingDrafts/RubyGem_with_C_code&oldid=83884 NM (In reply to comment #16) > I would rather keep myself focused on EPEL and not on Fedora which demands far > more effort. Okay. That's fine. It would be nice if someone was interested in working on the Fedora side in this ticket, but if not, we can work this ticket just for EPEL and leave Fedora to some other ticket. Tom, If no one takes it, I'll do it, but maybe only after the new year when I will have a bit more of free time (which is actually the handycap as the package has a lot of stuff to fix). It's a nice and challenging package which includes lots of stuff from different technology and from a packaging point of view it mixes a lot of fun stuff, but it does require quite a bit more of attention for the tiny details of every piece of different technology. This is maybe why it is somehow fun, but complicated and time demanding for the most unexperienced people like myself. If no one takes it, I'll take a look at it after the new year, meanwhile I'll just open a few bug reports on thrift JIRA. Meanwhile lets continue with EPEL one. NM I've started working on a package of thrift 0.8.0 to submit to Fedora supporting all the dependencies provided by Fedora. It's a bit later than I expected, but it's not been forgoten. I had to make a minor change to the package to get it to build for EPEL-6:
%if 0%{?rhel} <= 5
BuildRequires: php53-devel
%else
BuildRequires: php-devel
%endif
Let me know if you want me to just do an EPEL review at this point.
Heya, Good idea, lets get this moving, I can take Fedora package in the future and already take advantage from this review. There's a few issues I was unable to fix regarding erlang, but I'm not much proeficient with it also. Once we have this done, it's a bit easier to get it done later for Fedora with a more recent version. Account closed, activity terminated. |