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 767096 - Build script gstreamer-1.0.mk (android) does not work properly on OSX
Build script gstreamer-1.0.mk (android) does not work properly on OSX
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: cerbero
git master
Other Mac OS
: Normal blocker
: 1.8.3
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2016-06-01 08:42 UTC by Milos Pesic
Modified: 2016-07-01 01:13 UTC
See Also:
GNOME target: ---
GNOME version: ---

Attachments
Replace sed -i with cat | sed > temp and mv temp origin (4.16 KB, patch)
2016-06-01 08:42 UTC, Milos Pesic 		reviewed Details | Review
android: Don't use 'sed -i' to be compatible with BSD sed (4.09 KB, patch)
2016-06-06 12:43 UTC, Sebastian Dröge (slomo) 		committed Details | Review

Description Milos Pesic 2016-06-01 08:42:46 UTC 
Created attachment 328863 [details] [review]
Replace sed -i with cat | sed > temp and mv temp origin

sed command in gstreamer-1.0.mk file fails on OSX (BSD sed does not support -i option) resulting in Gstreamer.java and gstreamer_android-1.0.c generation failure.
Related to:
https://bugzilla.gnome.org/show_bug.cgi?id=763999

Although I am not sure if it is 100% correct (tested on OSX only so far) attached patch fixes the problem.
Comment 1 Sebastian Dröge (slomo) 2016-06-01 08:47:00 UTC 
Review of attachment 328863 [details] [review]:

Generally looks good but can be cleaned up a little bit :) Thanks!

::: data/ndk-build/gstreamer-1.0.mk
@@ +216,3 @@
+	mv $(PRIV_C).tmp $(PRIV_C)
+	cat $(PRIV_C) | $(SED) "s/@G_IO_MODULES_DECLARE@/$(PRIV_G_R)/g" > $(PRIV_C).tmp
+	mv $(PRIV_C).tmp $(PRIV_C)

Looks good but you can do all these replacements in one go:

cat orig | sed | sed | sed > tmp
mv tmp orig

@@ +260,3 @@
 else
+	cat $(GSTREAMER_JAVA_SRC_DIR)/org/freedesktop/gstreamer/GStreamer.java | $(SED) "s;@INCLUDE_COPY_FILE@;//;g" > $(GSTREAMER_JAVA_SRC_DIR)/org/freedesktop/gstreamer/GStreamer.java.tmp
+	mv $(GSTREAMER_JAVA_SRC_DIR)/org/freedesktop/gstreamer/GStreamer.java.tmp $(GSTREAMER_JAVA_SRC_DIR)/org/freedesktop/gstreamer/GStreamer.java

Also all these if you move the ifs outside the sed and e.g. do

ifneq ...
INCLUDE_COPY_FILE_SUBST=
else
INCLUDE_COPY_FILE_SUBST=//
endif

cat orig | sed | ... | sed 's;@INCLUDE_COPY_FILE@;$(INCLUDE_COPY_FILE_SUBST);g' > tmp
mv tmp orig
Comment 2 Sebastian Dröge (slomo) 2016-06-06 12:43:39 UTC 
Created attachment 329189 [details] [review]
android: Don't use 'sed -i' to be compatible with BSD sed

Based on a patch by Milos Pesic <msg4misa@gmail.com>
Comment 3 Sebastian Dröge (slomo) 2016-06-07 12:37:18 UTC 
Attachment 329189 [details] pushed as e928fda - android: Don't use 'sed -i' to be compatible with BSD sed
Comment 4 Olivier Crête 2016-06-29 20:12:55 UTC 
You seem to have broken font inclusion !
Comment 5 Olivier Crête 2016-06-29 20:14:22 UTC 
Can you please revert from 1.8 at least as it just breaks the build on Linux.
Comment 6 Sebastian Dröge (slomo) 2016-06-29 20:48:23 UTC 
Why not just fix it, should be trivial? :) How does the font build fail for you?
Comment 7 Sebastian Dröge (slomo) 2016-06-30 21:59:03 UTC 
commit 6505b5077baf645bec684892a5c81c0e131255ce
Author: Sebastian Dröge <sebastian@centricular.com>
Date:   Thu Jun 30 23:58:12 2016 +0200

    android: Fix sed substitutions to actually consider the configuration
    
    https://bugzilla.gnome.org/show_bug.cgi?id=767096
Comment 8 Olivier Crête 2016-07-01 01:13:43 UTC 
I can confirm this fixes it.. that said, we need a 1.8.2.1 for Android now, as currently most app builds won't work!