[vlc-devel] [PATCH 2/4] doc: add an option to build libvlc sample app from the documentation

Mon Mar 1 09:01:58 UTC 2021

Hi,

Again, I don't care if you use autotools, though it's probably
the worst choice for Qt, or anything else, really. It's not
possible to provide a sample for each buildsystem and simplicity
is better here than exhaustivity.

I do care if there are two buildsystem for the same project,
which is what your patch adds, and I do care about being able to
use the samples without recompiling libvlc from scratch, otherwise
they probably just don't belong in the doc/ folder, which -- to
remind you -- is installed.

> Otherwise you should advocate to add MSVC projects for libvlc and build windows samples with it since that's what a lot of people will use.

Well, that's not what I wrote, so no I shouldn't.

Regards,
--
Alexandre Janniaux
Videolabs

On Mon, Mar 01, 2021 at 07:41:19AM +0100, Steve Lhomme wrote:
> This is not a file we use in our build system. It's there for documentation purpose, to guide users.
> That doesn't mean there's only one way to build Qt project. We use autotools to build our own Qt module. In the past I've used others as well. I believe nowadays CMAKE is also a candidate. We'll probably use Meson for it as well.
> Pushing for qmake here when we already have all the autotools parts ready and solid to do it make no sense to me.
>
> Otherwise you should advocate to add MSVC projects for libvlc and build windows samples with it since that's what a lot of people will use.
>
>
> > On 27 Feb 2021, at 20:15, Alexandre Janniaux <ajanni at videolabs.io> wrote:
> >
> > On Sat, Feb 27, 2021 at 06:20:49PM +0100, Steve Lhomme wrote:
> >>> On 2/27/2021 3:56 PM, Alexandre Janniaux wrote:
> >>> Hi,
> >>>
> >>> On Sat, Feb 27, 2021 at 10:48:43AM +0100, Steve Lhomme wrote:
> >>>> On 2021-02-26 18:16, Alexandre Janniaux wrote:
> >>>>> Hi,
> >>>>>
> >>>>> As mentioned privately, I'm not fond of adding yet another build
> >>>>> system to build examples, especially since it won't even be testing
> >>>>> the buildsystem final users are supposed to use.
> >>>>
> >>>> There is not "yet another build system" in this patch.
> >>>
> >>> Sorry, what? you litterally create doc/libvlc/Makefile.am
> >>> typically to build QtGL and QtPlayer with new automake
> >>> targets and even new automake rules whereas there is already
> >>> a qmake-based build system.
> >>
> >> Our "build system" is autotools. I just use that.
> >> Please point me to the "qmake-based build system" we already use.
> >
> > https://code.videolan.org/videolan/vlc/-/blob/master/doc/libvlc/QtGL/QtGl.pro
> >
> >>> With that, both can now get out-of-sync with regards to the
> >>> other one, and one of them is even never used in the CI while
> >>> the other would always be used. Better remove the other one
> >>> like I mentioned then, but as I said:
> >>>
> >>>  - users will never build their application from the VLC build
> >>>    directory.
> >>>
> >>>  - most Qt users will typically never use automake to build a
> >>>    Qt application, so it seems like a bad target for
> >>>    documentation.
> >>>
> >>>  - in addition, it means that you need to build libvlc in order
> >>>    to build the examples, which is quite inconvenient since you
> >>>    could be able to ship the examples with a packaged libvlc.
> >>>
> >>>> The final users are free to use whatever they want and that's a good reason
> >>>> not to orientate them in any favored direction.
> >>>
> >>>  - and I completely agree with this point right above, which is
> >>>    why using automake within the same autoconf package is a bad
> >>>    idea, since it will never be the same environment for other
> >>>    buildsystem, by-pass potential dependency detection systems
> >>>    and thus is quite opinionated. But more on this in next
> >>>    paragraph.
> >>>
> >>>>
> >>>>> I'd prefer that we typically use make install to a given prefix and
> >>>>> then use qmake/foomake/etc with the correct PKG_CONFIG_PATH. The
> >>>>> alternative is to use only the automake buildsystem but to be fair,
> >>>>
> >>>> Last time I proposed a similar patch, Rémi advised to use "noinst_" instead
> >>>> of "bin_" prefix. And IMO it makes sense not to install sample code. The
> >>>> solution you propose is the opposite, it requires to install libvlc before
> >>>> building the samples. Something that is not needed by us nor the end user
> >>>> who will need to repackage the build anyway.
> >>>
> >>> There's a huge difference between "do not install the examples"
> >>> (which makes perfect sense because users generally don't need
> >>> them on their systems) and "do not install libvlc".
> >>>
> >>> Typically, if you don't install libvlc, but use the mock plugins
> >>> in the examples, it might work with the automake buildsystem you
> >>> wrote but won't work with a packaged VLC and an external build
> >>> system, because mock plugin is not installed.
> >>>
> >>> So I really fail to see how it's the opposite.
> >>>
> >>> In addition, if libvlc needs to be repackaged (which imho is
> >>> already a flaw in our buildsystem since it bypass the make
> >>> install behaviour and is another layer above the buildsystem
> >>> which actually mimic the layer below instead of using it), then
> >>> we just need to actually package libvlc in the CI before using
> >>> it in the examples.
> >>>
> >>>>> what are the odds that final users are building their application
> >>>>> inside libvlc buildsystem without having an installed package?
> >>>>
> >>>> Maybe it's customary on Linux (is there a separate libvlc package on Linux
> >>>> distros ?) but on Windows or Mac or Android or iOS they will need to package
> >>>> libvlc by their own means, with their build system. So the way we build it
> >>>> for ourself will not have much impact.
> >>>
> >>> I don't think many people wants to be compiling libvlc
> >>> themselves, even on Windows, and especially on Windows.
> >>>
> >>> But anyway say that this is true. Why use libvlc from
> >>> the build directory instead of using libvlc from the
> >>> packaged directory then?
> >>>
> >>> Then, yes, we have tools to detect dependencies much like
> >>> any other platforms. We don't typically use additional
> >>> layer of dependency management like Conan but the user
> >>> might use that to provide libvlc support. By using external
> >>> build system (and actually any external buildsystem), we
> >>> show the user how it can use it to build with libvlc with
> >>> minor tweaking to the dependency layer, be it cmake, meson,
> >>> qmake, conan or anything other used on Windows. How is using
> >>> uninstalled libtool library which almost none of the above
> >>> buildsystem understand is less opinionated than using the
> >>> idiomatic buildsystem used within the community the examples
> >>> are made for?
> >>>
> >>> Just so this message is clear, I'd be happy that we test the
> >>> examples in the CI and I'm glad that you initiated work
> >>> regarding that. But honestly even the previous buildsystem
> >>> was broken last time I used it and I had to fix it, I don't
> >>> want another maintenance burden there and especially if it
> >>> gives no additional value to the enduser.
> >>>
> >>> The --enable-doc-samples can still be in place in configure
> >>> with much less code and without duplicating the current work.
> >>>
> >>> Regards,
> >>> --
> >>> Alexandre Janniaux
> >>> Videolabs
> >>> _______________________________________________
> >>> vlc-devel mailing list
> >>> To unsubscribe or modify your subscription options:
> >>> https://mailman.videolan.org/listinfo/vlc-devel
> >>>
> >>
> >>
> >> _______________________________________________
> >> vlc-devel mailing list
> >> To unsubscribe or modify your subscription options:
> >> https://mailman.videolan.org/listinfo/vlc-devel
> > _______________________________________________
> > vlc-devel mailing list
> > To unsubscribe or modify your subscription options:
> > https://mailman.videolan.org/listinfo/vlc-devel
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel