Bug 1072319 - Macro %scl_files doesn't make metapackage own all dirs created by %scl_install
Summary: Macro %scl_files doesn't make metapackage own all dirs created by %scl_install
Keywords:
Status: CLOSED DUPLICATE of bug 1079203
Alias: None
Product: Red Hat Enterprise Linux 7
Classification: Red Hat
Component: scl-utils
Version: 7.0
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: rc
: ---
Assignee: Jan Zeleny
QA Contact: BaseOS QE - Apps
URL:
Whiteboard:
: 1049815 (view as bug list)
Depends On:
Blocks: 965565 1040470 1057169 1072281 1072401 1072789 1073458 1074012 1074337 1075025 1076159 1079203 1079968 1080036 1080048 1090361 1091008
TreeView+ depends on / blocked
 
Reported: 2014-03-04 11:29 UTC by Bohuslav "Slavek" Kabrda
Modified: 2014-04-24 15:11 UTC (History)
11 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2014-04-10 08:54:11 UTC


Attachments (Terms of Use)

Description Bohuslav "Slavek" Kabrda 2014-03-04 11:29:49 UTC
In scl-utils-20130529-5.el7, macro %scl_files doesn't make metapackage own all dirs created by %scl_install, see e.g. bug 1072281 (for example directories /opt/rh/python33/root/usr/share/locale/* are not owned by python33-runtime).

The reason for this is that macro %scl_files in RHEL 7 version contains
%dir %attr(555,root,root) %{_scl_scripts}
whereas in RHEL 6 version it contains just
%{_scl_scripts}

This causes the RHEL 6 version of scl-utils to print out bunch of "file listed twice" warnings during metapackage build, but all directories are properly owned.
On RHEL 7 however, the directories are not owned and metapackage build passes, since RPM doesn't consider empty directory to be a non-listed file and skips it silently in %files section (maybe that's a bug in RPM, too?).

Comment 1 Bohuslav "Slavek" Kabrda 2014-03-04 11:31:10 UTC
Also please note that this seems to have caused bug 1049815.

Comment 3 Jan Zeleny 2014-03-04 15:44:44 UTC
The state in RHEL7 is actually correct and RHEL6 should be changed accordingly. The "fix" you are requesting (going back to the RHEL6 way) would just mask the problem, it would not present a solution. The real solution will most likely be about including the missing files in the file list (i.e. solving the referenced two bugs separately).

Comment 4 Jan Zeleny 2014-03-04 16:11:44 UTC
Ok so after a deep dive, this doesn't seem to be a bug at all. It's probably just a documentation issue. Basically in your spec files you have:

%files runtime
%scl_files

whereas it should actually be:

%files runtime -f filesystem
%scl_files

Including the file list from the text file complements the file list in %scl_files and the whole thing starts working. I have tested this with the python33 metapackage, could you confirm my solution elsewhere as well?

Comment 5 Bohuslav "Slavek" Kabrda 2014-03-05 07:54:37 UTC
First of all, I'm not requesting to do this in the same way as in RHEL 6 - I know it's broken there, I just noted it works there.
Second, where is this behaviour documented? I have never seen it anywhere nor heard that it's supposed to work this way. (For example Fedora documentation [1], that I consider "upstream documentation" doesn't talk about this).
Third, why do we need to use both "%scl_files" and "-f filesystem"? Wouldn't it be logical to use just one of them? (I guess it'd be easier to just add everything from "%scl_files" to "filesystem" and use that only).

[1] http://docs.fedoraproject.org/en-US/Fedora_Contributor_Documentation/1/html/Software_Collections_Guide/sect-Creating_a_Meta_Package.html

Comment 6 Jan Zeleny 2014-03-05 08:08:25 UTC
(In reply to Bohuslav "Slavek" Kabrda from comment #5)
> First of all, I'm not requesting to do this in the same way as in RHEL 6 - I
> know it's broken there, I just noted it works there.
> Second, where is this behaviour documented? I have never seen it anywhere
> nor heard that it's supposed to work this way. (For example Fedora
> documentation [1], that I consider "upstream documentation" doesn't talk
> about this).
> Third, why do we need to use both "%scl_files" and "-f filesystem"? Wouldn't
> it be logical to use just one of them? (I guess it'd be easier to just add
> everything from "%scl_files" to "filesystem" and use that only).
> 
> [1]
> http://docs.fedoraproject.org/en-US/Fedora_Contributor_Documentation/1/html/
> Software_Collections_Guide/sect-Creating_a_Meta_Package.html

I'm sorry I haven't told you before but the truth is I found out myself just yesterday when I was looking for the root cause of this bug. That's why I mentioned it's a documentation issue.

