On Sun, Nov 01, 2009 at 07:06:46PM +0300, Dmitry V. Levin wrote:
> On Sun, Nov 01, 2009 at 06:15:50AM +0300, Alexey Tourbin wrote:
> > On Sun, Nov 01, 2009 at 05:14:42AM +0300, Dmitry V. Levin wrote:
> [...]
> > > I've just pushed sisyphus_check 0.8.14-alt1-4-gc78c3ee that implements
> > > this check.  Please have a look.
> > 
> > -rpm_requires=[%{requirename:shescape}" %{requireflags:depflags} "%{requireversion:shescape}"
> > +rpm_requires=[%{requirename:shescape}" %{requireflags} %{requireflags:depflags} "%{requireversion:shescape}"
> > 
> > %{requireflags:octal} may (or may not) be more convenient here,
> > possibly depending on whether awk can handle octals just as easy
> > as decimals.
> 
> I doubt that octals here may appear more convenient.  GNU awk definitely
> handles decimal numbers, and nothing else is going to handle this field
> yet.

Octals group bits (per three), and decimals do not.  Whenever you're in
a dire circumstances, octals might pay off a little bit.  So watch.
Smartass between dumbass is in the eye of the beholder. :)

> > +bad_prereqs='/sbin/install-info
> > +/sbin/ldconfig'
> > 
> > +       # 2^6: RPMSENSE_PREREQ
> > +       # 2^14: RPMSENSE_FIND_REQUIRES
> > +       bad=$(printf %s "$rpm_requires" |
> > +               awk '$2>=64 && $2!=16384 {print $1}' |
> > +               egrep -x "$bad_prereqs" |
> > +               sort -u)
> > 
> > Since %{requireflags} is a bitmap, numeric comparison looks puzzling.
> 
> It is result of optimization, so it may look puzzling and even a bit
> confusing.  The idea behind this optimization is simple: any high value flag
> except RPMSENSE_FIND_REQUIRES means that the requirement has a PREREQ sense.

Please do not optimise here.  It's a bitmap.  You need test a bit you
test a bit.  Got that dude?

> > What are exactly the rules we'd like to follow?  Basically, what we want
> > to do is to forbid the use of /sbin/install-info in %post-like scripts.
> > For such a script, the dependency on /sbin/install-info is generated
> > automatically.  But usually it is also accompanied with manual PreReq
> > dependency on %__install_info.  We should cover both cases -- for
> > example, one can forget to remove the accompanying PreReq dependency
> > while removing the scripts.
> > 
> > Now to the requireflags.  Older rpmbuild implementation (the one we use)
> > always sets PREREQ bit along with e.g. SCRIPT_POST (please check if this
> > is actually true).  Thus, to handle both cases (automatic scriptlet
> > dependencies and manual prereqs) it would suffice to perform the test
> > as simple as
> >     
> > 	bad = (requireflags & PREREQ);
> > 
> > Newer rpmbuild implementation does not set PREREQ bit along with
> > scriptlet bits, so the test should be extended to the scriptlet bits:
> > 
> > 	bad = (requireflags & (PREREQ | SCRIPT_POST | SCRIPT_...));
> 
> Prerequisites are not limited to RPMSENSE_PREREQ and RPMSENSE_SCRIPT_*
> bits.  There are also RPMSENSE_INTERP and RPMSENSE_TRIGGER* which should
> be treated the same way as other PREREQs.  As result, an appropriate awk
> expression grows like this:
> awk 'and($2,2^6+2^8+2^9+2^10+2^11+2^12+2^13+2^16+2^17+2^18)>0 {print $1}'
> 
> Of course it could be pre-calculated to
> awk 'and($2,474944)>0 {print $1}'
> 
> Does it look clearer than
> awk '$2>=64 && $2!=16384 {print $1}'
> ?
> 
> > Another idea looming around is to prohibit manual dependencies
> > on /sbin/install-info altogether.  I'm not quite sure if it is a good
> > idea, since it goes far beyond scriptlets and prereqs.  After all,
> > you name it "bad_prereqs", so it must be a prereq.
> 
> I was uncertain about this, so I made a check for /sbin/install-info and
> /sbin/ldconfig requirements in Sisyphus.  I found only one manual
> non-PREREQ requirement for /sbin/ldconfig (in
> libreadline-java-0.8.0-alt2_12jpp5) and no manual non-PREREQ requirements
> for /sbin/install-info.  So I decided to leave the check as is.
> Maybe I should reconsider this.
> 
> > Also consider adding 'info-install' to bad_prereqs (since they might
> > use bare 'install-info' command, not /sbin/install-info).
> 
> You mean the info-install package?  Yes, that makes sense.