Bug 1203018 - Review Request: baculum - WebGUI tool for Bacula Community program
Summary: Review Request: baculum - WebGUI tool for Bacula Community program
Keywords:
Status: CLOSED WONTFIX
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
unspecified
medium
Target Milestone: ---
Assignee: Nobody's working on this, feel free to take it
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: FE-DEADREVIEW
TreeView+ depends on / blocked
 
Reported: 2015-03-17 22:03 UTC by Marcin Haba
Modified: 2015-10-08 06:39 UTC (History)
6 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2015-09-18 10:38:49 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)

Description Marcin Haba 2015-03-17 22:03:54 UTC
Spec URL: http://www.bacula.pl/baculum.spec
SRPM URL: http://www.bacula.pl/baculum-7.0.20150317git-1.fc21.src.rpm
Description: Baculum is web based tool to manage, administrate and monitor Bacula services. From server side Baculum is written in PHP and works basing on PRADO Framework. Baculum interface enables e.g. to run Bacula backup and restore actions, take access to Bacula console via web, watch backup graphs and others.
Fedora Account System Username: I do not have Fedora account yet

Comment 1 Marcin Haba 2015-03-17 22:08:31 UTC
Default Baculum access is: http://localhost:9095
First time login is: admin
First time password is: admin

Comment 2 Marcin Haba 2015-03-18 10:06:52 UTC
Here are commands to make Baculum working with SELinux policies:

# semanage port -a -t http_port_t -p tcp 9095
# semanage fcontext -a -t httpd_sys_rw_content_t "/usr/share/baculum/htdocs/assets(/.*)?"
# restorecon "/usr/share/baculum/htdocs/assets"
# semanage fcontext -a -t httpd_sys_rw_content_t "/usr/share/baculum/htdocs/protected/Data(/.*)?"
# restorecon /usr/share/baculum/htdocs/protected/Data
# semanage fcontext -a -t httpd_sys_rw_content_t "/usr/share/baculum/htdocs/protected/logs(/.*)?"
# restorecon /usr/share/baculum/htdocs/protected/logs
# semanage fcontext -a -t httpd_sys_rw_content_t "/usr/share/baculum/htdocs/protected/runtime(/.*)?"
# restorecon /usr/share/baculum/htdocs/protected/runtime
# chcon -t httpd_user_content_rw_t /usr/share/baculum/htdocs/protected/Data/baculum.users
# chcon -t httpd_user_content_rw_t /etc/baculum/baculum.users
# setsebool -P httpd_can_network_connect 1

Comment 3 Marcin Haba 2015-03-19 18:27:37 UTC
I let know that just I created Fedora account. My Fedora account name is: ganiuszka

It is my first Review Request in this Bugzilla.

Comment 4 Marcin Haba 2015-03-22 02:14:02 UTC
Hello,

I built Baculum by Fedora Build System.

Here is link to task:
https://koji.fedoraproject.org/koji/taskinfo?taskID=9293633

Spec file and SRPM are here:

Spec URL: http://www.bacula.pl/baculum.spec
SRPM URL: http://www.bacula.pl/baculum-7.0.20150322git0be2b51a-1.fc21.src.rpm

One more info: I will need a sponsor.

Thank you in advance for review and advises.

Comment 5 Dominik 'Rathann' Mierzejewski 2015-03-23 13:21:18 UTC
Some quick comments.

1. You should give full URL to the source tarball in Source0:, not just the filename (it should be downloadable with curl/wget).
2. You should provide systemd units instead of SysV-style initscripts.
3. Does it work only with lighttpd? If not, you should provide configurations for all supported webservers.
4. Your versioning scheme is wrong, please check https://fedoraproject.org/wiki/Packaging:NamingGuidelines#Package_Versioning , especially the snapshot versioning part.
5. The service should work with SELinux in enforcing mode by default, you should either include and install the necessary policy in your package or file a bug against selinux-policy to include it there.
6. LICENSE file should be included with %license macro, not as %doc.

Comment 6 Marcin Haba 2015-03-23 14:04:31 UTC
Thank you for all comments and advises. I will work on them.