The reason why to use both file lists is that the content of locale and man directories is generated dynamically by a bash script and is therefore impossible to write down the file list statically. Having both lists merged in a file might be possible but definitely not something I would do willingly, as it would divert from the way how / filesystem is populated.

At this point I'm looking for confirmation that updating the spec file solves the issue. If it does, we can correctly document the behavior.

Comment 7 Bohuslav "Slavek" Kabrda 2014-03-05 08:21:14 UTC
Yes, with the fix, the -runtime package owns all the generated directories. There is however one more warning - the python33-runtime now contains a man file for the collection and I get:

"warning: File listed twice: /opt/rh/python33/root/usr/share/man/man7/python33.7.gz"

This makes me think that all directories in filesystem should be listed with %dir (and perhaps also all the other directories in %scl_filesystem to make it "correct", but change that big would probably be suitable for upstream only ATM).

Comment 8 Jan Zeleny 2014-03-05 09:18:26 UTC
The upstream doesn't have this information, it is only contained in distributions. I will look into this, it might as well be a bug in the distribution-wide directory ownership.

Anyway, closing this bug and opening bug 1072805 for closer inspection of the "listed twice" issue.

Comment 9 Vít Ondruch 2014-03-05 09:31:51 UTC
(In reply to Jan Zeleny from comment #6)
> The reason why to use both file lists is that the content of locale and man
> directories is generated dynamically by a bash script and is therefore
> impossible to write down the file list statically. Having both lists merged
> in a file might be possible but definitely not something I would do
> willingly, as it would divert from the way how / filesystem is populated.

You are moving responsibility of scl-utils on collection devels. This is error prone. Please pick either %scl_files or filesystem file list and stick only with one approach. The rest is implementation detail from my POV. Thank you.

Comment 10 Jan Zeleny 2014-03-05 17:58:23 UTC
I'm sorry but I have to stick with my original decision. We follow the distribution package with this approach, the reason being exactly the same you gave me - to reduce potential errors. Unless this is changed in the distribution itself (I can consult it with the maintainer if you insist on unifying the list), I will not change it in scl-utils, as it would divert from the original. I realize that it's extra work for you and I'm truly sorry but doing the change you requested would actually cause more harm than good.

Comment 11 Bohuslav "Slavek" Kabrda 2014-03-07 09:25:33 UTC
So I've talked about this with maintainer of "filesystem" package - Ondra Vasik - and here is what I've got:

The "filesystem" package uses an explicit list of "most important" directories in %files section (corresponds to %scl_files) to have an explicit double check of these (if he does a typo in %install section, rpm build will fail since this typo is most likely not repeated in %files). The rest of the directories are "not so important", e.g. the locale dirs etc, so he lists them using "-f filesystem".
Since scl-utils are supposed to make SCL packaging as little error-prone as possible, we've reached a conclusion that it'd make sense to append %scl_files content to "filesystem" as a part of %scl_install macro. This way, if a packager doesn't use "-f filesystem", the build will just fail (unlike the current situation, where a build passes if packager uses just %scl_files).

Note, that this solution both satisfies us as SCL packagers and it keeps the "most important" directories explicitly listed in macros.scl (e.g. protecting you as scl-utils developer from typos).

Comment 25 Vít Ondruch 2014-03-24 09:52:15 UTC
*** Bug 1049815 has been marked as a duplicate of this bug. ***

Comment 26 Vít Ondruch 2014-03-24 15:41:41 UTC
1) Speaking about compatibility, the only backward compatible way is usage of %scl_files macro. The -f filesystem might be new feature, supported in parallel, but %scl_files must work.

2) I am surprised, that the list of directories is not static. It should be generated during build of scl-utils and scl-utils should explicitly depend on specific filesystem package version to be able to accompany any changes in filesystem package into scl-utils.

In the meantime, I'm going to explicitly own man directories in metapackage and I would suggest to everybody else do the same.

Comment 27 Vít Ondruch 2014-03-24 15:44:54 UTC
BTW what was the point in extending the default SCL filesystem to this huge extent? It seems to be unreasonably complex and fragile.

Comment 29 Marcela Mašláňová 2014-03-24 17:54:42 UTC
(In reply to Vít Ondruch from comment #26)
> 1) Speaking about compatibility, the only backward compatible way is usage
> of %scl_files macro. The -f filesystem might be new feature, supported in
> parallel, but %scl_files must work.
> 
> 2) I am surprised, that the list of directories is not static. It should be
> generated during build of scl-utils and scl-utils should explicitly depend
> on specific filesystem package version to be able to accompany any changes
> in filesystem package into scl-utils.
> 
> In the meantime, I'm going to explicitly own man directories in metapackage
> and I would suggest to everybody else do the same.

