java-gnome-hackers — Hacking on the bindings themselves: design issues and co-ordination

Re: [java-gnome-hackers] Makefile and ProgressBar patch

[Andrew C. <an...@op...>]

On Sat, 2009-02-21 at 19:08 +0100, Bruno Dusausoy wrote:
> Here's a patch that include two separate fixes[1]

Merged to 'mainline'. Thanks!

++

> Sorry but I forgot to release a patch before commiting other
> changes.

So it's ok; it's only a problem as the collection of revisions grows
larger. The more unrelated things that are in a branch being submitted
means a larger patch [diff, whatever] to review, and a large likelihood
that something in one area will require work, thus causing the entire
bundle to be rejected along with all the other, unrelated, areas.

Since the likelihood of something needing revision is very high, it's
best to keep the submissions orthogonal and discrete. (There can still
be lots of revisions per branch; that's just DVCS in action)

This makes the maintainer's job easier. And that's a Good Thing™.

> I still have a lot to learn 

Sure. Distributed version control is powerful; it takes a bit of getting
used to. Even more, it's not always [easily] possible to create
orthogonal branches because of the fact that we work incrementally on
various things simultaneously.

If you've got unrelated things moving at the same time, one thing you
can try is:

a) create a bunch of branches

$ cd ~/src/java-gnome
$ bzr branch mainline fix-typo
$ bzr branch mainline improve-progress

b) switch to it

$ cd ~/workspace/java-gnome
$ bzr switch fix-typo
$ bzr add filename
$ bzr commit filename

c) go back to what you were working on

$ bzr switch improve-progress
$ edit filename
$ bzr commit filename

and repeat.

Now, this sequence means you won't see the revision created in
'fix-typo', so what I then find myself doing as a second layer is having
a working branch where I merge all the various things I'm working on,
switching back to a specific submission branch momentarily when it's
time to commit something that is [only] relevant to that feature.

$ cd ~/src/java-gnome
$ bzr branch mainline working      # you probably already have one
$ cd ~/workspace/java-gnome
$ bzr switch working
$ bzr merge improve-progress 
$ bzr merge fix-typo
$ make test
$ bzr commit

and then carry on with whatever application development you're doing
elsewhere, knowing that the Eclipse Project "java-gnome" at
~/workspace/java-gnome has everything you currently need in it.

Of course, if you're code formatting manually that means you're not in
Eclipse, which is fine of course; the principles all still apply - your
primary working branch, where ever it is, has all the pieces you need
merged into it, while you create individual branches for various
discrete orthogonal features you are aggregating so they can be
submitted as clean patches to the upstream project.

Anyway, this is all fairly advanced and it is not always necessary to go
to this level of complexity. `bzr switch` is magic, though, and if you
are doing unrelated things to unrelated files, if you take the trouble
to create separate branches you're more likely to get those changes
accepted.

AfC
Sydney

[java-gnome-hackers] Makefile and ProgressBar patch

[Bruno D. <bdu...@be...>]

Hi all,

Here's a patch that include two separate fixes[1] :

- A typo in the Makefile at line 43. It should read $(DESTDIR) instead
of $(DESDIR);
- The missing getText() method in ProgressBar class. 

I've formatted the source code as described on Andrew's blog[2].

Hope this patch is ok.

[1] Sorry but I forgot to release a patch before commiting other
changes. I still have a lot to learn from bzr :(
[2]
http://research.operationaldynamics.com/blogs/andrew/software/java-gnome/eclipse-code-format-from-command-line.html
-- 
Bruno Dusausoy <bdu...@be...>

[java-gnome-hackers] improve debug experience

[Andrew C. <an...@op...>]

Those of you who have followed the development of our thread safety
mechanisms over the years will be aware that we wrap every call into GTK
with a synchronized block. The key one turns out to be the one around
our call to gtk_main(), ie, in GtkMain.java we see:

    synchronized (lock) {
        gtk_main();
    }

nothing earth shattering here.

When debugging in Eclipse, I've been noticing something a bit strange of
late. Everytime I'm debugging something deep down inside some
application of mine, and I do something that throws an uncaught
exception (me throwing IllegalStateException or whatever) Eclipse's
debugger was stopping with its current instruction pointer at the
synchronzied block in GtkMain, not at whereever the exception was thrown
as we're all used to seeing.

Weird.

At first I thought it was a behaviour change on Eclipse's part, but
hunting around for a configuration flag to change it back to "normal"
didn't turn anything up. And then I noticed that this was only happening
in java-gnome programs, not others.

A lot of searching around the net led me to the conclusion that this is
deliberate on Eclipse's part - apparently this sort of stopping
behaviour helps when debugging thread lock problems.

Ok, fair enough.

But that's not the sort of problem we're dealing with here; and
meanwhile it's screwing up normal debugging (where it is really handy to
get a thread suspended right in the method where the exception gets
thrown, not umpteen layers back with most of the stack unwound)

