[PATCH] gstreamer1.0: Shorten __FILE__ in gst_debug_log output on all platforms.

[PATCH] gstreamer1.0: Shorten __FILE__ in gst_debug_log output on all platforms.

[Peter Urbanec]

On WIN32 the file argument to gst_debug_log_valist is shortened to just
the filename. This is useful not only for MSVC, but also with gcc/Linux
when doing cross-compilation builds and out-of-tree builds.

Signed-off-by: Peter Urbanec <openembedded-devel@urbanec.net>
---
 ...gstinfo-Shorten-__FILE__-on-all-platforms.patch | 55 ++++++++++++++++++++++
 .../gstreamer/gstreamer1.0_1.4.5.bb                |  1 +
 2 files changed, 56 insertions(+)
 create mode 100644 meta/recipes-multimedia/gstreamer/gstreamer1.0/0001-gstinfo-Shorten-__FILE__-on-all-platforms.patch

diff --git a/meta/recipes-multimedia/gstreamer/gstreamer1.0/0001-gstinfo-Shorten-__FILE__-on-all-platforms.patch b/meta/recipes-multimedia/gstreamer/gstreamer1.0/0001-gstinfo-Shorten-__FILE__-on-all-platforms.patch
new file mode 100644
index 0000000..8213c4b
--- /dev/null
+++ b/meta/recipes-multimedia/gstreamer/gstreamer1.0/0001-gstinfo-Shorten-__FILE__-on-all-platforms.patch
@@ -0,0 +1,55 @@
+From 81fecd367b016e5ac4fb0c04b84da5c474f30da6 Mon Sep 17 00:00:00 2001
+From: Peter Urbanec <git.user@urbanec.net>
+Date: Fri, 27 Feb 2015 01:16:58 +1100
+Subject: [PATCH 1/1] gstinfo: Shorten __FILE__ on all platforms.
+
+This is useful not only for MSVC, but also with gcc/Linux when doing
+cross-compilation builds and out-of-tree builds.
+
+Upstream-Status: Submitted [https://bugzilla.gnome.org/show_bug.cgi?id=745213]
+
+Signed-off-by: Peter Urbanec <git.user@urbanec.net>
+---
+ gst/gstinfo.c | 11 ++++-------
+ 1 file changed, 4 insertions(+), 7 deletions(-)
+
+diff --git a/gst/gstinfo.c b/gst/gstinfo.c
+index b2a3005..8b61d09 100644
+--- a/gst/gstinfo.c
++++ b/gst/gstinfo.c
+@@ -444,7 +444,6 @@ gst_debug_log (GstDebugCategory * category, GstDebugLevel level,
+   va_end (var_args);
+ }
+ 
+-#ifdef G_OS_WIN32
+ /* based on g_basename(), which we can't use because it was deprecated */
+ static inline const gchar *
+ gst_path_basename (const gchar * file_name)
+@@ -467,7 +466,6 @@ gst_path_basename (const gchar * file_name)
+ 
+   return file_name;
+ }
+-#endif
+ 
+ /**
+  * gst_debug_log_valist:
+@@ -497,12 +495,11 @@ gst_debug_log_valist (GstDebugCategory * category, GstDebugLevel level,
+   g_return_if_fail (function != NULL);
+   g_return_if_fail (format != NULL);
+ 
+-  /* The predefined macro __FILE__ is always the exact path given to the
+-   * compiler with MSVC, which may or may not be the basename.  We work
+-   * around it at runtime to improve the readability. */
+-#ifdef G_OS_WIN32
++  /* The predefined macro __FILE__ can be an absolute path in some build
++   * environments, such as MSVC or out-of-tree cross-compiles. This may
++   * be significantly longer than the filename.  We work around it at
++   * runtime to improve the readability. */
+   file = gst_path_basename (file);
+-#endif
+ 
+   message.message = NULL;
+   message.format = format;
+-- 
+2.3.0
+
diff --git a/meta/recipes-multimedia/gstreamer/gstreamer1.0_1.4.5.bb b/meta/recipes-multimedia/gstreamer/gstreamer1.0_1.4.5.bb
index 94be846..0a2ce86 100644
--- a/meta/recipes-multimedia/gstreamer/gstreamer1.0_1.4.5.bb
+++ b/meta/recipes-multimedia/gstreamer/gstreamer1.0_1.4.5.bb
@@ -6,6 +6,7 @@ LIC_FILES_CHKSUM = "file://COPYING;md5=6762ed442b3822387a51c92d928ead0d \
 SRC_URI = " \
     http://gstreamer.freedesktop.org/src/gstreamer/gstreamer-${PV}.tar.xz \
     file://0001-Fix-crash-with-gst-inspect.patch \
