[vlc-devel] [PATCH 5/6] contrib: protobuf: build protoc from contribs

[Steve Lhomme]

On 2020-03-23 17:23, Rémi Denis-Courmont wrote:
> Le maanantaina 23. maaliskuuta 2020, 17.27.26 EET Steve Lhomme a écrit :
>> Similar to how it's done for luac: we have a triplet-protoc in contrib/bin.
>> ---
>>   configure.ac                   | 10 +++++++++-
>>   contrib/src/protobuf/rules.mak | 19 +++++++++++++++++--
>>   2 files changed, 26 insertions(+), 3 deletions(-)
>>
>> diff --git a/configure.ac b/configure.ac
>> index bc1874cb97c..8af142d48bd 100644
>> --- a/configure.ac
>> +++ b/configure.ac
>> @@ -502,6 +502,12 @@ AS_IF([test -n "${CONTRIB_DIR}"], [
>>       ])
>>     ])
>>
>> +  AS_IF([test -z "$PROTOC"], [
>> +    AS_IF([test -x
>> "${CONTRIB_DIR}/../bin/${host_alias}-protoc${BUILDEXEEXT}"], [ +
>> PROTOC="${CONTRIB_DIR}/../bin/${host_alias}-protoc${BUILDEXEEXT}" +    ])
>> +  ])
>> +
>>     AS_IF([test "${SYS}" = "darwin"], [
>>       export LD_LIBRARY_PATH="${CONTRIB_DIR}/lib:$LD_LIBRARY_PATH"
>>       export DYLD_LIBRARY_PATH="${CONTRIB_DIR}/lib:$DYLD_LIBRARY_PATH"
>> @@ -3870,7 +3876,9 @@ dnl  Chromecast streaming support
>>   dnl
>>   m4_pushdef([protobuf_lite_version], 2.5.0)
>>   AC_ARG_VAR(PROTOC, [protobuf compiler])
>> -AC_CHECK_PROGS(PROTOC, protoc, no)
>> +AS_IF([test -z "$PROTOC"], [
>> +   AC_CHECK_TOOL(PROTOC, protoc, no)
>> +])
>>   PKG_WITH_MODULES([CHROMECAST],[protobuf-lite >= protobuf_lite_version], [
>>       AS_IF([test "x${PROTOC}" != "xno"], [
>>           build_chromecast="yes"
> 
> I don't know if this is right or wrong, but it belongs in a separate patch.

If you mean the lines above, it's related as in the case he host-protoc 
is found we use that and don't test it (like luac). Otherwise we revert 
to the system detection. No reason to change from AC_CHECK_PROGS to 
AC_CHECK_TOOL. I'll change that.

>> diff --git a/contrib/src/protobuf/rules.mak b/contrib/src/protobuf/rules.mak
>> index c505ef7548b..28f38445566 100644
>> --- a/contrib/src/protobuf/rules.mak
>> +++ b/contrib/src/protobuf/rules.mak
>> @@ -2,9 +2,10 @@
>>   PROTOBUF_VERSION := 3.1.0
>>   PROTOBUF_URL :=
>> https://github.com/google/protobuf/releases/download/v$(PROTOBUF_VERSION)/p
>> rotobuf-cpp-$(PROTOBUF_VERSION).tar.gz
>>
>> -PKGS += protobuf
>> +PKGS += protobuf protoc
>> +PKGS_PROGS += protoc
>>   ifeq ($(call need_pkg, "protobuf-lite >= 3.1.0 protobuf-lite < 3.2.0"),)
>> -PKGS_FOUND += protobuf
>> +PKGS_FOUND += protobuf protoc
>>   else
>>   # check we have a matching protoc to use
>>   PROTOC_ABSPATH = $(shell PATH="$(PATH)" which protoc)
> 
> Won't work. See how luac does it.

You mean adding protoc to PKGS_ALL ?


>> @@ -44,3 +45,17 @@ protobuf: protobuf-$(PROTOBUF_VERSION)-cpp.tar.gz
>> .sum-protobuf cd $< && $(HOSTVARS) ./configure $(HOSTCONF)
>> --with-protoc="$(PROTOC)" cd $< && $(MAKE) && $(MAKE) install
>>   	touch $@
>> +
>> +.sum-protoc: .sum-protobuf
>> +	touch $@
>> +
>> +protoc: protobuf-$(PROTOBUF_VERSION)-cpp.tar.gz .sum-protoc
>> +	$(UNPACK)
>> +	mv protobuf-$(PROTOBUF_VERSION) protobuf-$(PROTOBUF_VERSION)-cpp
>> +	$(APPLY) $(SRC)/protobuf/protobuf-win32.patch
>> +	$(MOVE)
>> +
>> +.protoc: protoc
>> +	cd $< && $(BUILDVARS) ./configure $(BUILDCONF)
>> +	cd $< && $(MAKE) && $(MAKE) install
>> +	touch $@
> 
> That will install plenty of crap that we most probably don't want to install.

As the great philosopher once said: storage is cheap.

And it's not that much. And there's no rule to only build and only 
install protoc from the whole protobuf package.