[Bug 1483339] Review Request: kiwi - A flexible operating system image builder

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

 



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



--- Comment #17 from Fabio Valentini <decathorpe@xxxxxxxxx> ---
(In reply to Neal Gompa from comment #16)
> (In reply to Fabio Valentini from comment #15)
> > There is one obvious error:
> > 
> > 0) The python2-kiwi package isn't installable on rawhide.
> > 
> > This is caused by using python-NAME Requires in the python2 subpackage. You
> > must use the fully qualified python2-NAME requires, they're all available as
> > far as I can tell:
> > 
> > BR: python-devel -> python2-devel
> > 
> > and
> > 
> > python-docopt -> python2-docopt
> > python-future -> python2-future
> > python-PyYAML -> python2-pyyaml
> > python-requests -> python2-requests
> > python-setuptools -> python2-setuptools
> > python-six -> python2-six
> > python-pyxattr -> python2-pyxattr
> > 
> > (Please double-check. You could even switch to using
> > "python2dist(docopt)"-style dependencies, if you want to be fancy.)
> > 
> > 
> 
> Fixed.

Well, not completely fixed, there are still "BR: python-devel" and "BR:
python-setuptools".

And where have the other dependencies gone? Are they now automatocally covered
by invoking the python dependency generator?

> > There are some other issues:
> > 
> > 1) Can you update the package to the latest version (9.13.9) for the final
> > review? A koji scratch build of the "final" package would be nice, too -
> > just to see if it builds correctly on all arches.
> > 
> 
> It's not yet uploaded to PyPI, so I've done it for 9.13.7 (the latest
> uploaded version).

That's fine.

> > 2) The kiwi-specific virtual provides look strange. Why not use a format
> > like kiwi(image:docker)? Additionally, they are unversioned, which rpmlint
> > complains about. And what is the "Provides: kiwi-schema" for?
> > 
> 
> This is used by obs-build for doing the correct substitution rules for
> certain things, so I can't really change them.

Ack.

> > 3) fedora-review complains about the %defattr(), and claims it's not needed.
> > Please double-check, since I suspect this is a false positive.
> > 
> 
> I've dropped it as tftp-server is packaged slightly differently in Fedora.

Good.

> > 4) Please don't write "%package -n %{name}-cli". Just use "%package cli"
> > (and %description cli, %files cli, etc.). Same goes for -tools and -pxeboot
> > subpackages. I suspect this is left-over from the previous
> > "python-kiwi"-named packaging.
> > 
> 
> Fixed.

Ack.

> > 5) The user and group creation scriptlet in "%pre -n kiwi-pxeboot (-> "%pre
> > pxeboot", btw) doesn't match the example scriptlet in the Packaging
> > Guidelines. Please check against [0]. Also, "Requires(pre): shadow-utils" is
> > missing from "%package pxeboot".
> > 
> 
> Dropped as it's not needed for our tftp-server packaging.

Nice :)

> > 6) The -pxeboot subpackage doesn't install a LICENSE file. Every other
> > combination pulls in a LICENSE file, as far as I can tell.
> > 
> 
> Fixed.

Ack.

> > 7) The file "kiwi/xml_parse.py" has "#!/usr/bin/env python" shebang, which
> > is wrong and must be replaced by "#!/usr/bin/python3" (sic!) and the file
> > marked as executable, or the shebang should be removed entirely (for both
> > the python2 and python3 version). That file isn't even compatible with
> > python2, so ... I guess that should be fixed upstream.
> > 
> 
> Issue filed: https://github.com/SUSE/kiwi/issues/666

I commented on the issue. It looks like you will have to fix this downstream.

> > 8) dracut modules (in "/usr/lib/drac7t/modules.d/99kiwi-lib/*" have bash
> > shebangs, but aren't marked as executable. Is that correct?
> > 
> 
> Issue filed: https://github.com/SUSE/kiwi/issues/668

Ack.

> > 9) The symlinks for "kiwi-ng" and "kiwicompat" don't look like they are
> > created proplerly. rpmlint complains that they are dangling symlinks.
> > 
> 
> This is because the versioned binaries exist in python3-kiwi and the
> unversioned ones are in kiwi-cli. kiwi-cli requires python3-kiwi, so they'll
> be satisfied on install.

OK, I trust that you have verified that this works as it is supposed to.

> > 10) "tools/kversion.c" has the wrong FSF address.
> > 
> 
> Issue filed: https://github.com/SUSE/kiwi/issues/667

Ack.

> > 11) There are unowned directories left:
> > 
> >      Note: Directories without known owners:
> >      /usr/share/bash-completion,
> >      /usr/lib/dracut,
> >      /usr/share/bash-completion/completions,
> >      /usr/lib/dracut/modules.d
> > 
> > Add the missing "Requires:" tags to the appropriate packages, or if that
> > doesn't work, co-own the directories.
> > 
> 
> Fixed.
> 
> > 12) The "/var/lib/tftpboot" directory and some files are already owned by
> > other packages (tftp-server and cobbler):
> > 
> >      Note: Dirs in package are owned also by:
> >      /var/lib/tftpboot(tftp-server),
> >      /var/lib/tftpboot/boot(cobbler),
> >      /var/lib/tftpboot/pxelinux.cfg(cobbler)
> > 
> 
> Fixed.
> 
> ----------
> 
> Spec URL:
> https://copr-be.cloud.fedoraproject.org/results/ngompa/KIWI/fedora-rawhide-
> x86_64/00728607-kiwi/kiwi.spec
> 
> SRPM URL:
> https://copr-be.cloud.fedoraproject.org/results/ngompa/KIWI/fedora-rawhide-
> x86_64/00728607-kiwi/kiwi-9.13.7-0.fc29.2.src.rpm
> 
> Koji scratch build:
> https://koji.fedoraproject.org/koji/taskinfo?taskID=25743697

If you look at the koji task log, you'll see that there was an error where
python2-kiwi built differently on ppc64 and x86_64, which I think will be an
error for real package builds. It looks like the noarch package virtually
provides archful python-kiwi, which is plain wrong. It looks like the 
%python_provide macro fails here ...

There are three additional, minor things I would suggest to change:

13) You could drop the leading "A" from the summary tag. It's just noise.

14) Since the -cli subpackage provides %{name}, you could just drop the cli
subpackage entirely and put everything from it into the "main" kiwi package.
But that's up to you.

15) Since the package builds binaries from .c files with a Makefile, you will
have to add "BR: gcc" and maybe even "BR: make" eventually.


Once fedora-review finally finishes I can conclude the review, since most
issues have been addressed AFAICT.

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