Re: [Libcdio-devel] [PATCH 2/5] Add extract ISO/UDF example

[Pete Batard]

On 2012.02.12 14:53, Rocky Bernstein wrote:
> I get a failure running the check_udf.sh created by this patch:

Well, there are some UDF patches missing from -pbatard, so the test is 
not supposed to work at this stage. It is added so that we can check 
that UDF extraction works _when_ it is actually meant to work.

My plan is to add the tests so that once we fix things, we can get some 
validation. But that doesn't mean that the test should PASS or even work 
right ahead. As previously indicated, I have quite a few fixes for the 
tests planned, that aren't in pbatard-integration yet, because I would 
really like us to first go through these 5 patches before processing 
some more.

> The test works  in the pbatard branch; one difference between the two are
> the unions added to inlcude/cdio/ecma_167.h and corresponding code
> adjustments.

Seems logical. Again, this is the beginning of integration and there is 
quite some way to go to get everything working...

Remember, that this first salvo of patches was meant to stop short of 
fixing the tests and doesn't include any of the necessary changes for core.

> Not strictly related. I have made some changes to extract.c to report
> filenames more often on error.

OK.

> And that reminds me, looking at extract.c I see that creates and writes are
> preformed using the full path. That's fine, but in those cases where there
> are lots of files to extract and the path is very long, it used to be the
> case that you can speed things up (at least on Unix) by doing a CD into the
> directory and then doing relative (not absolute) opens and writes. I have
> no idea if it will make a difference here, but if speed is a concern, this
> is one thing to consider.

Thanks for the pointer. Doesn't seem to be necessary right now, but I'll 
keep that in mind.

> Another change to pbatard-integration was to correct the failing
> check-cue.sh test. This change I think was already in pbatard, although it
> wasn't in master which it should have been. So master has been changed too.

I was planning to poduce a fix for check-cue.sh.in patch in the next salvo!

If there's anything already in pbatard that I haven't pushed into 
integration but that you want to see integrated, I'd really appreciate 
if you let me know, as I need to adjust my plans then...

This patch was not pushed because it pertains to the fixing of the 
tests, which is not the goal of the current 5 preliminary patches.

If you don't want to do it like this, that's fine, but please let me 
know so that I can produce patches in an order that we can all agree on.

Right now, I am still not planning to push anything more until the 5 
patches I have proposed have been reviewed and integrated (or redone if 
required), and this is regardless of whether some tests are failing or 
not. If this is not what you have in mind, then we need to review what 
the order should be...

> And finally in pbatard and pbatard-integration cdio_config.h how has the
> CDIO_ prefixes in #defines.

OK. I only see it in pbatard right now which is fine, as I'd rather you 
don't touch pbatard-integration actually (but you can push as many 
changes as you want to pbatard)

Whatever is in pbatard will end up into pbatard-integration one way or 
another, and subsequently as an official patch proposal for 
mainline/master. It may not be in the order you would prefer, but it 
will happen.
Also be mindful that I may very well revert commits from 
pbatard-integration on a whim if needed (or delete commit proposals and 
pick the ones from mainline once integrated) => the less you push there 
the better. In fact, I'd appreciate if you don't push anything in 
pbatard-integration without first letting me know. Better consider it a 
personal workdir that also happens to be public...

Now, if there are any modifications you would like to see on the 5 
proposed patches, let me know. Otherwise, I'll wait for you to integrate 
them before starting producing fixes for the tests.

Regards,

/Pete