[k3b] [Bug 372094] Please restore wodim support

[Leslie Zhai]

https://bugs.kde.org/show_bug.cgi?id=372094

Leslie Zhai  changed:

   What|Removed |Added

 Status|CONFIRMED   |RESOLVED
 Resolution|--- |FIXED

--- Comment #13 from Leslie Zhai  ---
https://quickgit.kde.org/?p=k3b.git=commit=abe673e2f1b9b016d3a9c3aaad42d808a80a0b4a

Migrating to libburnia ... https://bugs.kde.org/show_bug.cgi?id=137436

-- 
You are receiving this mail because:
You are watching all bug changes.

[k3b] [Bug 372094] Please restore wodim support

[Kevin Kofler]

https://bugs.kde.org/show_bug.cgi?id=372094

--- Comment #12 from Kevin Kofler  ---
Looks OK to me.

-- 
You are receiving this mail because:
You are watching all bug changes.

[k3b] [Bug 372094] Please restore wodim support

[Leslie Zhai]

https://bugs.kde.org/show_bug.cgi?id=372094

--- Comment #11 from Leslie Zhai  ---
Git commit abe673e2f1b9b016d3a9c3aaad42d808a80a0b4a by Leslie Zhai.
Committed on 09/11/2016 at 06:30.
Pushed by lesliezhai into branch 'cdrskin'.

Restore wodim support

Although wodim from cdrkit (CD only, DVD deprecated, unmaintained) was
unmaintained, Kevin Kofler suggested restoring it. Please review it, if
it looks good to you, I will merge to master.

CCMAIL: kevin.kof...@chello.at

M  +4-1libk3b/core/k3bdefaultexternalprograms.cpp
M  +1-1libk3b/jobs/k3bdvdcopyjob.cpp
M  +1-0libk3b/jobs/k3bmetawriter.cpp
M  +8-2libk3b/projects/datacd/k3bdatajob.cpp
M  +1-1libk3b/projects/k3bcdrecordwriter.cpp
M  +1-1src/k3bsystemproblemdialog.cpp
M  +2-1src/option/k3bexternalbinpermissionmodel.cpp

http://commits.kde.org/k3b/abe673e2f1b9b016d3a9c3aaad42d808a80a0b4a

-- 
You are receiving this mail because:
You are watching all bug changes.

[k3b] [Bug 372094] Please restore wodim support

[Kevin Kofler]

https://bugs.kde.org/show_bug.cgi?id=372094

--- Comment #10 from Kevin Kofler  ---
Also, please keep in mind that cdrtools/cdrkit is not only cdrecord/wodim, but
also mkisofs/genisoimage. You will also want to detect and use xorrisofs from
libburnia, which is likely a better mkisofs replacement than genisoimage.

-- 
You are receiving this mail because:
You are watching all bug changes.

[k3b] [Bug 372094] Please restore wodim support

[Kevin Kofler]

https://bugs.kde.org/show_bug.cgi?id=372094

--- Comment #9 from Kevin Kofler  ---
> but I want to ship still-maintaining && BUGFREE product to end-users.

I actually agree with you there. I really think we need to switch to libburnia.

You should just clearly communicate this to packagers when announcing the new
release, so we do not end up with things working poorly because wodim is
getting silently picked up due to outdated dependencies in the package.

You should also make sure that cdrskin is preferred over wodim even if wodim is
installed/symlinked as "cdrecord" (because updating K3b, even if we add a
Requires: cdrskin, is not magically going to make installed wodim packages go
away, we cannot really use Obsoletes or Conflicts to force it removed).

> Yes, I am editing libk3b/core/k3bdefaultexternalprograms.cpp for adding
> cdrskin to K3b::AbstractCdrtoolsProgram( QLatin1String( "cdrskin" ),
> QLatin1String("cdrecord") )

