[Bug 1359412] Review Request: gawkextlib - library providing support functions for gawk extension libraries

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

 



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

David Kaspar [Dee'Kej] <dkaspar@xxxxxxxxxx> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |ajschorr@alumni.princeton.e
                   |                            |du
              Flags|                            |needinfo?(ajschorr@alumni.p
                   |                            |rinceton.edu)



--- Comment #5 from David Kaspar [Dee'Kej] <dkaspar@xxxxxxxxxx> ---
Hello Andrew,

(In reply to Andrew J. Schorr from comment #4)
> Hi, sorry for the long delay -- I have been extremely busy with regular
> work.
Don't worry about it. As you can see, I have been also completely engaged with
my regular work... :-/ (I've become upstream maintainer/developer for
initscripts.)


> > 0) Is this project already packaged for some other distributions? Which ones?
> > Could you please provide the links those packages?
> 
> No. This will be the first distribution to include it.
In that case, I thank you for chosing Fedora as your first choice. I'm sorry
for the delay in my response, and I hope you will like our Fedora community at
some point! ;)

> I am open to other names, but please consider that comparing gawkextlib
> to gawk-xml is like comparing apples to oranges. The gawkextlib package
> provides a support library, not an actual gawk extenstion. The gawkextlib
> shared library provides APIs that are used by various gawk extension
> libraries such as gawk-xml, gawk-pgsql, etc. So it's really a different
> beast. The gawkextlib rpm provides no end-user functionality. It provides
> only a header file and shared library for use by actual gawk extension
> libraries.

Yeah, I'm aware that those are not same. :) I was just thinking to come up with
some naming schema to avoid some possible issues in the future. It's really
pain in the a** to be forced to create a pacake with new name just to solve
some problems. ;)

So, currently we have:
* gawk [the main package]
* gawk-devel [subpackage]
* gawk-doc [subpackage]
* gawk-debuginfo [automatically created subpackage]

In case we will have the 'gawkextlib' package (to follow FPG rule to have the
name as close to upstream name as possible), then I wouls suggest this naming
schema:
* gawkextlib [the main package, which is necessary for your other extensions]
* gawkextlib-devel [subpackage necessary for developers]
* gawkextlib-doc   [subpackage containing documentation, if any exists]
* gawkextlib-xml   [the module/extension used with gawkextlib]
* gawkextlib-pgsql [the module/extension used with gawkextlib]
* gawkextlib-mpfr  [the module/extension used with gawkextlib]

This way it would be clearly visible by the name that the modules/extensions
are part of the the bigger "collection", and that they cannot function without
the main (base) package.

I was even thinking that you could create these modules/extensions as
subpackages of the gawkextlib, but I see that you versioning is not united
across these packages. The maintenance for this approach might be simpler, but
updating a module would require rebuild of whole thing.

If you do not want to create this "collection" as subpackages, you can just
create each package separate as it is, which will look the same as above, but
will be versioned & build separately. You will have more specfiles to maintain,
but IMHO this might be the best course of action. :)

In case your modules/extensions are able to function *without* the
'gawkextlib', then I would follow this naming scheme:
* gawkextlib [optional library for gawk]
* gawkextlib-devel [subpackage necessary for developers]
* gawkextlib-doc   [subpackage containing documentation, if any exists]
* gawk-xml         [independent extension to gawkextlib used for gawk itself]
* gawk-pgsql       [independent extension to gawkextlib used for gawk itself]
* gawk-mpfr        [independent extension to gawkextlib used for gawk itself]

> > 3) Try to keep the 'Summary' more simple, like for example:
> > >  Extension libraries for gawk
> > ... Current summarry seems to me to vague, because using word library 2 times
> > in a simple sentence is kind of confusing.
> 
> Hmmm. "Extension libraries for gawk" is not accurate. This is a library that
> provides APIs for use by gawk extension libraries. I'm not sure how else to
> say
> it. Would it be more clear if "gawk extension libraries" were replaced by
> "gawk
> extension modules"? I could also replace "infrastructure" if that's somehow
> problematic. Here's a possible rewrite: "Library providing support functions
> used by several gawk extension modules." Is that better than "Library
> providing
> common infrastructure for gawk extension libraries"?

Yeah, I think
> Library providing common infrastructure for gawk extension modules
is OK. ;)

