Bug 226573
Summary: | Merge Review: xorg-x11-drivers | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Nobody's working on this, feel free to take it <nobody> |
Component: | Package Review | Assignee: | Adam Jackson <ajax> |
Status: | CLOSED RAWHIDE | QA Contact: | Fedora Package Reviews List <fedora-package-review> |
Severity: | medium | Docs Contact: | |
Priority: | medium | ||
Version: | rawhide | CC: | ajax, fedora |
Target Milestone: | --- | Flags: | fedora:
fedora-review+
|
Target Release: | --- | ||
Hardware: | All | ||
OS: | Linux | ||
Whiteboard: | |||
Fixed In Version: | Doc Type: | Bug Fix | |
Doc Text: | Story Points: | --- | |
Clone Of: | Environment: | ||
Last Closed: | 2007-02-26 22:43:46 UTC | Type: | --- |
Regression: | --- | Mount Type: | --- |
Documentation: | --- | CRM: | |
Verified Versions: | Category: | --- | |
oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
Cloudforms Team: | --- | Target Upstream Version: | |
Embargoed: |
Description
Nobody's working on this, feel free to take it
2007-01-31 21:21:39 UTC
* I understand the reasons for this package, but it looks like a maintaince nightmare as somebody needs to make sure this package is updated each time new drivers get added to the tree. That sucks :-/ -- for example currently there are at least xorg-x11-drv-amd and xorg-x11-drv-tek4957 available on i386, but not requires by this (I don't think that's on purpose). Suggestions to improve it (just for discussion, I'm unsure what the proper solution is): ship a script that generates the template for the specfile from cvs or "yum list" automatically. Or use something similar to the (ugly)trick that is used in the kmod packages to run a script and include it's output in the spec file instead of hardcoding the output in the spec -- Then a simple rebuild should do everything correctly. BTW, in case we stick to the current solution: the script mentioned in the comment would actually be more useful if one would know what what "xorg-all-drivers.txt" is or how it can be generated * Quoting the spec {{{ # This should match the list of architectures we build the Xorg server for. # Note the lack of s390{,x}. ExclusiveArch: %{ix86} x86_64 ia64 ppc ppc64 alpha sparc sparc64 }}} and all those {{{ %ifarch foo Requires: bar %endif }}} It IMHO would me wiser if we'd could use a ExcludeArch and ifnarch those packages and archs where we now those drivers don't exisit. But that's just my opinion and a detail and probably not worth the work... * rpmlint rpmlint on ./xorg-x11-drivers-7.1-3.i386.rpm W: xorg-x11-drivers invalid-license MIT/X11 -> MIT would be correct; But I fail what precisely is MIT licensed here... E: xorg-x11-drivers obsolete-not-provided xorg-x11 -> Why is that obsolete there in any case? E: xorg-x11-drivers no-binary - acceptable in this case W: xorg-x11-drivers no-documentation - might be a good idea to just create a small README that exaplains the purpose of this package (any maybe what might happen if you remove it) rpmlint on ./xorg-x11-drivers-7.1-3.src.rpm W: xorg-x11-drivers invalid-license MIT/X11 -> see above W: xorg-x11-drivers unversioned-explicit-obsoletes xorg-x11 -> if the obsoletes needs to stay better provide one with a version number -- that way we might be able to create a package with that name n the future * MISC: * "URL: http://www.redhat.com", I don't think that's helpful (might even be confusing), so maybe it should be removed * dist-tags are no must, but might be nice to use * Besides the stuff outlines above: package meets naming and packaging guidelines. specfile is properly named, is cleanly written and uses macros consistently. BuildRequires are proper. no shared libraries are present. package is not relocatable. no duplicates in %files. file permissions are appropriate. %clean is present. %check is present and all tests pass: (include the summary from the test suite, if any) no scriptlets present. code, not content. documentation is small, so no -docs subpackage is necessary. %docs are not necessary for the proper functioning of the package. no headers. no pkgconfig files. no libtool .la droppings. not a GUI app. not a web app. no open bugs > no open bugs Argh, no, that's untrue -- especially fixing Bug 198294 would be nice. The other should probably just moved where they belong or closed Okay, did some cleanup. First, Xorg is built on all arches except s390{,x}, so I flipped the ExclusiveArch at the top of the file to be ExcludeArch. In general I'd prefer to use ExcludeArch, since there's no intrinsic reason for most of X to be arch-specific. I did the same to all the driver packages I could, with the exception of a few that really are arch-specific. There's only ~8 of these, and they're clearly listed at the bottom of the Requires list now, along with a short comment for each explaining why they're %ifarch'd. Removed the Obsoletes: and fixed the License to be MIT. It's just a metapackage, there's really nothing to license, so picking the same license class as the drivers it's meta for seems appropriate. Which leaves us with: % rpmlint xorg-x11-drivers-7.2-2.fc7.src.rpm W: xorg-x11-drivers no-url-tag % rpmlint i686/xorg-x11-drivers-7.2-2.fc7.i686.rpm W: xorg-x11-drivers no-url-tag E: xorg-x11-drivers no-binary W: xorg-x11-drivers no-documentation As above, not having a binary is fine for this package. Removing the URL now generates an rpmlint warning for not having a URL. Eh. I think it's better without one. Also I don't see the point in making a trivial doc payload, given that it'd just be a repeat of the %description. i would like to see #199381 fixed with this also (In reply to comment #4) > i would like to see #199381 fixed with this also Do we even have those packaged yet? (In reply to comment #3) > Okay, did some cleanup. thx > I did the same to all the driver packages I could, with the exception of a few > that really are arch-specific. [...] thx, looks a lot better. But I think I spotted a bug: xorg-x11-drv-i810 should be required on x86_64, too, as there are x86_64 compatible boards with intel graphic chipsets out there. > [...] > Which leaves us with: > > % rpmlint xorg-x11-drivers-7.2-2.fc7.src.rpm > W: xorg-x11-drivers no-url-tag > % rpmlint i686/xorg-x11-drivers-7.2-2.fc7.i686.rpm > W: xorg-x11-drivers no-url-tag > E: xorg-x11-drivers no-binary > W: xorg-x11-drivers no-documentation > Also I don't see the point in making a trivial doc payload, given > that it'd just be a repeat of the %description. I'd say it would be worth the trouble. Regarding the bugs: - Fixing Bug 199381 would be nice and shouldn't do any harm, even if those drivers are are not (yet) packaged in main Fedora -- but I don't consider this a blocker (sorry dgilmore -- I'm counting on ajax cooperation in this case). - Bug 198294 will be fixed in another package APPROVED (In reply to comment #6) > > I did the same to all the driver packages I could, with the exception of a few > > that really are arch-specific. [...] > > thx, looks a lot better. But I think I spotted a bug: xorg-x11-drv-i810 should > be required on x86_64, too, as there are x86_64 compatible boards with intel > graphic chipsets out there. Worse than that, there are ia64 boards with i810 chips too. Fixed now. > Regarding the bugs: > > - Fixing Bug 199381 would be nice and shouldn't do any harm, even if those > drivers are are not (yet) packaged in main Fedora -- but I don't consider this a > blocker (sorry dgilmore -- I'm counting on ajax cooperation in this case). Until I know that the drivers are actually packaged, I don't see the need to break the metapackage on sparc. (In reply to comment #7) > > Regarding the bugs: > > - Fixing Bug 199381 would be nice and shouldn't do any harm, even if those > > drivers are are not (yet) packaged in main Fedora -- but I don't consider this > > a blocker (sorry dgilmore -- I'm counting on ajax cooperation in this case). > Until I know that the drivers are actually packaged, I don't see the need to > break the metapackage on sparc. Well, Fedora is not build for Sparc afaics, so no deps will be broken in our tree. But the community guys that are building Fedora for Sparc ask for this -- so I suppose they know what they are doing and have those drivers packaged in their tree, so deps will probably be satisfied there. So when they ask for it I'd say that it is a good sign of collaboration with the community to add this deps, as it doesn't do any harm to us, and saves them the work to adjust this specfile in their tree. |