+    file://0001-gstinfo-Shorten-__FILE__-on-all-platforms.patch \
 "
 SRC_URI[md5sum] = "88a9289c64a4950ebb4f544980234289"
 SRC_URI[sha256sum] = "40801aa7f979024526258a0e94707ba42b8ab6f7d2206e56adbc4433155cb0ae"
-- 
2.3.0

Re: [PATCH] gstreamer1.0: Shorten __FILE__ in gst_debug_log output on all platforms.

[Richard Purdie]

On Fri, 2015-02-27 at 04:31 +1100, Peter Urbanec wrote:
> On WIN32 the file argument to gst_debug_log_valist is shortened to just
> the filename. This is useful not only for MSVC, but also with gcc/Linux
> when doing cross-compilation builds and out-of-tree builds.

Ultimately I think we need to address this issue in gcc itself, probably
setting some kind of base path in the environment which it removes from
__FILE__ (set to ${S}). There were more complex discussions about using
the same mapping code as can be used with the debug symbols code too.

Cheers,

Richard

Re: [PATCH] gstreamer1.0: Shorten __FILE__ in gst_debug_log output on all platforms.

[Andreas Oberritter]

On 26.02.2015 22:47, Richard Purdie wrote:
> On Fri, 2015-02-27 at 04:31 +1100, Peter Urbanec wrote:
>> On WIN32 the file argument to gst_debug_log_valist is shortened to just
>> the filename. This is useful not only for MSVC, but also with gcc/Linux
>> when doing cross-compilation builds and out-of-tree builds.
> 
> Ultimately I think we need to address this issue in gcc itself, probably
> setting some kind of base path in the environment which it removes from
> __FILE__ (set to ${S}). There were more complex discussions about using
> the same mapping code as can be used with the debug symbols code too.

FWIW, here's a patch I'm using with gcc 4.8, which just cuts off the
whole directory part.

http://git.openembedded.org/openembedded-core-contrib/commit/?h=obi/master&id=a1e8d6cfb71367d745f2478c13a7250e41ca4f1b

Regards,
Andreas

Re: [PATCH] gstreamer1.0: Shorten __FILE__ in gst_debug_log output on all platforms.

[Otavio Salvador]

On Thu, Feb 26, 2015 at 6:47 PM, Richard Purdie
<richard.purdie@linuxfoundation.org> wrote:
> On Fri, 2015-02-27 at 04:31 +1100, Peter Urbanec wrote:
>> On WIN32 the file argument to gst_debug_log_valist is shortened to just
>> the filename. This is useful not only for MSVC, but also with gcc/Linux
>> when doing cross-compilation builds and out-of-tree builds.
>
> Ultimately I think we need to address this issue in gcc itself, probably
> setting some kind of base path in the environment which it removes from
> __FILE__ (set to ${S}). There were more complex discussions about using
> the same mapping code as can be used with the debug symbols code too.

Agreed; this should indeed be a global fix as this affects virtually
all packages.

