After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 629232 - Multiple syntax errors in file mutter-message.c when building Mutter for GNOME Shell dependencies
Multiple syntax errors in file mutter-message.c when building Mutter for GNOM...
Status: RESOLVED FIXED
Product: mutter
Classification: Core
Component: general
unspecified
Other Linux
: Normal blocker
: ---
Assigned To: mutter-maint
mutter-maint
Depends on:
Blocks:
 
 
Reported: 2010-09-10 04:13 UTC by Kenny Strawn
Modified: 2010-09-11 12:25 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
The source file that is causing the errors (4.94 KB, text/x-csrc)
2010-09-10 04:13 UTC, Kenny Strawn
  Details
Replace gdk_display (33.75 KB, patch)
2010-09-10 08:12 UTC, Florian Müllner
none Details | Review
Remove usage of 'gdk_display', a removed symbol in gtk3. (33.19 KB, patch)
2010-09-10 11:27 UTC, Jasper St. Pierre (not reading bugmail)
reviewed Details | Review
Remove usage of 'gdk_display', a removed symbol in gtk3. (33.26 KB, patch)
2010-09-10 16:47 UTC, Jasper St. Pierre (not reading bugmail)
reviewed Details | Review
Remove usage of 'gdk_display', a removed symbol in gtk3. (32.90 KB, patch)
2010-09-10 19:03 UTC, Jasper St. Pierre (not reading bugmail)
committed Details | Review

Description Kenny Strawn 2010-09-10 04:13:33 UTC
Created attachment 169922 [details]
The source file that is causing the errors

When I tried building the latest GNOME Shell using jhbuild: the build got up to the point where it had to build Mutter and then errored out:

make (in directory: /home/kenny/gnome-shell/source/mutter/src/tools)
  CC     mutter-message.o
mutter-message.c: In function ‘send_restart’:
mutter-message.c:41: error: ‘gdk_display’ undeclared (first use in this function)
mutter-message.c:41: error: (Each undeclared identifier is reported only once
mutter-message.c:41: error: for each function it appears in.)
mutter-message.c: In function ‘send_reload_theme’:
mutter-message.c:68: error: ‘gdk_display’ undeclared (first use in this function)
mutter-message.c: In function ‘send_set_keybindings’:
mutter-message.c:96: error: ‘gdk_display’ undeclared (first use in this function)
mutter-message.c: In function ‘send_toggle_verbose’:
mutter-message.c:125: error: ‘gdk_display’ undeclared (first use in this function)
make: *** [mutter-message.o] Error 1

