[fpc-pascal] Re: Yet more make file problems: make works, make install fails

[Reinier Olislagers]

On 6-10-2012 14:34, 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.
>>>
>>> I can get it to compile with fpc make all
>>> However make install fails with
<snip>
>>> Patch including instructions at
>>> https://bitbucket.org/reiniero/fpc_laz_patch_playground/downloads/dblistener.zip
>>>
>>>
>>> What am I doing wrong?
> 
> The change you did in fpmake.pp looks correct.
> Do not change the Makefile. Just change fpmake.pp, though. That should
> be enough.
Ok.
> 
> But about the rest, there is lots to say :/
No problem.

> 
> 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.
Ok.
> 
> 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.
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?

> 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)
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>
> 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.
Ah, nice one, thanks :)

> 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.
Certainly, thanks.

Thanks for having a look.