Take 1: This is a plain bug, short options should work. Needs investigation. Short options are heavily used in the unit tests and works there.

Take2: This message is probably straight from mock. Could you please attach the file ~/.cache/fedora-review.log after running this? My provisional idea about this is that mock works this way, and that f-r should not try to change these semantics

Take3: The user is the only one who knows what to do with the review directory (move or delete). To leave this decision to her is the Right Thing IMHO.

Take 4: As Take2: please attach ~/.cache/fedora-review.log for this case. Probably just how mock works(?)

This is where it gets interesting.

2) This works fine if I run mock from the CLI directly. That is in fact exactly what I did prior to running f-r.

You can supply a full path to mock for the config file. I do it all the time.

3) If you use mkstemp (or any other number of ways), the old directory will remain intact without any conflicts at all. Mandating that a specifically named directory exist and never be altered is not going to work. I can cause f-r to fail miserably this way .. Trivial. Please reconsider and use a new directory each time f-r is run upon the package.

4) No. Look at the path that f-r is trying to use. It is not using the file specified.

Again, all this works if you use mock directly.

I really want to see f-r work for everyone in a clean manner so there is a very short time between "please review my request" and "finished reviewing". This would be spectacularly awesome.

This code is not ready yet. I can pypuke it on demand. I have run it 8 times, and crashed it 8 times differently. Please, please review.

Several issues, indeed. First, let's focus on what's in updates-testing (the release candidate) since this is quite urgent:
{{{
sudo yum enablerepo=updates-testing update fedora-review
fedora-review <args>
}}}
If you use git, please use meaningful branches (probably release-0.2.0 or devel)

Using release candidate or git devel branch I can't reproduce the take1: statement about short options not being recognized:

{{{
$ fedora-review -m /home/user1/.mock/default.cfg -o '--uniqueext=user1 --resultdir=/home/user1/mock' -b 785606
Processing bugzilla bug: 785606
Bugzilla v0.6.2 initializing
Chose subclass RHBugzilla v0.3
Trying bugzilla cookies for authentication
Getting .spec and .srpm Urls from : 785606
--> SRPM url: http://nb.fedorapeople.org/horde-reviews/php-horde-Horde-Test-1.3.0-3.fc16.src.rpm
--> Spec url: http://nb.fedorapeople.org/horde-reviews/php-horde-Horde-Test.spec
[cut]
}}}

Secondly, whatever 'works', the mock manual page describes the -r argument (reflected from --mock-config) as

{{{
-r CHROOT, --root=CHROOT
Uses specified chroot configuration as defined in
/etc/mock/<chroot>.cfg. If none specified, uses the chroot
linked to by /etc/mock/default.cfg

}}}

The command line executed form f-r is

{{{
$ mock -r /home/user1/.mock/default.cfg --rebuild --uniqueext=user1 --resultdir=/home/user1/mock --no-cleanup-after /home/al/785606-php-horde-Horde-Test/srpm/php-horde-Horde-Test-1.3.0-3.fc16.src.rpm
}}}

Which gives the same result: Could not find required config file: /etc/mock//home/user1/.mock/default.cfg.cfg. So, this message is from mock, not f-r. Mock version is 1.1.21

Basically I think the situation is:
- Your mock installation seems to accept other arguments than 1.1.21. This is about take2 and take4.
- You want another handling of the "review dir exists" situation in take3. This is a separate issue. Please file separate RFE bug for this, it's hard to have many issues in the same bug.
- The "short options not recognized" error in take1: is not present in the release candidate or the git devel branch. Please file a separate bug also on this for the relevant version/branch for the same reasons.
- The review dir is created in current directory, this has nothing to do with the --resultdir mock options parameter (which is about mock's output dir, not f-r's). If you feel this is wrong, please file a separate bug also on this.

EDIT: update bottom summary (twice), mock version, git branch references.

== Take9: finally got it to run .. sort of. ==