-- 
Otavio Salvador                             O.S. Systems
http://www.ossystems.com.br        http://code.ossystems.com.br
Mobile: +55 (53) 9981-7854            Mobile: +1 (347) 903-9750

Re: [PATCH] gstreamer1.0: Shorten __FILE__ in gst_debug_log output on all platforms.

[Otavio Salvador]

On Thu, Feb 26, 2015 at 6:56 PM, Andreas Oberritter
<obi@opendreambox.org> wrote:
> On 26.02.2015 22:47, Richard Purdie wrote:
>> On Fri, 2015-02-27 at 04:31 +1100, Peter Urbanec wrote:
>>> On WIN32 the file argument to gst_debug_log_valist is shortened to just
>>> the filename. This is useful not only for MSVC, but also with gcc/Linux
>>> when doing cross-compilation builds and out-of-tree builds.
>>
>> Ultimately I think we need to address this issue in gcc itself, probably
>> setting some kind of base path in the environment which it removes from
>> __FILE__ (set to ${S}). There were more complex discussions about using
>> the same mapping code as can be used with the debug symbols code too.
>
> FWIW, here's a patch I'm using with gcc 4.8, which just cuts off the
> whole directory part.
>
> http://git.openembedded.org/openembedded-core-contrib/commit/?h=obi/master&id=a1e8d6cfb71367d745f2478c13a7250e41ca4f1b

Ideally it could drop the S from the string. So the internal path
would be respected.

What do you think?

-- 
Otavio Salvador                             O.S. Systems
http://www.ossystems.com.br        http://code.ossystems.com.br
Mobile: +55 (53) 9981-7854            Mobile: +1 (347) 903-9750

Re: [PATCH] gstreamer1.0: Shorten __FILE__ in gst_debug_log output on all platforms.

[Andreas Oberritter]

On 26.02.2015 23:11, Otavio Salvador wrote:
> On Thu, Feb 26, 2015 at 6:56 PM, Andreas Oberritter
> <obi@opendreambox.org> wrote:
>> On 26.02.2015 22:47, Richard Purdie wrote:
>>> On Fri, 2015-02-27 at 04:31 +1100, Peter Urbanec wrote:
>>>> On WIN32 the file argument to gst_debug_log_valist is shortened to just
>>>> the filename. This is useful not only for MSVC, but also with gcc/Linux
>>>> when doing cross-compilation builds and out-of-tree builds.
>>>
>>> Ultimately I think we need to address this issue in gcc itself, probably
>>> setting some kind of base path in the environment which it removes from
>>> __FILE__ (set to ${S}). There were more complex discussions about using
>>> the same mapping code as can be used with the debug symbols code too.
>>
>> FWIW, here's a patch I'm using with gcc 4.8, which just cuts off the
>> whole directory part.
>>
>> http://git.openembedded.org/openembedded-core-contrib/commit/?h=obi/master&id=a1e8d6cfb71367d745f2478c13a7250e41ca4f1b
> 
> Ideally it could drop the S from the string. So the internal path
> would be respected.
> 
> What do you think?

I thought about that before, but preferred not to do it for the sake of
simplicity. Usually __FILE__ is used for debug output, so it's printed
together with a more or less unique debug message. I guess it should be
easy to find the right location inside the source tree in almost every
case, even if some filenames might appear more than once in a project.

Note that generated source files may live in ${B}, which is not below
${S}. You could cut off ${WORKDIR} instead, though.

Regards,
Andreas

Re: [PATCH] gstreamer1.0: Shorten __FILE__ in gst_debug_log output on all platforms.

[Randy Witt]

On 02/26/2015 02:00 PM, Otavio Salvador wrote:
> On Thu, Feb 26, 2015 at 6:47 PM, Richard Purdie
> <richard.purdie@linuxfoundation.org> wrote:
>> On Fri, 2015-02-27 at 04:31 +1100, Peter Urbanec wrote:
>>> On WIN32 the file argument to gst_debug_log_valist is shortened to just
>>> the filename. This is useful not only for MSVC, but also with gcc/Linux
>>> when doing cross-compilation builds and out-of-tree builds.
>>
>> Ultimately I think we need to address this issue in gcc itself, probably
>> setting some kind of base path in the environment which it removes from
>> __FILE__ (set to ${S}). There were more complex discussions about using
>> the same mapping code as can be used with the debug symbols code too.