But that is only the easy part. You need to look at all the places where a
decision what burning program to prefer is made (or checks for "if burning
program == cdrecord, apply workaround foo"), make it prefer cdrskin for things
it is actually better at (remember that growisofs is unmaintained too, though
not as long as wodim, so cdrskin is the better choice if it works), disable
cdrecord-specific workarounds (start by looking at those workarounds already
disabled for wodim in the code you removed, but keep in mind that cdrskin is
not a fork of cdrecord as wodim is, so probably does not need ANY of the
cdrecord workarounds), etc.

I have seen a bunch of cdrecord workarounds while browsing K3b code in the
past, but unfortunately I do not remember the exact locations in the code. For
example, there is something that says that cdrecord fails at doing some things
for audio CDs in DAO mode, so cdrdao (another unmaintained tool (no news since
2009), that in addition also uses libraries from cdrtools/cdrkit, so also uses
the unmaintained wodim code) is used for those instead. (Searching for "cdrdao"
will probably find that particular one.) You really want to go through all the
burning code and check whether what it is doing still makes sense if cdrecord
is actually cdrskin.

-- 
You are receiving this mail because:
You are watching all bug changes.

[k3b] [Bug 372094] Please restore wodim support

[Leslie Zhai]

https://bugs.kde.org/show_bug.cgi?id=372094

--- Comment #8 from Leslie Zhai  ---
(In reply to Kevin Kofler from comment #7)
> And we will default to libburnia if you add tested support for it. But I
> object to removing wodim support before a replacement is ready. We cannot
> use cdrecord in Fedora.

Yes, I am editing libk3b/core/k3bdefaultexternalprograms.cpp for adding cdrskin
to K3b::AbstractCdrtoolsProgram( QLatin1String( "cdrskin" ),
QLatin1String("cdrecord") )

I might be too anxious ;-) sorry for my expression! but I want to ship
still-maintaining && BUGFREE product to end-users.
for example, transcode was unmaintained too, please see KDEBUG-360170
http://www.leetcode.cn/2016/08/k3b.html#bug I migrated it, *NOT* be merged
again, so deprecated transcode resolutely, end-users do *NOT* need unuseful
software even it still be packaged again and again...

-- 
You are receiving this mail because:
You are watching all bug changes.

[k3b] [Bug 372094] Please restore wodim support

[Kevin Kofler]

https://bugs.kde.org/show_bug.cgi?id=372094

--- Comment #7 from Kevin Kofler  ---
And we will default to libburnia if you add tested support for it. But I object
to removing wodim support before a replacement is ready. We cannot use cdrecord
in Fedora.

-- 
You are receiving this mail because:
You are watching all bug changes.

[k3b] [Bug 372094] Please restore wodim support

[Kevin Kofler]

https://bugs.kde.org/show_bug.cgi?id=372094

--- Comment #6 from Kevin Kofler  ---
Removing working detection code just because you did not understand what it
really does is not helpful. Especially when your "fixes" actually make the code
misbehave on our distribution. And answering a request to restore the code with
a "NOPE!" in ALL CAPS is also not helpful.

-- 
You are receiving this mail because:
You are watching all bug changes.

[k3b] [Bug 372094] Please restore wodim support

[Leslie Zhai]

https://bugs.kde.org/show_bug.cgi?id=372094

Leslie Zhai  changed:

   What|Removed |Added

 CC||xiangzha...@gmail.com

--- Comment #5 from Leslie Zhai  ---
Hi Kevin,

I am a newbie of K3b maintainer http://www.leetcode.cn/2016/08/k3b.html so
welcome to report bug to me ;-)

But Thomas Schmitt , the author of libburnia, have already reviewed my patch
https://bugs.kde.org/show_bug.cgi?id=367639#c14 *including*:

> Isn't your patch
>  "Use cdrecord for burning blueray instead of wodim."
>  http://commits.kde.org/k3b/fcb0ff1f36c319fd1e2b4bd92c2c08fdc690212c
> mislabelled ?
> Shouldn't it rather be
> "Use cdrecord for burning blueray and DVD instead of growisofs"
>?

