[Bug 1485458] Review Request: orangefs - parallel network file system ( formerly PVFS2)

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

Alexander Ploumistos <alex.ploumistos@xxxxxxxxx> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |alex.ploumistos@xxxxxxxxx



--- Comment #1 from Alexander Ploumistos <alex.ploumistos@xxxxxxxxx> ---
Hello Martin and welcome aboard,

I think you would get more help if you posted your questions on the devel
mailing list. I won't take this review, as it is quite out of my league, but
I'll try to give some pointers. I know that the mere number of guidelines and
policies in place can be daunting. Since you've run fedora-review yourself,
you've already seen most of what needs to be taken care of.

Having read through your entire message, I believe you will find the answers to
a lot of your questions in https://fedoraproject.org/wiki/Packaging:Guidelines


(In reply to Martin Brandenburg from comment #0)> 
> This is my first package.  I represent the upstream developers.  I have
> fixed many problems since our last release 2.9.6 while making this
> package.  We intend to fix any further problems uncovered by this review
> and make a new release 2.9.7 if this package is accepted.  As such, my
> RPM currently builds our SVN trunk.

See https://fedoraproject.org/wiki/Packaging:Versioning and
https://fedoraproject.org/wiki/Package_Versioning_Examples and mind the
prerelease versioning schemes in particular.


> The OrangeFS server and client (which is all I have packaged) are
> intended to be LGPL v2.1 or later.  There are a few files which are
> under various open licenses:
> 
> *No copyright* LGPL (v2)
> ------------------------
> orangefs-2.9.7/src/apps/admin/pvfs2-config.in
> 
> BSD (2 clause)
> --------------
> orangefs-2.9.7/maint/config/ssl.m4
> 
> BSD (3 clause)
> --------------
> orangefs-2.9.7/src/client/usrint/fts.c
> orangefs-2.9.7/src/client/usrint/fts.h
> 
> ISC
> ---
> orangefs-2.9.7/src/common/lmdb/mdb.c
> 
> LGPL (v2.1)
> -----------
> orangefs-2.9.7/src/common/dotconf/dotconf.c
> 
> NTP
> ---
> orangefs-2.9.7/maint/config/install-sh

Isn't this one under MIT?

> 
> zlib/libpng
> -----------
> orangefs-2.9.7/src/common/misc/md5.c
> orangefs-2.9.7/src/common/misc/md5.h

Your License tag may contain as many licenses as needed, see:
https://fedoraproject.org/wiki/Packaging:LicensingGuidelines?rd=Packaging/LicensingGuidelines#Multiple_Licensing_Scenarios

Here is a somewhat comprehensive list of compatible and incompatible licenses:
https://fedoraproject.org/wiki/Licensing:Main


> Then there is code under other licenses in our source tree but which are
> not built by this package.  The kernel module is GPL v2.  Parts of our
> Hadoop integration are Apache v2.0.  Parts of the webpack (a set of
> Apache modules) is GPL v3.  None of this is built.

I'm guessing these (sub)packages should have a requirement on Apache and such,
but you don't need to worry about their licenses.

> OrangeFS includes a copy of LMDB.  It does not link against any external
> package.  Does this need to be changed in upstream?

Is there a reason that you can't use the version of LMDB already in Fedora
(e.g. forked code, older upstream snapshot etc.)? Depending on your needs, you
could have something like
BuildRequires:   lmdb-devel
or
Requires:        lmdb
in your spec file.

> 
> OrangeFS requires some configuration to start and creates some files at
> runtime.  A simple configuration file that configures a one-machine file
> system still requires the local hostname.  We generally use a tool to
> generate configuration files.  Then the storage database must be
> initialized.  I have a commented postinstall script.  How do I handle
> this?

Sorry, no idea. Perhaps you could take a look at something like owncloud and
see how the maintainer has handled first time setup there?

> 
> The storage database is currently /usr/storage (i.e. $PREFIX/storage).
> Obviously this is not right.  I suppose it should go in /var somewhere.
> Where should it go?  I also don't know how to mark this directory as
> owned by the package.  Should it be deleted on package uninstall since
> it contains user data?

https://fedoraproject.org/wiki/Packaging:Guidelines#File_and_Directory_Ownership
In general, I think that whatever might contain user data is left intact when
the package gets uninstalled. There is a similar provisions for user-made
configurations.

> 
> The storage database contains a number called the FSID (file system
> identifier).  It is generally different for each installation
> (calculated randomly), so distributing one base storage directory would
> be impractical.
> 
> I have written a systemd file for the server.  I'm not really sure how
> to make it not run if the file system has not yet been initialized.

Maybe these could help?
https://fedoraproject.org/wiki/Packaging:Systemd
https://fedoraproject.org/wiki/Packaging:Scriptlets#Systemd

> 
> The utility programs distributed and others which can be linked against
> our libraries will run against the server.  It is also possible to use a
> userspace client program along with the kernel module (distributed by
> kernel.org and already present in Fedora).  This would require running
> the client program and mounting the filesystem through the kernel.  I
> suppose systemd scripts are needed, but I'm not sure what to distribute.
> 
> The server logs to /var/log/orangefs-server.log and the client logs to
> /tmp/pvfs2-client.log.  Obviously /tmp/pvfs2-client.log should be moved
> to /var/log.  The packaging system does not know about either file yet.

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


> [!]: Changelog in prescribed format.

I think rpmlint complains about your changelog because there is no
(epoch-)version-release in the entry. See
 https://fedoraproject.org/wiki/Packaging:Guidelines#Changelogs but keep in
mind what I wrote before about prerelease/snapshot versioning.
Btw, I think that the changelog should go at the end of the spec file.

> [?]: Package contains systemd file(s) if in need.
> 
> I'd like someone to review my systemd file.

If you don't get definitive answers in the wiki, ask on the mailing lists.

> [!]: Description and summary sections in the package spec file contains
>      translations for supported Non-English languages, if available.
> 
> I can write some German badly, but I can't write anything else.

You don't have to, that is in case there are translations included with the
code upstream.

> [!]: %check is present and all tests pass.

A %check section contains test suites or units. If you have these in your
source code, you should enable them there (if it makes sense).

> I intend to deal with the manpage issues, but I'm at the point now where
> I'm looking for advice on most of it.

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

> 
> The macro-in-comment lines will go away when those lines are deleted
> from the spec.
> 
> Rpmlint
> -------
> Checking: orangefs-2.9.7-1.fc26.x86_64.rpm
>           orangefs-debuginfo-2.9.7-1.fc26.x86_64.rpm
>           orangefs-devel-2.9.7-1.fc26.x86_64.rpm
>           orangefs-server-2.9.7-1.fc26.x86_64.rpm
>           orangefs-2.9.7-1.fc26.src.rpm
> orangefs.x86_64: W: no-version-in-last-changelog
> orangefs.x86_64: W: unstripped-binary-or-object /usr/lib64/libpvfs2.so.2.9.6
> orangefs.x86_64: E: no-ldconfig-symlink /usr/lib64/libpvfs2.so.2.9.6
> orangefs.x86_64: W: shared-lib-calls-exit /usr/lib64/libpvfs2.so.2.9.6
> exit@GLIBC_2.2.5
> orangefs.x86_64: W: manual-page-warning
> /usr/share/man/man1/pvfs2-drop-caches.1.gz 13: warning: numeric expression
> expected (got `f')
> orangefs.x86_64: W: manual-page-warning
> /usr/share/man/man1/pvfs2-fs-dump.1.gz 15: warning: numeric expression
> expected (got `m')
> orangefs-debuginfo.x86_64: W: no-version-in-last-changelog
> orangefs-devel.x86_64: W: no-dependency-on orangefs/orangefs-libs/liborangefs
> orangefs-devel.x86_64: W: no-version-in-last-changelog
> orangefs-devel.x86_64: W: no-documentation
> orangefs-devel.x86_64: W: no-manual-page-for-binary pvfs2-config
> orangefs-server.x86_64: W: no-version-in-last-changelog
> orangefs-server.x86_64: W: no-manual-page-for-binary pvfs2-start-all
> orangefs-server.x86_64: W: no-manual-page-for-binary pvfs2-stop-all
> orangefs.src: W: no-version-in-last-changelog
> orangefs.src:12: W: unversioned-explicit-provides libofs.so()(64bit)
> orangefs.src:12: W: unversioned-explicit-provides liborangefs.so()(64bit)
> orangefs.src:12: W: unversioned-explicit-provides libpvfs2.so()(64bit)
> orangefs.src:19: W: macro-in-comment %{version}
> orangefs.src:41: W: macro-in-comment %{_bindir}
> orangefs.src:42: W: macro-in-comment %{_bindir}
> orangefs.src:43: W: macro-in-comment %{_bindir}
> orangefs.src:44: W: macro-in-comment %{_bindir}
> orangefs.src:45: W: macro-in-comment %{_bindir}
> orangefs.src:64: W: macro-in-comment %{_bindir}
> orangefs.src:80: W: macro-in-comment %{_bindir}
> orangefs.src:83: W: macro-in-comment %{_libdir}
> orangefs.src:84: W: macro-in-comment %{_libdir}
> orangefs.src:85: W: macro-in-comment %{_libdir}
> orangefs.src:86: W: macro-in-comment %{_libdir}
> orangefs.src:87: W: macro-in-comment %{_libdir}
> orangefs.src:88: W: macro-in-comment %{_libdir}
> orangefs.src:164: W: macro-in-comment %{_libdir}
> orangefs.src:165: W: macro-in-comment %{_libdir}
> orangefs.src:166: W: macro-in-comment %{_libdir}
> orangefs.src:167: W: macro-in-comment %{_libdir}
> orangefs.src:168: W: macro-in-comment %{_libdir}
> orangefs.src:169: W: macro-in-comment %{_libdir}
> orangefs.src:197: W: macro-in-comment %config
> orangefs.src: W: invalid-url Source0: orangefs-2.9.7.tar.gz
> 5 packages and 0 specfiles checked; 1 errors, 39 warnings.

You can find some useful info on rpmlint warnings and errors here:
https://fedoraproject.org/wiki/Common_Rpmlint_issues

I hope I have helped a little bit. Happy reading and I'm sure you'll find more
qualified people to answer your questions on devel or IRC.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
_______________________________________________
package-review mailing list -- package-review@xxxxxxxxxxxxxxxxxxxxxxx
To unsubscribe send an email to package-review-leave@xxxxxxxxxxxxxxxxxxxxxxx




[Index of Archives]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite Conditions]     [KDE Users]

  Powered by Linux