| Message ID | 20220820080744.30325-1-tomi.valkeinen@ideasonboard.com |
|---|---|
| State | New |
| Headers | show |
| Series |
|
| Related | show |
Hi Tomi, With the recent discussions on "versioning", I would actually argue that this should be removed entirely, and we should only rely on the semver versioning. Best, Christian Am 20.08.22 um 10:07 schrieb Tomi Valkeinen via libcamera-devel: > libcamera builds the git hash version into the libcamera library. This > makes ninja create the generated version.cpp on every build, and if the > git hash has changed, compilation of version.cpp and linking of > libcamera library. > > This patch adds a meson option to disable this behavior and instead use > the meson project-version as the version built into the library, which > is only generated at meson configuration time. > > With this change, ninja will nicely say "no work to do" if there's > nothing to compile, instead of starting the build and generating > version.cpp. > > But, more importantly, it often reduces build time. For example, this > takes a particular git range compilation (~20 py bindings patches) from > 43 seconds to 10 seconds. Obviously the improvement depends very much on > the patches in question. If every commit changes libcamera itself, there > is no difference. > > Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> > --- > meson_options.txt | 5 +++++ > src/libcamera/meson.build | 29 +++++++++++++++++++++-------- > 2 files changed, 26 insertions(+), 8 deletions(-) > > diff --git a/meson_options.txt b/meson_options.txt > index 7a9aecfc..6a0bd0be 100644 > --- a/meson_options.txt > +++ b/meson_options.txt > @@ -63,3 +63,8 @@ option('pycamera', > type : 'feature', > value : 'disabled', > description : 'Enable libcamera Python bindings (experimental)') > + > +option('disable-vcs-versioning', > + type : 'boolean', > + value : false, > + description : 'Disable automatic libcamera library versioning with the git hash') > diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build > index ce1f0f2f..6fd6f084 100644 > --- a/src/libcamera/meson.build > +++ b/src/libcamera/meson.build > @@ -121,14 +121,27 @@ endforeach > > libcamera_sources += control_sources > > -gen_version = meson.project_source_root() / 'utils' / 'gen-version.sh' > - > -# Use vcs_tag() and not configure_file() or run_command(), to ensure that the > -# version gets updated with every ninja build and not just at meson setup time. > -version_cpp = vcs_tag(command : [gen_version, meson.project_build_root(), meson.project_source_root()], > - input : 'version.cpp.in', > - output : 'version.cpp', > - fallback : meson.project_version()) > +if get_option('disable-vcs-versioning') > + cdata = configuration_data() > + cdata.set('VCS_TAG', meson.project_version()) > + > + # For some reason using 'version.cpp' as output file works fine for generation > + # but causes meson to delete the file before build. Any other file name > + # seems to work. > + version_cpp = configure_file(input : 'version.cpp.in', > + output : 'version_static.cpp', > + configuration : cdata) > + > +else > + gen_version = meson.project_source_root() / 'utils' / 'gen-version.sh' > + > + # Use vcs_tag() and not configure_file() or run_command(), to ensure that the > + # version gets updated with every ninja build and not just at meson setup time. > + version_cpp = vcs_tag(command : [gen_version, meson.project_build_root(), meson.project_source_root()], > + input : 'version.cpp.in', > + output : 'version.cpp', > + fallback : meson.project_version()) > +endif > > libcamera_sources += version_cpp >
On Sat, Aug 20, 2022 at 03:29:48PM +0200, Christian Rauch via libcamera-devel wrote: > Hi Tomi, > > With the recent discussions on "versioning", I would actually argue that > this should be removed entirely, and we should only rely on the semver > versioning. I'd rather not. It has been proven very useful multiple times to have the exact git commit ID in bug reports. If we were to drop it, it would increase our support burden. I'm fine omitting the commit ID when the libcamera version exactly matches a released version, but not otherwise. > Am 20.08.22 um 10:07 schrieb Tomi Valkeinen via libcamera-devel: > > libcamera builds the git hash version into the libcamera library. This > > makes ninja create the generated version.cpp on every build, and if the > > git hash has changed, compilation of version.cpp and linking of > > libcamera library. > > > > This patch adds a meson option to disable this behavior and instead use > > the meson project-version as the version built into the library, which > > is only generated at meson configuration time. > > > > With this change, ninja will nicely say "no work to do" if there's > > nothing to compile, instead of starting the build and generating > > version.cpp. I'd prefer a way to skip recompilation (and relinking) if the version hasn't changed since the last build. > > But, more importantly, it often reduces build time. For example, this > > takes a particular git range compilation (~20 py bindings patches) from > > 43 seconds to 10 seconds. Obviously the improvement depends very much on > > the patches in question. If every commit changes libcamera itself, there > > is no difference. > > > > Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> > > --- > > meson_options.txt | 5 +++++ > > src/libcamera/meson.build | 29 +++++++++++++++++++++-------- > > 2 files changed, 26 insertions(+), 8 deletions(-) > > > > diff --git a/meson_options.txt b/meson_options.txt > > index 7a9aecfc..6a0bd0be 100644 > > --- a/meson_options.txt > > +++ b/meson_options.txt > > @@ -63,3 +63,8 @@ option('pycamera', > > type : 'feature', > > value : 'disabled', > > description : 'Enable libcamera Python bindings (experimental)') > > + > > +option('disable-vcs-versioning', > > + type : 'boolean', > > + value : false, > > + description : 'Disable automatic libcamera library versioning with the git hash') > > diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build > > index ce1f0f2f..6fd6f084 100644 > > --- a/src/libcamera/meson.build > > +++ b/src/libcamera/meson.build > > @@ -121,14 +121,27 @@ endforeach > > > > libcamera_sources += control_sources > > > > -gen_version = meson.project_source_root() / 'utils' / 'gen-version.sh' > > - > > -# Use vcs_tag() and not configure_file() or run_command(), to ensure that the > > -# version gets updated with every ninja build and not just at meson setup time. > > -version_cpp = vcs_tag(command : [gen_version, meson.project_build_root(), meson.project_source_root()], > > - input : 'version.cpp.in', > > - output : 'version.cpp', > > - fallback : meson.project_version()) > > +if get_option('disable-vcs-versioning') > > + cdata = configuration_data() > > + cdata.set('VCS_TAG', meson.project_version()) > > + > > + # For some reason using 'version.cpp' as output file works fine for generation > > + # but causes meson to delete the file before build. Any other file name > > + # seems to work. > > + version_cpp = configure_file(input : 'version.cpp.in', > > + output : 'version_static.cpp', > > + configuration : cdata) > > + > > +else > > + gen_version = meson.project_source_root() / 'utils' / 'gen-version.sh' > > + > > + # Use vcs_tag() and not configure_file() or run_command(), to ensure that the > > + # version gets updated with every ninja build and not just at meson setup time. > > + version_cpp = vcs_tag(command : [gen_version, meson.project_build_root(), meson.project_source_root()], > > + input : 'version.cpp.in', > > + output : 'version.cpp', > > + fallback : meson.project_version()) > > +endif > > > > libcamera_sources += version_cpp > >
On Mon, Aug 22, 2022 at 05:17:05PM +0300, Laurent Pinchart via libcamera-devel wrote: > On Sat, Aug 20, 2022 at 03:29:48PM +0200, Christian Rauch via libcamera-devel wrote: > > Hi Tomi, > > > > With the recent discussions on "versioning", I would actually argue that > > this should be removed entirely, and we should only rely on the semver > > versioning. > > I'd rather not. It has been proven very useful multiple times to have > the exact git commit ID in bug reports. If we were to drop it, it would > increase our support burden. I'm fine omitting the commit ID when the > libcamera version exactly matches a released version, but not otherwise. > > > Am 20.08.22 um 10:07 schrieb Tomi Valkeinen via libcamera-devel: > > > libcamera builds the git hash version into the libcamera library. This > > > makes ninja create the generated version.cpp on every build, and if the > > > git hash has changed, compilation of version.cpp and linking of > > > libcamera library. > > > > > > This patch adds a meson option to disable this behavior and instead use > > > the meson project-version as the version built into the library, which > > > is only generated at meson configuration time. > > > > > > With this change, ninja will nicely say "no work to do" if there's > > > nothing to compile, instead of starting the build and generating > > > version.cpp. > > I'd prefer a way to skip recompilation (and relinking) if the version > hasn't changed since the last build. Which of course doesn't work for Tomi's use case. /me needs to wake up How can we get the best of both worlds ? > > > But, more importantly, it often reduces build time. For example, this > > > takes a particular git range compilation (~20 py bindings patches) from > > > 43 seconds to 10 seconds. Obviously the improvement depends very much on > > > the patches in question. If every commit changes libcamera itself, there > > > is no difference. > > > > > > Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> > > > --- > > > meson_options.txt | 5 +++++ > > > src/libcamera/meson.build | 29 +++++++++++++++++++++-------- > > > 2 files changed, 26 insertions(+), 8 deletions(-) > > > > > > diff --git a/meson_options.txt b/meson_options.txt > > > index 7a9aecfc..6a0bd0be 100644 > > > --- a/meson_options.txt > > > +++ b/meson_options.txt > > > @@ -63,3 +63,8 @@ option('pycamera', > > > type : 'feature', > > > value : 'disabled', > > > description : 'Enable libcamera Python bindings (experimental)') > > > + > > > +option('disable-vcs-versioning', > > > + type : 'boolean', > > > + value : false, > > > + description : 'Disable automatic libcamera library versioning with the git hash') > > > diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build > > > index ce1f0f2f..6fd6f084 100644 > > > --- a/src/libcamera/meson.build > > > +++ b/src/libcamera/meson.build > > > @@ -121,14 +121,27 @@ endforeach > > > > > > libcamera_sources += control_sources > > > > > > -gen_version = meson.project_source_root() / 'utils' / 'gen-version.sh' > > > - > > > -# Use vcs_tag() and not configure_file() or run_command(), to ensure that the > > > -# version gets updated with every ninja build and not just at meson setup time. > > > -version_cpp = vcs_tag(command : [gen_version, meson.project_build_root(), meson.project_source_root()], > > > - input : 'version.cpp.in', > > > - output : 'version.cpp', > > > - fallback : meson.project_version()) > > > +if get_option('disable-vcs-versioning') > > > + cdata = configuration_data() > > > + cdata.set('VCS_TAG', meson.project_version()) > > > + > > > + # For some reason using 'version.cpp' as output file works fine for generation > > > + # but causes meson to delete the file before build. Any other file name > > > + # seems to work. > > > + version_cpp = configure_file(input : 'version.cpp.in', > > > + output : 'version_static.cpp', > > > + configuration : cdata) Do I understand correctly that this will cause a build failure if you don't first run meson setup with the disable-vcs-versioning option disabled ? > > > + > > > +else > > > + gen_version = meson.project_source_root() / 'utils' / 'gen-version.sh' > > > + > > > + # Use vcs_tag() and not configure_file() or run_command(), to ensure that the > > > + # version gets updated with every ninja build and not just at meson setup time. > > > + version_cpp = vcs_tag(command : [gen_version, meson.project_build_root(), meson.project_source_root()], > > > + input : 'version.cpp.in', > > > + output : 'version.cpp', > > > + fallback : meson.project_version()) > > > +endif > > > > > > libcamera_sources += version_cpp > > >
On 22/08/2022 17:39, Laurent Pinchart via libcamera-devel wrote: > On Mon, Aug 22, 2022 at 05:17:05PM +0300, Laurent Pinchart via libcamera-devel wrote: >> On Sat, Aug 20, 2022 at 03:29:48PM +0200, Christian Rauch via libcamera-devel wrote: >>> Hi Tomi, >>> >>> With the recent discussions on "versioning", I would actually argue that >>> this should be removed entirely, and we should only rely on the semver >>> versioning. >> >> I'd rather not. It has been proven very useful multiple times to have >> the exact git commit ID in bug reports. If we were to drop it, it would >> increase our support burden. I'm fine omitting the commit ID when the >> libcamera version exactly matches a released version, but not otherwise. >> >>> Am 20.08.22 um 10:07 schrieb Tomi Valkeinen via libcamera-devel: >>>> libcamera builds the git hash version into the libcamera library. This >>>> makes ninja create the generated version.cpp on every build, and if the >>>> git hash has changed, compilation of version.cpp and linking of >>>> libcamera library. >>>> >>>> This patch adds a meson option to disable this behavior and instead use >>>> the meson project-version as the version built into the library, which >>>> is only generated at meson configuration time. >>>> >>>> With this change, ninja will nicely say "no work to do" if there's >>>> nothing to compile, instead of starting the build and generating >>>> version.cpp. >> >> I'd prefer a way to skip recompilation (and relinking) if the version >> hasn't changed since the last build. > > Which of course doesn't work for Tomi's use case. > > /me needs to wake up > > How can we get the best of both worlds ? I think there are two different build types: for publishing and for your own use (development usually). With dev builds I'm fine with omitting versioning info, skipping any signing processes, and so forth. I think that can be a meson option. Which one should be the default, that's a more difficult question (maybe publishing). >>>> But, more importantly, it often reduces build time. For example, this >>>> takes a particular git range compilation (~20 py bindings patches) from >>>> 43 seconds to 10 seconds. Obviously the improvement depends very much on >>>> the patches in question. If every commit changes libcamera itself, there >>>> is no difference. >>>> >>>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> >>>> --- >>>> meson_options.txt | 5 +++++ >>>> src/libcamera/meson.build | 29 +++++++++++++++++++++-------- >>>> 2 files changed, 26 insertions(+), 8 deletions(-) >>>> >>>> diff --git a/meson_options.txt b/meson_options.txt >>>> index 7a9aecfc..6a0bd0be 100644 >>>> --- a/meson_options.txt >>>> +++ b/meson_options.txt >>>> @@ -63,3 +63,8 @@ option('pycamera', >>>> type : 'feature', >>>> value : 'disabled', >>>> description : 'Enable libcamera Python bindings (experimental)') >>>> + >>>> +option('disable-vcs-versioning', >>>> + type : 'boolean', >>>> + value : false, >>>> + description : 'Disable automatic libcamera library versioning with the git hash') >>>> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build >>>> index ce1f0f2f..6fd6f084 100644 >>>> --- a/src/libcamera/meson.build >>>> +++ b/src/libcamera/meson.build >>>> @@ -121,14 +121,27 @@ endforeach >>>> >>>> libcamera_sources += control_sources >>>> >>>> -gen_version = meson.project_source_root() / 'utils' / 'gen-version.sh' >>>> - >>>> -# Use vcs_tag() and not configure_file() or run_command(), to ensure that the >>>> -# version gets updated with every ninja build and not just at meson setup time. >>>> -version_cpp = vcs_tag(command : [gen_version, meson.project_build_root(), meson.project_source_root()], >>>> - input : 'version.cpp.in', >>>> - output : 'version.cpp', >>>> - fallback : meson.project_version()) >>>> +if get_option('disable-vcs-versioning') >>>> + cdata = configuration_data() >>>> + cdata.set('VCS_TAG', meson.project_version()) >>>> + >>>> + # For some reason using 'version.cpp' as output file works fine for generation >>>> + # but causes meson to delete the file before build. Any other file name >>>> + # seems to work. >>>> + version_cpp = configure_file(input : 'version.cpp.in', >>>> + output : 'version_static.cpp', >>>> + configuration : cdata) > > Do I understand correctly that this will cause a build failure if you > don't first run meson setup with the disable-vcs-versioning option > disabled ? Isn't this piece of code always run when you change (enable) the option, even if you do it with "meson configure" after the initial setup? Tomi
On Mon, Aug 22, 2022 at 09:44:20PM +0300, Tomi Valkeinen wrote: > On 22/08/2022 17:39, Laurent Pinchart via libcamera-devel wrote: > > On Mon, Aug 22, 2022 at 05:17:05PM +0300, Laurent Pinchart via libcamera-devel wrote: > >> On Sat, Aug 20, 2022 at 03:29:48PM +0200, Christian Rauch via libcamera-devel wrote: > >>> Hi Tomi, > >>> > >>> With the recent discussions on "versioning", I would actually argue that > >>> this should be removed entirely, and we should only rely on the semver > >>> versioning. > >> > >> I'd rather not. It has been proven very useful multiple times to have > >> the exact git commit ID in bug reports. If we were to drop it, it would > >> increase our support burden. I'm fine omitting the commit ID when the > >> libcamera version exactly matches a released version, but not otherwise. > >> > >>> Am 20.08.22 um 10:07 schrieb Tomi Valkeinen via libcamera-devel: > >>>> libcamera builds the git hash version into the libcamera library. This > >>>> makes ninja create the generated version.cpp on every build, and if the > >>>> git hash has changed, compilation of version.cpp and linking of > >>>> libcamera library. > >>>> > >>>> This patch adds a meson option to disable this behavior and instead use > >>>> the meson project-version as the version built into the library, which > >>>> is only generated at meson configuration time. > >>>> > >>>> With this change, ninja will nicely say "no work to do" if there's > >>>> nothing to compile, instead of starting the build and generating > >>>> version.cpp. > >> > >> I'd prefer a way to skip recompilation (and relinking) if the version > >> hasn't changed since the last build. > > > > Which of course doesn't work for Tomi's use case. > > > > /me needs to wake up > > > > How can we get the best of both worlds ? > > I think there are two different build types: for publishing and for your > own use (development usually). With dev builds I'm fine with omitting > versioning info, skipping any signing processes, and so forth. I think > that can be a meson option. Which one should be the default, that's a > more difficult question (maybe publishing). I'd like to make this difficult to enable by mistake, extending the definition of "by mistake" to enabling the option on purpose without understanding it will hurt bug reports. We should at least print a very big warning in this case, and I wonder if it would be worth it trying to hide the option (for instance using an environment variable that gen-version.sh would interpret instead of a meson option). I'll be quite cross if we start seeing libcamera logs without a proper version number. > >>>> But, more importantly, it often reduces build time. For example, this > >>>> takes a particular git range compilation (~20 py bindings patches) from > >>>> 43 seconds to 10 seconds. Obviously the improvement depends very much on > >>>> the patches in question. If every commit changes libcamera itself, there > >>>> is no difference. > >>>> > >>>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> > >>>> --- > >>>> meson_options.txt | 5 +++++ > >>>> src/libcamera/meson.build | 29 +++++++++++++++++++++-------- > >>>> 2 files changed, 26 insertions(+), 8 deletions(-) > >>>> > >>>> diff --git a/meson_options.txt b/meson_options.txt > >>>> index 7a9aecfc..6a0bd0be 100644 > >>>> --- a/meson_options.txt > >>>> +++ b/meson_options.txt > >>>> @@ -63,3 +63,8 @@ option('pycamera', > >>>> type : 'feature', > >>>> value : 'disabled', > >>>> description : 'Enable libcamera Python bindings (experimental)') > >>>> + > >>>> +option('disable-vcs-versioning', > >>>> + type : 'boolean', > >>>> + value : false, > >>>> + description : 'Disable automatic libcamera library versioning with the git hash') > >>>> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build > >>>> index ce1f0f2f..6fd6f084 100644 > >>>> --- a/src/libcamera/meson.build > >>>> +++ b/src/libcamera/meson.build > >>>> @@ -121,14 +121,27 @@ endforeach > >>>> > >>>> libcamera_sources += control_sources > >>>> > >>>> -gen_version = meson.project_source_root() / 'utils' / 'gen-version.sh' > >>>> - > >>>> -# Use vcs_tag() and not configure_file() or run_command(), to ensure that the > >>>> -# version gets updated with every ninja build and not just at meson setup time. > >>>> -version_cpp = vcs_tag(command : [gen_version, meson.project_build_root(), meson.project_source_root()], > >>>> - input : 'version.cpp.in', > >>>> - output : 'version.cpp', > >>>> - fallback : meson.project_version()) > >>>> +if get_option('disable-vcs-versioning') > >>>> + cdata = configuration_data() > >>>> + cdata.set('VCS_TAG', meson.project_version()) > >>>> + > >>>> + # For some reason using 'version.cpp' as output file works fine for generation > >>>> + # but causes meson to delete the file before build. Any other file name > >>>> + # seems to work. > >>>> + version_cpp = configure_file(input : 'version.cpp.in', > >>>> + output : 'version_static.cpp', > >>>> + configuration : cdata) > > > > Do I understand correctly that this will cause a build failure if you > > don't first run meson setup with the disable-vcs-versioning option > > disabled ? > > Isn't this piece of code always run when you change (enable) the option, > even if you do it with "meson configure" after the initial setup? What happens if you run mkdir build cd build meson setup -Ddisable-vcs-versioning=true .. ninja
diff --git a/meson_options.txt b/meson_options.txt index 7a9aecfc..6a0bd0be 100644 --- a/meson_options.txt +++ b/meson_options.txt @@ -63,3 +63,8 @@ option('pycamera', type : 'feature', value : 'disabled', description : 'Enable libcamera Python bindings (experimental)') + +option('disable-vcs-versioning', + type : 'boolean', + value : false, + description : 'Disable automatic libcamera library versioning with the git hash') diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build index ce1f0f2f..6fd6f084 100644 --- a/src/libcamera/meson.build +++ b/src/libcamera/meson.build @@ -121,14 +121,27 @@ endforeach libcamera_sources += control_sources -gen_version = meson.project_source_root() / 'utils' / 'gen-version.sh' - -# Use vcs_tag() and not configure_file() or run_command(), to ensure that the -# version gets updated with every ninja build and not just at meson setup time. -version_cpp = vcs_tag(command : [gen_version, meson.project_build_root(), meson.project_source_root()], - input : 'version.cpp.in', - output : 'version.cpp', - fallback : meson.project_version()) +if get_option('disable-vcs-versioning') + cdata = configuration_data() + cdata.set('VCS_TAG', meson.project_version()) + + # For some reason using 'version.cpp' as output file works fine for generation + # but causes meson to delete the file before build. Any other file name + # seems to work. + version_cpp = configure_file(input : 'version.cpp.in', + output : 'version_static.cpp', + configuration : cdata) + +else + gen_version = meson.project_source_root() / 'utils' / 'gen-version.sh' + + # Use vcs_tag() and not configure_file() or run_command(), to ensure that the + # version gets updated with every ninja build and not just at meson setup time. + version_cpp = vcs_tag(command : [gen_version, meson.project_build_root(), meson.project_source_root()], + input : 'version.cpp.in', + output : 'version.cpp', + fallback : meson.project_version()) +endif libcamera_sources += version_cpp
libcamera builds the git hash version into the libcamera library. This makes ninja create the generated version.cpp on every build, and if the git hash has changed, compilation of version.cpp and linking of libcamera library. This patch adds a meson option to disable this behavior and instead use the meson project-version as the version built into the library, which is only generated at meson configuration time. With this change, ninja will nicely say "no work to do" if there's nothing to compile, instead of starting the build and generating version.cpp. But, more importantly, it often reduces build time. For example, this takes a particular git range compilation (~20 py bindings patches) from 43 seconds to 10 seconds. Obviously the improvement depends very much on the patches in question. If every commit changes libcamera itself, there is no difference. Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> --- meson_options.txt | 5 +++++ src/libcamera/meson.build | 29 +++++++++++++++++++++-------- 2 files changed, 26 insertions(+), 8 deletions(-)