Still fighting makefiles with my db fpcunit listener.

I can get it to compile with fpc make all
However make install fails with

Installation package fcl-extra for target x86_64-linux succeeded Start compiling package fcl-fpcunit for target x86_64-linux. Compiling fcl-fpcunit/BuildUnit_fcl_fpcunit.pp
Compiled package fcl-fpcunit
Installing package fcl-fpcunit
The installer encountered the following error:
Unable to open file "units/x86_64-linux/fpcunit.o"

Patch including instructions at
https://bitbucket.org/reiniero/fpc_laz_patch_playground/downloads/dblistener.zip

What am I doing wrong?

Thanks,
Reinier

Just to clarify:
1. I'd rather learn what I'm doing wrong and fixing it rather than dump my dblistener patch in the bugtracker for somebody else to fix
2. Given my notoriously bad memory & benefit to others, I'd be happy to update the fpmake wiki page if possible

Thanks,
Reinier

The change you did in fpmake.pp looks correct.
Do not change the Makefile. Just change fpmake.pp, though. That should be enough.

But about the rest, there is lots to say :/

For starters, you are creating a dependency on fcl-db in fcl-fpcunit. That's not correct, since there is an (implicit) dependency in fcl-db on fcl-fpcunit for testing, so you create a circular dependency.

The listener should reside in fcl-db, preferrably in a directory src/sqldb/fpcunit. This will make the dependency of fcl-db on fcl-fpcunit an explicit one.

Regarding the dbreporter unit itself:

I think the explicit dependency on IBConnection is misplaced. What is more, it will ONLY work on IB, the way you wrote it. Neither mysql or postgres or ms-sql or oracle will eat any of the statements that you use.

I would suggest only letting it be dependent on sqldb. It's the duty of the user to provide a connection class preferably by creating a descendent. You can provide descendents for the various TSQLConnection classes in different units, and let the user use the unit which uses the connection of his choice. (the descendents can also provide 'correct' SQL statements for that particular connection)

Concerning the routine:

procedure TDBResultsWriter.SetupCPU_OS;
begin
    //CPU/OS detectionM
   FCPU:='unknown';M
   {$IFDEF CPUI386}M
   FCPU:='i386';M
   {$ENDIF}M
   {$IFDEF CPUM68K}M
   // etc.
end;

It's far more compact to write this:

procedure TDBResultsWriter.SetupCPU_OS;
begin
   FCPU:={$I %FPCTARGETCPU%};
   FOS:={$I %FPCTARGETOS%};
end;

It has the benefit of remaining correct when new CPU/OS combinations are added.

I have a lot more remarks, which I will not write here in public.

From your zip, it looks like you wanted to submit the unit for inclusion in FPC ?

I'm sorry to say that, as it is, this unit will not make it in FPC. It needs a major redesign from the ground up.
I'm prepared to discuss it in private, if you're interested.

Michael.

<snip>

Ok.

But about the rest, there is lots to say :/

No problem.

Ok.

Completely correct. It is meant for automatically creating an embedded database.
It does not mess with client/server database servers as the DBA is supposed to set up the database there.
I don't mind having it ifdefed out but see no reason why this extra functionality for use with embedded dbs should be disabled?

As indicated in the code, it currently works with all sqldb connection components, using an ini file to specify connection string much like the db test framework.

The SQL that is used is kept basic on purpose to maintain compatibility. If people want to extend that, patches welcome.

Concerning the routine:

<snip>

Ah, nice one, thanks :)

Certainly, thanks.

Thanks for having a look.

Michael Van Canneyt wrote

On Sat, 6 Oct 2012, Reinier Olislagers wrote: > On 6-10-2012 6:52, Reinier Olislagers wrote: >> Still fighting makefiles with my db fpcunit listener.

<snip>

I don't really understand that.

Just confirmed: works fine on PostgreSQL[1] (as you may have noticed dbwriteraddlist.pas contains a reference to pqconnection). As far as I can see there's no explicit dependency on ibconnection: when running with PostgreSQL backend, process monitor shows no Firebird dlls being loaded (or attempted to be loaded), only PostgreSQL dlls. pgadmin shows the test results end up in the database.

Yes, I do need to add the other sqldb units to the uses clause of the listener class and tell the user in the readme he needs to modify his own test code uses clause to add non-FPC supplied sqldb units (if any) in order to load the relevant connector.

It is modelled after the db test framework for a reason: it allows easy setup by testers without having to change code in order to use a different database backend.

I see your point about inheritance and actually the code could quite well be split into a db selection (with support for ini files/db selection)/manager part, and the core with the rest of the functionality. DB specific core descendents could be set up if required.

However, I used simple SQL on purpose in order to make it as compatible as possible. It shouldn't be necessary to split out the code - but perhaps tests on other dbs may make that more desirable.

<snip>

Having read your private comments I think it could fit in quite well with FPC with some work, not a total rewrite.

However, it's not up to me and actions speak louder than words, so let's wait until I have improved it.

Regardless of what happens, I'll move the code from my general repository to one of it's own, together with a recent db test framework in order to get further feedback from people when running real-world tests (as well as improvements in adding views/SQL for regression detection).

Thanks,
Reinier

[1] Although fixes like this had to be made for PostgreSQL reporting "unknown column type"... these fixes work well on Firebird and I expect them to work elsewhere as well
- FQuery.Params[1].Clear //null
+ begin
+ FQuery.Params[1].Clear; //null
+ FQuery.Params[1].Datatype:=ftstring;
+ end

I have just read this thread and found the points mentioned interesting. I think it would have been better if the points were not discussed in private, that way everyone could learn from it.

Too late though :(