I tested Baculum with Apache and Lighttpd. I chose Lighttpd, because it supports HTTP Basic auth with plain text user:password format stored on a authfile. It is useful and used to internal communication between Baculum's layers and for multi-users mode. It also makes available to modify Baculum HTTP Basic users from Baculum interface and "on fly".

In case Apache webserver does not support this typy of user:password storing as plain text and it causes that Baculum can have only one pre-defined user, without multi-user mode and without possibility to modify auth params from Baculum interface.

Summing up, Baculum can work with other webservers. If webserver supports plain text user:password format, then works in multi user mode and with possibility to change password from Baculum interface. If webserver does not support this type of HTTP Basic auth, user need to modify auth file, for example by htpasswd binary for Apache.

I described Apache configuration in README file.

Anyway, I will follow on your advice about provide other servers configuration files, at least sample configuration for these webservers.

About SELinux, that was a point that I was not sure. Thank you for indicateing me what should I do.

I will back here when I am ready with all changes.

Thanks.

Comment 7 Marcin Haba 2015-03-24 13:02:51 UTC
New Spec file and SRPM location:

Spec URL: http://www.bacula.pl/baculum.spec
SRPM URL: http://www.bacula.pl/baculum-7.0.6-0.1.a.fc21.src.rpm

I fixed pointed things to change:

1) I added Source0:,
2) I added systemd unit file,
3) I did not add other webservers config files. If it is possible, for now I would assume that only Lighttpd is supported. It could be a bit difficult in support if on Lighttpd it works with full functionality, on other webservers Baculum works with limited functionality. Instruction how to configure Baculum manually with Apache is in INSTALL file. Is it OK for you?
4) I changed release tag according to pre-release snapshot versions,
5) I added support for SELinux as subpackage "baculum-selinux" defined in Spec file,
6) I moved license file to %license macro.

From other things, I have started use /var/cache/baculum/ directory for Baculum cache data. Additionally I used more macros in Spec file instead of static paths.

Thank you in advance for next comments and advises.

Comment 8 Marcin Haba 2015-03-27 20:14:27 UTC
I updated new Spec file and SRPM. Location is the same as previously:

Spec URL: http://www.bacula.pl/baculum.spec
SRPM URL: http://www.bacula.pl/baculum-7.0.6-0.1.a.fc21.src.rpm


I also built on Koji the Baculum. Here is this task:

https://koji.fedoraproject.org/koji/taskinfo?taskID=9349633

After three failed builds, now the build finished successfully. I needed to add to build requirements "systemd-units".

Comment 9 Marcin Haba 2015-06-14 15:38:20 UTC
Any news in this ticket?

Thank you in advance for any information.

Comment 10 Marcin Haba 2015-07-10 19:30:22 UTC
Hello,

From March till now I have worked a bit on Spec file. I prepared SELinux module (for not using SELinux booleans calls in Spec). In the meantime I also prepared Baculum 7.0.6-0.1.b version.

Spec and SRPM are available here:

Spec URL: http://bacula.pl/baculum/baculum.spec
SRPM URL: http://bacula.pl/baculum/baculum-7.0.6-0.1.b.fc22.src.rpm

Comment 12 Matthias Runge 2015-07-13 08:33:15 UTC
I really like to see selinux integrated here too. Unfortunately, that doesn't happen that often.

some minor nits:

you have a leftover requires on chkconfig and on /usr/bin/service.

About apache integration, does http://httpd.apache.org/docs/2.4/mod/mod_authn_file.html help you here?

Comment 13 Marcin Haba 2015-07-13 09:21:24 UTC
Hello,

Thanks for point me these not needed requires (chkconfig and service). I removed them and prepared new Spec and SRPM (attached below).

About Apache integration, thanks for show me link to mod_authn_file. Some time ago I have done private research about Apache auth. Then I found info that storing plain text passwords for Apache works only for selected platforms. From htpasswd man page:

-p     Use plaintext passwords. Though htpasswd will support creation on all platforms, the httpd daemon will only accept plain text passwords on Windows and Netware.

However, if I remember good, I have not tried this way for check with Apache.

Spec URL: http://bacula.pl/baculum-2/baculum.spec
SRPM URL: http://bacula.pl/baculum-2/baculum-7.0.6.0.1.b.fc22.src.rpm

