Bug 1315935

+++ This bug was initially created as a clone of Bug #1315024 +++

+++ This bug was initially created as a clone of Bug #1312374 +++

Description of problem:
When upgrading from f23->f24, f23 glusterfs-libs postun scriptlet failed

  Cleanup     : glusterfs-libs-3.7.8-1.fc23.x86_64                                                                                                                                                                                 2985/3742 
/sbin/ldconfig: relative path `1' used to build cache
warning: %postun(glusterfs-libs-3.7.8-1.fc23.x86_64) scriptlet failed, exit status 1
Non-fatal POSTUN scriptlet failure in rpm package glusterfs-libs
Non-fatal POSTUN scriptlet failure in rpm package glusterfs-libs



Not sure how important this is, or is it fixed already, I see in package git 
this new releases have changes to calls for ldconfig in package scritptlets, but I don't know how this works.


If this is already fixed, or can be ignored, please close the bug, and sorry for the noise.

Version-Release number of selected component (if applicable):
libs-3.7.8-1.fc23.x86_64

How reproducible:


Steps to Reproduce:
1.
2.
3.

Actual results:


Expected results:


Additional info:

--- Additional comment from Niels de Vos on 2016-02-26 11:36:14 EST ---

This was fixed with upstream bug 1145989, change http://review.gluster.org/8835 and needs to be included in the glusterfs.spec for Fedora.

--- Additional comment from Niels de Vos on 2016-02-26 11:51:24 EST ---

Change pushed to the master branch (for Rawhide) and f24.

--- Additional comment from Fedora Update System on 2016-02-28 08:28:21 EST ---

glusterfs-3.7.8-2.fc23 has been submitted as an update to Fedora 23. https://bodhi.fedoraproject.org/updates/FEDORA-2016-f974e3a4d8

--- Additional comment from Fedora Update System on 2016-02-29 06:24:48 EST ---

glusterfs-3.7.8-2.fc23 has been pushed to the Fedora 23 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2016-f974e3a4d8

--- Additional comment from Fedora Update System on 2016-03-05 01:21:23 EST ---

glusterfs-3.7.8-2.fc23 has been pushed to the Fedora 23 stable repository. If problems still persist, please make note of it in this bug report.

--- Additional comment from Ville Skyttä on 2016-03-05 12:51:02 EST ---

While this does fix the immediate problem, it introduces another: now all packages are missing the necessary scriptlet dependencies on ldconfig.

The root cause of the issue is the attempt to use comment blocks with lines starting with #'s, and specifically the last one between %postun libs and %files caused the failure. # is not really a comment character in specfile syntax.

Removing all of them, or at least the following attempted comment between %postun libs and %files would have been the better fix. The other alternative is to sprinkle a bunch of manual "Requires(something): /sbin/ldconfig"s where necessary.

##-----------------------------------------------------------------------------
## All %%files should be placed here and keep them sorted by groups
##

--- Additional comment from Kaleb KEITHLEY on 2016-03-05 14:47:45 EST ---


http://www.rpm.org/max-rpm/ch-rpm-inside.html

and

https://docs.fedoraproject.org/en-US/Fedora_Draft_Documentation/0.1/html/RPM_Guide/ch-specfile-syntax.html

would seem to suggest that # is a comment character in specfile syntax.

Why do you say it's not?

--- Additional comment from Vijay Bellur on 2016-03-05 16:28:43 EST ---

REVIEW: http://review.gluster.org/13613 (packaging: postun scriptlet fail /sbin/ldconfig: relative path `1') posted (#1) for review on master by Kaleb KEITHLEY (kkeithle)

--- Additional comment from Ville Skyttä on 2016-03-06 03:46:07 EST ---

