https://bugzilla.redhat.com/show_bug.cgi?id=1485458 Jonathan Dieter <jdieter@xxxxxxxxx> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED Assignee|nobody@xxxxxxxxxxxxxxxxx |jdieter@xxxxxxxxx --- Comment #8 from Jonathan Dieter <jdieter@xxxxxxxxx> --- (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. As Alexander pointed out, at least until 2.9.7 has been built you really need to use proper pre-release versioning (https://fedoraproject.org/wiki/Package_Versioning_Examples#Complex_versioning_examples, scroll down to pkg pre-release svn checkout) This allows us to verify that the code submitted matches the upstream code. > I have several questions which have arison from my reading of the wiki > and fedora-review's output. > > 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 > > zlib/libpng > ----------- > orangefs-2.9.7/src/common/misc/md5.c > orangefs-2.9.7/src/common/misc/md5.h > > 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. If it's not built, you don't need to list the licenses. But I would wonder why you're not building these modules (obviously excepting the kernel module), and splitting them into subpackages so your users can easily get the parts of OrangeFS that they want. I noticed the following options not being set by configure: PVFS2 configured to build karma gui : no PVFS2 configured to build visualization tools : no PVFS2 configured to perform coverage analysis : no PVFS2 configured to use FUSE : no PVFS2 configured for the 2.6/3 kernel module : no PVFS2 configured for the 2.4.x kernel module : no PVFS2 configured for using the ra-cache : no PVFS2 configured for resetting file position : no PVFS2 will use workaround for redhat 2.4 kernels : no PVFS2 will use workaround for buggy NPTL : no PVFS2 server capability cache enabled : no PVFS2 server credential cache enabled : no PVFS2 server certificate cache enabled : no PVFS2 configured with key-based security : no PVFS2 configured with certificate-based security : no PVFS2 configured with profiling enabled : no PVFS2 user interface libraries will be built : no PVFS2 symbolic libraries will be built : no PVFS2 user environment variables enabled : no PVFS2 user interface library cache enabled : no PVFS2 client JNI enabled : no Obviously some of those are irrelevant to Fedora, but what about the karma gui, visualization tools, ra-cache, server caches, key-based security, certificate-based security, etc. Are these all obsolete or irrelevant to Fedora, or would some of those be useful to your users? > I am not sure how to handle this. I assume that the BSD/MIT style > licenses will not pose a problem, but I don't know where to document it. Normally like this: GPLv3 and BSD and ... > OrangeFS includes a copy of LMDB. It does not link against any external > package. Does this need to be changed in upstream? According to the Guidelines, if upstream *can't* be built against the system version of LMDB, then you can bundle it, while following the guidelines on how to do that (https://fedoraproject.org/wiki/Packaging:Guidelines#Bundling_and_Duplication_of_system_libraries), but, given that you are upstream, it would be *much* better if you could make the changes required to build against the system library. You really don't want to be in a position where your bundled library has a vulnerability that's been patched in the system library, but not in your bundled library. I've been there. It sucks. > 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? My recommendation would be to put pvfs2-genconfig in /usr/bin, as you already have, but possibly throw a simple config in /etc/orangefs/orangefs.conf. If the config could contain minimal (even if it doesn't work out of the box) configuration and a comment explaining the best way to properly generate it using pvfs2-genconfig, that would make it easy enough for even a non-documentation-reading sysadmin like myself to figure out what to do. Whatever you do, it looks like there are enough configuration files (certificates, etc as well as orangefs.conf) that it would be worth creating and owning /etc/orangefs/ and using that as your default configuration location. > 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? /var/lib/orangefs or /var/lib/orangefs/storage if there are other state files kept in /var/lib/orangefs. I'm assuming the admin can change this location in the config file. > 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'm assuming storage database generation is manual. If so, I would use /var/lib/orangefs as the default location (as generated by pvfs2-genconfig), but when the sysadmin runs pvfs2-genconfig, they will obviously be given the chance to change this. > 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. If the server exits with a sane enough message ("Your file system has not yet been initialized. Please read the configuration file/documentation for more details"), then I wouldn't worry about it. > 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. If the client needs to be run with arguments to mount the filesystem, I'd probably not bother with a systemd service, but if it reads the configuration from /etc, a service might make sense. > 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. Can both log to the journal? Or to syslog (which will get forwarded to the journal)? I believe that would be the most sane option. > We have a component called the usrint which is a libc interposer. > Several of our utility programs require it now. It does not build on > all platforms. It is not required to run either the server or the > client. I have disabled it completely. If some of your utilities require it and it's important, I would probably check that it builds on all primary architectures. If it does, I'd enable it, and then use conditionals to disable building it on the arches it crashes on. Then open a bug for each arch it crashes on, (similar to what's described at https://fedoraproject.org/wiki/Packaging:Guidelines#Architecture_Build_Failures). It's possible someone who knows those arches may fix the bugs. If it doesn't build on all primary architectures (which I believe are currently ARM and x86_64), then I'd try to get it working at least for those, but it's not required by the guidelines. The one other thing I noticed is that you're packaging .a files in your -devel package. I'd remove them as part of your build process. There are other issues, but I'd like to see the above dealt with before we start moving through the detailed review. -- 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