Comment 14 Dominik 'Rathann' Mierzejewski 2015-07-13 10:01:27 UTC
Typos and errors in %description:

> The Baculum program allows the user to administrate and manage Bacula work.
                                         ^- administer                  ^- jobs, tasks?  
> By using Baculum is possible to execute backup/restore operations, monitor
                  ^- missing 'it'
> current Bacula jobs, media management and others. Baculum has integrated web
                                                               ^- missing 'an'
> console that communicates with Bacula bconsole program.

Given that it's possible to make this work with Apache, it'd be better to put lighttpd-specific files (and Requires:) into a subpackage, so that it's possible to install this without pulling in lighttpd.

The ordering of the spec file sections is weird. Please put all %files sections at the bottom, just before %changelog.

Comment 15 Jonathan Underwood 2015-07-13 10:06:01 UTC
In support of Marcin's Sponsor request, I'd like to point out that he contributed very helpfully to the review of python-importmagic (BZ #1241944) - if I were a sponsor I'd not hesitate sponsoring him.

Comment 16 Jonathan Underwood 2015-07-13 10:16:11 UTC
Hi Macin, some points fro looking over the spec file:

1) Please update the Release tag each time you make a change, and add a %changelog entry

2) What is the rationale behind the %post and %preun snippets that create/remove those symlinks? Why can they not simply be created in %install and packaged? At the very least the spec file needs some comments explaining why these operations are done in %post and %preun, but better would be find a way to not have to do that there.

3) It's not obligatory, but it strikes me that you do a lot of work manually in %install that would more normally be handled with a Makefile shipped with the upstream tarball. Since you're also upstream, have you considered using a Makefile (or autotools) to simplify installation for users and other distros in general?

4) adding a sub-package for apache configuration would adviseable

5) See all of Dominik's points above :)

Comment 17 Dominik 'Rathann' Mierzejewski 2015-07-13 10:17:32 UTC
Regarding selinux policy, you should not import the included binary policy module directly:
semodule -i %{_datadir}/selinux/packages/%{name}/%{name}.pp 2>/dev/null || :
but build it from source (.te file) first, instead.

make -f %{datadir}/selinux/devel/Makefile %{name}.pp

Of course, you need selinux-policy-devel for that.

Comment 18 Dominik 'Rathann' Mierzejewski 2015-07-13 10:22:18 UTC
Also, allowing the web server user unfettered access to the bconsole command as root seems like a security hole waiting to be exploited. Isn't there a way to authenticate users against PAM so that sudoers can be configured only for those users?

Comment 19 Dominik 'Rathann' Mierzejewski 2015-07-13 10:23:18 UTC
@Jonathan, I also like Marcin's contributions and I intend to sponsor him once this review is completed.

Comment 20 Marcin Haba 2015-07-13 11:51:22 UTC
@Dominik, @Jonathan, Matthias:

Thank you for all your advises and recommendations in this Baculum request. It is precious knowledge for me, specially as a beginner in RPM packaging.

@Dominik
Thanks for your correction English texts in Baculum package description. Thanks also for indicate me proper sections ordering.

About Baculum working with Apache. It is good idea to have separate subpackages for Apache and Lighttpd. I will work on it and I try to find common way in Baculum to full support both web servers.

Generally Baculum works with Apache fine, but in this usage is not possible to serve some Baculum functionalities (that I mentioned in "Comment 6"). If finding solution for full Apache support be impossible, then I modify Baculum code to hide these functionalities not supported in Apache environment and inform about this fact in message on Baculum interface.

About SELinux compilation by Spec file. Thanks for show me proper sample, I will use it and I will add selinux-policy-devel to requirements.

You said about unfettered access to web server users to bconsole with root privilages. Baculum does not provide sudoers.d definition for bconsole. Users have to choose in Baculum install wizard if they want to sudo or they want to not sudo. If sudo access checked, then user need to provide prepared sudoers.d file self. If sudo access not checked, then user need to make bconsole accessible also self. Both solutions have their disadventages. Sudo for lighttpd to execute bconole may sound OK specially that lighttpd user has default as shell: /sbin/nologin. However it is root access to bconsole so it may be not safe. Access without sudo also may sound OK, but customer need to provide way to read bconsole.conf file by web server user for execute bconsole.