(In reply to Vijay Bellur from comment #1)
> REVIEW: http://review.gluster.org/13613

Related to this, the end of a scriptlet body is the start of another, or another section such as %files (ignoring empty lines).

In the Fedora docs, "exit 0" is not a pattern for ensuring where a script ends, it is there for ensuring zero exit status from the script, more about that in https://fedoraproject.org/wiki/Packaging:Scriptlets

Preventing non-zero exit status pretty much at all costs was necessary some 10 years ago, and I'm fairly confident that it is no longer so if an actual error for which it is good to notify users occurs. No use in letting harmless non-zero exit statuses through though. The "Non-zero exit codes from scriptlets break..." chapter in the above packaging doc is outdated, I'll file a bug for removing it.

--- Additional comment from Ville Skyttä on 2016-03-06 04:10:13 EST ---

(In reply to Ville Skyttä from comment #2)
> I'll file a bug for removing it.

Looks like %pre is still something to pay special attention to, but otherwise the info seems outdated indeed. https://fedorahosted.org/fpc/ticket/606

--- Additional comment from Kaleb KEITHLEY on 2016-03-06 11:09:48 EST ---

(In reply to Ville Skyttä from comment #2)
> (In reply to Vijay Bellur from comment #1)
> > REVIEW: http://review.gluster.org/13613
> 
> Related to this, the end of a scriptlet body is the start of another, or
> another section such as %files (ignoring empty lines).
> 
> In the Fedora docs, "exit 0" is not a pattern for ensuring where a script
> ends, it is there for ensuring zero exit status from the script, more about
> that in https://fedoraproject.org/wiki/Packaging:Scriptlets

And nothing I wrote says that I expect it to determine where the scriptlet ends. I.e. where rpm/rpmbuild stops copying the scriptlet.

It is, exactly as I wrote, simply a way to ensure that the script exits reliably, where it is expected to exit; regardless of how much of the scriptlet was copied.

--- Additional comment from Vijay Bellur on 2016-03-06 13:35:17 EST ---

REVIEW: http://review.gluster.org/13613 (packaging: postun scriptlet fail /sbin/ldconfig: relative path `1') posted (#2) for review on master by Kaleb KEITHLEY (kkeithle)

--- Additional comment from Vijay Bellur on 2016-03-07 16:11:26 EST ---

REVIEW: http://review.gluster.org/13613 (packaging: rpm scriptlet cleanup, handle -p /sbin/ldconfig) posted (#3) for review on master by Kaleb KEITHLEY (kkeithle)

--- Additional comment from Vijay Bellur on 2016-03-07 18:22:55 EST ---

REVIEW: http://review.gluster.org/13613 (packaging: rpm scriptlet cleanup, handle -p /sbin/ldconfig) posted (#4) for review on master by Kaleb KEITHLEY (kkeithle)

--- Additional comment from Vijay Bellur on 2016-03-08 00:43:25 EST ---

REVIEW: http://review.gluster.org/13613 (packaging: rpm scriptlet cleanup, handle -p /sbin/ldconfig) posted (#5) for review on master by Kaleb KEITHLEY (kkeithle)

--- Additional comment from Vijay Bellur on 2016-03-08 22:32:16 EST ---

COMMIT: http://review.gluster.org/13613 committed in master by Kaleb KEITHLEY (kkeithle) 
------
commit 5cb80ac63b17a3c34d4deef1816e1782757687d9
Author: Kaleb S KEITHLEY <kkeithle>
Date:   Sat Mar 5 16:06:18 2016 -0500

    packaging: rpm scriptlet cleanup, handle -p /sbin/ldconfig
    
    The RPM documention indicates that during an rpm install or erase, the
    script(lets): %post, %preun, and %postun (and %pre, %build, %install,
    etc.) are copied to a temp file, and then the temp file is run as a
    (/bin/sh or bash) script.
    
    Unfortunately the documentation is not clear about how rpmbuild and/or
    rpm determine where the end of any scriptlet is when it is copied to
    the file.
    
    Most things in the glusterfs.spec work correctly as is. These are the
    %preun, %post, and %postun scriptlets that are "closed" by a following
    %preun, %post, and %postun, or poetentially another scriptlet, e.g.
    %file.
    
    The ones that don't work correctly (only one actually) are those where
    there is a comment in the spec file before it is closed by another
    scriptlet. Further complicating things is that the type of scriptlet
    affects what rpm does and what `rpm -qp --scripts ...` shows.
    
    The specific one that didn't work was the "%postun libs" scriptlet. It
    is followed by a comment before being "closed" by the %files section (or
    scriptlet). It can be written two ways: "%postun libs\n/sbin/ldconfig"
    or "%postun libs -p /sbin/ldconfig" Either way it's written, `rpm -qp
    --scripts glusterfs-libs...` will include the comment lines between the
    %postun libs line and the following %files line.
    
    But the way rpm executes these depends on how they're written. If
    written as "%postun libs\n/sbin/ldconfig" rpm will simply run
    /sbin/ldconfig with no command line options, i.e.
      execve ("/sbin/ldconfig", [ "/sbin/ldconfig" ], [ ]);
    
    But when written as "%postun libs -p /sbin/ldconfig", it will copy
    the comment lines to a temp file, and pass the temp file name and "1"
     as (command line) parameters, i.e.
      execve ("/sbin/ldconfig", [ "/sbin/ldconfig", "/tmp/tmpXXXXXX", "1" ],
              [ ]);
    
    Which results in ldconfig exiting with an error. (Remember, both ways show
    the comment in `rpm -qp --scripts ...`)
    
    (Note though, that the similar "%postun api -p /sbin/ldconfig" is run
    correctly, because it is "closed" by the following "%postun server"
    scriptlet.)
    
    Finally, through trial and error, it appears that rpm can be tricked with
    a hack, and "closure" of the scriptlet forcedlike this:
    
     %postun libs -p /sbin/ldconfig
    
     %if ( 0%{?_undocumented_hack_closes_scriptlets} )
     %postun
     %endif
    
    in which case ldconfig appears to run correctly. Note also that here too
    the comment will be included in the output of `rpm -qp --scripts ...`
    But that's very ugly hack.
    
    Change-Id: I587a490ddcdf47d01605479bc8ef8b0e439108fb
    BUG: 1315024
    Signed-off-by: Kaleb S KEITHLEY <kkeithle>
    Reviewed-on: http://review.gluster.org/13613
    Smoke: Gluster Build System <jenkins.com>
    Reviewed-by: Niels de Vos <ndevos>
    NetBSD-regression: NetBSD Build System <jenkins.org>
    CentOS-regression: Gluster Build System <jenkins.com>
    Reviewed-by: Raghavendra Talur <rtalur>