I agree.

Comment 30 Jan Zeleny 2014-03-25 13:08:45 UTC
(In reply to Vít Ondruch from comment #27)
> BTW what was the point in extending the default SCL filesystem to this huge
> extent? It seems to be unreasonably complex and fragile.

The reason was coherence - we wanted to provide the same environment to SCLized packages as / filesystem offers to standard packages.

Comment 32 Vít Ondruch 2014-03-26 09:56:04 UTC
(In reply to Jan Zeleny from comment #30)
> (In reply to Vít Ondruch from comment #27)
> > BTW what was the point in extending the default SCL filesystem to this huge
> > extent? It seems to be unreasonably complex and fragile.
> 
> The reason was coherence - we wanted to provide the same environment to
> SCLized packages as / filesystem offers to standard packages.

I thought so ... but now I see it makes more problems then it solves. It is false promise. For my packages it does not cover at least two areas:

1) Metapacakge has to own pkgconfig directory. It is originally owned by pkgconfig package.

2) Recently QA discovered, that ror40 package has to recreate and own the RubyGems directory strucutre (bug 1080036).

I am afraid there will be more issues like this (as an example, it could be Python collection using system python, but providing additional Python packages). May be we should resort back to own just the root directory and each packager should pay attention to his directory structure and our tooling should provide the tools to discover the weak points.

Comment 33 Jan Zeleny 2014-03-26 13:42:51 UTC
I have no problem with that approach - it will make the root directories of SCLs much cleaner and more transparent. On the other hand it will transfer more responsibilities to SCL maintainers, are you guys sure you're ok with that?

As for the tooling, I have no idea what exactly are you talking about, could you elaborate?

Comment 34 Honza Horak 2014-03-26 14:02:14 UTC
(In reply to Jan Zeleny from comment #33)
> I have no problem with that approach - it will make the root directories of
> SCLs much cleaner and more transparent. On the other hand it will transfer
> more responsibilities to SCL maintainers, are you guys sure you're ok with
> that?

Actually I have no idea about how many paths more I would have to specify comparing what we do now. Would it be hard to provide some proof-of-concept build, pls? Or can I simulate the new functionality within my meta package somehow easily like removing %scl_files or so?

Comment 35 Jan Zeleny 2014-03-26 15:28:14 UTC
Yes, replacing %scl_files with [1] and replacing %scl_install with [2] should do the trick:

[1]
%defattr(-,root,root,-)
%dir %_scl_prefix
%dir %attr(555,root,root) %{_scl_root}
%dir %attr(555,root,root) %{_scl_scripts}
%{_scl_scripts}/enable
%{_root_sysconfdir}/scl/prefixes/%scl

[2]
# scl specific stuff
mkdir -p %{buildroot}%{_root_sysconfdir}/{rpm,scl/prefixes}
echo -n '%' > %{buildroot}%{_root_sysconfdir}/rpm/macros.%{scl}-config
cat >> %{buildroot}%{_root_sysconfdir}/rpm/macros.%{scl}-config << EOF
scl %scl
EOF
cat >> %{buildroot}%{_root_sysconfdir}/scl/prefixes/%{scl} << EOF
%_scl_prefix
EOF

Let me know if you need any further assistance.

Comment 36 Vít Ondruch 2014-03-27 08:01:15 UTC
(In reply to Jan Zeleny from comment #33)
> As for the tooling, I have no idea what exactly are you talking about, could
> you elaborate?

My idea is to have something, which actually tests if all directories of my collection are owned. QA is currently installing the whole collection and later removes the metapackage-runtime. What lefts on the filesystem should be of concern.

This is big hammer of course. I am not sure if we can provide some lightweight and more robust tool.

Comment 37 Pavel Raiskup 2014-04-08 12:26:26 UTC
Just an idea (hopefully not completely OT):  couldn't we use %() construct or
lua to just "include" the 'filelist' contents via %scl_files into %files?
Cons:  I tried that and it seems I (silently) hit the 8K limit for the size of
generated output [1], but we could make the limit larger possibly (or include
the filelist split into multiple chunks via %()).  There may be some conceptual
problem I don't see (apart from that it may be seen like too "hacky").

[1] http://www.rpm.org/wiki/PackagerDocs/Macros

Comment 38 Jan Zeleny 2014-04-10 08:54:11 UTC
This is a part of the %scl_files vs -f filelist conversation. On scl-utils planning meeting we have agreed to leave the current state as it is and aim at SCL 2.0 with the general fix. Closing as duplicate to have just one tracking bug.

*** This bug has been marked as a duplicate of bug 1079203 ***


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