Bug 1241383

Summary: Review Request: mkchroot - Fedora Chroot Directory Maker
Product: [Fedora] Fedora Reporter: Mosaab Alzoubi <moceap>
Component: Package ReviewAssignee: Nobody's working on this, feel free to take it <nobody>
Status: NEW --- QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: i, marcin.haba, package-review, zbyszek
Target Milestone: ---   
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:

Description Mosaab Alzoubi 2015-07-09 06:55:19 UTC
Spec URL: http://ojuba.org/test/mkchroot.spec
SRPM URL: http://ojuba.org/test/mkchroot-0.0.1-1.oj35.noarch.rpm
Description: Fedora Chroot Directory Maker
Fedora Account System Username: moceap

Comment 1 Mosaab Alzoubi 2015-07-09 06:56:51 UTC
*** Bug 1241382 has been marked as a duplicate of this bug. ***

Comment 2 Marcin Haba 2015-07-10 21:39:54 UTC
Hello,

It is informal review due to I am not Fedora packages maintainer.

Few comments:

License:
- License is in PDF format instead of text format (waqf2-ar.pdf)
- License WAQFv2 is provided in an Arabic language.
- I have no idea about this license WAQF2. Could you tell if this license is pointed on following link: https://fedoraproject.org/wiki/Licensing:Main?rd=Licensing#SoftwareLicenses
- In LICENSE file is text "Licened under WAQFv2 or GPLv3". When your script is provided on WAQFv2 and when on GPLv3?
- In file LICENSE in text "Licened under WAQFv2 or GPLv3" the word "Licened" should be "Licensed".
- 

SRPM:
- You attached link to RPM, not SRPM.

Script:
- Yum command has been deprecated. You can try to use dnf.
- mkchroot requires root account, so the script should be placed in /usr/sbin, not /usr/bin
- script does not provide any interface (at least like: usage:, --help, --version ...etc.).
- script requres to run (or look inside script) for know how it works and what it do.
- this command: "yum -y --nogpg ..." can be dangerous because it skips checking GPG signatures.
- in case validation error always is returned exitcode 0 instead of exitcode > 0

Comment 3 Mosaab Alzoubi 2015-07-13 22:16:57 UTC
Changed when I process another bug :)

Comment 4 Zbigniew Jędrzejewski-Szmek 2015-07-16 14:54:01 UTC
Why --nogpgcheck?

Comment 5 Marcin Haba 2015-07-16 15:19:01 UTC
@Zbigniew
Is it question to Mosaab or to me?

Comment 6 Zbigniew Jędrzejewski-Szmek 2015-07-16 15:33:17 UTC
To Mosaab.

Comment 7 Mosaab Alzoubi 2015-07-17 05:01:50 UTC
Where is it ?

Comment 8 Marcin Haba 2015-07-17 11:25:09 UTC
@Mosaab

It is in rpm in /usr/bin/mkchroot:47:

yum -y --nogpg --installroot="$directory" --disablerepo=* --enablerepo=fcdm install yum

Comment 9 Mosaab Alzoubi 2015-07-17 16:28:11 UTC
Mmmmmmmmmmm I thought in SPEC.

Using --nogpg due to errors in GPG checks when building chrootdir for another Fedora version.

Comment 10 Zbigniew Jędrzejewski-Szmek 2015-07-17 17:09:04 UTC
I don't think this makes sense to make this into a package. We are moving to dnf, so new dependencies on yum should not be added. dnf deals with chroot installation just fine, without any futher configuration:

On F21:
$ sudo dnf --installroot=`pwd`/test/ --disablerepo=* --enablerepo=fedora --releasever=22 install dnf 
Fedora 22 - x86_64                                              5.0 MB/s |  41 MB     00:08    
Using metadata from Fri Jul 17 12:56:43 2015 (0:00:28 hours old)
Dependencies resolved.
===========================================================================================
 Package             Arch              Version                  Repository         Size
===========================================================================================
Installing:
 acl                x86_64            2.2.52-7.fc22              fedora             76 k
...
Total                                                           2.8 MB/s |  68 MB     00:24     
warning: /var/tmp/test/var/cache/dnf/x86_64/22/fedora/packages/dnf-1.0.0-1.fc22.noarch.rpm: Header V3 RSA/SHA256 Signature, key ID 8e1431d5: NOKEY
Importing GPG key 0x8E1431D5:
 Userid     : "Fedora (22) <fedora@fedoraproject.org>"
 Fingerprint: C527 EA07 A934 9B58 9C35 E1BF 11AD C094 8E14 31D5
 From       : /etc/pki/rpm-gpg/RPM-GPG-KEY-fedora-22-x86_64
Is this ok [y/N]: y
Key imported successfully
Running transaction check
Transaction check succeeded.
Running transaction test
Transaction test succeeded.
Running transaction
  Installing  : libgcc-5.1.1-1.fc22.x86_64                            1/142 
...

So instead, this right command should be made more widely known. It can be added
to the wiki, maybe to dnf man page.

And the right command seems to be (note added updates repo):
dnf --installroot=<somewhere>/ --disablerepo=* --enablerepo=fedora --enablerepo=updates --releasever=<version> install <packages>