And I am double sure I fixed it!

> DVD support in wodim is crap and BluRay support is nonexistent, but that's 
> why the code you removed, which prefers growisofs, is there!
> The only reason we need wodim at all is that growisofs does not support CDs

NOPE!
[R.E.Q]I will add libburnia backup backend W.I.P. *BUT* I will never ever use
wodim! I *hate* meaningless forked then unmaintained!

Regards,
Leslie Zhai - a KDE developer https://git.reviewboard.kde.org/users/lesliezhai/

-- 
You are receiving this mail because:
You are watching all bug changes.

[k3b] [Bug 372094] Please restore wodim support

[Rex Dieter]

https://bugs.kde.org/show_bug.cgi?id=372094

Rex Dieter  changed:

   What|Removed |Added

 Ever confirmed|0   |1
 CC||rdie...@math.unl.edu
 Status|UNCONFIRMED |CONFIRMED

--- Comment #4 from Rex Dieter  ---
marking confirmed, fedora will likely need this fixed before we can consider
shipping this new version

-- 
You are receiving this mail because:
You are watching all bug changes.

[k3b] [Bug 372094] Please restore wodim support

[Kevin Kofler]

https://bugs.kde.org/show_bug.cgi?id=372094

--- Comment #3 from Kevin Kofler  ---
Also, your change is labeled "Use cdrecord for burning blueray instead of
wodim.", but K3b did not attempt to use wodim to burn BluRay even before your
change. If you look more closely at the code you removed, you will notice that
it took care to prefer growisofs over wodim for all DVD and BluRay burning
tasks. (The only reason we need wodim at all is that growisofs does not support
CDs.) In fact, ironically, your change actually made K3b attempt to use wodim
for BluRay! (Then you added a followup to just unconditionally prefer
growisofs:
http://quickgit.kde.org/?p=k3b.git=commitdiff=f3f4602e30c67bf622b5a52ea39c78817b1dd020
, which I'm not sure is a good idea either. It is much better than wodim, but
is it better than current versions of cdrecord? Probably not. You will need to
revert that first so that you can revert fcb0ff1f.)

-- 
You are receiving this mail because:
You are watching all bug changes.

[k3b] [Bug 372094] Please restore wodim support

[Kevin Kofler]

https://bugs.kde.org/show_bug.cgi?id=372094

--- Comment #2 from Kevin Kofler  ---
And in the long run, to work with something maintained, see wishlist bug
#137436.

-- 
You are receiving this mail because:
You are watching all bug changes.

[k3b] [Bug 372094] Please restore wodim support

[Kevin Kofler]

https://bugs.kde.org/show_bug.cgi?id=372094

--- Comment #1 from Kevin Kofler  ---
I will also note that your changes have several logic errors:

-: K3b::AbstractCdrtoolsProgram( QLatin1String( "cdrecord" ),
QLatin1String( "wodim" ) )
+: K3b::AbstractCdrtoolsProgram( QLatin1String( "cdrecord" ),
QLatin1String("cdrecord") )

(2 times the same command, probably not what you want)

-// but prefer growisofs to wodim for DVDs
+// but prefer growisofs for DVDs
 if ( d->usedWritingApp == K3b::WritingAppAuto ) {
-if
(k3bcore->externalBinManager()->binObject("cdrecord")->hasFeature( "wodim" ))
-d->usedWritingApp = K3b::WritingAppGrowisofs;
-else
-d->usedWritingApp = K3b::WritingAppCdrecord;
+d->usedWritingApp = K3b::WritingAppCdrecord;

(The comment says "prefer growisofs", the code says "prefer cdrecord".)

but instead of fixing those errors, please revert this commit wholesale, or K3b
will have to be removed from Fedora.

-- 
You are receiving this mail because:
You are watching all bug changes.