> > 6) (In reply to Andrew Schorr from comment #1)
> > > FYI, I updated the spec file to make this minor change:
> > > 
> > > @@ -5,7 +5,7 @@ Release:        1%{?dist}
> > >  License:       GPLv3+
> > >  Group:         Development/Libraries
> > >  URL:           http://sourceforge.net/projects/gawkextlib
> > > -Source0:      
> > > http://sourceforge.net/projects/gawkextlib/files/%{name}-%{version}.tar.gz
> > > +Source0:       %{url}/files/%{name}-%{version}.tar.gz
> > >  BuildRequires: /usr/include/gawkapi.h
> > >  Requires:      gawk
> > 
> > Actually, I think this change make the Source URL shorter, but for new
> > maintainers in the future, it might be a little bit obfuscating. My personal
> > preference is to keep the full URL for the source tarball, with only %{name}
> > and %{version} in it. However, this is just my opinion, the final decision
> > about this will be on you... :)
> 
> I made this change because I searched for other new packages currently in the
> review process and noted that another maintainer had requested that this
> change
> be made. I can't remember the name of that package at the moment. It seems
> that
> this may be a matter of personal preference. I prefer to avoid repetition,
> so I
> like using %{url} in the Source definition.

Noted, thank you for stating that. It's OK to keep it like this. ;)

> > 7) I see you have used the /usr/include/gawkapi.h as the BuildRequires. Again,
> > this is too much obfuscated, because other people would have to find out from
> > where this header file comes from (is it from gawk or gawkextlib?). Since this
> > is normally part of the gawk package (according to 'dnf whatprovides
> > <path/file>), you should just use simple form of:
> > > BuildRequires:  gawk
> > 
> > UPDATE: Looking at the gawk package itself, I will most likely split it into
> > base package and devel supbackage. The /usr/include/gawkapi.h will be located
> > inside the gawk-devel subpackage.
> 
> I am changing this to say "BuildRequires: gawk-devel".

Thank you for this, 'gawk-devel' already exists. :)

> > 8) %description - it is generally more save to use the name of 'gawkextlib' in
> > the description. According to Fedora Package Guidelines (FPG), there might be
> > instances where the %{name} might not expand properly, which is not correct.
> 
> I don't see where that's mentioned. Particularly since you seem to dislike
> the
> name "gawkextlib", it seems much safer to use %{name} where possible in case
> it
> changes in the future.  Why might it not expand properly?

To be honest, I really don't remember where exactly did I find this in FPG.
This is nothing serious, so lets keep this. In case this proves being
problematic in the future, we can do the change if it happens. Right now we
should focus on getting your package into Fedora as soon as possible. :)

> > And I wouldn't be afraid to use more specific description, like the one you
> > have on the sourceforge webpage:
> > 
> > "The gawkextlib project provides several extension libraries for gawk (GNU
> > AWK), as well as libgawkextlib containing some APIs that are useful for
> > building gawk extension libraries."
> > 
> > In that case, it's the same for people comparing if the source code @
> > sourceforg is the same as the source code packaged in Fedora. :)
> 
> Sorry -- I know this is confusing, but we have made the mistake of using
> gawkextlib in 2 different ways. This particular package's purpose is
> to provide libgawkextlib, an API library used by various gawk extension
> libraries. In addition, the "gawkextlib" project is a collection of gawk
> extension libraries. It's a naming mess. But the bottom line is that the
> description from the web site refers to the collection of gawk modules
> and is not appropriate for this rpm.
Ah, OK. In that case I would just slightly rewrite the current %description to
match the *Summary*, maybe like this? :)

"%{name} is a library providing common support infrastructure for gawk
extensions. This particular package provides the 'libgawkextlib', which is used
by various gawk extension modules -- for example gawk-xml, gawk-pgsql, and
more."

> > 10) The 'Requires' tag for /usr/include/gawkapi.h in devel supbackage is not
> > necessary, because the gawkextlib (base package) already requires gawk, and the
> > devel subpackage is depending on its base package.
> 
> Hmmm. I'm confused about this one. The gawkextlib base package requires gawk,
> but BUILDING the gawkextlib package requires gawk-devel. On the other hand,
> INSTALLING gawkextlib-devel requires gawk-devel (which is not required for
> installing gawkextlib). So I don't see why the Requires: gawk-devel is
> superflous for the devel package. What am I misunderstanding? Perhaps you
> wrote this before realizing that the header file belongs in a separate
> gawk-devel subpackage...

