Bug 1476839 - Fedora Packaging Guideline cleanups for glibc.spec
Fedora Packaging Guideline cleanups for glibc.spec
Status: CLOSED RAWHIDE
Product: Fedora
Classification: Fedora
Component: glibc (Show other bugs)
27
Unspecified Unspecified
unspecified Severity unspecified
: ---
: ---
Assigned To: Carlos O'Donell
Fedora Extras Quality Assurance
: Reopened
: 1476840 (view as bug list)
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2017-07-31 11:02 EDT by Tomasz Kłoczko
Modified: 2017-08-17 14:32 EDT (History)
8 users (show)

See Also:
Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2017-08-17 14:20:57 EDT
Type: Bug
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)
0001-release-31.patch (3.35 KB, text/plain)
2017-07-31 11:06 EDT, Tomasz Kłoczko
no flags Details
0002-remove-Group-and-defatr-o~cial-Fedora-Packaging-Po.patch (9.00 KB, application/mbox)
2017-07-31 11:06 EDT, Tomasz Kłoczko
no flags Details
0003-remove-in-files-redundant~nfig-missingok-noreplace.patch (2.66 KB, application/mbox)
2017-07-31 11:07 EDT, Tomasz Kłoczko
no flags Details
0004-added-execute-sbin-ldcofi~n-transfiletriggerin-and.patch (3.08 KB, application/mbox)
2017-07-31 11:07 EDT, Tomasz Kłoczko
no flags Details

  None (edit)
Description Tomasz Kłoczko 2017-07-31 11:02:14 EDT
Summary changes (copy from %changelog):

* Mon Jul 31 2017 Tomasz Kłoczko <kloczek@fedoraproject.org> - 2.25.90-31
- added execute /sbin/ldcofig in %%transfiletriggerin and
  %%transfiletriggerpostun and removed %%post/%%postun with /sbin/ldconfig
  executions as those scripttlets are no longer needed
  %%verify(not md5 size mtime) if %%ghost is used
- remove in %%files redundant %%config(missingok,noreplace) and
  %%verify(not md5 size mtime) if %%ghost is used
- remove Group, %%defatr and %%clean (official Fedora Packaging Policy)
- remove redundant %%ifing

Additional comments:
* With introduction %%transfiletriggerin and %%transfiletriggerpostun will be possible dramatically reduce number of scrip lets executed during multiple packages installation or upgrade. Every package which installs/upgrades/removes some libraries are executing /sbin/ldconfig in %post and %postun. With %triggers all those execution can be removed and post install/upgrade/remove %trigger package will be executed at the end of the whole sequence of packages changes.
This will:
 - reduce overall time of packages installation/upgrades/removes
 - reduce number of dependencies from /sbin/ldconfig

If this patch will be commited it can be added massive change in all packages (on trunk only) to start remove execution /sbin/ldconfig in %post/postun.
In mean time this will no generate any conflicts with any packages which will be executing ldconfig in those scriptlets.

* None of the spec file subpackages headers needs to be surrounded by %f .. %endif if those packages are conditional. Only exact packages %files sections needs to be withing %if .. %endif

* %ghost disables checking content of the file because such content and digest is not included in the binary package. By this weakening file verification policy by add %%verify(not md5 size mtime) is completely redundant because %ghost disables this by default. Fragments from rpm source code from lib/verify.c:

    if (fileAttrs & RPMFILE_GHOST)
        flags &= ~(RPMVERIFY_FILEDIGEST | RPMVERIFY_FILESIZE |
                   RPMVERIFY_MTIME | RPMVERIFY_LINKTO);

The same is %%config(missingok,noreplace).

Attached patches are generated from my cloned git repo.
Comment 1 Tomasz Kłoczko 2017-07-31 11:06 EDT
Created attachment 1307159 [details]
0001-release-31.patch
Comment 2 Tomasz Kłoczko 2017-07-31 11:06 EDT
Created attachment 1307160 [details]
0002-remove-Group-and-defatr-o~cial-Fedora-Packaging-Po.patch
Comment 3 Tomasz Kłoczko 2017-07-31 11:07 EDT
Created attachment 1307170 [details]
0003-remove-in-files-redundant~nfig-missingok-noreplace.patch
Comment 4 Tomasz Kłoczko 2017-07-31 11:07 EDT
Created attachment 1307175 [details]
0004-added-execute-sbin-ldcofi~n-transfiletriggerin-and.patch
Comment 5 Tomasz Kłoczko 2017-07-31 11:08:53 EDT
*** Bug 1476840 has been marked as a duplicate of this bug. ***
Comment 6 Carlos O'Donell 2017-07-31 15:26:45 EDT
This is not as simple as making the change.

Please see:

https://bugzilla.redhat.com/show_bug.cgi?id=1380878

and:

https://pagure.io/packaging-committee/issue/654