Hm.

I experimented with things a bit, and came to a somewhat surprising
notion for how to work around this.

People who are *really* detail oriented might have seen the notes about
how the stack thinks that that thread holds the Gdk lock when it
doesn't. We did EXTENSIVE testing on that a couple years ago, and it's
all good - the actual blocked, holding, and waiting states are all
correct [even better, Eclipse gets it right, actually; it knows which
thread really holds the lock].

For some time I've mumbled in the back of my mind that maybe we should
take the cosmetic step to take the monitor on the JNI side before
calling the gtk_main() function, which would result in the thread stack
dumps looking a smidge more correct. I resisted the temptation because
entering that lock before calling gtk_main() is *SO* important and
having it appear exactly the same as every other native call seemed a
good idea.

But having run into this strange debugging behaviour, I remembered this
temptation and for the heck of it tried it out.

Sure enough, Eclipse's debugger now behaves the way I'd expect it to. I
tested rather extensively.


=== modified file 'src/bindings/org/gnome/gtk/GtkMain.java'
     static final void main() {
-        synchronized (lock) {
-            gtk_main();
-        }
+        // monitor entry on JNI side
+        gtk_main();
+        // monitor exited
     }
 
     private static native final void gtk_main();


=== modified file 'src/bindings/org/gnome/gtk/GtkMain.c'
 {
 	// call function
+	gdk_threads_enter();
 	gtk_main();
+	gdk_threads_leave();
 }


So the question I have to ask: is this a good idea?

That it seems to result in a better debugging experience for people
developing an application that happens to have its UI in java-gnome is
obviously something very desirable.

The fact that it breaks the parallel clarity that we strive incredibly
hard to maintain throughout the library bothers me very much.

Doing it this way is still "correct" as far as thread safety goes; I
have no worries about that. It just breaks symmetry. Ok, no big deal,
but given that more than any other call in the library having this call
inside the lock is the most crucial aspect of achieving thread safety,
it bothers me to be other than crystal clear about it. And while
development-experience-in-IDE is something we've heavily optimized for,
I don't necessarily want to see us jumping though hoops like this just
because of strange things Eclipse is doing this week.

But before accepting it for 'mainline' I thought I would mention what I
had run into and see if anyone else actively developing java-gnome
applications or hacking on the library itself had any thoughts.

[If someone can tell me an Eclipse setting I've overlooked then that
would simplify matters :)]

The patch is at:
bzr://research.operationaldynamics.com/bzr/java-gnome/hackers/andrew/monitor-entry-jni/

AfC
Sydney

Re: [java-gnome-hackers] Adding switch-page signal

[Andrew C. <an...@op...>]

On Wed, 2009-02-18 at 00:10 +0100, Stefan Schweizer wrote:
> I did the documentation and the test.

Terrific!

Should you do work like this in the future, please add things like
@since tags. I took care of it for you this time - you've already put a
huge amount of work into this and it's otherwise ready. So,

Merged to 'mainline'.

Well done. Thanks for your persistence on this one.

AfC
Sydney

Re: [java-gnome-hackers] Adding switch-page signal

[Stefan S. <ste...@gm...>]

On Tue, 2009-02-17 at 19:34 +1100, Andrew Cowie wrote:
> Stefan, if you take that branch and do the work to document the public
> API you wish to expose and to finish up the test case you started on,
> then I'd say we can merge it to 'mainline' fairly quickly.

I did the documentation and the test.

	Stefan

Re: [java-gnome-hackers] Adding switch-page signal

[Andrew C. <an...@op...>]

On Tue, 2009-02-17 at 13:42 +1100, Andrew Cowie wrote:
> My guesses are

Ok, it's mostly us, with a tinsy bit of GTK:

> a) something GTK is doing

They marshal the parameter as a G_TYPE_POINTER which is as about as
opaque and unusable as you can get. Certainly it is not the
GtkNotebookPage* that they said it was in the .defs data.

> b) something in our signal marshaling code is choking and causing us to
> skip that parameter. That'd be good if we were doing it on purpose.

But we're not. We were skipping it as a fallback in a default block in a
switch statement, and worse losing track of the warning message. That's
all bad, more or less, so I did something about it. Building on Stefan's
work,
bzr://research.operationaldynamics.com/bzr/java-gnome/hackers/andrew/notebook-signals/

Stefan, if you take that branch and do the work to document the public
API you wish to expose and to finish up the test case you started on,
then I'd say we can merge it to 'mainline' fairly quickly.

