Bug 518766
Summary: | Review Request: auto-destdir - Automate DESTDIR support for "make install" | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | David A. Wheeler <dwheeler> |
Component: | Package Review | Assignee: | Rahul Sundaram <sundaram> |
Status: | CLOSED ERRATA | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
Severity: | medium | Docs Contact: | |
Priority: | medium | ||
Version: | rawhide | CC: | fedora-package-review, notting, smohan, sundaram, susi.lehtola |
Target Milestone: | --- | Flags: | sundaram:
fedora-review+
kevin: fedora-cvs+ |
Target Release: | --- | ||
Hardware: | All | ||
OS: | Linux | ||
Whiteboard: | |||
Fixed In Version: | 1.11-1.fc11 | Doc Type: | Bug Fix |
Doc Text: | Story Points: | --- | |
Clone Of: | Environment: | ||
Last Closed: | 2009-09-04 04:01:45 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
David A. Wheeler
2009-08-22 15:46:32 UTC
If the program uses autotools, use %configure instead of ./configure %configure also sets a lot of other stuff, such as the optimization flags. (In reply to comment #0) > This warning is irrelevant; this only installs bash scripts and documentation, > so there's no libdir to use. Correct, but it uses bindir and datadir, two configuration options you don't pass to configure (MUSTFIX). (In reply to comment #1) > %configure also sets a lot of other stuff, such as the optimization flags. Very bad explanation, optimization flags are irrelevant on noarch-packages (such as this). What is not irrelevant are installation directories (c.f. comment above) Finally, I am having doubts that this package works and will ever work. It doesn't use autotools, and optimization rules are irrelevant since this is implemented entirely in bash. It *does* use bindir and datadir. I can set those, and if %configure is the preferred way to set them, that's fine. As far as the doubts that this works, it "works for me", with a variety of programs. Let's let users decide if it works for them :-). Okay, here's a new package release that I believe addresses the issues above. Basically, I changed ./configure to %configure. The %configure macro includes settings for bindir and datadir, addressing the MUSTFIX. This eliminates all rpmlint complaints, too, because the macro sets libdir. Updated package here: Spec URL: http://www.dwheeler.com/auto-destdir/auto-destdir.spec SRPM URL: http://www.dwheeler.com/auto-destdir/auto-destdir-1.4-2.fc10.src.rpm Anything else? (In reply to comment #4) > Anything else? Yes, * MUSTFIX: remove this from your spec: ... # This only has simple scripts, so there's nothing to put in a debug package: %define _enable_debug_package 0 %define debug_package %{nil} %define __os_install_post /usr/lib/rpm/brp-compress %{nil} ... This doesn't make any sense. (In reply to comment #3) > As far as the doubts that this works, it "works for me", with a variety of > programs. Let's let users decide if it works for them :-). OK, then elaborate how this package works. * "MUSTFIX: remove this from your spec:" Okay, removed. Thanks for noticing. I made a few more tweaks while doing it. * "OK, then elaborate how this package works." For that, see its "man" page, esp. the "implementation approach" section. This is available as PDF here: http://www.dwheeler.com/auto-destdir/make-redir.pdf A longer paper, discussing alternative approaches, is here: http://www.dwheeler.com/essays/automating-destdir.html Here's the new version, which addresses all previously-noted MUSTFIX-es: Spec URL: http://www.dwheeler.com/auto-destdir/auto-destdir.spec SRPM URL: http://www.dwheeler.com/auto-destdir/auto-destdir-1.5-1.fc10.src.rpm FYI, I think the "_enable_debug_package 0" stuff was to make Fedora 9 or 8 happy; I wrote the original .spec file a long time ago. Obviously that's irrelevant today. In any case, it's gone now. Here's the new version, which addresses all previously-noted MUSTFIX-es *AND* improves the test suite. rpmlint gives no warnings, no errors: Spec URL: http://www.dwheeler.com/auto-destdir/auto-destdir.spec SRPM URL: http://www.dwheeler.com/auto-destdir/auto-destdir-1.6-1.fc11.src.rpm Let me know of anything else that needs doing; I don't know of anything. A few minor things: Remove the # commented out lines. The description deosn't seem aligned and seems too long. Unless you are branching out for EPEL, you don't need to define the buildroot or remove it in the %install section anymore. IIUC, the mini scripts that are in /usr/share are really helper apps that should be in /usr/libexec instead. (In reply to comment #6) > * "OK, then elaborate how this package works." > For that, see its "man" page, esp. the "implementation approach" section. > This is available as PDF here: > http://www.dwheeler.com/auto-destdir/make-redir.pdf Well, if you think this is a nice approach, I can't avoid to disagree. I will not approve this package and recommend other reviewers to do the same. Thanks for commenting! * "The description deosn't seem aligned and seems too long". I can shorten it, sure! But I don't understand the 'aligned' comment. Can you explain what you mean? * "Remove the # commented out lines... Unless you are branching out for EPEL, you don't need to define the buildroot or remove it in the %install section anymore." Actually, I *am* hoping to have the same .spec file for other RPM-based systems, including RHEL/EPEL. These lines are critically necessary for many systems, and they cause no harm to Fedora. So, I'd rather leave them in. Please tell me if removing them is really critical. * "IIUC, the mini scripts that are in /usr/share are really helper apps that should be in /usr/libexec instead." If they were binaries, I'd 100% agree with you. But these files do NOT depend on the specific architecture being used. Unfortunately, /usr/libexec isn't in FHS, so it's hard to find really good rules to make a clear determination. The FHS DOES say /usr/lib is for architecture-dependent data (e.g., ELF files like .so files), while /usr/share is for architecture-independent data. I think /usr/libexec is intended to be like /usr/lib, namely, it stores architecture-specific files, as suggested by the name similarity. Following that line of thought, private executables that are architecture-independent would go into /usr/share instead. Obviously, if you take the position that "all private executables go into /usr/libexec, architecture-neutral or not", then /usr/libexec would be the answer. The GNU coding standards talk about libexecdir, but don't make it entirely clear (to me) if private scripts would go here too. Anyone know of a semi-official Fedora position (either way) on this? If they should be moved, that would be trivial to do, by just changing %configure to: %configure --scriptdir=%{_libexecdir}/%{name} On alignment, there is a lot of white space in between lines and makes it harder to read the text and text does get displayed in many places including PackageKit and therefore writing a shorter description makes sense. It is mostly cosmetic however. Removing the buildroot stuff is not critical at all. If you want, you can keep them and branch for EPEL as well https://fedoraproject.org/wiki/EPEL The guidelines for libexec are at https://fedoraproject.org/wiki/Packaging/Guidelines#Libexecdir The majority of packages in /usr/libexec are binaries but I found a few shell scripts as well. rpm -qf /usr/libexec/xscreensaver-autostart xscreensaver-base-5.08-12.fc12.i686 This is recommended but I won't insist on it. Btw, Fedora did try to get it into FHS but it appears there are no owners to keep it updated anymore. APPROVED Okay, I per comment #12, I moved the scripts to libexecdir and shortened the description. (You didn't REQUIRE this, but they made sense). I left the buildroot stuff, as discussed above. rpmlint still reports 0 errors, 0 warnings. The new versions, with the optional changes suggested, are here: Spec URL: http://www.dwheeler.com/auto-destdir/auto-destdir.spec SRPM URL: http://www.dwheeler.com/auto-destdir/auto-destdir-1.7-1.fc11.src.rpm Thanks for the approval! (see comment #12) New Package CVS Request ======================= Package Name: auto-destdir Short Description: Automate DESTDIR support for "make install" Owners: dwheeler Branches: F-10 F-11 InitialCC: CVS done. auto-destdir-1.10-1.fc11 has been submitted as an update for Fedora 11. http://admin.fedoraproject.org/updates/auto-destdir-1.10-1.fc11 auto-destdir-1.10-1.fc10 has been submitted as an update for Fedora 10. http://admin.fedoraproject.org/updates/auto-destdir-1.10-1.fc10 Package Change Request ====================== Package Name: auto-destdir New Branches: EL-4 EL-5 Owners: dwheeler cvs done. auto-destdir-1.10-1.fc10 has been pushed to the Fedora 10 stable repository. If problems still persist, please make note of it in this bug report. auto-destdir-1.10-1.fc11 has been pushed to the Fedora 11 stable repository. If problems still persist, please make note of it in this bug report. auto-destdir-1.11-1.fc11 has been submitted as an update for Fedora 11. http://admin.fedoraproject.org/updates/auto-destdir-1.11-1.fc11 auto-destdir-1.11-1.fc11 has been pushed to the Fedora 11 stable repository. If problems still persist, please make note of it in this bug report. |