Re: wxWidgets 2.9 build

From: Dave Page <dpage(at)pgadmin(dot)org>
To: Peter Geoghegan <peter(dot)geoghegan86(at)gmail(dot)com>
Cc: Guillaume Lelarge <guillaume(at)lelarge(dot)info>, pgadmin-hackers(at)postgresql(dot)org
Subject: Re: wxWidgets 2.9 build
Date: 2011-01-17 22:19:11
Message-ID: AANLkTin+KR6Ond9BBzUSNFst0pL2bAHPKZpRvxKSjjUE@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

On Mon, Jan 17, 2011 at 3:30 PM, Peter Geoghegan
<peter(dot)geoghegan86(at)gmail(dot)com> wrote:
>>> in pgsDictionaryGen.cpp, why do we do this?:
>>>
>>>                        wxString line;
>>>                        while ((line = text.ReadLine()) && !input.Eof())
>>>                        {
>>>                                ++result;
>>>                        }
>>>
>
>> It's getting late and I've had an extremely stressful weekend, so
>> maybe I'm missing something obvious but I don't see an issue there.
>> It's reading from the stream (text) line by line into a dummy variable
>> (line), and counting the lines until it gets to the end of the file
>> (input).
>
> Sorry to hear that. I just thought that it might not be prudent to
> rely on the implicit cast, and that it superficially looked like the
> classic "assignment operator in place of equality operator" mistake.
> Perhaps I should have been clearer on that.

Oh, I see what you mean. I was staring right through those additional
brackets :-). I'd break it out for clarity.

>> How did the OGL port go? I looked at that briefly, and had a rough
>> build in 10 minutes or so iirc. Oh, and in answer to your previous
>> comments on the distribution; you're quite correct - the wxWidgets
>> licence isn't suitable for distribution with pgAdmin. We'd have to get
>> the patch applied upstream, or host our own port. If we need to do the
>> latter, then I guess we should also setup our own GIT repo.
>
> I didn't look at it yet. I thought that it would be better to leave it
> until there was a consensus on how to deal with it, and just tackle
> the landslide of other compiler errors. Hopefully the fact that
> compilation of certain TUs is terminated when ogl.h can't be included
> isn't masking a whole lot more errors at this point. Have we
> considered how maintaining OGL ourselves will affect package
> maintainers and so on?

It'll annoy them for sure, but we don't really have any choice unless
we want to rip out the graphical explain tool, and query builder :-(

> IANAL, but it seems like a separate git repo is the way to go to
> protect ourselves from copyleft implications. It might be worth asking
> the author/maintainer to change the licence. That's highly unlikely to
> happen, but it's still worth asking the question.

Yeah - I imagine the code had a few contributors over the years. The
wxWidgets licence is a weird one - bizarrely, it allows closed source
developers to incorporate the code in their source trees, but not open
source guys using liberal licences like ours.

> We're down to just 25kb of make output when I haven't done a make
> clean first; otherwise, there'd be some additional warnings which I'll
> fix later. Plus, as I said, I still haven't looked at OGL, and we can
> take it that once we've dealt with that issue, more unrelated errors
> will occur as we get through those additional TUs.

:-)

> The fact that wxString::c_str() now returns a special proxy object is
> a pain. Because it doesn't have a compiler generated copy constructor
> or is composed of objects that don't, you cannot pass it to vararg
> functions, of which there seems to be a few. I'm cleaning that up by
> explicitly casting to const wxChar* (and generally not calling c_str()
> and not going through the proxy object). That seems to work fine.

I imagine that makes for a big (and ugly) patch - the main reason we
use c_str() is so we can pass wxStrings to variadics.

> However, it's not immediately clear how to do this in the bison
> grammar file pgsParser.yy; changing something like this:
>
> @@ -886,8 +886,8 @@ assign_element
>        | PGS_IDENTIFIER '[' expression ']' '[' expression ']'
> PGS_EQ_OP expression
>                                                                {
>
>  wxLogScriptVerbose(wxT("SET %s[%s][%s] = %s"),
> -
>                 $1->c_str(), $3->value().c_str(),
> -
>                 $6->value().c_str(), $9->value().c_str());
> +
>                  ((const wxChar*) $1), ((const wxChar*) $3->value()),
> +
>                  ((const wxChar*) $6->value()), ((const wxChar*)
> $9->value()));
>
> doesn't fix the problem. I still see the same error, like
> "pgscript/pgsParser.yy:494:92: error: cannot pass objects of
> non-trivially-copyable type ‘class wxCStrData’ through ‘...’".

I've never even looked at that code I don't think.

> This code in  ctlSQLBox::RegexFindText() errors:
>
> wxWX2MBbuf buf = (wxWX2MBbuf)wx2stc(text);
>
> wx2stc is now in private.h . See
> http://groups.google.com/group/wx-dev/browse_thread/thread/1477c44701f792ea/8205f32f8433b03b
> and please advise.

Hmm, I need to look into that some more when I'm a little more awake.
Probably not for a couple of days though unfortunately.

> For your information, I've attached the text of remaining errors. A
> few mysteries remain, such as why some classes that we were
> instantiating have become abstract. I haven't really looked into that
> yet. I'm going to give OGL some attention now.

OK - thanks!

--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Dave Page 2011-01-17 22:20:45 Re: wxWidgets 2.9 build
Previous Message Guillaume Lelarge 2011-01-17 22:13:28 Re: wxWidgets 2.9 build