AfC
Sydney

Re: [java-gnome-hackers] Adding switch-page signal

[Andrew C. <an...@op...>]

On Mon, 2009-02-16 at 20:30 +0100, Stefan Schweizer wrote:
> I am attaching a bundle with what I have
> so far, also a test that currently fails.

You're a star.

> Uncommenting the debug message reveals the following:
> Java method receiveSwitchPage
> with signature (Lorg/gnome/glib/Signal;JI)V
> to handle signal switch-page
> 
> So the page parameter is missing? I have no idea.

Wow. That is weird. Of all the things I expected to see, that was
certainly not it.

I mean, in essence, that's what we _want_ it to be, right? With that as
the signature we're being handed we could just edit that parameter out
of the (define-virtual) and it'd work, and no need for the private
handler class anything to trim the parameter. Which would be great.

So we're done!

Except for one thing.

I don't know why we're getting handed a parameter sequence that is
resulting in us calculating that signature. Where's the GtkNotebookPage*
parameter?

My guesses are that either,

a) something in GTK is acting to hide that parameter (unlikely, given
that pygtk see it). I read the source code in GTK for gtk/gtknotebook.c
and I don't see anything changing the signal at registration or emission
time.

or

b) something in our signal marshaling code is choking and causing us to
skip that parameter. That'd be good if we were doing it on purpose.

If we can work out which it is, then we can probably sort this out.

There's still a lingering doubt about whether this signal is actually
supposed to be public. Unfortunately GLib × gtkdoc doesn't have a
mechanism for not presenting signals that are actually internal.

AfC
Sydney

P.S. No need to address me as well. I'm subscribed to the mailing list.



> 
> 	Stefan
> ------------------------------------------------------------------------------
> Open Source Business Conference (OSBC), March 24-25, 2009, San Francisco, CA
> -OSBC tackles the biggest issue in open source: Open Sourcing the Enterprise
> -Strategies to boost innovation and cut costs with open source participation
> -Receive a $600 discount off the registration fee with the source code: SFAD
> http://p.sf.net/sfu/XcvMzF8H
> _______________________________________________ java-gnome-hackers mailing list jav...@li... https://lists.sourceforge.net/lists/listinfo/java-gnome-hackers
-- 
Andrew Frederick Cowie

Operational Dynamics is an operations and engineering consultancy
focusing on IT strategy, organizational architecture, systems
review, and effective procedures for change management: enabling
successful deployment of mission critical information technology in
enterprises, worldwide.

http://www.operationaldynamics.com/

Sydney   New York   Toronto   London

Re: [java-gnome-hackers] Adding switch-page signal

[Stefan S. <ste...@gm...>]

Thanks for your help, Andrew. I am attaching a bundle with what I have
so far, also a test that currently fails.

Uncommenting the debug message reveals the following:
Java method receiveSwitchPage
with signature (Lorg/gnome/glib/Signal;JI)V
in class FIXME
to handle signal switch-page

So the page parameter is missing? I have no idea.

	Stefan

Re: [java-gnome-hackers] Adding switch-page signal

[Andrew C. <an...@op...>]

On Sun, 2009-02-15 at 13:55 +0100, Stefan Schweizer wrote:
> On Sat, 2009-02-14 at 22:03 +1100, Andrew Cowie wrote:
> > That _might_ work. I kinda doubt it, actually, but it might do as a
> > kludge. A patch bundle to this effect is attached.
> 
> Unfortunately changing the parameter to glong did not work. I get a
> "java.lang.NoSuchMethodError: receiveSwitchPage" when connecting to the
> signal.

Do you have a TestCase that runs through this? 

testReactingToCursorPositionChanges() in ValidateTextBuffer is an
example of hooking up a signal that reacts to a programmatic event. If
you coded up something like that and sent us a bundle with it, then we
could see what you're seeing.

As for the NoSuchMethodError, that is what happens when the signature
doesn't precisely match. Java VMs are very precise that way. You might
have had something incorrect on the other side.

If you manually remove the comments hiding lines 521-522 in
src/jni/bindings_java_signal.c you'll see some debug info about this.

For example, GtkWidget.receiveDeleteEvent() is invoked with a signature
of

(Lorg/gnome/glib/Signal;JJ)Z

with the first J being the long for the pointer address of Widget
"source", and the second J being the long for the boxed Event "event". I
imagine you'll find it will attempt to find a method called
receiveSwitchPage() with a signature of

(Lorg/gnome/glib/Signal;JJI)V

if you change to glong like I suggested. So then the question would be
why the generated receiver method doesn't have a matching signature.

