Note: This is a public test instance of Red Hat Bugzilla. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback at bugzilla.redhat.com.
Bug 174021
Summary: | Review Request: aplus-fsf - Advanced APL Interpreter | ||||||
---|---|---|---|---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Jochen Schmitt <jochen> | ||||
Component: | Package Review | Assignee: | Jason Tibbitts <j> | ||||
Status: | CLOSED NEXTRELEASE | QA Contact: | Fedora Package Reviews List <fedora-package-review> | ||||
Severity: | medium | Docs Contact: | |||||
Priority: | medium | ||||||
Version: | rawhide | CC: | gemi | ||||
Target Milestone: | --- | Keywords: | Reopened | ||||
Target Release: | --- | ||||||
Hardware: | All | ||||||
OS: | Linux | ||||||
Whiteboard: | |||||||
Fixed In Version: | Doc Type: | Bug Fix | |||||
Doc Text: | Story Points: | --- | |||||
Clone Of: | Environment: | ||||||
Last Closed: | 2006-09-13 16:39:16 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: | |||||||
Bug Depends On: | |||||||
Bug Blocks: | 163779 | ||||||
Attachments: |
|
Description
Jochen Schmitt
2005-11-23 19:15:24 UTC
Errornous: The second Link should be: http://www.herr-schmitt.de/pub/apluf-fsf/aplus-fsf-4.20.2-1.src.rpm Sorry for any inconvinience. Best Regards: Jochen Schmitt For some reason I get a bad MD5 digest. What should the md5sum/sha1sum of the src rpm be? Also, note that with modular X your X defines and BuildRequires need another look. http://mharris.ca/xorg/xorg-modularization-status.html Anything happening with this ticket? The SRPM is corrupt and this package has little chance of building on FC5 or development because of modular X issues. It shouldn't be hard to fix, but first we'd have to see a clean SRPM. I have done some work on the package. The new version you may find at Spec: http://www.herr-schmitt.de/pub/aplus-fsf/aplus-fsf.spec SRPM: http://www.herr-schmitt.de/pub/aplus-sfs/aplus-fsf-4.20.2-2.src.rpm Adding back in the review that was lost in the crash: ------- Additional Comments From tibbs.edu 2006-06-11 00:19 EST ------- This review assumes you switch the dist tag around as necessary to build. I find it rather odd that the upstream tarfile is ends in .tar.gz but isn't actually compressed. I'm surprised rpmbuild handled that, but it did. You include the COPYING file as %doc, but it just refers to the LICENSE file which you don't package. I suggest packaging LICENSE and dropping COPYING. There's not really any reason to include a copy of COPYING (or LICENSE) in every subpackage although it doesn't hurt. If you want to do so, include LICENSE instead of COPYING as above. Your %post script for the truetype fonts calls ttmkfdir, but you only require it for postun. It seems to me that the fonts-truetype-apl subpackage should have the same list of requirements for both post and postun, since it calls the same programs. You drop a file into /usr/share/X11/app-defaults without owning that directory, yet none of your dependencies will create it for you. (In fact, currently the libX11.so dependency will pull in nx if the libX11 package isn't already installed, although that's not a problem this package should try to solve.) I think it's best to own that directory. By the way, just what is that app-default file for? I understand it specifies and alternate set of key mappings for xterm, but how would it get used? Review: * package meets naming and packaging guidelines. * specfile is properly named, is cleanly written and uses macros consistently. * dist tag is present. * build root is correct. * license field matches the actual license. X license is open source-compatible; text of license included upstream but not packaged. * source files match upstream: 2366264664c0b352b907b411af48e5aa aplus-fsf-4.20-2.tar.gz 2366264664c0b352b907b411af48e5aa aplus-fsf-4.20-2.tar.gz-srpm * latest version is being packaged. * BuildRequires are proper. * package builds in mock (development, x86_64). * rpmlint is silent. * final provides and requires are sane: aplus-fsf-4.20.2-2.fc6.x86_64.rpm libAplusGUI-4.20.2.so()(64bit) libIPC-4.20.2.so()(64bit) libMSGUI-4.20.2.so()(64bit) libMSIPC-4.20.2.so()(64bit) libMSTypes-4.20.2.so()(64bit) liba-4.20.2.so()(64bit) libadap-4.20.2.so()(64bit) libcxb-4.20.2.so()(64bit) libcxc-4.20.2.so()(64bit) libcxs-4.20.2.so()(64bit) libcxsys-4.20.2.so()(64bit) libesf-4.20.2.so()(64bit) aplus-fsf = 4.20.2-2.fc6 = /sbin/ldconfig fonts-apl libAplusGUI-4.20.2.so()(64bit) libIPC-4.20.2.so()(64bit) libMSGUI-4.20.2.so()(64bit) libMSIPC-4.20.2.so()(64bit) libMSTypes-4.20.2.so()(64bit) libX11.so.6()(64bit) liba-4.20.2.so()(64bit) libadap-4.20.2.so()(64bit) libcxb-4.20.2.so()(64bit) libcxc-4.20.2.so()(64bit) libcxs-4.20.2.so()(64bit) libcxsys-4.20.2.so()(64bit) libesf-4.20.2.so()(64bit) aplus-fsf-devel-4.20.2-2.fc6.x86_64.rpm aplus-fsf-devel = 4.20.2-2.fc6 = aplus-fsf = 4.20.2 libAplusGUI-4.20.2.so()(64bit) libIPC-4.20.2.so()(64bit) libMSGUI-4.20.2.so()(64bit) libMSIPC-4.20.2.so()(64bit) libMSTypes-4.20.2.so()(64bit) liba-4.20.2.so()(64bit) libadap-4.20.2.so()(64bit) libcxb-4.20.2.so()(64bit) libcxc-4.20.2.so()(64bit) libcxs-4.20.2.so()(64bit) libcxsys-4.20.2.so()(64bit) libesf-4.20.2.so()(64bit) aplus-fsf-el-4.20.2-2.fc6.x86_64.rpm aplus-fsf-el = 4.20.2-2.fc6 = aplus-fsf xemacs fonts-truetype-apl-4.20.2-2.fc6.x86_64.rpm fonts-apl fonts-truetype-apl = 4.20.2-2.fc6 = /bin/sh /usr/bin/mkfontdir chkfontpath fontconfig ttmkfdir fonts-x11-apl-4.20.2-2.fc6.x86_64.rpm fonts-apl fonts-x11-apl = 4.20.2-2.fc6 = /bin/sh /usr/bin/mkfontdir chkfontpath fontconfig * shared libraries are present; ldconfig is called properly. * package is not relocatable. * owns the directories it creates. * doesn't own any directories it shouldn't. * no duplicates in %files. * file permissions are appropriate. * %clean is present. * %check is not present; no test suite upstream (that I could find). ? many scriptlets present; I'm not sure about the dependencies. * code, not content. * documentation is small, so no -docs subpackage is necessary. * %docs are not necessary for the proper functioning of the package. * headers present, in -devel package. * no pkgconfig files. * no libtool .la droppings. * not a GUI app. Any comment on the review? This package is pretty close; just a few minor issues to fix. I will close this ticket if I don't hear back soon. /usr/share/X11/app-default own to the X11 System, so I can't get ownership to this directory. The problem with the xterm file is, that apl has its own font with special characters which will be maped to the keyboard. Next Release: Spec: http://www.herr-schmitt.de/pub/aplus-fsf/aplus-fsf.spec SRPM: http://www.herr-schmitt.de/pub/aplus-sfs/aplus-fsf-4.20.2-3.src.rpm If you don't own this directory and you depend on nothing that does, then you cannot under any circumstances put files there. (Yes, there are packages in core that violate this; that is a bug.) The packages in FC5 which provide this directory are: xorg-x11-apps xorg-x11-resutils xorg-x11-server-utils xorg-x11-utils xorg-x11-xdm xorg-x11-xsm xscreensaver-base You will either have to own the directory, require one of those packages or require the directory itself, which will have the effect of requiring xorg-x11-xdm unless one of the other packages is already installed. (Personally I think that xorg-x11-filesystem should own this directory, but that's another ticket.) In any case, this package still doesn't even start build, because you put %{?dist} at the end of your "rel" define and not at the end of release. How are you able to build this package to test it? During the mock run, I have read http://www.redhat.com/archives/fedora-packaging/2006-July/msg00126.html Now, I own /usr/share/X11/app-defaults in the current release. But I agree with you, that /usr/share/X11/app-defaults should go into xorg-x11-filesystem and all packages which put files into /usr/share/X11/app-defaults should depend on it. Next release: Spec: http://www.herr-schmitt.de/pub/aplus-fsf/aplus-fsf.spec SRPM: http://www.herr-schmitt.de/pub/aplus-sfs/aplus-fsf-4.20.2-4.src.rpm Caution: In acording to the naming guidelines I have rename the subpackage aplus-fsf-el to xemacs-aplus-fsf. You're right about renaming the xemacs subpackage; the naming guidelines have changed since this review started. Unfortunately my testing infrastructure has changed as well, and I'm now doing a proper rpmlint on the installed package, which turns up an additional category of problems. Firstly there's this warning about the srpm which I don't really have a problem with: W: aplus-fsf setup-not-quiet It wants you to use "-q" on the %setup line. Then there's three no-documentation warnings, which are OK: W: fonts-truetype-apl no-documentation W: fonts-x11-apl no-documentation W: xemacs-aplus-fsf no-documentation And finally there are nearly 3800 undefined-non-weak-symbol warnings, of the form: W: aplus-fsf undefined-non-weak-symbol /usr/lib64/libesf-4.20.2.so P1 W: aplus-fsf undefined-non-weak-symbol /usr/lib64/libesf-4.20.2.so APL I will attach a full list, or you can just run "rpmlint aplus-fsf" with the package already installed. It looks like the libraries themselves are merely compiled but not linked against each other; the loading program has to be set up to link in all of the necessary libraries in the proper order. It seems this not allowed in Fedora, but I'm not sure how reasonable it is to fix it. I'm asking around to find how strong the prohibition is. Created attachment 132489 [details]
List of undefined-non-weak-symbol warnings
I asume, that is a 64 related problem. Unfortunately, I don't have a 64 bit system. It will be nice if anyone have a hint to solve the problem reported by Jason. Well, I don't think there's anything specifically 64-bit related here. What does "rpmlint aplus-fsf" tell you with the package installed on an i386 machine? (rpmlint 0.77 running in my build setup.) Unfortunately, I can't reproduced the reported effect. I have make a local build on my maschine and done a rpmlint on the binary rpm using rpmlint-0.77. In Opposite of your results, I didn't get any complaints from rpmlint. Just to be completely, absolutely clear, what rpmlint command did you run? You say you ran it on the binary rpm, which seems to imply that you ran something like: rpmlint aplus-fsf-4.20.2-4.fc5.i386.rpm which is not what I'm talking about. I'm saying to install that package and its dependencies, and then to run rpmlint aplus-fsf which will check the installed package. This does a different set of checks. I have try to solve the problem by reordering the libraries in src/Makefile.am. Unfortunately, the rpmlint will no been gone after installation of the new build version. It will be nice, if anyone have a hint for me. (In reply to comment #10) > During the mock run, I have read > > http://www.redhat.com/archives/fedora-packaging/2006-July/msg00126.html > > Now, I own /usr/share/X11/app-defaults in the current release. That is a bug, your package should _not_ own a system directory that is part of the X Window System. > But I agree with you, that /usr/share/X11/app-defaults should go into > xorg-x11-filesystem and all packages which put files into > /usr/share/X11/app-defaults should depend on it. libXt is the canonical owner of the app-defaults directory at the bottom of the app-defaults-user food chain. app-defaults ^ | libXt ^ ^ libXaw libXm This was a bug in the libXt package, in that because the libXt package build does not create the directory or put files in it, it was not owned by the package. Since libXt really is the canonical owner of the directory however, the package _should_ be owning this dir, and that has been fixed in the latest rawhide libXt package. When you discover packaging problems of this nature, it is a good idea to post questions to the Fedora development mailing lists, or if you suspect a given package or subsystem is missing something, to report it in bugzilla, rather than to put ad-hoc hacks into other packages. The xorg-x11-filesystem package is nothing more than an unfortunately necessary ugly hack to work around a bug in rpm (or misfeature if one prefers...). Currently the xorg-x11-filesystem package serves no other purpose, and after FC6/RHEL5 we no longer need the ugly hack, so the package can be removed from the OS. I definitely do not want to _add_ more junk to the packaging than is necessary. Generally speaking, I agree with jkeating - that every directory in the OS /should/ have a canonical owner. For the X Window System, there is a canonical owner for every directory, however if a given directory that should be owned by some component of Xorg is not owning it, file a bug report against the proper component if you know which one is to blame, or file a bug against xorg-x11 and we'll figure it out. There may be other issues of this nature yet undiscovered in the modular X packaging. Anyhoo... please remove ownership of the app-defaults dir from any spec files now after upgrading libXt. Hope this helps. (In reply to comment #19) > That is a bug, your package should _not_ own a system directory that > is part of the X Window System. Here's a list of all packages in FC5 which own that directory: xscreensaver-base xorg-x11-resutils xorg-x11-server-utils xorg-x11-utils xorg-x11-xsm xorg-x11-apps xorg-x11-xdm Mike, if you think these are buggy, I'll happily file bugs against them. Honestly, I think it's reasonable to see that list and conclude that it's expected behavior for a package that adds an app-defaults file to own /usr/share/X11/app-defaults. Jochen, do you think it would be reasonable to back out of this mess by just adding a dependency on xterm? That's what the app-defaults file is for, isn't it? In FC6, the xterm dependency will pull in libXt which will own this directory; in FC5 the directory will still be unowned in the dependency tree but at least that's not a bug in this package. (In reply to comment #20) > (In reply to comment #19) > > That is a bug, your package should _not_ own a system directory that > > is part of the X Window System. > > Here's a list of all packages in FC5 which own that directory: > > xscreensaver-base > xorg-x11-resutils > xorg-x11-server-utils > xorg-x11-utils > xorg-x11-xsm > xorg-x11-apps > xorg-x11-xdm > > Mike, if you think these are buggy, I'll happily file bugs against them. I fixed all the xorg-x11 packages, but please file one against xscreensaver. I looked at the spec file and it's list is autogenerated, so I'd rather if the package owner resolved that one... > Honestly, I think it's reasonable to see that list and conclude that it's > expected behavior for a package that adds an app-defaults file to own > /usr/share/X11/app-defaults. Technically, it wasn't a problem previously. In fact, it was considered by many people to be a sensible thing to do, at least in certain specific circumstances. However other people believe it is very wrong and never appropriate, and so it was made into official Fedora policy which we must now adhere to religiously. ;o) For Fedora Core at least, any case which would be a special exception to this rule, generally should have the "filesystem" package own the given directory to avoid issues. For packages outside of Core, the problem still exists and nobody claims to have an adequate solution. I believe it is considered to be a minor issue that people are expected to just live with, if they uninstall a group of packages all at once and there is no package which can be deemed to be the canonical owner of a directory. Some examples might be: - povray data files - video games which provide data files which can be shared by multiple games packaged by different people, which there's no really good way to choose an owner for the dir (ie: DOOM .wad files), and the other games which use the data, do not have a real dependency on the game (DOOM), nor any particular set of data files. - any other non-FHS type of data file which does not have a standardized location in the filesystem hierarchy. But for things that do have a canonical owner which can easily be determined, such as libXt in this case, the current Fedora policy makes sense. > Jochen, do you think it would be reasonable to back out of this mess by just > adding a dependency on xterm? No. With the latest libXt package, there is no problem anymore. If your package contains an app-defaults file, then it must also have software which links to libXm, libXaw, or libXt directly, the first two of which are linked also to libXt. As such, all software either directly or indirectly already depends on libXt, and one way or another, rpm will automatically pick up the proper library dependencies without needing to specify them. This will ensure that libXt is guaranteed to be installed either prior to these packages, or as part of the same transaction which installs these packages, thus guaranteeing that the app-defaults dir has a legitimate and canonical owner (libXt). Of course this only applies to current rawhide, not FC5, however it will get fixed in FC5 when we release 7.1 for FC5 also. > That's what the app-defaults file is for, isn't it? They are X resources for Xt apps. > In FC6, the xterm dependency will pull in libXt which will own this > directory; in FC5 the directory will still be unowned in the dependency tree but > at least that's not a bug in this package. Correct. Will be fixed in future FC5 update, so IMHO at least, 3rd party packagers should not own the dir, and shouldn't worry about it now. Thanks again for bringing up these issues. (In reply to comment #21) > > Jochen, do you think it would be reasonable to back out of this mess by just > > adding a dependency on xterm? > > No. With the latest libXt package, there is no problem anymore. If your > package contains an app-defaults file, then it must also have software which > links to libXm, libXaw, or libXt directly, the first two of which are linked > also to libXt. This package does not have software which is linked to any of those libraries. It drops in an alternate app-default file that xterm can use to set up the proper APL key bindings, but it doesn't actually use the app-default file itself. This is why I suggested the dependency on xterm. Next release for review: Spec: http://www.herr-schmitt.de/pub/aplus-fsf/aplus-fsf.spec SRPM: http://www.herr-schmitt.de/pub/aplus-sfs/aplus-fsf-4.20.2-5.src.rpm Hey, you managed to fix those undefined-non-weak-symbol errors. I see only two things: W: aplus-fsf incoherent-version-in-changelog 4.20-2.5 4.20.2-5.fc6 I think you meant to use "4.20.2-5" and not "4.20-2.5". You should probably change ":" to "-" in your initial changelog entry as well. You have "BuildRequires: xterm" when you should have "Requires: xterm". The point is to make sure that xterm is there when the package is installed, because that's what the Xterm-apl app-defaults file is for. So there's really nothing substantive wrong at this point. I'll go ahead and approve; if you agree with the above fixes then go ahead and make them and check in. APPROVED, provided you make the above changes. The release has the following problems: - aplus.el contains /usr/local/bin/a+ instead of /usr/bin/a+ - on startup of a+ /usr/lib/idap.+: No such file or directory somethere a path /usr/lib must be changed to /usr/lib/a+ Further problems with font packages: - file /usr/share/X11/fonts/apl is not owned by any package file /usr/share/X11/fonts/apl/pcf is not owned by any package file /usr/share/X11/fonts/apl/TTF is not owned by any package - The fonts.dir and fonts.scale files should be shipped with the packages. - AFAIK, /usr/share/X11/fonts is not in the font path. The KAPL font is not seen by fontconfig, better put it in a subdirectory of /usr/share/fonts. Thank you for your report. I have tried to solve the reported issues. aplus-fsf-4.20-6 should be available on the mirrors in the next days. (In reply to comment #25) > The release has the following problems: > - aplus.el contains /usr/local/bin/a+ instead of /usr/bin/a+ This is not fixed. > - on startup of a+ > /usr/lib/idap.+: No such file or directory > somethere a path /usr/lib must be changed to /usr/lib/a+ Fixed. As to fonts: file /usr/share/fonts/apl is not owned by any package the x11 fonts should remain in /usr/share/X11/fonts, only the truetype fonts need to go to /usr/share/fonts. I propose the following: fonts-truetype-apl (is it really necessary to include this in the X11 font path?): /usr/share/fonts/apl /usr/share/fonts/apl/KAPL.TTF fonts-x11-apl: /usr/share/X11/fonts/apl /usr/share/X11/fonts/apl/fonts.alias /usr/share/X11/fonts/apl/fonts.dir /usr/share/X11/fonts/apl/Kaplcour-14.pcf.gz /usr/share/X11/fonts/apl/Kaplgallant-19.pcf.gz /usr/share/X11/fonts/apl/Kaplscreen-11.pcf.gz /usr/share/X11/fonts/apl/Kaplscreen-Bold-14.pcf.gz Please make sure that the correct files and also the directories are included in the files list. This package was approved and imported and built. This bugzilla ticket should be closed as NEXTRELEASE. Bugs in this package should be filed against the aplus-fsf componet in bugzilla directly. I have try to fix the reported problems for devel and FC-5. Becouse there are no any complaints now I will close this bug. |