Comment 11 Marcin Haba 2015-08-05 17:10:47 UTC
@Mosaab,

What do you think about the idea from Zbigniew about add this modified command from your script to wiki and about propose dnf maintainers to put this command in dnf man page?

This ticket is not moving forward and without any action from your side the ticket will be still open for a long time.

Comment 12 Christopher Meng 2015-08-06 04:49:15 UTC
I don't think this package is useful, it's only your script, from my view I only see you enjoy pushing your own almost useless contents to repo and such scripts shouldn't be packaged.

+1 for Zbigniew and dnf is preferred.

Comment 13 Mosaab Alzoubi 2015-08-07 23:31:41 UTC
This project hosted on Github , All your suggestion could be merged .

Any fixes welcomed .

I don't think it's useless :)

Comment 14 Mosaab Alzoubi 2015-08-07 23:32:41 UTC
You can submit any change you think important on Github.

Comment 15 Christopher Meng 2015-08-08 01:03:32 UTC
I won't submit anything to things I consider useless, and your github project is not a project at all. Also I won't recommend installing it if people can do with bash directly. Make chroot environment can be done in various ways, your github one line script is worst.

Comment 16 Mosaab Alzoubi 2015-08-08 01:55:01 UTC
It's a try .. not perfect but works .. if you want to improve it , do that ..

Comment 17 Christopher Meng 2015-08-08 09:33:48 UTC
(In reply to Mosaab Alzoubi from comment #16)
> It's a try .. not perfect but works .. if you want to improve it , do that ..

The main point is, review queue in Fedora is being overrun by more and more stuffs like this. Fedora repo doesn't really need any 'try', the main aim is to provide high quality softwares instead of 'try'. Copr is the best place for such kind of stuffs. I don't see points of pushing any stuffs like this to official repo, or I would have long pushed tons of packages like this one.

Comment 18 Michael Schwendt 2015-08-08 10:42:02 UTC
> URL:		http://ojuba.org

If there's no web page for this project, pointing at the github repo home would be the way to go.


There are more capable tools like "mock" and "mach".

Disabling GPG check and bypassing mirror manager is problematic with regard to security.

The "SRPM URL:" line during review is supposed to point at the src.rpm package.


> Summary: Fedora Chroot Directory Maker

Ambiguous. Misleading. Directories are created with commands like "mkdir". The %description also doesn't expand on it. The included README is too brief. "to enabling yum command" is very vague. The script's goal is not explained. It uses Yum to install package "yum" (and any dependencies) into the chroot directory, but what exactly that will pull in and what it will result in is undefined. Does it guarantee that Fedora repo definition files will be installed? The script contains a couple of lines at the end that are commented out.


> License: GPLv3

That doesn't match %license.


/usr/bin/mkchroot seems under-developed, half-baked. Overwriting /etc/yum.repos.d/fcdm.repo and removing it afterwards is highly questionable, too. This is _not_ inside the chroot.

Else I agree with comment 17.

Comment 19 Marcin Haba 2015-08-08 11:34:16 UTC
I am very curious how many people will come here and will repeat the same points:

- used yum instead of dnf
- Srpm package is not real srpm
- disabled gpg check
- license tag does not match
...etc.

Dear reviewers, I would ask you about read all thread and answer in its context, before you write your comment. Then you save your time and time all people that monitors this ticket.

I like Zbigniew idea which shows what we CAN do with this package instead of what we CANNOT do.

If you think that there is not possible to do anything here with this package - please close this ticket.

Thank you in advance for taking any action with this ticket.

Comment 20 Michael Schwendt 2015-08-08 12:03:14 UTC
It doesn't work like that. I had read all comments before I've taken an independent look nevertheless. It has not been a full review, but some of the things I've pointed out have not been commented on before.

Comment 21 Marcin Haba 2015-08-08 12:12:04 UTC
(In reply to Michael Schwendt (Fedora Packager Sponsors Group) from comment #20)
> It has not been a full review, but some of
> the things I've pointed out have not been commented on before.

Yes, some of your points are new, thanks for them, rest is doubled.

Comment 22 Zbigniew Jędrzejewski-Szmek 2015-10-22 18:19:53 UTC
#1246701 is now finished, and it is possible to cleanly make a chroot with:

dnf -y --releasever=N --installroot=PATH --disablerepo=* --enablerepo=fedora install PACKAGE...

This command was already documented in systemd-nspawn(1), but I submitted https://github.com/systemd/systemd/pull/1646 to remove --nogpg and add --enablerepo=updates, based on the discussion in this ticket.

This script also supports the architecture. dnf still cannot do that, but there's a bug open.

Comment 23 Upstream Release Monitoring 2015-11-13 18:37:28 UTC
csutherl's scratch build of fedora-tomcat-epel?#9661f8da0d7e82b63ef65ebe467e0eb103ae624f for el6-candidate and git://pkgs.fedoraproject.org/fedora-tomcat-epel?#9661f8da0d7e82b63ef65ebe467e0eb103ae624f failed http://koji.fedoraproject.org/koji/taskinfo?taskID=11823023