Steps to do by user after install Baculum are:

1) Choose in install wizard how Baculum should connect to bconsole
2) If connection using sudo then there is need provide sudoers.d
3) If connection using direct access (no sudo) then there is need to make available reading bconsole.conf file by web server user (for exec('/usr/sbin/bconsole -c /etc/bacula/....etc.) and change web server user shell.

All above three points are to choose by user.

There is at least a few additional barriers to force for get access to Baculum and then bconsole.

1) Access to Baculum is realized by web server HTTP Basic auth (no internal Bauclum login pages)
2) In Baculum is one admin role and can be a lot of normal users. All they have own HTTP Basic credentials.
3) Normal users have assigned dedicated restricted consoles access defined as Bacula Console ACL. It is restricted access from Bacula side (not Baculum side), for example user1 can check only Bacula status, user2 can only run specific one backup job. All these actions are done by Bacula administrator.
4) Baculum has defined possibility to execute only pre-defined commands in bconsole. (no pipes, injections, etc..).

@Jonathan:

Thanks for info about update Release tag. I will have it on mind for next Spec and SRPM.

About symlinks, I will check what can I do with them. Maybe you are right about using them in %install way. I wll check it and I will add comments to Spec file.

Using Makefile is really good idea. I will prepare this Makefile. Thank you for this your idea :)

@Dominik, @Jonathan and Matthias:

Thank you again for your help. With each your comment the Baculum becomes better :)

I will back here with new Spec and SRPM when I have all changes ready.

Comment 21 Marcin Haba 2015-07-14 20:05:34 UTC
Hello,

I prepared all pointed errors and improvements. They are:

 - Separate to subpackage Lighttpd support
 - Add Apache subpackage
 - Use upstream Makefile to prepare build files
 - Cache symlbolic links only in install section
 - Add comments to Spec
 - Compile SELinux policies instead of install pre-compiled
 - Add source files: baculum.users, baculum-apache.conf
   baculum-lighttpd.conf and baculum-lighttpd.service
 - Change Source0 URL

I successfully prepared changes for Apache support even without disabling/hidding Lighttpd specified features.

In place creating symlinks to cache directory I considered using:

https://fedoraproject.org/wiki/Packaging:Alternatives

but I think that it might be strage to have two cache dirs for one application.

I changed Source0 to bacula.pl because today I am not able to upload upstream to bacula.org.

Thank you in advance for look on these changes and review. Latest changes are here:

Spec URL: http://bacula.pl/downloads/baculum/baculum.spec
SRPM URL: http://bacula.pl/downloads/baculum/baculum-7.0.6-0.3.b.fc22.src.rpm

Comment 22 Marcin Haba 2015-07-17 00:30:26 UTC
Hello,

I moved more work to Makefile for make Spec easier.

Here are latest changes:

 - Remove source files: baculum.users, baculum-apache.conf
   baculum-lighttpd.conf and baculum-lighttpd.service
 - Use reorganized upstream Makefile


Spec URL: http://bacula.pl/downloads/baculum/baculum.spec
SRPM URL: http://bacula.pl/downloads/baculum/baculum-7.0.6-0.4.b.fc22.src.rpm

Comment 23 Marcin Haba 2015-07-18 19:25:34 UTC
Spec URL: http://bacula.pl/downloads/baculum/baculum.spec
SRPM URL: http://bacula.pl/downloads/baculum/baculum-7.0.6-0.5.b.fc22.src.rpm

Changes:
 - Change baculum.users and Data/ directory permissions to more
   restrictive
 - Add noreplace param to Lighttpd config file
 - Add systemd macros for httpd subpackage
 - Fix systemd action in post section
 - Move DESTDIR target and languages to global variables
 - Do not remove settings file when a web server specific package
   is removed (used move action)
 - Drop storing Lighttpd logs in separate logs directory
 - Define locale files

Hello,

I have next spec and srpm update. I hope that this update is last one before review process.

