search

juhp / cabal-rpm

Haskell Cabal RPM packaging tool
https://hackage.haskell.org/package/cabal-rpm
GNU General Public License v3.0
25 stars 8 forks source link

Do not write empty spec file when error occurs. #7

Closed jan-matejka closed 10 years ago

jan-matejka commented 10 years ago

Do not write empty spec file when error occurs.

yac@deathstar % ./dist/build/cblrpm/cblrpm spec xmobar
Warning: The package list for 'hackage.haskell.org' is 40 days old. Run 'cabal update' to get the latest list of available packages. xmobar.spec exists: creating xmobar.spec.cblrpm cblrpm: user error (repoquery: command not found) yac@deathstar % cat xmobar.spec yac@deathstar %

juhp commented 10 years ago

Thanks for the report.

I think I fixed this with commit 0451f4d. Can you try to see if that works for you?

I suspect there are probably more problems still lurking with cblrpm local and requires perhaps...

juhp commented 10 years ago

BTW is there an equivalent of repoquery for querying SuSE rpm repos?

I further improved the above fallback to rpm -q with a check that the devel library is installed in commit f718ad2 !

jan-matejka commented 10 years ago

Not fixed.

yac@deathstar % ls | gr xmoba  
--------------------------------------------------------------------------------
~/data/suse/cabal-rpm.git git:master [1]
yac@deathstar % ./dist/build/cblrpm/cblrpm spec xmobar
Warning: The package list for 'hackage.haskell.org' is 40 days old.
Run 'cabal update' to get the latest list of available packages.
Warning: repoquery: command not found
cblrpm: readProcess: rpm "-q" "--qf=%{name}" "-f" "/usr/lib64/libXrandr.so" (exit 1): failed
--------------------------------------------------------------------------------
~/data/suse/cabal-rpm.git git:master [1]
yac@deathstar % ls | gr xmoba
(standard input):19:xmobar.spec
--------------------------------------------------------------------------------
~/data/suse/cabal-rpm.git git:master
yac@deathstar % cat xmobar.spec
jan-matejka commented 10 years ago

There is repoquery in SuSE and it works for me.

Though it has to be installed extra and maybe some extra magic involving creating an index or something because it did not work for me right after install, but next day it just worked.

Anyway, it might be better to call rpm -qf on suse, however I'd say that's out of scope of this bug.

jan-matejka commented 10 years ago

Oh I see what you did. You can't handle an error condition by fixing it for one more system in ideal conditions.

There are more cases where the IO can fail and in that case, cabal-rpm should NOT write an empty file. This is what the bug is about, not about repoquery not working on suse.

juhp commented 10 years ago

Not sure if our comments crossed... did you test with commit f718ad2 ? which I believe fixes your "Not fixed." comment.

(I see: the Fedora package of cabal-rpm requires yum-utils which provides repoquery.)

juhp commented 10 years ago

Though maybe I should try to catch error during spec file generation: but I would rather no errors occurred. :)

roman-neuhauser commented 10 years ago

but I would rather no errors occurred.

i also want a pony :)

juhp commented 10 years ago

Or maybe line buffering would prevent the file from being empty :)

jan-matejka commented 10 years ago

Yes, I tested with latest master, however, on Gentoo ;p

Wheter there is repoquery or rpm doesn't matter. If an error occurs during generating spec file contents, there should be no file written on the file system at all.

juhp commented 10 years ago

Okay I take your point - I will think a bit more about how to improve this.

Maybe I will move all such processing to before opening the spec file.

These are still corner/edges cases in my opinion so I am not so worried about them.

juhp commented 10 years ago

Okay - how about now? :)

commit 043d51c

jan-matejka commented 10 years ago

fix confirmed. Thanks.

~/data/suse/cabal-rpm.git git:master 
yac@deathstar % ls | gr xmoba 
--------------------------------------------------------------------------------
~/data/suse/cabal-rpm.git git:master [1]
yac@deathstar % ./dist/build/cblrpm/cblrpm spec xmobar
Warning: The package list for 'hackage.haskell.org' is 40 days old.
Run 'cabal update' to get the latest list of available packages.
Warning: repoquery: command not found
cblrpm: readProcess: rpm "-q" "--qf=%{name}" "-f" "/usr/lib64/libXrandr.so" (exit 1): failed
--------------------------------------------------------------------------------
~/data/suse/cabal-rpm.git git:master [1]
yac@deathstar % ls | gr xmoba                         
--------------------------------------------------------------------------------
~/data/suse/cabal-rpm.git git:master [1]
yac@deathstar %
juhp commented 10 years ago

Thanks

BTW I don't really like this error:

cblrpm: readProcess: rpm "-q" "--qf=%{name}" "-f" "/usr/lib64/libXrandr.so" (exit 1): failed

but I am not quite sure what is causing it?

jan-matejka commented 10 years ago

Hum, yeah there's a number of things.

  1. the error you see is because I ran it on gentoo as that's simplest to reproduce the functionality this bug refers to. That errors is ok.
  2. You shouldn't have added rpm command as part of this fix as it's completely unrelated to this issue.

  3. If I understand correctly the code checks if repoquery is available and if not fallbacks to rpm. However, while this seems to work, it also shows 

    yac@linux-1e2q.dhcp.scz.suse.com % ./dist/build/cblrpm/cblrpm spec xmobar
Warning: repoquery: command not found

    Which is a little confusing. Also the logic of the solution seems too ad-hoc/whaterver-works-first. It appears rpm exists on fedora 20 as well, by default. Why not just use rpm and forget about repoquery or eventually fallback from rpm to repoquery in case older fedoras don't have rpm?

  4. Next time you could develop new code on a branch until the code is correct and then merge it into master.
juhp commented 10 years ago

Yeah I understand you tested on gentoo: good idea. But the point is I test for availability of the commands and the existence of the file before running rpm -qf .... So I am still puzzled.

Rpm is much older than repoquery and I suspect repoquery only exists on rpm based distros? Further I don't really expect normal users to use cabal-rpm on non-RPM systems, though if it can be easily supported I don't mind. Anyway I like your idea to use rpm first and then fallback to repoquery: this makes sense here since the results should be the same (assuming the system is up to date) and rpm is much cheaper than a cold repoquery of course. (Historic note: RPM was first introduced in Red Hat Linux long ago. :)

I implemented the reverse fallback (rpm -> repoquery) in commit ff652d8. I think this logic is improved at least and it is good since I had thought before about using rpm -q too but then forgotten about it. Thanks for the critical feedback.

jan-matejka commented 10 years ago

Cool. That seems to be working nicely.

So I am still puzzled.

I do have rpm installed on gentoo. It's not used for package management but can be used for other development purposes for rpm-based distros without having it actually installed.

juhp commented 10 years ago

Ah okay so then maybe "rpm -qf" is really failing on your Gentoo if there is no rpmdb say, then I agree the error is reasonable I guess.