{{{
$ git pull
Enter passphrase for key '/home/user1/.ssh/id_rsa_fas':
Already up-to-date.

$ ./try-fedora-review --version
fedora-review version 0.2.0 2fb2447 2012-07-22 18:33:00 +0200

$ mkdir ~/.cache

$ rm -rf munin;

$ ./try-fedora-review --mock-config fedora-16-x86_64-rpmfusion_nonfree --mock-options '--configdir=/home/user1/.mock --uniqueext=user1 --resultdir=/home/user1/mock' -n munin
Processing local files: munin
Getting .spec and .srpm Urls from : Local files in /home/user1/fedora-scm/FedoraReview
--> SRPM url: file:///home/user1/fedora-scm/FedoraReview/munin-2.0.4-1.fc16.src.rpm
--> Spec url: file:///home/user1/fedora-scm/FedoraReview/munin.spec
Using review directory: /home/user1/fedora-scm/FedoraReview/munin
Downloading (Source10): http://downloads.sourceforge.net/sourceforge/munin/munin-2.0.4.tar.gz.sha256sum
No upstream for (Source11): munin-node.service-privatetmp
No upstream for (Source2): munin-1.2.5-hddtemp_smartctl-config
No upstream for (Source3): munin-node.logrotate
Downloading (Source0): http://downloads.sourceforge.net/sourceforge/munin/munin-2.0.4.tar.gz
No upstream for (Source1): munin-1.2.4-sendmail-config
No upstream for (Source6): munin-1.2.6-postfix-config
No upstream for (Source7): munin-1.4.5-df-config
No upstream for (Source4): munin.logrotate
No upstream for (Source8): munin-node.service
No upstream for (Source9): munin.conf
Running checks and generate report

Rebuilding /home/user1/fedora-scm/FedoraReview/munin-2.0.4-1.fc16.src.rpm using mock root fedora-16-x86_64-rpmfusion_nonfree

INFO: Results and/or logs in: /home/user1/mock
Build completed
Install command returned error code 30
Exception down the road...
}}}

Manually creating ~/.cache/ in advance is still required. This was supposed to be fixed already, so is this a regression?

Again, this builds fine in mock but fails in f-r.

Actual mock command used:

{{{
$ rpm -q mock
mock-1.1.21-1.fc16.noarch

$ time mock -r fedora-16-x86_64-rpmfusion_nonfree --configdir=/home/user1/.mock --uniqueext=user1 --resultdir=/home/user1/mock
}}}

In theory, the mock command should match what f-r is using.

However, f-r is pulling in everything it finds in ~/mock/ for some strange reason and failing.

~/.cache/fedora-review.log file from the only exec that actually sort of almost ran, 9.
fedora-review.log

To be frank, current code just doesn't support --configdir (BTW, I note that this option wasn't there in your first attempts). On the contrary, current code always looks in /etc/mock.

Also, current code presumes that the resultdir is empty when f-r is invoked i. e., that anything in this directory is created by current f-r run. The behaviour for you is clearly a bug.

OTOH, the title of this bug is "Fails to use options listed". Since the discussion now is very different from this I suggest that we close this bug and that you

• File a new bug about the missing support for --configdir.
• File a new bug about failures when resultdir already has rpms in place.

BTW, seems that you still is running the master branch. As I said in comment:3, the state of this is more or less undefined. Try using the devel or a release branch (sorry if you already are doing that).

With this said, the status in comment:4 is likely to be the same IMHO: --configdir is unsupported, and a resultdir already containing rpm:s will cause problems.

re/--configdir mock option: You are correct, I am trying various options to have f-r actually run.

The release branch does not get as far as the git version. It insta-pukes instead.

To date, this has never actually run properly once nor generated any report.

Hm... it's still something in your environment and/or the way you use the thing which creates this. We actually have some users, and you are the only one reporting these serious problems. So, question is why (that's not to say there are no f-r bugs!).

To be fair, your statement "To date, this has never actually run properly once nor generated any report" is certainly true for you, but other people does not share this experience. Just look into some fresh review tickets...

It would be helpful if you started using f-r in a way which is more tested, say just using the -b option and then adding to your usecase until the bug(s) occur.

It would also be helpful if you closed this bug. After all, we are not discussing the title in any way (or can you still reproduce this?!) And then file more specific bugs e. g. as proposed in comment:5

Filed a new bug #117 for the --configdir issue

Filed yet another bug #125 for missing check that resultdir is empty.

I cannot see that this bug will lead to further progress in this shape. In particular, I have never been able to reproduce the very title for the bug.

Thanks for this report. I'm closing it, because I don't really know what to do otherwise. Please feel free to reopen bug you think this is necessary - however, as long as the command line parsing problem in the header cannot be reproduced, it's would be an advantage to file new bug(s) as required instead.

The invalid resolution should be interpreted as what's left after #125 and #117 is invalid - these issues are certainly valid.

Perhaps, also #127 is involved in this