[vlc-devel] [PATCH 2/4] use angle brackets for all public headers
Alexandre Janniaux
ajanni at videolabs.io
Fri Oct 2 11:39:57 CEST 2020
Hi,
I don't think having multiple directories to basically vlc/plugins
in the include directory path would be a good idea from the developer
itself at this point, there is no «good version» of the file you
want to include.
Typically, if a file only exists in one of the directory, and the
other directory is the one where files are taken first, you would
end up with one file including from the second that would require
either files from the first or the second. Which one is the best
given that most of the time, you include those from the first?
Issues like with libvpx (?) happen because the include path is not
namespaced, ie. you include <libvlc/foo.x> where you have
/usr/include/libvpx/foo.x which would break contribs here, but that
is not really the case for libvlc/libvlccore.
This patch makes a lot of noise, but it seems pretty legitimate in
my opinion. I would not do the same with modules though since it's
not installed. The issue mentioned by Steve could be «silenced» by
the previous state of the code, but it doesn't actually fix the
original issue (peeking files from multiples installations).
Regards,
--
Alexandre Janniaux
Videolabs
On Fri, Oct 02, 2020 at 10:28:25AM +0200, Thomas Guillem wrote:
> Hello,
>
> On Fri, Oct 2, 2020, at 09:05, Steve Lhomme wrote:
> > IMO it should be the other way around. If for some reason you have
> > another VLC directory in your PATH you want to make sure it's using the
> > local ones.
> >
> > I had some issues with some contribs (libvpx?) which included their own
> > headers as public headers. But the general contrib path comes before the
> > project being built so it's using the already installed headers. When
> > the API changes, the new headers are not used. So I think in general
> > it's a good rule of thumb to only use angle bracket includes for headers
> > outside of the project you build.
>
> I agree with Steve.
>
> >
> > On 2020-10-02 7:25, Lyndon Brown wrote:
> > > From: Lyndon Brown <jnqnfe at gmail.com>
> > > Date: Thu, 1 Oct 2020 22:17:11 +0100
> > > Subject: use angle brackets for all public headers
> > >
> > >
> > > diff --git a/include/vlc_aout.h b/include/vlc_aout.h
> > > index bbad114bec..3335ecd62e 100644
> > > --- a/include/vlc_aout.h
> > > +++ b/include/vlc_aout.h
> > > @@ -58,7 +58,7 @@
> > > /* Max acceptable resampling (in %) */
> > > #define AOUT_MAX_RESAMPLING 10
> > >
> > > -#include "vlc_es.h"
> > > +#include <vlc_es.h>
> > >
> > > #define AOUT_FMTS_IDENTICAL( p_first, p_second ) ( \
> > > ((p_first)->i_format == (p_second)->i_format) \
> > > diff --git a/include/vlc_common.h b/include/vlc_common.h
> > > index 22a6c089d7..c0f24c790d 100644
> > > --- a/include/vlc_common.h
> > > +++ b/include/vlc_common.h
> > > @@ -37,7 +37,7 @@
> > > /*****************************************************************************
> > > * Required vlc headers
> > > *****************************************************************************/
> > > -#include "vlc_config.h"
> > > +#include <vlc_config.h>
> > >
> > > /*****************************************************************************
> > > * Required system headers
> > > @@ -527,8 +527,8 @@ typedef int ( * vlc_list_callback_t ) ( vlc_object_t *, /* variable's objec
> > > # include <os2.h>
> > > #endif
> > >
> > > -#include "vlc_tick.h"
> > > -#include "vlc_threads.h"
> > > +#include <vlc_tick.h>
> > > +#include <vlc_threads.h>
> > >
> > > /**
> > > * \defgroup intops Integer operations
> > > @@ -1205,10 +1205,10 @@ VLC_API const char * VLC_Compiler( void ) VLC_USED;
> > > /*****************************************************************************
> > > * Additional vlc stuff
> > > *****************************************************************************/
> > > -#include "vlc_messages.h"
> > > -#include "vlc_objects.h"
> > > -#include "vlc_variables.h"
> > > -#include "vlc_configuration.h"
> > > +#include <vlc_messages.h>
> > > +#include <vlc_objects.h>
> > > +#include <vlc_variables.h>
> > > +#include <vlc_configuration.h>
> > >
> > > #if defined( _WIN32 ) || defined( __OS2__ )
> > > # define DIR_SEP_CHAR '\\'
> > > diff --git a/include/vlc_extensions.h b/include/vlc_extensions.h
> > > index 94af501728..638d83a88e 100644
> > > --- a/include/vlc_extensions.h
> > > +++ b/include/vlc_extensions.h
> > > @@ -23,8 +23,8 @@
> > > #ifndef VLC_EXTENSIONS_H
> > > #define VLC_EXTENSIONS_H
> > >
> > > -#include "vlc_common.h"
> > > -#include "vlc_arrays.h"
> > > +#include <vlc_common.h>
> > > +#include <vlc_arrays.h>
> > >
> > > /* Structures */
> > > typedef struct extensions_manager_sys_t extensions_manager_sys_t;
> > > diff --git a/modules/gui/qt/dialogs/help/aboutmodel.cpp b/modules/gui/qt/dialogs/help/aboutmodel.cpp
> > > index 3322f76989..2aa267798a 100644
> > > --- a/modules/gui/qt/dialogs/help/aboutmodel.cpp
> > > +++ b/modules/gui/qt/dialogs/help/aboutmodel.cpp
> > > @@ -21,7 +21,7 @@
> > >
> > > #include "aboutmodel.hpp"
> > >
> > > -#include "vlc_about.h"
> > > +#include <vlc_about.h>
> > >
> > >
> > > AboutModel::AboutModel(QObject *parent) : QObject(parent)
> > > diff --git a/modules/gui/qt/maininterface/main_interface.cpp b/modules/gui/qt/maininterface/main_interface.cpp
> > > index 4472b6754d..1ef30f77ea 100644
> > > --- a/modules/gui/qt/maininterface/main_interface.cpp
> > > +++ b/modules/gui/qt/maininterface/main_interface.cpp
> > > @@ -49,7 +49,7 @@
> > >
> > > #include "menus/menus.hpp" // Menu creation
> > >
> > > -#include "vlc_media_library.h"
> > > +#include <vlc_media_library.h>
> > >
> > > #include <QCloseEvent>
> > > #include <QKeyEvent>
> > > diff --git a/modules/gui/qt/maininterface/videosurface.hpp b/modules/gui/qt/maininterface/videosurface.hpp
> > > index 5ff2cd4814..933f443ed2 100644
> > > --- a/modules/gui/qt/maininterface/videosurface.hpp
> > > +++ b/modules/gui/qt/maininterface/videosurface.hpp
> > > @@ -23,7 +23,7 @@
> > > #include <QMutex>
> > > #include <util/qml_main_context.hpp>
> > > #include "qt.hpp"
> > > -#include "vlc_vout_window.h"
> > > +#include <vlc_vout_window.h>
> > >
> > > class VideoSurfaceProvider : public QObject
> > > {
> > > diff --git a/modules/gui/qt/medialibrary/mlalbum.hpp b/modules/gui/qt/medialibrary/mlalbum.hpp
> > > index 95ca6088a5..230cc3b99b 100644
> > > --- a/modules/gui/qt/medialibrary/mlalbum.hpp
> > > +++ b/modules/gui/qt/medialibrary/mlalbum.hpp
> > > @@ -21,13 +21,13 @@
> > > #ifdef HAVE_CONFIG_H
> > > #include "config.h"
> > > #endif
> > > -#include "vlc_common.h"
> > > +#include <vlc_common.h>
> > >
> > > #include <QObject>
> > > #include <QString>
> > > #include <QList>
> > > #include <memory>
> > > -#include "vlc_media_library.h"
> > > +#include <vlc_media_library.h>
> > > #include "mlhelper.hpp"
> > > #include "mlqmltypes.hpp"
> > >
> > > diff --git a/modules/gui/qt/medialibrary/mlalbumtrack.hpp b/modules/gui/qt/medialibrary/mlalbumtrack.hpp
> > > index c9abbed671..beebcfa0c6 100644
> > > --- a/modules/gui/qt/medialibrary/mlalbumtrack.hpp
> > > +++ b/modules/gui/qt/medialibrary/mlalbumtrack.hpp
> > > @@ -21,7 +21,7 @@
> > > #ifdef HAVE_CONFIG_H
> > > #include "config.h"
> > > #endif
> > > -#include "vlc_common.h"
> > > +#include <vlc_common.h>
> > >
> > > #include <QObject>
> > > #include <QString>
> > > diff --git a/modules/gui/qt/medialibrary/mlartist.hpp b/modules/gui/qt/medialibrary/mlartist.hpp
> > > index f9a9e5a31b..0d524e3197 100644
> > > --- a/modules/gui/qt/medialibrary/mlartist.hpp
> > > +++ b/modules/gui/qt/medialibrary/mlartist.hpp
> > > @@ -22,7 +22,7 @@
> > > #ifdef HAVE_CONFIG_H
> > > #include "config.h"
> > > #endif
> > > -#include "vlc_common.h"
> > > +#include <vlc_common.h>
> > >
> > > #include <QObject>
> > > #include <QString>
> > > diff --git a/modules/gui/qt/medialibrary/mlbasemodel.hpp b/modules/gui/qt/medialibrary/mlbasemodel.hpp
> > > index 4d7c87b574..661630931f 100644
> > > --- a/modules/gui/qt/medialibrary/mlbasemodel.hpp
> > > +++ b/modules/gui/qt/medialibrary/mlbasemodel.hpp
> > > @@ -22,12 +22,12 @@
> > > #ifdef HAVE_CONFIG_H
> > > #include "config.h"
> > > #endif
> > > -#include "vlc_common.h"
> > > +#include <vlc_common.h>
> > >
> > > #include <memory>
> > > #include <QObject>
> > > #include <QAbstractListModel>
> > > -#include "vlc_media_library.h"
> > > +#include <vlc_media_library.h>
> > > #include "mlqmltypes.hpp"
> > > #include "medialib.hpp"
> > > #include <memory>
> > > diff --git a/modules/gui/qt/medialibrary/mlgenre.hpp b/modules/gui/qt/medialibrary/mlgenre.hpp
> > > index 66b5773be4..5309dd0dfe 100644
> > > --- a/modules/gui/qt/medialibrary/mlgenre.hpp
> > > +++ b/modules/gui/qt/medialibrary/mlgenre.hpp
> > > @@ -22,7 +22,7 @@
> > > #ifdef HAVE_CONFIG_H
> > > #include "config.h"
> > > #endif
> > > -#include "vlc_common.h"
> > > +#include <vlc_common.h>
> > >
> > > #include <memory>
> > > #include <QObject>
> > > diff --git a/modules/gui/qt/medialibrary/mlhelper.hpp b/modules/gui/qt/medialibrary/mlhelper.hpp
> > > index df84d2b2a1..e7375cf292 100644
> > > --- a/modules/gui/qt/medialibrary/mlhelper.hpp
> > > +++ b/modules/gui/qt/medialibrary/mlhelper.hpp
> > > @@ -25,7 +25,7 @@
> > > #include "config.h"
> > > #endif
> > >
> > > -#include "vlc_media_library.h"
> > > +#include <vlc_media_library.h>
> > > #include <QString>
> > >
> > > template<typename T>
> > > diff --git a/modules/gui/qt/playlist/playlist_controller.cpp b/modules/gui/qt/playlist/playlist_controller.cpp
> > > index 4215e774e6..36aee44c9b 100644
> > > --- a/modules/gui/qt/playlist/playlist_controller.cpp
> > > +++ b/modules/gui/qt/playlist/playlist_controller.cpp
> > > @@ -22,8 +22,8 @@
> > >
> > > #include "playlist_controller.hpp"
> > > #include "playlist_controller_p.hpp"
> > > -#include "vlc_player.h"
> > > -#include "vlc_url.h"
> > > +#include <vlc_player.h>
> > > +#include <vlc_url.h>
> > > #include <algorithm>
> > > #include <QVariant>
> > > #include <QDesktopServices>
> > > diff --git a/modules/gui/qt/playlist/playlist_item.hpp b/modules/gui/qt/playlist/playlist_item.hpp
> > > index c9be09555c..faddd4f803 100644
> > > --- a/modules/gui/qt/playlist/playlist_item.hpp
> > > +++ b/modules/gui/qt/playlist/playlist_item.hpp
> > > @@ -22,7 +22,7 @@
> > > # include "config.h"
> > > #endif
> > >
> > > -#include "vlc_player.h"
> > > +#include <vlc_player.h>
> > > #include <vlc_cxx_helpers.hpp>
> > > #include <vlc_input_item.h>
> > > #include <vlc_playlist.h>
> > > diff --git a/modules/gui/skins2/events/evt_input.cpp b/modules/gui/skins2/events/evt_input.cpp
> > > index eb6e5d4499..3190874ee9 100644
> > > --- a/modules/gui/skins2/events/evt_input.cpp
> > > +++ b/modules/gui/skins2/events/evt_input.cpp
> > > @@ -22,7 +22,7 @@
> > > *****************************************************************************/
> > >
> > > #include "evt_input.hpp"
> > > -#include "vlc_actions.h"
> > > +#include <vlc_actions.h>
> > >
> > > const int
> > > EvtInput::kModNone=0,
> > > diff --git a/modules/services_discovery/upnp-wrapper.hpp b/modules/services_discovery/upnp-wrapper.hpp
> > > index f692552eae..ee7c6e20bb 100644
> > > --- a/modules/services_discovery/upnp-wrapper.hpp
> > > +++ b/modules/services_discovery/upnp-wrapper.hpp
> > > @@ -340,7 +340,7 @@ done:
> > >
> > > #if defined(TARGET_OS_OSX) && TARGET_OS_OSX
> > > #include <SystemConfiguration/SystemConfiguration.h>
> > > -#include "vlc_charset.h"
> > > +#include <vlc_charset.h>
> > >
> > > inline char *getPreferedAdapter()
> > > {
> > > diff --git a/src/config/chain.c b/src/config/chain.c
> > > index 608de1b3af..4458d4cbce 100644
> > > --- a/src/config/chain.c
> > > +++ b/src/config/chain.c
> > > @@ -35,7 +35,7 @@
> > > #include <vlc_charset.h>
> > > #include <vlc_plugin.h>
> > >
> > > -#include "vlc_interface.h"
> > > +#include <vlc_interface.h>
> > > #include "configuration.h"
> > >
> > > /*****************************************************************************
> > > diff --git a/src/config/core.c b/src/config/core.c
> > > index b24ef59ab4..13495e751b 100644
> > > --- a/src/config/core.c
> > > +++ b/src/config/core.c
> > > @@ -29,7 +29,7 @@
> > > #include <vlc_modules.h>
> > > #include <vlc_plugin.h>
> > >
> > > -#include "vlc_configuration.h"
> > > +#include <vlc_configuration.h>
> > >
> > > #include <errno.h>
> > > #include <assert.h>
> > > diff --git a/src/input/info.h b/src/input/info.h
> > > index c76903a9db..7ce4e3c530 100644
> > > --- a/src/input/info.h
> > > +++ b/src/input/info.h
> > > @@ -23,7 +23,7 @@
> > > #ifndef LIBVLC_INPUT_INFO_H
> > > #define LIBVLC_INPUT_INFO_H 1
> > >
> > > -#include "vlc_input_item.h"
> > > +#include <vlc_input_item.h>
> > >
> > > static inline info_t *info_New(const char *name)
> > > {
> > > diff --git a/src/libvlc-module.c b/src/libvlc-module.c
> > > index 1cb548ff5f..e1ac07df41 100644
> > > --- a/src/libvlc-module.c
> > > +++ b/src/libvlc-module.c
> > > @@ -39,8 +39,8 @@
> > > #include "modules/modules.h"
> > >
> > > //#define Nothing here, this is just to prevent update-po from being stupid
> > > -#include "vlc_actions.h"
> > > -#include "vlc_meta.h"
> > > +#include <vlc_actions.h>
> > > +#include <vlc_meta.h>
> > > #include <vlc_aout.h>
> > > #include <vlc_vout.h>
> > > #include <vlc_player.h>
> > > diff --git a/src/misc/objects.c b/src/misc/objects.c
> > > index 7b15ae91d0..d7285358a5 100644
> > > --- a/src/misc/objects.c
> > > +++ b/src/misc/objects.c
> > > @@ -43,8 +43,8 @@
> > > #include <vlc_aout.h>
> > > #include "audio_output/aout_internal.h"
> > >
> > > -#include "vlc_interface.h"
> > > -#include "vlc_codec.h"
> > > +#include <vlc_interface.h>
> > > +#include <vlc_codec.h>
> > >
> > > #include "variables.h"
> > >
> > > diff --git a/src/misc/update_crypto.c b/src/misc/update_crypto.c
> > > index d3d555fc60..f91bfe6b43 100644
> > > --- a/src/misc/update_crypto.c
> > > +++ b/src/misc/update_crypto.c
> > > @@ -37,7 +37,7 @@
> > > #include <assert.h>
> > > #include <limits.h>
> > >
> > > -#include "vlc_common.h"
> > > +#include <vlc_common.h>
> > > #include <vlc_stream.h>
> > > #include <vlc_strings.h>
> > > #include <vlc_fs.h>
> > > diff --git a/src/modules/modules.c b/src/modules/modules.c
> > > index 2b9ed30fe0..dc7b2fddf4 100644
> > > --- a/src/modules/modules.c
> > > +++ b/src/modules/modules.c
> > > @@ -37,9 +37,10 @@
> > >
> > > #include <vlc_common.h>
> > > #include <vlc_modules.h>
> > > +#include <vlc_arrays.h>
> > > +
> > > #include "libvlc.h"
> > > #include "config/configuration.h"
> > > -#include "vlc_arrays.h"
> > > #include "modules/modules.h"
> > >
> > > bool module_provides (const module_t *m, const char *cap)
> > > diff --git a/src/test/dictionary.c b/src/test/dictionary.c
> > > index 3c00a50dff..f9ca63c50d 100644
> > > --- a/src/test/dictionary.c
> > > +++ b/src/test/dictionary.c
> > > @@ -27,7 +27,7 @@
> > > #include <assert.h>
> > >
> > > #include <vlc_common.h>
> > > -#include "vlc_arrays.h"
> > > +#include <vlc_arrays.h>
> > >
> > > #include <stdio.h>
> > > #include <stdlib.h>
> > > diff --git a/src/test/i18n_atof.c b/src/test/i18n_atof.c
> > > index 49f0beba30..07cb5058a0 100644
> > > --- a/src/test/i18n_atof.c
> > > +++ b/src/test/i18n_atof.c
> > > @@ -23,7 +23,7 @@
> > > #endif
> > >
> > > #include <vlc_common.h>
> > > -#include "vlc_charset.h"
> > > +#include <vlc_charset.h>
> > >
> > > #undef NDEBUG
> > > #include <assert.h>
> > > diff --git a/src/text/iso_lang.c b/src/text/iso_lang.c
> > > index 247287eb6f..f90ec7feda 100644
> > > --- a/src/text/iso_lang.c
> > > +++ b/src/text/iso_lang.c
> > > @@ -33,7 +33,7 @@
> > >
> > > #include <vlc_common.h>
> > >
> > > -#include "vlc_iso_lang.h"
> > > +#include <vlc_iso_lang.h>
> > >
> > > /*****************************************************************************
> > > * Local tables
> > >
> > > _______________________________________________
> > > 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
More information about the vlc-devel
mailing list