The bug https://gcc.gnu.org/bugzilla/show_bug.cgi?id=47047 looks to still be 
open. Should we open a Yocto bug to update that patch to apply to a more recent gcc?

> Agreed; this should indeed be a global fix as this affects virtually
> all packages.
>

Re: [PATCH] gstreamer1.0: Shorten __FILE__ in gst_debug_log output on all platforms.

[Otavio Salvador]

On Thu, Feb 26, 2015 at 7:37 PM, Randy Witt
<randy.e.witt@linux.intel.com> wrote:
> On 02/26/2015 02:00 PM, Otavio Salvador wrote:
>>
>> On Thu, Feb 26, 2015 at 6:47 PM, Richard Purdie
>> <richard.purdie@linuxfoundation.org> wrote:
>>>
>>> On Fri, 2015-02-27 at 04:31 +1100, Peter Urbanec wrote:
>>>>
>>>> On WIN32 the file argument to gst_debug_log_valist is shortened to just
>>>> the filename. This is useful not only for MSVC, but also with gcc/Linux
>>>> when doing cross-compilation builds and out-of-tree builds.
>>>
>>>
>>> Ultimately I think we need to address this issue in gcc itself, probably
>>> setting some kind of base path in the environment which it removes from
>>> __FILE__ (set to ${S}). There were more complex discussions about using
>>> the same mapping code as can be used with the debug symbols code too.
>
>
> The bug https://gcc.gnu.org/bugzilla/show_bug.cgi?id=47047 looks to still be
> open. Should we open a Yocto bug to update that patch to apply to a more
> recent gcc?

Even nicer :D

-- 
Otavio Salvador                             O.S. Systems
http://www.ossystems.com.br        http://code.ossystems.com.br
Mobile: +55 (53) 9981-7854            Mobile: +1 (347) 903-9750

Re: [PATCH] gstreamer1.0: Shorten __FILE__ in gst_debug_log output on all platforms.

[Peter Urbanec]

On 27/02/15 09:00, Otavio Salvador wrote:
> On Thu, Feb 26, 2015 at 6:47 PM, Richard Purdie
> <richard.purdie@linuxfoundation.org> wrote:
>> On Fri, 2015-02-27 at 04:31 +1100, Peter Urbanec wrote:
>>> On WIN32 the file argument to gst_debug_log_valist is shortened to just
>>> the filename. This is useful not only for MSVC, but also with gcc/Linux
>>> when doing cross-compilation builds and out-of-tree builds.
>>
>> Ultimately I think we need to address this issue in gcc itself, probably
>> setting some kind of base path in the environment which it removes from
>> __FILE__ (set to ${S}). There were more complex discussions about using
>> the same mapping code as can be used with the debug symbols code too.
>
> Agreed; this should indeed be a global fix as this affects virtually
> all packages.

I also agree, but this patch can go ahead regardless of what the global 
solution is. The patch enables a mechanism that already exists in 
gstreamer (but is only enabled on WIN32) and solves an immediate need in 
that context. Whatever the global solution is, this patch is very likely 
to continue working correctly.

Having a proper solution at the gcc level will significantly reduce the 
size of the generated files, so there is certainly a win to be had 
there. I had a look at both C99 and C++98 standards and the semantics of 
the __FILE__ macro are implementation defined and do not stipulate any 
particular interpretation. In particular, there's no requirement to have 
path information preserved.

I think a simple patch to gcc to transform a full pathname to basename 
is probably sufficient for oe-core - the less complexity is introduced 
the better.