Actually, the real question is what *actually* it is being invoked as
(since you can't change what GLib thinks the parameters are); presumably
it's not what I speculated above since presumably having changed that
parameter to glong would have caused a receive of that signature no
problem.

Once you find out what is actually being composed, then can you come up
with something else for the .defs data that you can cause the signature
of the generated receiveSwitchPage() to match. Should be fairly straight
forward once you know what it's feeding you.

AfC
Sydney

Re: [java-gnome-hackers] Adding switch-page signal

[Stefan S. <ste...@gm...>]

On Sat, 2009-02-14 at 22:03 +1100, Andrew Cowie wrote:
> That _might_ work. I kinda doubt it, actually, but it might do as a
> kludge. A patch bundle to this effect is attached.

Unfortunately changing the parameter to glong did not work. I get a
"java.lang.NoSuchMethodError: receiveSwitchPage" when connecting to the
signal.

> Or, finally, the worst case scenario is that we have to create a new
> Boxed type NotebookPage. THAT will certainly work, and indeed is the
> most "proper" solution, although it'd be kinda silly to have to do so
> if
> we can avoid it. I've been working fairly hard to get _rid_ of no-op
> type declarations like this.

I also tried to create NotebookPage as new type but it resulted in a
NoSuchMethodError as well. Here is the defs file that I used:

(define-boxed NotebookPage
  (in-module "Gtk")
  (c-name "GtkNotebookPage")
  (gtype-id "G_TYPE_POINTER")
)

Maybe something is missing or wrong, especially I do not know what to
use for copy-func and release-func.

	Stefan

Re: [java-gnome-hackers] Adding switch-page signal

[Andrew C. <an...@op...>]

On Sat, 2009-02-14 at 10:50 +0100, Stefan Schweizer wrote:
> I would like to add the switch-page signal [1] of GtkNotebook but I got
> stuck. The generated interface has a FIXME data type for the page
> parameter which should be a GtkNotebookPage. The documentation says that
> it's an "opaque implementation detail".

Yup. Pretty awful of them to have exposed this in their public API.

[You often here us going on about how hard doing a language binding of a
GNOME library is; in particular, C sugar leaking as public interfaces is
common and horrid to deal with]

>  From what I can see, this
> parameter is quite useless

From what I can tell it is entirely an implementation detail internal to
GtkNotebook.


> I already tried to create a defs-file for GtkNotebookPage but did not
> know which parent class I should use.

Right. So, I think the right way to deal with this is:

        a) not to expose it,

and

        b) do whatever minimum kludging is necessary to let us marshal
        it successfully.

The pygtk docs give me thought in the right direction: they point out
that it's a gpointer and unusable. No kidding. Anyway,

So to achieve (a) we need to do a wrapper like is done any time we need
to change the signature of a signal handler callback interface. See the
TextBuffer.InsertText signal for a recent example of trimming off an
unnecessary parameter. Follow that pattern and you'll be able to lose
the 2nd parameter of the 'switch-page' signature.

Dealing with the not-public type (b) will be a little tougher. This is
where we'll have to do something bad to trick out our code generator
into doing something that we can then ignore with (a). Picking a
compatible type and changing the .defs data for the switch-page virtual
*might* work.

The worrisome part here is that it's not just a matter of casting like
the forward direction usually is, but rather ensuring that the args
placed on the call stack for a C -> Java invocation (as happens when we
marshal a signal) don't cause a large splat when that invoke happens.

Fortunately, by careful design we only pass primitives across the JNI
boundary, so it's entirely possible that anything that (still) results
in a jlong being passed across (ie, what we wrap pointers in) will work.

So maybe try:

=== modified file 'src/defs/GtkNotebook.defs'
--- src/defs/GtkNotebook.defs	2009-02-01 05:36:52 +0000
+++ src/defs/GtkNotebook.defs	2009-02-14 10:50:45 +0000
@@ -343,7 +343,7 @@
   (of-object "GtkNotebook")
   (return-type "none")
   (parameters
-    '("GtkNotebookPage*" "page")
+    '("glong" "page")
     '("guint" "page_num")
   )
 )

That _might_ work. I kinda doubt it, actually, but it might do as a
kludge. A patch bundle to this effect is attached.

Slightly better would be to define something that would automatically
result in 0L being passed back for that parameter, regardless of what
type it is.

Or, finally, the worst case scenario is that we have to create a new
Boxed type NotebookPage. THAT will certainly work, and indeed is the
most "proper" solution, although it'd be kinda silly to have to do so if
we can avoid it. I've been working fairly hard to get _rid_ of no-op
type declarations like this.

See how you go.

++

Bad GTK hackers, letting internal structs be publicly visible. Bad. No
milk bone for you today.