Almost every day I get to know something new from Packaging Guidlines and from time to time I am seeing something to change, fix or improve in my Spec file. From this reason also every comment or notice is important for me.

I used rpmlint on packages. I attached results below.

For now I would solve this error:
baculum-httpd.noarch: E: non-readable /usr/share/baculum/htdocs/protected/Data/baculum.users 0600L

In this "baculum.users" file are stored HTTP Basic auth credentials. From this reason privilages are 0600. I think that I cannot move the file to /etc because the file is modifying via Baculum web interface and it might be strange that web application make modification in /etc file. And of course it is danterous.


$ rpmlint ./baculum.spec 
0 packages and 1 specfiles checked; 0 errors, 0 warnings.

$ rpmlint ./baculum-7.0.6-0.5.b.fc22.noarch.rpm 
baculum.noarch: W: spelling-error %description -l en_US bconsole -> console, b console, consolable
baculum.noarch: W: dangerous-command-in-%post ln
baculum.noarch: W: dangerous-command-in-%preun rm
1 packages and 0 specfiles checked; 0 errors, 3 warnings.

$ rpmlint ./baculum-selinux-7.0.6-0.5.b.fc22.noarch.rpm 
baculum-selinux.noarch: W: no-documentation
1 packages and 0 specfiles checked; 0 errors, 1 warnings.

$ rpmlint ./baculum-httpd-7.0.6-0.5.b.fc22.noarch.rpm 
baculum-httpd.noarch: W: no-documentation
baculum-httpd.noarch: E: non-readable /usr/share/baculum/htdocs/protected/Data/baculum.users 0600L
baculum-httpd.noarch: W: dangerous-command-in-%preun mv
1 packages and 0 specfiles checked; 1 errors, 2 warnings.

$ rpmlint ./baculum-lighttpd-7.0.6-0.5.b.fc22.noarch.rpm 
baculum-lighttpd.noarch: W: no-documentation
baculum-lighttpd.noarch: W: non-standard-uid /usr/share/baculum/htdocs/protected/Data lighttpd
baculum-lighttpd.noarch: W: non-standard-gid /usr/share/baculum/htdocs/protected/Data lighttpd
baculum-lighttpd.noarch: W: non-standard-uid /usr/share/baculum/htdocs/protected/Data/baculum.users lighttpd
baculum-lighttpd.noarch: W: non-standard-gid /usr/share/baculum/htdocs/protected/Data/baculum.users lighttpd
baculum-lighttpd.noarch: E: non-readable /usr/share/baculum/htdocs/protected/Data/baculum.users 0600L
baculum-lighttpd.noarch: W: non-standard-uid /var/cache/baculum lighttpd
baculum-lighttpd.noarch: W: non-standard-gid /var/cache/baculum lighttpd
baculum-lighttpd.noarch: W: dangerous-command-in-%preun mv
1 packages and 0 specfiles checked; 1 errors, 8 warnings.

$ rpmlint ./baculum-7.0.6-0.5.b.fc22.src.rpm 
baculum.src: W: spelling-error %description -l en_US bconsole -> console, b console, consolable
1 packages and 0 specfiles checked; 0 errors, 1 warnings.

Comment 25 Marcin Haba 2015-07-19 15:58:51 UTC
Just I successully built Baculum on Koji:
https://koji.fedoraproject.org/koji/taskinfo?taskID=10406385