Attached is the full source code. Please fix these errors ASAP, as I don't want to have to fix them myself. Modify the attached source if you wish and then reattach it.
Comment 1 Florian Müllner 2010-09-10 08:10:39 UTC
(In reply to comment #0)
> Attached is the full source code. [...] Modify the attached source if you wish and then
> reattach it.

Please don't do that - attach modifications as patches, never attach the whole modified file. And attaching an unmodified file from a git checkout is pointless.

Also note that the API break affects the following files too:

 src/core/main.c
 src/ui/frames.c
 src/ui/menu.c
 src/ui/tabpopup.c
 src/ui/ui.c
Comment 2 Florian Müllner 2010-09-10 08:12:38 UTC
Created attachment 169924 [details] [review]
Replace gdk_display

The variable has been removed in GTK+-3. Thanks sed!
Comment 3 Jasper St. Pierre (not reading bugmail) 2010-09-10 11:27:05 UTC
Created attachment 169940 [details] [review]
Remove usage of 'gdk_display', a removed symbol in gtk3.
Comment 4 Florian Müllner 2010-09-10 14:15:00 UTC
Review of attachment 169940 [details] [review]:

Looks mostly good to me, comments below.

::: src/tools/mutter-message.c
@@ +30,3 @@
 #define N_(x) x
 
+static Display *display;

I would prefer using a local display variable in each function, but well ...

::: src/ui/ui.c
@@ +127,3 @@
   ui->xscreen = screen;
 
+  g_assert (xdisplay == meta_ui_get_display ());

Hmm, meta_ui_get_display() is intended for files in the core/ subdirectory which may not include gtk/gdk. It is not wrong to use it here, but note how you don't do so in the other files either (and neither did the original code). So I'd prefer a direct GDK_DISPLAY_XDISPLAY() call here ...

@@ +409,3 @@
     {
       gtk_widget_hide (iw->window);
+      meta_core_increment_event_serial (meta_ui_get_display ());

... and here.
Comment 5 Jasper St. Pierre (not reading bugmail) 2010-09-10 16:47:36 UTC
Created attachment 169973 [details] [review]
Remove usage of 'gdk_display', a removed symbol in gtk3.
Comment 6 Jasper St. Pierre (not reading bugmail) 2010-09-10 16:49:25 UTC
Fixed meta_ui_get_display references

for the static Display, it should be a simple tool, so I'm not
sure if it's worth it to add local references in each function.
Comment 7 Florian Müllner 2010-09-10 17:05:12 UTC
Review of attachment 169973 [details] [review]:

(In reply to comment #6)
> for the static Display, it should be a simple tool, so I'm not
> sure if it's worth it to add local references in each function.

I don't like it but yeah, it's not that big of a deal. OK to commit with the style fix mentioned below.

::: src/ui/tabpopup.c
@@ +494,3 @@
       gdk_window_hide (window);
+      meta_core_increment_event_serial (
+      GDK_DISPLAY_XDISPLAY (gdk_display_get_default ()));

Missing indentation
Comment 8 Marina Zhurakhinskaya 2010-09-10 17:54:11 UTC
Review of attachment 169973 [details] [review]:

There is also a trailing whitespace in the patch.

/home/mazik/gnome-shell/source/mutter/.git/rebase-apply/patch:840: trailing whitespace.
  (* md->menu->func) (md->menu, 
warning: 1 line adds whitespace errors.
Comment 9 Jasper St. Pierre (not reading bugmail) 2010-09-10 19:03:42 UTC
Created attachment 169984 [details] [review]
Remove usage of 'gdk_display', a removed symbol in gtk3.
Comment 10 Kenny Strawn 2010-09-11 02:20:20 UTC
Was able to solve the problem by adding:

+  static Display *gdk_display;

at the beginning of the file, so that essentially gdk_display is a variable of type Display. Now, it compiles successfully.
Comment 11 Kenny Strawn 2010-09-11 03:15:14 UTC
Was able to find out that running jhbuild again replaced the file with one that has more errors:

mutter-message.c:33: error: expected identifier or ‘(’ before ‘<<’ token
mutter-message.c:35: error: expected identifier or ‘(’ before ‘==’ token
mutter-message.c:37: error: expected identifier or ‘(’ before ‘>>’ token
mutter-message.c: In function ‘send_reload_theme’:
mutter-message.c:74: error: ‘display’ undeclared (first use in this function)
mutter-message.c:74: error: (Each undeclared identifier is reported only once
mutter-message.c:74: error: for each function it appears in.)
mutter-message.c: In function ‘send_set_keybindings’:
mutter-message.c:102: error: ‘display’ undeclared (first use in this function)
mutter-message.c: In function ‘send_toggle_verbose’:
mutter-message.c:131: error: ‘display’ undeclared (first use in this function)
mutter-message.c: In function ‘main’:
mutter-message.c:170: error: ‘display’ undeclared (first use in this function)
cc1: warnings being treated as errors
mutter-message.c:173: error: implicit declaration of function ‘send_restart’
mutter-message.c:173: error: nested extern declaration of ‘send_restart’

How could the file have possibly compiled on your machines??
Comment 12 Kenny Strawn 2010-09-11 03:23:39 UTC
I found out the problem:

>>>>>>> Updated upstream
static Display *display;
=======
static Display *gdk_display;
<<<<<<< Stashed changes

W
T
F?

The compiler treated these as identifiers because the comment keyword wasn't used (//). This is BOUND to cause errors, not to mention break the variables that actually declare something around them.

So, I just simplified it to this:

//Updated upstream
static Display *display;
Comment 13 Florian Müllner 2010-09-11 07:20:47 UTC
(In reply to comment #11)
> How could the file have possibly compiled on your machines??

Because we did not do the modifications you did - fix your tree with "git checkout -f HEAD" (if you didn't commit your change) or "git reset --hard origin/master" (if you did).
Comment 14 Florian Müllner 2010-09-11 07:22:26 UTC
(In reply to comment #12)
> <<<<<<< Stashed changes

OK, git checkout -f HEAD it is.
Comment 15 Kenny Strawn 2010-09-11 12:25:31 UTC
Done.