944506 - Update media/libopus/update.sh for bug 874266

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Description

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

As mentioned in bug 874266 comment 55.

Comment 1

[Ralph Giles (:rillian)]

Attachment: replace version string in moz.build

Comment 2

[Timothy B. Terriberry (:derf)]

Comment on attachment 8340125 [details] [diff] [review]
replace version string in moz.build

Review of attachment 8340125 [details] [diff] [review]:
-----------------------------------------------------------------

LGTM, but while you're here, I notice that our command for extracting the version does not match upstream. Specifically, upstream uses
  git describe --tags --match 'v*' --dirty
while you use just
  git describe --tags
It would be nice to fix that (feel free to do it in a separate bug).

::: media/libopus/update.sh
@@ +62,5 @@
>      mv ${TARGET}/README_MOZILLA+ ${TARGET}/README_MOZILLA
>  # update compiled-in version string
> +sed -e "s/DEFINES\['OPUS_VERSION'\][ \t]*=[ \t]*'\".*\"'/DEFINES['OPUS_VERSION'] = '\"${version}-mozilla\"'/" \
> +    ${TARGET}/moz.build > ${TARGET}/moz.build+ && \
> +    mv ${TARGET}/moz.build+ ${TARGET}/moz.build

I'd prefer sed -i (as used by kiss_fft/update.sh, libnestegg/update.sh, and libcubeb/update.sh) than the mv dance, but that's up to you.

Comment 3

[Chris Pearce [:cpearce (Not reading bugmail)]]

Comment on attachment 8340125 [details] [diff] [review]
replace version string in moz.build

Review of attachment 8340125 [details] [diff] [review]:
-----------------------------------------------------------------

::: media/libopus/update.sh
@@ +62,5 @@
>      mv ${TARGET}/README_MOZILLA+ ${TARGET}/README_MOZILLA
>  # update compiled-in version string
> +sed -e "s/DEFINES\['OPUS_VERSION'\][ \t]*=[ \t]*'\".*\"'/DEFINES['OPUS_VERSION'] = '\"${version}-mozilla\"'/" \
> +    ${TARGET}/moz.build > ${TARGET}/moz.build+ && \
> +    mv ${TARGET}/moz.build+ ${TARGET}/moz.build

I don't have an strong opinion here, but I'll note that sed -i doesn't work with the version of sed that ships in MozillaBuild for Windows.

Comment 4

[Ralph Giles (:rillian)]

Thanks for the review. Good catch about the version string. I'll fix that in bug 945419.

I prefer 'sed -i' too, but it doesn't work on MacOS either (BSD sed). We could at least fix MozillaBuild for Windows. :)

https://hg.mozilla.org/integration/mozilla-inbound/rev/cc923aaab73e

Comment 5

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/cc923aaab73e