I don't know what the h*ll I wrote there... :D Seriously, that sentence from me
does not make any sense. I'm not surprised you were confused... :D Looking at
the current specfile, it seems OK. ;)

> 
> > 11) Similar to point 9), you can just use "%description devel".
> 
> Done. Thanks.

Here's my proposal of updated %description for devel subpackage, to unite it
with the %description proposed above:

"The %{name}-devel package contains the header files and libraries
needed to develop gawk extension modules that use %{name} facilities."

> > 15) You're not calling the ldconfig for (un)installation of devel subpackages,
> > which is not correct:
> > https://fedoraproject.org/wiki/Packaging:Guidelines#Shared_Libraries
> > 
> > So, basically, you're missing something like this:
> > > %post devel -p /sbin/ldconfig
> > > %postun devel -p /sbin/ldconfig
> 
> I'm confused about that one. The gawkextlib-devel package installs only
> the symlink /usr/lib64/libgawkextlib.so.

Ah, that explains everything. :) I haven't tried compiling the source code
myself, so I didn't realize it was just a symlink and header file. Initially I
thought it was another shared library. ;) This part can stay as it is.

> > 17) Please, consider if your package should use hardened build:
> > https://fedoraproject.org/wiki/Packaging:Guidelines#PIE
> > 
> > You know it better than I do.
> 
> Isn't this currently the default behavior in Fedora? Please take a look at
>    https://fedoraproject.org/wiki/Changes/Harden_All_Packages
Yes, you are right. It is by default enabled in current Fedora releases.
However, there might come a point where we might like to get this package in
RHEL or EPEL. In that case AFAIK the package should be build with preconfigured
environment for RHEL/EPEL, which will most likely not have the hardened build
enabled by default.

So I personally add the hardened build anywhere, just to be safe. :)

> > 19) Looking at your last changelog, I would suggest make it more descriptive in
> > the future. In case it was a rebase to newer version (my guess), than it should
> > be stated there explicitly. It helps in case someone is trying to find changes
> > that caused some problems.
> 
> While in heavy development, I have not bothered to update the changelog
> carefully. Once we get to a working version, I will add thorough comments
> about subsequent changes. But it is changing too much right now for this
> to be useful...

OK, noted. :) 

> > 20) Since this package provides a library and other packages might be linking
> > to it, is there a reason why this software is not packaged under some LGPL
> > license? Legal stuff is generally problematic, this is just a question.
> 
> The gawk documentation requires that all extensions use the GPL license.
> The gawk docs say:
> 
>    "Every dynamic extension must be distributed under a license that is
>    compatible with the GNU GPL (*note Copying::)."
> 
> I'm not sure if that includes LGPL or not. It seems safe to me to use GPL
> since gawk is GPL. I'm not a licensing expert, but I don't see why LGPL
> would make more sense in this context. These libraries are explicitly
> for use with gawk, which is under the GPL, and the extension libraries
> bundled with gawk are also under the GPL.

I'm not a legal expert myself, but your clarification makes sense. :) And
changing licenses for FOSS projects can really be very painful, I wouldn't
advise that much. That's why it was just a question I was intrigued by. ;)

> 
> > 21) I see the m4/ folder inside the source package. Are you using automate
> > (autoconf) for anything? Because if you do, you probably need additional
> > BuildRequires for 'autoconf'...
> 
> The tarball uses standard './configure && make && make check && make
> install'.
> The autoconf stuff is required for developers, but it's not
> needed after the tarball has been built. This is the same as for gawk.
> If gawk.spec doesn't need it, then neither do we.

Yes, the 'gawk' does not require the 'autoconf' anymore. Thank you for
clarification. :)

> > 24) Do you have any documentation for the library, for example in some markdown
> > format? If so, it would be good to add it into the sourceball, because after it
> > we could transform the markdown into man page... :)
> 
> No, I don't have any additional documentation. This is intended for gawk
> extension library developers, not end users, so I must confess that the
> documentation is not great. There are extensive comments in the gawkextlib.h
> header file. That should be adequate for a developer trying to use this,
> particularly when combined with examples from other projects.

Thanks for the info, and all the changes you have made to the spec to confirm
to FPG rules. I really like it. :)

The main (only?) question right now remains on how do you decide to proceed
with the naming of packages (see above).

I will be looking for hearing from you! ;)

Dee'Kej

-- 
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