*** This bug has been marked as a duplicate of bug 1380878 ***
Comment 7 Tomasz Kłoczko 2017-07-31 16:15:10 EDT
Please have look on other changes because I've intentionally provided all this ticket changes in separated patches.
Change about use triggers is only one of those changes.
Comment 8 Carlos O'Donell 2017-07-31 19:09:19 EDT
(In reply to Tomasz Kłoczko from comment #7)
> Please have look on other changes because I've intentionally provided all
> this ticket changes in separated patches.
> Change about use triggers is only one of those changes.

Your other changes have nothing to do with this bug description. Please do not file multiple bugs in a single bug.

You have two options:

(1) Post your patches to Fedora glibc list to discuss changes and get your patches accepted:

https://lists.fedoraproject.org/admin/lists/glibc.lists.fedoraproject.org/

(2) File another bug about glibc.spec file cleanup.

I do appreciate the cleanup help, particularly the Fedora Packaing guideline does say not to use Group, so the removal is particularly relevant.
Comment 9 Tomasz Kłoczko 2017-08-01 03:42:07 EDT
So moment .. you are not glibc package maintainer?
Comment 10 Carlos O'Donell 2017-08-01 09:35:54 EDT
(In reply to Tomasz Kłoczko from comment #9)
> So moment .. you are not glibc package maintainer?

I am the glibc package maintainer.
Comment 11 Carlos O'Donell 2017-08-01 09:36:51 EDT
(In reply to Carlos O'Donell from comment #10)
> (In reply to Tomasz Kłoczko from comment #9)
> > So moment .. you are not glibc package maintainer?
> 
> I am the glibc package maintainer.

Sorry, to put it more clearly: The Fedora glibc is maintained by a *team* of developers. I am the steward for that team.
Comment 12 Tomasz Kłoczko 2017-08-01 15:55:16 EDT
So could you please be so kind and forward to the group my ticket?
Comment 13 Carlos O'Donell 2017-08-02 09:29:06 EDT
(In reply to Tomasz Kłoczko from comment #12)
> So could you please be so kind and forward to the group my ticket?

Certainly. Posted with review.

https://lists.fedoraproject.org/archives/list/glibc@lists.fedoraproject.org/thread/JLLFOFYHBMY7ESPG4J2SVOWU4NA63UQT/
Comment 14 Tomasz Kłoczko 2017-08-02 11:01:52 EDT
(In reply to Carlos O'Donell from comment #13)
> (In reply to Tomasz Kłoczko from comment #12)
> > So could you please be so kind and forward to the group my ticket?
> 
> Certainly. Posted with review.
> 
> https://lists.fedoraproject.org/archives/list/glibc@lists.fedoraproject.org/
> thread/JLLFOFYHBMY7ESPG4J2SVOWU4NA63UQT/

From you email:

"0003-remove-in-files-redundant~nfig-missingok-noreplace.patch 
- Remove specific %config and %verify in the presence of %ghost.

I'm rejecting this patch because it relies on what appears to be implementation
dependent behaviour of %ghost, specifically that it ignores certain changes.
While it seems logical that %ghost should ignore certain changes I would rather
leave the belt-and-suspenders check with %verify, and %config."

Seems you do not understand how %ghost is working. If %ghost is specified in generated binary package description there is no file mtime, md5sum. Specify %verify is pointless/redundant because those rules are part of the %ghost.
In other words you are rejecting how %ghost is working and applies how you are thinking that this token is working.

From http://ftp.rpm.org/max-rpm/s1-rpm-inside-files-list-directives.html
"The way to achieve this, is to use the %ghost directive. By adding this directive to the line containing a file, RPM will know about the ghosted file, but <b>will not add it to the package</b>. However it still needs to be in the buildroot. Here's an example of %ghost in action."

Why do you want disable verification of the files which are physically not included in the package!!!!????
%ghost file as well cannot be replaced by new file because:
a) new file will never will be installed by package because it is %ghost file
b) even if file could be installed it is not possible to find out is file with new content because there is not in package description size, mtime and md5sum.

If you do not understand how %ghost is working just try to make short experiment with test package with only one file marked those tokens.
Comment 15 Tomasz Kłoczko 2017-08-02 11:04:33 EDT
"I am rejecting patch #1 outright, the %if'ing I find a matter of taste.
The NEVRA change always happens anyway."

Do you know what it is Ockham razor or KISS principle?
https://en.wikipedia.org/wiki/Occam%27s_razor
Comment 16 Carlos O'Donell 2017-08-03 11:34:44 EDT
(In reply to Tomasz Kłoczko from comment #14)
> If you do not understand how %ghost is working just try to make short
> experiment with test package with only one file marked those tokens.

(In reply to Tomasz Kłoczko from comment #15)
> Do you know what it is Ockham razor or KISS principle?
> https://en.wikipedia.org/wiki/Occam%27s_razor

Please continue this discussion on the Fedora glibc mailing list.

I plan to close this bug once I have integrated the accepted suggetions.
Comment 17 Tomasz Kłoczko 2017-08-03 12:06:15 EDT
Sorry if you have no time to work on the package please do not ask me to waste my time on convincing bunch of people who don't know enough about rpm packaging (those people already approved change which completely does not make any sense!!!).
Do what you want but I'm more and more convinced already that trying contribute to the Fedora is pointless.
Comment 18 Carlos O'Donell 2017-08-03 12:52:35 EDT
(In reply to Tomasz Kłoczko from comment #17)
> Sorry if you have no time to work on the package please do not ask me to
> waste my time on convincing bunch of people who don't know enough about rpm
> packaging (those people already approved change which completely does not
> make any sense!!!).
> Do what you want but I'm more and more convinced already that trying
> contribute to the Fedora is pointless.

I'm sorry you feel that way. My suggestion to send your comments to the mailing list is so that we can continue the discussion more naturally via email, rather than back and forth on this particular bug. Your contributions are not pointless, several of your suggestions are good, and I'll include them in a glibc.spec update. Thank you for that.
Comment 19 Tomasz Kłoczko 2017-08-04 03:48:57 EDT
Look .. in discussion people are using arguments. You've already refused one of the patches because "I find a matter of taste".
How can we continue discussion if you are refusing arguments and sending me to /dev/tree? (sign to mailing list on which people approved completely pointless patch)
glibc.spec is one of the scariest spec I've ever seen. It uses 100% generating %files lists in %install making content of the packages completely not unreadable. Amount of the code in %install is few times larger than what is possible to have in case straight listing what needs to be in each subpackages.

Again: if you are going to discourage anyone who will be sending you patches you are doing very good job.
I'm not going to do your job convincing group of people on the glibc mailing list if it is part of the process,
Comment 20 Jan Kurik 2017-08-15 03:07:51 EDT
This bug appears to have been reported against 'rawhide' during the Fedora 27 development cycle.
Changing version to '27'.
Comment 21 Carlos O'Donell 2017-08-17 14:20:57 EDT
This is now fixed in rawhide.
Comment 22 Carlos O'Donell 2017-08-17 14:21:30 EDT
commit c6e992763d9bc1bd5a46499c08bb8342ef3a25b6
Author: Tomasz Kłoczko <kloczek@fedoraproject.org>
Date:   Thu Aug 17 14:14:18 2017 -0400

    Resolves: #1476939
    
    - Remove 'Buildroot' tag, 'Group' tag, and '%clean' section, and don't
      remove the buildroot in %install, all per Fedora Packaging Guidelines
      (#1476839)
Comment 23 Carlos O'Donell 2017-08-17 14:32:44 EDT
(In reply to Tomasz Kłoczko from comment #19)
> Look .. in discussion people are using arguments. You've already refused one
> of the patches because "I find a matter of taste".

That's just one part of the patch.

> How can we continue discussion if you are refusing arguments and sending me
> to /dev/tree? (sign to mailing list on which people approved completely
> pointless patch)

I don't follow. I'm not refusing any arguments. I'm refusing to apply certain patches that don't match my own tastes, particularly when the patch isn't technical in nature but just a cleanup. Similarly I would refuse patches that remove large block quote comments in the spec file because I like such comments.

> glibc.spec is one of the scariest spec I've ever seen. It uses 100%
> generating %files lists in %install making content of the packages
> completely not unreadable. Amount of the code in %install is few times
> larger than what is possible to have in case straight listing what needs to
> be in each subpackages.

Yes, we could probably switch to a static list of files, but the list of headers changes when the kernel changes, and some files vary by architecture, so in the end it is simpler from a maintenance perspective to have a fully 100% automatically generated %files list.

If you have an interest in converting glibc to a static file list I'd be interested in seeing the consequences on the glibc.spec file and the %ifarch variances you'd need to maintain the list.

This sounds fairly low priority given how many other things there are to work on in the project, and we already have the existing method working without problems.

> Again: if you are going to discourage anyone who will be sending you patches
> you are doing very good job.

I apologize, my intent was not to discourage. My intent was to move this discussion to a mailing list where we can have an easier time communicating instead of in bugzilla.

> I'm not going to do your job convincing group of people on the glibc mailing
> list if it is part of the process,

This is a community project, you always have to convince other people. The glibc team draws from a variety of volunteers, including contributors from IBM, Intel, Red Hat, and Linaro. I want you to email the list so I can have additional help reviewing your patch, and perhaps getting broader input on the changes.

Again, in summary, please keep sending patches. Not all of them will be accepted, but some will, and that's positive progress.

Thanks again for the patches to cleanup the Fedora packaging policy violations.

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