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 730859 - MinGW build fails on missing atk.def
MinGW build fails on missing atk.def
Status: RESOLVED FIXED
Product: atk
Classification: Platform
Component: build
git master
Other Linux
: Normal normal
: ---
Assigned To: ATK maintainer(s)
ATK maintainer(s)
Depends on:
Blocks:
 
 
Reported: 2014-05-28 03:30 UTC by Benjamin Gilbert
Modified: 2014-06-03 16:00 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Fix for atk/Makefile.am (1.06 KB, patch)
2014-05-28 03:30 UTC, Benjamin Gilbert
committed Details | Review

Description Benjamin Gilbert 2014-05-28 03:30:06 UTC
Created attachment 277351 [details] [review]
Fix for atk/Makefile.am

Since 41442d82 we no longer generate atk.def, but we still read it when linking libatk on MinGW.  This causes a build failure:

      CC       atkwindow.lo
      CC       atk-enum-types.lo
    /usr/bin/i686-w64-mingw32-windres atk.rc atk-win32-res.o
      CCLD     libatk-1.0.la
    libtool: link: symbol file `atk.def' does not exist
    make[3]: *** [libatk-1.0.la] Error 1
Comment 1 Alejandro Piñeiro Iglesias (IRC: infapi00) 2014-05-28 15:31:19 UTC
Review of attachment 277351 [details] [review]:

Just testing, and the patch fails to apply with the last master. Could you update it?
Comment 2 Alejandro Piñeiro Iglesias (IRC: infapi00) 2014-05-28 15:33:26 UTC
CCing Chun-wei Fan, that is the one that have more experience with Win32 related compilation.
Comment 3 Benjamin Gilbert 2014-05-28 17:02:13 UTC
The patch was developed against 22d344af, which is still the latest master, so I'm not sure why it's not applying for you.  I just double-checked and it applies for me.
Comment 4 Alejandro Piñeiro Iglesias (IRC: infapi00) 2014-05-28 18:13:33 UTC
(In reply to comment #3)
> The patch was developed against 22d344af, which is still the latest master, so
> I'm not sure why it's not applying for you. 

My bad, I was applying on the wrong branch.

> I just double-checked and it applies for me.

Yes, it applies correctly, and I was able to compile and install atk without problems on a Linux environment. But I don't have the proper environment to test this on Win32, so if you don't mind, we will wait a little to see if Chun-wei Fan have something to say about the patch.

Again, sorry for my mistake.
Comment 5 Fan, Chun-wei 2014-05-30 09:37:32 UTC
Hi,

Sorry, I was away for a few days, so I wasn't able to see this bug until now.  I would not be the one that would know the best on MinGW builds of ATK (as I don't usually use it at all), so I can't really say much about it, so I think I might CC LRN (who does the MinGW builds) to see whether this would be okay for him as well.

With blessings, thank you!
Comment 6 LRN 2014-05-30 11:48:51 UTC
May i suggest using
> -export-symbols-regex '^atk_'
instead?
Comment 7 Benjamin Gilbert 2014-05-30 15:26:50 UTC
Since ATK is now using visibility-based symbol exporting (as of 41442d82, the commit that introduced the bug), isn't it true that we no longer need -export-symbols* at all?
Comment 8 LRN 2014-05-30 16:06:34 UTC
If ATK uses visibility the same way GTK does - then yes, no need for -export-symbols* of any kind.
Comment 9 Fan, Chun-wei 2014-06-03 08:31:37 UTC
Hi,

The original intent of the mentioned patch is to use visibility, just as what GLib and GTK+ does... the -export-symbols* (for MinGW builds) should not be needed, as far as I can tell.

With blessings!
Comment 10 LRN 2014-06-03 09:02:08 UTC
So, patch is OK. What do we do now? Its status is "reviewed" currently. Should someone change it to "ready to push" or something?
Comment 11 Alejandro Piñeiro Iglesias (IRC: infapi00) 2014-06-03 09:42:47 UTC
Review of attachment 277351 [details] [review]:

(In reply to comment #10)
> So, patch is OK. What do we do now? Its status is "reviewed" currently. Should
> someone change it to "ready to push" or something?

Just doing. Thanks everybody.
Comment 12 Fan, Chun-wei 2014-06-03 16:00:54 UTC
Hi,

I have pushed the patch as cbf8e493f, so closing this bug.

Thanks, with blessings!