Comment 26 Dominik 'Rathann' Mierzejewski 2015-07-21 10:34:28 UTC
(In reply to Marcin Haba from comment #23)
> For now I would solve this error:
> baculum-httpd.noarch: E: non-readable
> /usr/share/baculum/htdocs/protected/Data/baculum.users 0600L
> 
> In this "baculum.users" file are stored HTTP Basic auth credentials. From
> this reason privilages are 0600. I think that I cannot move the file to /etc
> because the file is modifying via Baculum web interface and it might be
> strange that web application make modification in /etc file. And of course
> it is danterous.

If this file is modified over time, you should put it somewhere in /var (/var/lib/baculum perhaps) and mark it with %verify macro accordingly so that rpm -V doesn't complain when it's modified. /usr/share is for static content.
/etc/baculum could also be used. See https://fedoraproject.org/wiki/Packaging:Guidelines#Filesystem_Layout .

[...]
> $ rpmlint ./baculum-httpd-7.0.6-0.5.b.fc22.noarch.rpm 
[...]
> baculum-httpd.noarch: E: non-readable
> /usr/share/baculum/htdocs/protected/Data/baculum.users 0600L
> 
> $ rpmlint ./baculum-lighttpd-7.0.6-0.5.b.fc22.noarch.rpm 
[...]
> baculum-lighttpd.noarch: E: non-readable
> /usr/share/baculum/htdocs/protected/Data/baculum.users 0600L

Please note that you should not duplicate files between two subpackages:
https://fedoraproject.org/wiki/Packaging:Guidelines#Duplicate_Files

Comment 27 Marcin Haba 2015-07-21 22:10:17 UTC
Hello,

Thanks for your suggestions.

(In reply to Dominik 'Rathann' Mierzejewski from comment #26)
> If this file is modified over time, you should put it somewhere in /var
> (/var/lib/baculum perhaps) and mark it with %verify macro accordingly so
> that rpm -V doesn't complain when it's modified. /usr/share is for static
> content.

/var/lib/ is for applications state data. From this reason I do not think that auth data may be stored there.

/etc/baculum is better for me, however it is a bit strage for me that web application is allowed to modify /etc/* data.

If it is OK for Fedora and there does not exist better candidate for storing these auth data, then I have no choice and I will use /etc/baculum/

> > baculum-httpd.noarch: E: non-readable
> > /usr/share/baculum/htdocs/protected/Data/baculum.users 0600L
> > 
> > $ rpmlint ./baculum-lighttpd-7.0.6-0.5.b.fc22.noarch.rpm 
> [...]
> > baculum-lighttpd.noarch: E: non-readable
> > /usr/share/baculum/htdocs/protected/Data/baculum.users 0600L
> 
> Please note that you should not duplicate files between two subpackages:
> https://fedoraproject.org/wiki/Packaging:Guidelines#Duplicate_Files

Yes, it is true that these two packages contain the same file. Nevertheless these files are in subpackages that cannot be installed together. I used "Conclicts" tag for them and I explained it in Spec comment.

I know that "Conflicts" tag is not too desirable in Fedora due to general design  requirements. In this case I think that it is reasonable (please see comment in Spec).

Comment 28 Marcin Haba 2015-07-22 02:33:14 UTC
Spec URL: http://bacula.pl/downloads/baculum/baculum.spec
SRPM URL: http://bacula.pl/downloads/baculum/baculum-7.0.6-0.6.b.fc22.src.rpm

Hello,

@Dominik: You are right. Duplicate files are not desirable in any case.

In Makefile from upstream side I splitted Baculum data directory to two directories:

- /etc/baculum/Data-apache (provided by baculum-httpd subpackage)
- /etc/baculum/Data-lighttpd (provided by baculum-lighttpd subpackage)

and next in %post I link one from these directories to framework specific path in /usr/share...

By this move, both points are solved:

1) Variable data stored in /etc/baculum directory, not in /usr/share
2) Files are not doubled in particular subpackages

Thanks for comments.

Comment 29 Marcin Haba 2015-07-25 14:47:57 UTC
Hello,

I have a doubt about proper way for providing PHP framework in Baculum together in one package. From this reason and for not overload this feature request I sent question to fedora-join mailing list here:

https://lists.fedoraproject.org/pipermail/fedora-join/2015-July/000356.html

Comment 30 Dominik 'Rathann' Mierzejewski 2015-07-25 21:05:08 UTC
fedora-join is probably not the best place to ask such questions, The devel or the packaging list would have been better places in my opinion. However, the answer to your question there is actually quite short: you have to unbundle and submit each bundled project as a separate package or request an exception from the Fedora Packaging Committee. See https://fedoraproject.org/wiki/Packaging:Guidelines#Duplication_of_system_libraries .

Comment 31 Marcin Haba 2015-07-26 19:01:28 UTC
Thanks for the information.

So, I guess that now there can be only better :-)

I will try to prepare each project in a separate feature request. I estimate that it will take a few weeks or even months when finally I am ready with Baculum.

Anyway, it looks that I have much more work than I expected.

Comment 32 Dominik 'Rathann' Mierzejewski 2015-07-26 22:09:03 UTC
Please check the currently open review requests first. Some of your dependencies may already be under review.

Comment 33 Marcin Haba 2015-07-27 06:47:52 UTC
Right.

I will check it before open every review request.

Thanks for the tip.

Comment 34 Marcin Haba 2015-08-01 04:08:46 UTC
Just I added first package on way to unbundling PRADO Framework and finally to add Baculum. This package is FirePHPCore and my request is here:

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

Comment 35 Michael Schwendt 2015-08-17 17:28:22 UTC
Skimming over the comments in this ticket, I see comment 19 where Dominik wrote:

 | I intend to sponsor him once this review is completed.

So why the fuss on devel@ list?

[...]

I find no mentioning of the fedora-review tool.

Please point it at this ticket: fedora-review -b 1203018

It will download the latest package files from the "Spec URL:" and "SRPM URL:" lines it can find and perform many helpful checks that are relevant during review. Using it is a basic exercise for new packagers.

[...]

Only displaying the spec files, what's up with the directory ownership?

> %files -f %{name}.lang
> %defattr(-,root,root)
> # directory excluded here, because it needs to be provided
> # with selected web server privileges (lighttpd or apache)
> %{_datadir}/%{name}
> %license LICENSE
> %doc AUTHORS README

That comment is unclear, because there is no %exclude statement in this %files list.

> %files selinux
> %defattr(-,root,root)
> %{_datadir}/selinux/packages/%{name}/%{name}.pp

Directory %{_datadir}/selinux/packages/%{name} is not included.

https://fedoraproject.org/wiki/Packaging:Guidelines#File_and_Directory_Ownership

> %files httpd
> %defattr(-,apache,apache)
> %config(noreplace) %{_sysconfdir}/httpd/conf.d/%{name}.conf
> %config(noreplace) %{_sysconfdir}/%{name}/Data-apache/%{name}.users
> # Apache logs are stored in /var/log/httpd/
> %attr(755,apache,apache) %{_localstatedir}/cache/%{name}/
> %{_sysconfdir}/%{name}/Data-apache/

Directory %{_sysconfdir}/%{name} is not included.

> %files lighttpd
> %defattr(-,lighttpd,lighttpd)
> %config(noreplace) %{_sysconfdir}/%{name}/%{name}-lighttpd.conf
> %config(noreplace) %{_sysconfdir}/%{name}/Data-lighttpd/%{name}.users
> # Lighttpd logs are stored in /var/log/lighttpd
> %attr(755,lighttpd,lighttpd) %{_localstatedir}/cache/%{name}/
> %attr(-,root,root) %{_unitdir}/%{name}-lighttpd.service
> %{_sysconfdir}/%{name}/Data-lighttpd/

Same here.

Since you have both subpackages conflict with eachother, it is clearly a case of another unowned directory.


The %files lists are ambiguous, too. Watch these two lines as an example:

%config(noreplace) %{_sysconfdir}/%{name}/Data-lighttpd/%{name}.users
%{_sysconfdir}/%{name}/Data-lighttpd/

The second line includes the directory %{_sysconfdir}/%{name}/Data-lighttpd/ and everything below it. That includes also the .users file from the first line.

  https://fedoraproject.org/wiki/Packaging:Guidelines#Duplicate_Files

There are warnings in the build.log file about those cases.

$ rpmls -p baculum-httpd-7.0.6-0.6.b.fc24.noarch.rpm
drwx------  /etc/baculum/Data-apache
-rw-------  /etc/baculum/Data-apache/baculum.users
-rw-r-----  /etc/httpd/conf.d/baculum.conf
drwxr-xr-x  /var/cache/baculum

And since the directory includes only the .users file, the correct way would be to turn it into a %dir line in the files list.

Same for the -lighttpd subpackage.

Comment 36 Marcin Haba 2015-08-17 20:05:35 UTC
Please close this ticket. I feel discouraged to continue work on this package. Thank you every person who participated in this ticket for effort in review process and for valuable advices.


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