AfC
Sydney

[java-gnome-hackers] Adding switch-page signal

[Stefan S. <ste...@gm...>]

Hi java-gnome hackers,

currently I am working on a personal application using java-gnome and I
found some parts of the toolkit that I need which are not yet exposed. 

I would like to add the switch-page signal [1] of GtkNotebook but I got
stuck. The generated interface has a FIXME data type for the page
parameter which should be a GtkNotebookPage. The documentation says that
it's an "opaque implementation detail". From what I can see, this
parameter is quite useless because the new page can also be obtained
from the page_num parameter. In gtk# this type is even deprecated.

I already tried to create a defs-file for GtkNotebookPage but did not
know which parent class I should use.

Does anyone have an idea what I can do about this parameter? Or can we
omit this parameter completely in the public API?

The other thing I am interested in is GtkSourceView. Is anyone working
or planning to work on coverage for this library?

	Stefan


[1]
http://library.gnome.org/devel/gtk/stable/GtkNotebook.html#GtkNotebook-switch-page

Re: [java-gnome-hackers] release candidate

[Serkan K. <se...@ge...>]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Serkan Kaba yazmış:
>> Thanks for the remarks and hints. After googling I found 2 solutions.
>> One[1] being yours and the other[2] being typecasting to a intermediate
>> void pointer. I tried the latter which at least stops the warning. But
>> Diego Petteno said that might be just silencing and hiding the real problem)
> 
>> 1: http://osdir.com/ml/java.classpath.patches/2004-08/msg00034.html
>> 2: http://mail.opensolaris.org/pipermail/tools-gcc/2005-August/000048.html
> I'm attaching a diff to show what I did
> 
I'm reattching it as a bundle.


- --
Sincerely,
Serkan KABA
Gentoo/Java
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.9 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iEYEARECAAYFAkmWgPAACgkQRh6X64ivZaJcswCfa/5UTaRZj4ndB/T5KFL2d/GA
OLwAn1wFJ1Ll2aH4RRdxhnlk9SCRppw7
=4/O4
-----END PGP SIGNATURE-----

[java-gnome-hackers] release candidate 2

[Andrew C. <an...@op...>]

I cut another release candidate; java-gnome 4.0.10-rc2 is up. The
library is in "works for me" state (I am fairly actively hacking on a
large code base which uses a lot of the things that entered java-gnome
this cycle, so I always keep an eye out to see if all our assumptions
are holding up).

All that really need doing now is for someone to write a nice
comprehensive NEWS entry reflecting what we've been up to. That, of
course, will likely be me, but I'm kinda busy right now so it might have
to wait a bit.

Anyway, we're otherwise release ready.

Naturally if someone has a bug or problem they want addressed there is
always no time like the present to speak up.

AfC
Sydney

-- 
Andrew Frederick Cowie

Operational Dynamics is an operations and engineering consultancy
focusing on IT strategy, organizational architecture, systems
review, and effective procedures for change management: enabling
successful deployment of mission critical information technology in
enterprises, worldwide.

http://www.operationaldynamics.com/

Sydney   New York   Toronto   London

Re: [java-gnome-hackers] release candidate

[Serkan K. <se...@ge...>]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

> Thanks for the remarks and hints. After googling I found 2 solutions.
> One[1] being yours and the other[2] being typecasting to a intermediate
> void pointer. I tried the latter which at least stops the warning. But
> Diego Petteno said that might be just silencing and hiding the real problem)
> 
> 1: http://osdir.com/ml/java.classpath.patches/2004-08/msg00034.html
> 2: http://mail.opensolaris.org/pipermail/tools-gcc/2005-August/000048.html
I'm attaching a diff to show what I did

- --
Sincerely,
Serkan KABA
Gentoo/Java
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.9 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iEYEARECAAYFAkmJ12gACgkQRh6X64ivZaIdSwCdHd/RnHJJoW7A6xVLXoSiRI67
bE4AmwY1M5Sc+j4QJo7wgwI5MJ6wza6s
=t1ND
-----END PGP SIGNATURE-----

Re: [java-gnome-hackers] release candidate

[Andrew C. <an...@op...>]

On Tue, 2009-02-03 at 10:16 +0100, Mark Wielaard wrote:

> The problem is the definition of GetEnv which takes a (void **) as
> second argument, but you are pointing it to a (JNIEnv **). I am not sure
> why GetEnv is defined like that. It doesn't seem particular useful....

I'm sure the way we're using it was copy & pasted from some example in
the book somewhere.

>  But
> there is nothing you can do about that now since it is a standard
> interface definition.

That made me wonder if nonetheless we were doing it wrong.

