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

[Alexandre Janniaux]

Hi,

On Fri, Mar 05, 2021 at 08:18:15AM +0100, Steve Lhomme wrote:
> On 3/4/2021 3:34 PM, Alexandre Janniaux wrote:
> > Hi,
> >
> > On Thu, Mar 04, 2021 at 11:42:02AM +0100, Steve Lhomme wrote:
> > > On 3/1/2021 11:15 AM, Marvin Scholz wrote:
> > > >
> > > >
> > > > On 1 Mar 2021, at 7:41, 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.
> > > > >
> > > >
> > > > I agree with what Alexandre already wrote, duplicating the build in
> > > > autotools here seems incredibly awful choice
> > > > for nearly no benefit at all… Just use qmake to build the sample
> > > > project, like most users of it would do?
> > > > I am not sure who would benefit from having another build for this in
> > > > autotools, which can be painful to use
> > > > properly with Qt, or nearly impossible even, on macOS. And additionally
> > > > can easily get out of sync when
> > > > someone is unaware of the other build system for it and makes changes
> > > > only updating the autotools build for it…
> > > >
> > > > Your point about MSVC does not really make sense to me, as this is a
> > > > completely different topic, really.
> > >
> > > Nope. this is exactly the confusion you are both making between building the
> > > samples when building VLC and building the samples in an external context
> > > with the purpose of integrating libvlc in an app.
> >
> > No, I think you're confusing what a sample project is and what
> > a test is. Using samples for the coverage of libvlc API is not
> > correct.
> >
> > Don't get me wrong, checking that the libvlc samples are correct
> > _is_ a good move IMHO, like I already mentioned, but if you're
> > not testing the buildsystem you're shipping in the documentation
> > then the check makes no sense and it's quite easy to understand
>
> I agree we can remove the dummy build project since we don't use it anyway.
> Other samples don't have this luxury. And as explained before we can't
> assume what build system people will use for their own system (ie we're not
> adding msvc+cmake+meson for win32 samples just because someone might use
> that).
>
> > why having two buildsystems for the samples can actually lead to
> > breakage of the shipped samples, especially if you develop using
> > the non-shipped buildsystem. Just think missing dependency, source
> > file not added, etc.
> >
> > Despite of the different buildsystem for the new sample project,
> > you can still compile them within the CI and even add them in the
> > automake project.
>
> This is a nightmare to manage, no thank you.

Why?

> > > Building the samples with the VLC build is useful for the CI to make sure
> > > changes to the API are also reflected in the sample code that users are
> > > supposed to use.
> > > It also helps tracking regression when doing a git bisect. Otherwise at
> > > every step you need to apply/unapply this patch(set) in order to test the
> > > git commit.
> >
> > That's poor management excuses to be honest. If you need the
>
> Thanks.

I don't mean to attack you, but I don't see how that is helping
tracking regression more than calling make in the VLC build
directory and _then_ calling make in the sample directory, ie.
I don't see why you would need to apply/unapply this patchset.

> > latest version of the sample, you can just copy the project
> > somewhere out of the version control system, and install libVLC
>
> You probably never worked on Windows. Installing libvlc everytime you make a
> change in the code is taking forever, when in fact there's no reason to do
> it at all. The changes I propose allow developping the sample code without
> having to install anything. Just like making changes in VLC don't require it
> to install before testing the code. We have VLC_PLUGIN_PATH for that.

Thanks², but you know very well that I work on Windows too.

I refer you to autotools manuel mentioning this very problem
and also mentionning the solution, static linking during
development, which happens to be exactly what I'm currently
working on (though for iOS/android currently since it's first
goal is to remove additional home-made bash layers of build
systems that were there for bootstrapping those platforms).

If you know what you're doing, you can also use the non-installed
libtool libraries and configure the PATH correctly.

Windows is not an alien and is not that different than other
platforms. It does need PATH wrapper for executable because it
doesn't support runtime rpath, but that's moslty all. Everything
else works just like Linux and the same problems are happening
on both platforms except that point, so I don't understand why
we would __require__ to change everything for Windows. It already
has specific packaging targets instead of using autoconf builtin
which can result typically in some parts not being installed, like
from share/.

> > while bisecting. If you need to sync the sample revision with
> > the bisect, then you have nothing to change from now. If you
>
> What ? This is exactly the thing I am missing and adding: having the samples
> build in sync with master. And what I repeatdly add to do manually in the
> last few years I worked on the samples.

I'm not talking about «building in sync», whatever that mean. You
can do that in one line with the following command so that's not
really a problem.

    make -C build && make -C doc/libvlc/yousample/build

I'm talking about revision in the version control system.

> > need to have different bisect between the sample and libvlc,
>
> No idea who would ever want that.

Having different revision between the code for the sample and
the code for libvlc is just for completness. I just don't see
what enabler it is when you include the sample in automake
instead of using its project build system.

> > then you need two VCS, potentially apply a filter-branch to
> > one of them to only keep doc/ commits (but it's not even
> > needed, it's just to reduce the final complexity), and then
> > you actually need to use external build system to be able to
> > compile against the installed libvlc version.
> >
> > > This is not meant to be used by external developers to integrate in their
> > > own code.
> >
> > I mostly criticize the way you do it, duplicating build system
> > instead of relying on the existing supplied ones, not the intent.
>
> Duplicating build system would be calling qmake,cmake,msvc,meson from
> autotools we are using today. You're still confused between internal and
> external use of the code.

Please explain internal and external use of code then, because
I don't think it makes sense at all.

The point of writing buildsystem for sample project is that it
gets installed __with__ the samples, as explain earlier. The
fact some samples don't have buildsystem is just that they are
poorly maintained, which is just another argument for not having
another buildsystem to replace proper envvironment setups.

Please note that I don't want to incriminate you for that so
don't take it personnally. The main reason it is like this is
probably that having additional layers makes it really hard to
go from one platform to the other and increase a lot the quantity
of work, so a lot of people seems to be afraid to just compile
and test. Samples were also not compiled in the CI, so basically
didn't work back then when I fixed the buildsystem and didn't even
work afterwards before you fix it in your previous patchsets.
It's just that we need less layers of workarounds and more common
proper buildsystem improvements.

Again, if it's about testing, write a test, just like I did with
the iosvlc.m file. You can perfectly integrate this afterwards,
it will be built and handled by autotools without packaging step,
you can integrate it with `make test` and it is not confusing for
the documentation, and it's actually clear that it's dealing with
«internal code».

> > > There is no new complexity in autotools, it's using variables from the same
> > > configure run.
> >
> > You litterally add 100 lines of Makefile.am, a custom rule to
> > build player.moc.cpp, add a strong dependency on the orders of
> > SUBDIRS, and actually add a new AC_CONFIG_FILES which generates
> > a whole new Makefile in the end. I fail to see how you don't
> > add new complexity in autotools.
>
> If you consider autotools complex sure. Calling qmake,cmake,msvc,meson from
> autotools will likely create *more* complexity. The MOC part here can
> probably be shared with the Qt modules we want to reduce duplicates. But as
> I said I don't care about that part so we can drop it.

I'm not sure how calling

    PKG_CONFIG_PATH="$(PREFIX)/$(libdir)" qmake .. && $(MAKE)

is more complex than the automake counterpart + the duplication of
buildsystem or impoverishment of the documentation.

> > > But given I don't care too much about the Qt samples I can just do it for
> > > the Win32 sample app which don't have a pretend makefile that we're
> > > supposedly meant to 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
> > > > _______________________________________________
> > > > 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