Quantcast

[PATCH] gbuild conversion: registry module

classic Classic list List threaded Threaded
5 messages Options
David Ostrovsky David Ostrovsky
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate
star

[PATCH] gbuild conversion: registry module

Hi,

unfortunately gerrit is out of sync now, so i can not push it for review.
I have some questions to this patch:

1.
Test and workben directories are not used: let the old makefiles in place?

2.
What is this for:
CDEFS += -DDLL_VERSION=$(EMQ)"$(DLLPOSTFIX)$(EMQ)"

3.
Where these versions are used?
.INCLUDE :  ..$/version.mk

4. Have no idea what is this for => ignored ;-)
makedocpp: $(DOCPPFILES)
      docpp -H -m -f  -u -d $(OUT)$/doc$/$(PRJNAME) $(DOCPPFILES)

5. how to handle this?
TARGET=reg
[...]
.IF "$(COM)" == "MSC"
SHL1IMPLIB= ireg
.ELSE
SHL1IMPLIB= $(TARGET)
.ENDIF

Thanks
David


_______________________________________________
LibreOffice mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice

0001-gbuild-conversion-registry-module.patch (31K) Download Attachment
Matúš Kukan Matúš Kukan
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate
star

Re: [PATCH] gbuild conversion: registry module

Hi David,

On 5 May 2012 00:21, David Ostrovsky <[hidden email]> wrote:
> I have some questions to this patch:
>
> 1.
> Test and workben directories are not used: let the old makefiles in place?

Either that or remove the directories completely.
I've been keeping them untouched.

> 2.
> What is this for:
> CDEFS += -DDLL_VERSION=$(EMQ)"$(DLLPOSTFIX)$(EMQ)"

probably nothing

> 3.
> Where these versions are used?
> .INCLUDE :  ..$/version.mk

nowhere I guess

> 4. Have no idea what is this for => ignored ;-)
> makedocpp: $(DOCPPFILES)
>     docpp -H -m -f  -u -d $(OUT)$/doc$/$(PRJNAME) $(DOCPPFILES)

hmm, this is probably useless, so ok to ignore

> 5. how to handle this?
> TARGET=reg
> [...]
> .IF "$(COM)" == "MSC"
> SHL1IMPLIB= ireg
> .ELSE
> SHL1IMPLIB= $(TARGET)
> .ENDIF

You don't need to do anything about this.
'i' prefix is default for .lib file. Otherwise you would need to touch
RepositoryFixes.mk.

Your patch looks good, only few comments:
 - AFAICS rdbedit is not delivered so can't be used (see prj/d.lst)
 - there was no regcpp library, only archive used to create reg
library (which is already registered)
     -> I think that it has worked for you because you have not done
build from clean ?
- maybe registry_helper could be called registry_common or just
registry ? But I don't care too much.

Otherwise it could be good, though I can't get it build on windows and
it does not print reasonable error.
ireg.lib is missing but I don't know why.

Thanks,
Matus
_______________________________________________
LibreOffice mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice
Matúš Kukan Matúš Kukan
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate
star

Re: [PATCH] gbuild conversion: registry module

On 5 May 2012 02:47, Matúš Kukan <[hidden email]> wrote:
> Otherwise it could be good, though I can't get it build on windows and
> it does not print reasonable error.
> ireg.lib is missing but I don't know why.

Oh, and of course there is this sometimes painful DLLPUBLIC change.
You need to do something as in
http://cgit.freedesktop.org/libreoffice/core/commit/?id=cf77e8a0b9dc26d5007c76388c3f09231f048bdd

Create registrydllapi.h file, use it, deliver, ...

Thanks,
Matus
_______________________________________________
LibreOffice mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice
David Ostrovsky David Ostrovsky
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate
star

Re: [PATCH] gbuild conversion: registry module

Hi Matúš,

thank you very much for your review and your help (on IRC).
I updated my patch accordingly.

Ciao
David

On 05.05.2012 16:46, Matúš Kukan wrote:

> On 5 May 2012 02:47, Matúš Kukan<[hidden email]>  wrote:
>> Otherwise it could be good, though I can't get it build on windows and
>> it does not print reasonable error.
>> ireg.lib is missing but I don't know why.
> Oh, and of course there is this sometimes painful DLLPUBLIC change.
> You need to do something as in
> http://cgit.freedesktop.org/libreoffice/core/commit/?id=cf77e8a0b9dc26d5007c76388c3f09231f048bdd
>
> Create registrydllapi.h file, use it, deliver, ...
>
> Thanks,
> Matus

_______________________________________________
LibreOffice mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice

0001-gbuild-conversion-registry-module.patch (55K) Download Attachment
David Tardon David Tardon
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate
star

Re: [PATCH][PUSHED] gbuild conversion: registry module

On Sat, May 05, 2012 at 11:09:33PM +0200, David Ostrovsky wrote:
> Hi Matúš,
>
> thank you very much for your review and your help (on IRC).
> I updated my patch accordingly.

Pushed, thanks!

D.
_______________________________________________
LibreOffice mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice
Loading...