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 741359 - check.h distributed unnecessarily
check.h distributed unnecessarily
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gstreamer (core)
git master
Other Linux
: Normal normal
: 1.5.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2014-12-10 20:43 UTC by Sebastian Rasmussen
Modified: 2014-12-16 15:33 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Proposed patch. (1008 bytes, patch)
2014-12-10 20:47 UTC, Sebastian Rasmussen
none Details | Review
Proposed patch. (7.05 KB, patch)
2014-12-14 13:11 UTC, Sebastian Rasmussen
none Details | Review
Proposed patch (6.97 KB, patch)
2014-12-15 21:33 UTC, Sebastian Rasmussen
committed Details | Review

Description Sebastian Rasmussen 2014-12-10 20:43:04 UTC
I found recently that check.h is distributed in the tarballs: ./gstreamer-1.5.0.1/libs/gst/check/libcheck/check.h This file is actually generated by configure from check.h.in in the same directory so it is not necessary to distribute it.

Moreover since the latest libcheck 0.9.14 updates the contents of this file have actually changed which means that there is a slim chance that you may see compilation errors.
Comment 1 Sebastian Rasmussen 2014-12-10 20:47:11 UTC
Created attachment 292484 [details] [review]
Proposed patch.
Comment 2 Sebastian Dröge (slomo) 2014-12-12 08:49:00 UTC
If that file is autogenerated, we should replace our internal-check.h file with that one. At least it looks like that is the same file :)
Comment 3 Tim-Philipp Müller 2014-12-13 13:45:36 UTC
I don't remember why I added the internal-check.h, but I'm sure there was a reason :)

Anyway, what's to be done here now?
Comment 4 Sebastian Dröge (slomo) 2014-12-14 10:35:28 UTC
I think we should just get rid of the internal-check.h then and use libcheck/check.h. Less risk of things in the build system breaking.
Comment 5 Sebastian Rasmussen 2014-12-14 11:16:44 UTC
Alright, then I'll try to whip up a patch that does that. :)
Comment 6 Sebastian Rasmussen 2014-12-14 13:11:40 UTC
Created attachment 292693 [details] [review]
Proposed patch.

Well... I failed in my attempts to generate a check.h as it would clobber an existing check.h. So instead I opted for generating an internal-check.h directly. This had the conequence of requiring libcheck code to include internal-check.h instead. But maybe this is ok?

Compiles on Linux, builds and runs the check tests, distcheck works fine, as does compiling the contents of the tarball. I most likely have missed something, but I'm not sure of what exactly.
Comment 7 Sebastian Rasmussen 2014-12-14 13:12:17 UTC
Comment on attachment 292484 [details] [review]
Proposed patch.

I forgot to obsolete the old patch.
Comment 8 Sebastian Dröge (slomo) 2014-12-14 19:42:48 UTC
It clobbered which existing check.h?
Comment 9 Sebastian Rasmussen 2014-12-14 20:57:49 UTC
libs/gst/check/check.h. This is really difficult to explain, but let me try:

So we are talking about these files:
libs/gst/check/libcheck/check.h.in --generates--> libs/gst/check/libcheck/check.h
libs/gst/check/libcheck/check.h --included-by-> libs/gst/check/libcheck/*.[ch]
libs/gst/check/libcheck/check.h --copied-to--> libs/gst/check/internal-check.h
libs/gst/check/internal-check.h --included-by--> libs/gst/check/gstcheck.h
libs/gst/check/gstcheck.h --included-by--> libs/gst/check/check.h
libs/gst/check/check.h --included-by-> all unit tests

By using autotools I can change this to:
libs/gst/check/libcheck/check.h.in --generates--> libs/gst/check/check.h
libs/gst/check/libcheck/check.h --included-by-> libs/gst/check/libcheck/*.[ch]
libs/gst/check/check.h --included-by--> libs/gst/check/gstcheck.h
libs/gst/check/gstcheck.h --included-by--> libs/gst/check/check.h (This is the problem)
libs/gst/check/check.h --included-by-> all unit tests (This is also the problem)

But since check.h can not be used to contain both the generated contents
and the manually created check.h I opted to keep the file name internal-check.h:
libs/gst/check/libcheck/check.h.in --generates--> libs/gst/check/internal-check.h
libs/gst/check/internal-check.h --included-by-> libs/gst/check/libcheck/*.[ch] (Diverges more from upstream libcheck)
libs/gst/check/internal-check.h --included-by--> libs/gst/check/gstcheck.h
libs/gst/check/gstcheck.h --included-by--> libs/gst/check/check.h
libs/gst/check/check.h --included-by-> all unit tests

In this last example we refrain from copying internal-check.h to check.h, but
at the cost of diverging from upstream a bit more.
Comment 10 Sebastian Dröge (slomo) 2014-12-15 09:02:26 UTC
Ah I see, it conflicts with our own single-include check.h. Seems fine to me then. Tim?
Comment 11 Tim-Philipp Müller 2014-12-15 10:47:14 UTC
Sure, whatever works.
Comment 12 Sebastian Rasmussen 2014-12-15 21:33:17 UTC
Created attachment 292772 [details] [review]
Proposed patch

As per slomo's suggestion move the generated internal-check.h from libs/gst/check/libcheck to libs/gst/check.

Appears to compile well, make check works as does make distcheck and compiling the resulting tarball, making check from it as well as installing the compiled tarball. I hope I have tested everything now.
Comment 13 Sebastian Dröge (slomo) 2014-12-16 15:33:00 UTC
commit 0b09573bbec99a2474d5ff41e2cf1be8147ecd60
Author: Sebastian Rasmussen <sebras@hotmail.com>
Date:   Sun Dec 14 12:54:32 2014 +0100

    check: Have autotools generate internal-check.h
    
    Previously GStreamer got access to the libcheck interface by including
    libs/gst/check/check.h which in turn included internal-check.h in the
    same directory. internal-check.h was generated by copying
    libs/gst/check/libcheck/check.h which in turn was generated from
    check.h.in in the same directory. In this case generating
    libs/gst/check/libcheck/check.h is unnecessary, in addition this file
    was accidentally distributed in generated project tarballs.
    
    Now libs/gst/check/internal-check.h is generated directly from
    libs/gst/check/libcheck/check.h.in by configure. This means that the
    libcheck source must include internal-check.h instead of the previously
    generated libs/gst/check/libcheck/check.h. However the unnecessary
    intermediate step is now skipped.
    
    Fixes https://bugzilla.gnome.org/show_bug.cgi?id=741359