>   union env_union
>   {
>     void **void_env;
>     JNIEnv **jni_env;
>   };
> 
>   e.jni_env = &env;
>   result = (*cachedJavaVM)->GetEnv (cachedJavaVM,
>                                     e.void_env,
>                                     JNI_VERSION_1_4);


The idea of using a union to hint the compiler is really interesting.
Thanks!

Instead of that, however, can we not just cast? I tried this:

=== modified file 'src/jni/bindings_java_util.c'
--- src/jni/bindings_java_util.c	2009-02-01 14:53:00 +0000
+++ src/jni/bindings_java_util.c	2009-02-04 06:54:56 +0000
@@ -50,11 +50,13 @@
 bindings_java_getEnv()
 {
 	JNIEnv* env = NULL;
+	void* ptr = NULL;
 	JavaVMAttachArgs args = { 0, };
 	static int i = 0; 
 	jint result;
 
-	result = (*cachedJavaVM)->GetEnv(cachedJavaVM, (void **) &env, JNI_VERSION_1_4);
+	result = (*cachedJavaVM)->GetEnv(cachedJavaVM, &ptr, JNI_VERSION_1_4);
+	env = (JNIEnv*) ptr;
 	if (env != NULL) {
 		return env;
 	}
@@ -64,7 +66,8 @@
 		args.version = JNI_VERSION_1_4;
 		args.name = g_strdup_printf("NativeThread%d", i++);
 
-		result = (*cachedJavaVM)->AttachCurrentThreadAsDaemon(cachedJavaVM, (void **) &env, &args);
+		result = (*cachedJavaVM)->AttachCurrentThreadAsDaemon(cachedJavaVM, &ptr, &args);
+		env = (JNIEnv*) ptr;
 		if ((result == JNI_OK) && (env != NULL)) {
 			g_free(args.name);




and the warnings have gone away for me. Any reason that isn't ok? This
sort of casting is, after all, how the entire java-gnome library works.

A bundle with this patch is attached. Give it a try?

AfC
Sydney

-- 
Andrew Frederick Cowie

Operational Dynamics is an operations and engineering consultancy
focusing on IT strategy, organizational architecture, systems
review, and effective procedures for change management: enabling
successful deployment of mission critical information technology in
enterprises, worldwide.

http://www.operationaldynamics.com/

Sydney   New York   Toronto   London

Re: [java-gnome-hackers] release candidate

[Serkan K. <se...@ge...>]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Mark Wielaard yazmış:
> On Tue, 2009-02-03 at 17:47 +1100, Andrew Cowie wrote:
>> On Tue, 2009-02-03 at 07:25 +0200, Serkan Kaba wrote:
>>
>>> Sorry I was wrong in the initial finding -O2 reveals the issue.
>> Yup, I can duplicate it now. Well done.
>>
>> Of course I'm none the wiser what that problem means. I wonder what
>> we're doing wrong.
>>
>>> I'll investigate why it does in the evening.
>> Thanks! This is just the sort of thing that can really screw up other
>> people, so I'd like to clear it up if we can.
>>
>> My guess would be that we're doing the idiom of using the JavaVM pointer
>> incorrectly, or are caching it incorrectly, or...
> 
> The problem is the definition of GetEnv which takes a (void **) as
> second argument, but you are pointing it to a (JNIEnv **). I am not sure
> why GetEnv is defined like that. It doesn't seem particular useful. But
> there is nothing you can do about that now since it is a standard
> interface definition. So, this is triggering alarm bells with gcc, with
> -O2 or higher you get:
> 
> -fstrict-aliasing
>      Allows the compiler to assume the strictest aliasing rules
>      applicable to the language being compiled.  For C (and C++), this
>      activates optimizations based on the type of expressions.  In
>      particular, an object of one type is assumed never to reside at
>      the same address as an object of a different type, unless the
>      types are almost the same.
>      [... more examples in the gcc info and man pages ...]
> http://gcc.gnu.org/onlinedocs/gcc/Optimize-Options.html#index-fstrict_002daliasing-751
> 
> You can help gcc out by telling it "yes, this (void **) and (JNIEnv **)
> really point at the same thing" by putting them in a union and accessing
> both through that union:
> 
>   union env_union
>   {
>     void **void_env;
>     JNIEnv **jni_env;
>   };
> 
>   e.jni_env = &env;
>   result = (*cachedJavaVM)->GetEnv (cachedJavaVM,
>                                     e.void_env,
>                                     JNI_VERSION_1_4);
> 
> Then gcc knows that &env and e.void_env really point to the same thing.
> 
> Cheers,
> 
> Mark
Thanks for the remarks and hints. After googling I found 2 solutions.
One[1] being yours and the other[2] being typecasting to a intermediate
void pointer. I tried the latter which at least stops the warning. But
Diego Petteno said that might be just silencing and hiding the real problem)

1: http://osdir.com/ml/java.classpath.patches/2004-08/msg00034.html
2: http://mail.opensolaris.org/pipermail/tools-gcc/2005-August/000048.html
- --
Sincerely,
Serkan KABA
Gentoo/Java

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.9 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iEYEARECAAYFAkmIqmwACgkQRh6X64ivZaIZTQCfW0qvLgpmu2VwaPX0qj3GuL6c
BWwAn0eSpIUWUedlngBdiFeI0MBzdSxN
=pgao
-----END PGP SIGNATURE-----

Re: [java-gnome-hackers] release candidate

[Mark W. <ma...@kl...>]

On Tue, 2009-02-03 at 17:47 +1100, Andrew Cowie wrote:
> On Tue, 2009-02-03 at 07:25 +0200, Serkan Kaba wrote:
> 
> > Sorry I was wrong in the initial finding -O2 reveals the issue.
> 
> Yup, I can duplicate it now. Well done.
> 
> Of course I'm none the wiser what that problem means. I wonder what
> we're doing wrong.
> 
> > I'll investigate why it does in the evening.
> 
> Thanks! This is just the sort of thing that can really screw up other
> people, so I'd like to clear it up if we can.
> 
> My guess would be that we're doing the idiom of using the JavaVM pointer
> incorrectly, or are caching it incorrectly, or...

The problem is the definition of GetEnv which takes a (void **) as
second argument, but you are pointing it to a (JNIEnv **). I am not sure
why GetEnv is defined like that. It doesn't seem particular useful. But
there is nothing you can do about that now since it is a standard
interface definition. So, this is triggering alarm bells with gcc, with
-O2 or higher you get:

-fstrict-aliasing
     Allows the compiler to assume the strictest aliasing rules
     applicable to the language being compiled.  For C (and C++), this
     activates optimizations based on the type of expressions.  In
     particular, an object of one type is assumed never to reside at
     the same address as an object of a different type, unless the
     types are almost the same.
     [... more examples in the gcc info and man pages ...]
http://gcc.gnu.org/onlinedocs/gcc/Optimize-Options.html#index-fstrict_002daliasing-751

You can help gcc out by telling it "yes, this (void **) and (JNIEnv **)
really point at the same thing" by putting them in a union and accessing
both through that union:

  union env_union
  {
    void **void_env;
    JNIEnv **jni_env;
  };

  e.jni_env = &env;
  result = (*cachedJavaVM)->GetEnv (cachedJavaVM,
                                    e.void_env,
                                    JNI_VERSION_1_4);

Then gcc knows that &env and e.void_env really point to the same thing.

Cheers,

Mark

Re: [java-gnome-hackers] release candidate

[Andrew C. <an...@op...>]

On Tue, 2009-02-03 at 07:25 +0200, Serkan Kaba wrote:

> Sorry I was wrong in the initial finding -O2 reveals the issue.

Yup, I can duplicate it now. Well done.

Of course I'm none the wiser what that problem means. I wonder what
we're doing wrong.

> I'll investigate why it does in the evening.

Thanks! This is just the sort of thing that can really screw up other
people, so I'd like to clear it up if we can.

My guess would be that we're doing the idiom of using the JavaVM pointer
incorrectly, or are caching it incorrectly, or...

AfC
Sydney

Re: [java-gnome-hackers] release candidate

[Serkan K. <se...@ge...>]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Sorry I was wrong in the initial finding -O2 reveals the issue. I'll
investigate why it does in the evening.

- --
Sincerely,
Serkan KABA
Gentoo/Java
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.9 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iEYEARECAAYFAkmH1UcACgkQRh6X64ivZaKuSgCdFYXDNtDINnvlPrPbe/8HjW/n
S74AnA4xXnec7vd3hqJ0Rgmw9BqKGbby
=k2yQ
-----END PGP SIGNATURE-----

Re: [java-gnome-hackers] release candidate

[Andrew C. <an...@op...>]

On Tue, 2009-02-03 at 06:49 +0200, Serkan Kaba wrote:
> optimizing for core2 (-march=nocona) reveals the warning.

Hm. I just tried that (adding it manually to the CC variable in .config)
and it again compiles here with no warning, sorry.

I wonder what else it might be. 32bit vs 64bit, maybe?

Perhaps if you were to post the full GCC command that your are running
(trying using

$ V=1 make

to see) we might spot something. Mine was:

$ rm tmp/objects/bindings_java_util.o
$ V=1 make
build/faster
GCC	src/jni/bindings_java_util.c
/usr/bin/ccache /usr/bin/gcc -march=nocona -g -Wall -fPIC
-I/opt/sun-jdk-1.6.0.11/include -I/opt/sun-jdk-1.6.0.11/include/linux
-Wno-int-to-pointer-cast -Wno-pointer-to-int-cast
-Werror-implicit-function-declaration -Wfatal-errors -Isrc/jni
-Itmp/include -pthread -I/usr/include/glib-2.0
-I/usr/lib/glib-2.0/include -I/usr/include/gtk-2.0
-I/usr/lib/gtk-2.0/include -I/usr/include/atk-1.0 -I/usr/include/cairo
-I/usr/include/pango-1.0 -I/usr/include/pixman-1
-I/usr/include/freetype2 -I/usr/include/libpng12
-I/usr/include/gtk-unix-print-2.0 -I/usr/include/libglade-2.0
-I/usr/include/libxml2 -o tmp/objects/bindings_java_util.o -c
src/jni/bindings_java_util.c
$

AfC
Sydney

Re: [java-gnome-hackers] release candidate

[Serkan K. <se...@ge...>]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Andrew Cowie yazmış:
> I just did:
> 
> moonglow:~/workspace/java-gnome $ rm tmp/objects/bindings_java_util.o 
> moonglow:~/workspace/java-gnome $ make 
> GCC	src/jni/bindings_java_util.c
> moonglow:~/workspace/java-gnome $
> 
> No warnings.
> 
> So what is Portage doing that a straight up from-source build is not?
> 
> As for the lines in question, as far as I can tell those are the correct
> idiom for making calls through the JavaVM interface struct.
> 
> AfC
> Sydney
optimizing for core2 (-march=nocona) reveals the warning.
- --
Sincerely,
Serkan KABA
Gentoo/Java
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.9 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iEYEARECAAYFAkmHzNgACgkQRh6X64ivZaJ61ACfWqRTXdOAGfk8lbNwAkzd4KQw
LigAn1oDWKbDQrCkTRAne7W0EpjkHwvs
=9dmE
-----END PGP SIGNATURE-----

Re: [java-gnome-hackers] release candidate

[Andrew C. <an...@op...>]

On Mon, 2009-02-02 at 21:02 +0200, Serkan Kaba wrote:
> Portage extracted following warning out of gcc. Can we ignore them?
>  * QA Notice: Package has poor programming practices which may compile
>  *            fine but exhibit random runtime failures.
>  * src/jni/bindings_java_util.c:57: warning: dereferencing type-punned
> pointer will break strict-aliasing rules
>  * src/jni/bindings_java_util.c:67: warning: dereferencing type-punned
> pointer will break strict-aliasing rules

I just did:

moonglow:~/workspace/java-gnome $ rm tmp/objects/bindings_java_util.o 
moonglow:~/workspace/java-gnome $ make 
GCC	src/jni/bindings_java_util.c
moonglow:~/workspace/java-gnome $

No warnings.

So what is Portage doing that a straight up from-source build is not?

As for the lines in question, as far as I can tell those are the correct
idiom for making calls through the JavaVM interface struct.

AfC
Sydney

Re: [java-gnome-hackers] release candidate

[Serkan K. <se...@ge...>]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Andrew Cowie yazmış:
> I merged a number of pending branches and did a release candidate.
> Version 4.0.10-rc1 is up at
> http://ftp.gnome.org/pub/gnome/sources/java-gnome/4.0/
> 
> As ever,
> 
> $ bzr diff -r tag:v4.0.9
> 
> is your friend.
> 
> AfC
> Sydney
Portage extracted following warning out of gcc. Can we ignore them?
 * QA Notice: Package has poor programming practices which may compile
 *            fine but exhibit random runtime failures.
 * src/jni/bindings_java_util.c:57: warning: dereferencing type-punned
pointer will break strict-aliasing rules
 * src/jni/bindings_java_util.c:67: warning: dereferencing type-punned
pointer will break strict-aliasing rules

- --
Sincerely,
Serkan KABA
Gentoo/Java
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.9 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iEYEARECAAYFAkmHQ1kACgkQRh6X64ivZaJFbgCfaUR9kIkV6ZSXTgegqER2WHzH
X6cAmwbB9erebnRclJOqNYIwBdO5qajr
=r452
-----END PGP SIGNATURE-----

[java-gnome-hackers] release candidate

[Andrew C. <an...@op...>]

I merged a number of pending branches and did a release candidate.
Version 4.0.10-rc1 is up at
http://ftp.gnome.org/pub/gnome/sources/java-gnome/4.0/

As ever,

$ bzr diff -r tag:v4.0.9

is your friend.

AfC
Sydney