Bug 312571 - lvm2: missing dependency on which
Summary: lvm2: missing dependency on which
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: lvm2
Version: rawhide
Hardware: All
OS: Linux
low
low
Target Milestone: ---
Assignee: Petr Rockai
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2007-09-29 20:50 UTC by Ville Skyttä
Modified: 2007-11-30 22:12 UTC (History)
4 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2007-11-16 12:25:44 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)

Description Ville Skyttä 2007-09-29 20:50:04 UTC
lvmdump uses "which", but the dependency on it is missing in the lvm2 package. 
Fix: "Requires: which"

Also, a comment in lvmdump line 8 says "# which, echo and test are internal in
bash at least" but actually "which" is not.

Comment 1 Milan Broz 2007-09-30 09:02:51 UTC
Yes... 
maybe "type -p" as internal bash command can be used in lvmdump instead
(to avoid another dependency).



Comment 2 Alasdair Kergon 2007-09-30 10:49:47 UTC
which?  Yes, that's quite broken, and mustn't have been tested.

  which $LVM >& /dev/null || die 2 "Fatal: could not find lvm binary '$LVM'"
  test -x `which $LVM` || die 2 "Fatal: lvm binary '$LVM' not executable"

The *only* 100% reliable way to check is to attempt to run the binary, and
that's all the code should be doing here.  Trying to pre-empt certain failures
just causes new problems.  (Simple exercise for the reader: create a situation
when the existing tests give you an error even though $LVM can be run by the
script just fine.)

# lvm_dump.sh
lvm_dump.sh: line 49: test: too many arguments
Fatal: lvm binary 'lvm' not executable
# lvm version 2>/dev/null
  LVM version:     2.02.28-cvs (2007-07-17)


Comment 3 Alasdair Kergon 2007-09-30 11:08:49 UTC
My test case behaves OK on the latest Fedora BTW - but that's due to
distribution-specific customisation.  But lvm_dump.sh is upstream and must not
assume that.

Comment 4 Petr Rockai 2007-10-02 15:50:50 UTC
Yes and no. For lvm, yes, running "$LVM" version is a much better test, 
however, since we support non-root invocations of lvm_dump.sh, there is a 
problem with dmsetup, since dmsetup version fails for non-root. What can be 
done is either running dmsetup help, or dropping support for (incomplete) 
non-root invocations altogether.

I have commited the lvm version + dmsetup help kind of fix for now, 
corrections welcome. Also, dropped the wrong "which is internal in bash" 
comment (which is internal in zsh, but not bash).

Comment 5 Petr Rockai 2007-11-16 12:25:44 UTC
This has been fixed in CVS some time ago, I don't have the fedora/devel 
packages handy so don't know if the fix has already propagated, but if it 
hasn't, it will certainly do so soon. I am closing the bug.

Comment 6 Alasdair Kergon 2007-11-16 12:41:53 UTC
Simply look in the WHATS_NEW file for the change.  If you can't find it easily
then it needed a better description:-)

Fix a bug in lvm_dump.sh checks for lvm/dmsetup binaries.

That's in the unreleased section - for 2.02.29 - so is not yet in Fedora, which
tracks the releases.


Note You need to log in before you can comment on or make changes to this bug.