[Bug 1195279] Review Request: preupgrade-assistant - Preupgrade assistant a tool for assess system before an upgrade

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

 



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



--- Comment #22 from Zbigniew Jędrzejewski-Szmek <zbyszek@xxxxxxxxx> ---
(In reply to Petr Hracek from comment #21)
> Spec URL: https://phracek.fedorapeople.org/preupgrade-assistant.spec
> SRPM URL:
> https://phracek.fedorapeople.org/preupgrade-assistant-0.11.7-4.fc21.src.rpm
It seems that the problem from comment #c3 has resurfaced. In F23 mock:

running build_ext
Traceback (most recent call last):
  File "setup.py", line 61, in <module>
    test_suite      = 'tests.suite',
  File "/usr/lib64/python3.4/distutils/core.py", line 148, in setup
    dist.run_commands()
  File "/usr/lib64/python3.4/distutils/dist.py", line 955, in run_commands
    self.run_command(cmd)
  File "/usr/lib64/python3.4/distutils/dist.py", line 974, in run_command
    cmd_obj.run()
  File "/usr/lib/python3.4/site-packages/setuptools/command/test.py", line 142,
in run
    self.with_project_on_sys_path(self.run_tests)
  File "/usr/lib/python3.4/site-packages/setuptools/command/test.py", line 122,
in with_project_on_sys_path
    func()
  File "/usr/lib/python3.4/site-packages/setuptools/command/test.py", line 163,
in run_tests
    testRunner=self._resolve_as_ep(self.test_runner),
  File "/usr/lib64/python3.4/unittest/main.py", line 92, in __init__
    self.parseArgs(argv)
  File "/usr/lib64/python3.4/unittest/main.py", line 139, in parseArgs
    self.createTests()
  File "/usr/lib64/python3.4/unittest/main.py", line 146, in createTests
    self.module)
  File "/usr/lib64/python3.4/unittest/loader.py", line 146, in
loadTestsFromNames
    suites = [self.loadTestsFromName(name, module) for name in names]
  File "/usr/lib64/python3.4/unittest/loader.py", line 146, in <listcomp>
    suites = [self.loadTestsFromName(name, module) for name in names]
  File "/usr/lib64/python3.4/unittest/loader.py", line 105, in
loadTestsFromName
    module = __import__('.'.join(parts_copy))
  File "/builddir/build/BUILD/preupgrade-assistant-0.11.7/tests/__init__.py",
line 2, in <module>
    from tests import test_preup
  File "/builddir/build/BUILD/preupgrade-assistant-0.11.7/tests/test_preup.py",
line 6, in <module>
    from preup.application import Application
  File
"/builddir/build/BUILD/preupgrade-assistant-0.11.7/preup/application.py", line
18, in <module>
    from preuputils.compose import XCCDFCompose
  File
"/builddir/build/BUILD/preupgrade-assistant-0.11.7/preuputils/compose.py", line
12, in <module>
    from preuputils.oscap_group_xml import OscapGroupXml
  File
"/builddir/build/BUILD/preupgrade-assistant-0.11.7/preuputils/oscap_group_xml.py",
line 13, in <module>
    from preuputils.xml_utils import print_error_msg, XmlUtils
  File
"/builddir/build/BUILD/preupgrade-assistant-0.11.7/preuputils/xml_utils.py",
line 11, in <module>
    from preuputils import script_utils
  File
"/builddir/build/BUILD/preupgrade-assistant-0.11.7/preuputils/script_utils.py",
line 6, in <module>
    import xml_utils
ImportError: No module named 'xml_utils'

> (In reply to Zbigniew Jędrzejewski-Szmek from comment #20)
> > - Why BR:pykickstart R:pykickstart in F23?
> > 
> You are right python3-kickstart should require pykickstart in F23
Are you sure? The only reason to do that would be if python3-kickstart called
binaries (ksflatten, ksshell, ksvalidator, ksverdiff) provided by pykickstart.

The reason for my original question was that this dependency seems spurious
since pykickstart is for Python2, and preupgrade-asistant is running under
Python3 in F23. In the spec file I see you moved R/BR:pykicstart underneath %if
0%{?fedora} == 22, so this seems fine now.

> > - preupgrade-assistant.rpm has an empty /usr/share/doc/preupgrade directory
> > and README files are in /usr/share/preupgrade.
> > 
> README is in /usr/share/doc/preupgrade directory.
> README file should be in both directories. The file is copied to
> /root/preupgrade/ directory after an assessment.
> User can see after a upgrade/migration what results mean.
> But I will corrected it after package review so that only one README file is
> enough.
I think it is fine as is. You have to be careful here because of the rule that
packages cannot require anything in %doc at runtime. So having two copies of
this very small file is probably the best option.

> > - Looking at the first message:
> > The Preupgrade Assistant is a diagnostics tool 
> > and does not perform the actual upgrade.
> > Make sure you back up your system and all of your data now,
> > before using the Upgrade Tool to avoid potential data loss.
> > Do you want to continue? y/n
> > 
> > If it is only a diagnostic tool, why is it unsafe to run?
> > Also, why does it require root privileges? Isn't the RPM database public?
> We need to have a access to all files and directories. Root access is really
> needed. Some data are accessible only under root account.
OK, so it needs to run as root. By why the scary warning?

> > - In the man page: "All common log files are stored in
> > /var/cache/preupgrade/common". Isn't /var/log/preupgrade used?
> Those directories are different.
> - /var/log/preupgrade directory is used for logs generated by
> preupgrade-assistant itself as a program
> - /var/cache/preupgrade/common directory contains special files used by
> contents
OK.

> > - I installed the rpm on F21. /usr/bin/preupg fails with:
> > Traceback (most recent call last):
> >   File "/usr/bin/preupg", line 8, in <module>
> >     from preup.application import Application
> >   File "/usr/lib/python2.7/site-packages/preup/application.py", line 18, in
> > <module>
> >     from preuputils.compose import XCCDFCompose
> > ImportError: No module named preuputils.compose
> > 
> > Installing also -devel subpackage fixes that. Maybe you should merge -devel
> > back into the main package. They are both really tiny, so the split is
> > probably not worth the trouble.
> > 
> Merged
Hm, I don't see this. In the spec file there's still "%package devel".

> > - This package seems useless without preupgrade-assistant-contents. Is this
> > available somewhere?
> 
> Draft for Packaging Guidelines is already finished
> (https://fedoraproject.org/wiki/User:Phracek/Draft:Packaging:
> PreupgradeAssistant)
> Currently now I would like to add it to
> http://fedoraproject.org/wiki/Packaging:
> Guidelines#Application_Specific_Guidelines
> 
> I will send a mail to devel list which describes how to create a content for
> preupgrade-assistant.
> 
> I will create several bugzilla's to specific components for creating
> contents. Likie mariadb, postgresql, etc.
OK, thanks.

Two comments on the guidelines:
The Python example does not take into account the Python2/Python3 split. I
think you must explain that the script must run under Python2 in F22 and under
Python3 in F23 (and have #!/usr/bin/python3). Alternatively, you could package
the module for Python2 in F23 too, in a python-preupgrade-assistant subpackage,
to allow people to use Python2 for the scripts in F22 and F23, or Python2 in
F22 and Python3 in F23.

%doc %{preupgrade_dir}/%{name}/*.txt → this looks wrong, packages are not
allowed to use files in %doc at runtime. The whole section can be simplified
to:
%files -n preupgrade-assistant-%{name}
%{preupgrade_dir}/%{name}/

-- 
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
https://admin.fedoraproject.org/mailman/listinfo/package-review





[Index of Archives]     [Fedora Legacy]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]