| Message ID | mailman.114.1668011787.939.libcamera-devel@lists.libcamera.org |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
On Wed, Nov 09, 2022 at 04:36:14PM +0000, Barnabás Pőcze via libcamera-devel wrote: > Date: Wed, 09 Nov 2022 16:36:14 +0000 > From: Barnabás Pőcze <pobrn@protonmail.com> > To: libcamera-devel@lists.libcamera.org > Subject: [PATCH v1] libcamera: tracing: fix header generation when built as > subproject > > The previously used > > path = output.replace('include/', '', 1) > > logic is not sufficient to correctly determine the proper path > when libcamera is built as a subproject. Fix it by using Python's > pathlib to calculate the relative path of the output file with > respect to the "include" directory of libcamera. > > Signed-off-by: Barnabás Pőcze <pobrn@protonmail.com> Reviewed-by: Paul Elder <paul.elder@ideasonboard.com> > --- > include/libcamera/internal/meson.build | 2 +- > include/meson.build | 2 ++ > utils/tracepoints/gen-tp-header.py | 13 +++++++------ > 3 files changed, 10 insertions(+), 7 deletions(-) > > diff --git a/include/libcamera/internal/meson.build b/include/libcamera/internal/meson.build > index 7a780d48ee57..f8be86e040c5 100644 > --- a/include/libcamera/internal/meson.build > +++ b/include/libcamera/internal/meson.build > @@ -6,7 +6,7 @@ libcamera_tracepoint_header = custom_target( > 'tp_header', > input: ['tracepoints.h.in', tracepoint_files], > output: 'tracepoints.h', > - command: [gen_tracepoints_header, '@OUTPUT@', '@INPUT@'], > + command: [gen_tracepoints_header, include_build_dir, '@OUTPUT@', '@INPUT@'], > ) > > libcamera_internal_headers = files([ > diff --git a/include/meson.build b/include/meson.build > index 27ce2f41c534..19b93a7bd753 100644 > --- a/include/meson.build > +++ b/include/meson.build > @@ -1,4 +1,6 @@ > # SPDX-License-Identifier: CC0-1.0 > > +include_build_dir = meson.current_build_dir() > + > subdir('android') > subdir('libcamera') > diff --git a/utils/tracepoints/gen-tp-header.py b/utils/tracepoints/gen-tp-header.py > index bbd472d972d0..a454615e4625 100755 > --- a/utils/tracepoints/gen-tp-header.py > +++ b/utils/tracepoints/gen-tp-header.py > @@ -8,22 +8,23 @@ > > import datetime > import jinja2 > +import pathlib > import os > import sys > > def main(argv): > - if len(argv) < 3: > - print(f'Usage: {argv[0]} output template tp_files...') > + if len(argv) < 4: > + print(f'Usage: {argv[0]} include_build_dir output template tp_files...') > return 1 > > - output = argv[1] > - template = argv[2] > + output = argv[2] > + template = argv[3] > > year = datetime.datetime.now().year > - path = output.replace('include/', '', 1) > + path = pathlib.Path(output).absolute().relative_to(argv[1]) > > source = '' > - for fname in argv[3:]: > + for fname in argv[4:]: > source += open(fname, 'r', encoding='utf-8').read() + '\n\n' > > template = jinja2.Template(open(template, 'r', encoding='utf-8').read()) > -- > 2.38.1 > >
Quoting Paul Elder via libcamera-devel (2022-11-10 03:27:36) > On Wed, Nov 09, 2022 at 04:36:14PM +0000, Barnabás Pőcze via libcamera-devel wrote: > > Date: Wed, 09 Nov 2022 16:36:14 +0000 > > From: Barnabás Pőcze <pobrn@protonmail.com> > > To: libcamera-devel@lists.libcamera.org > > Subject: [PATCH v1] libcamera: tracing: fix header generation when built as > > subproject > > > > The previously used > > > > path = output.replace('include/', '', 1) > > > > logic is not sufficient to correctly determine the proper path > > when libcamera is built as a subproject. Fix it by using Python's > > pathlib to calculate the relative path of the output file with > > respect to the "include" directory of libcamera. > > > > Signed-off-by: Barnabás Pőcze <pobrn@protonmail.com> > > Reviewed-by: Paul Elder <paul.elder@ideasonboard.com> This looks ok to me, and doesn't break the builds for my integration tests - but I haven't easily tested as a submodule yet. However, given that I expect that has been tested by Barnabás, Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > > --- > > include/libcamera/internal/meson.build | 2 +- > > include/meson.build | 2 ++ > > utils/tracepoints/gen-tp-header.py | 13 +++++++------ > > 3 files changed, 10 insertions(+), 7 deletions(-) > > > > diff --git a/include/libcamera/internal/meson.build b/include/libcamera/internal/meson.build > > index 7a780d48ee57..f8be86e040c5 100644 > > --- a/include/libcamera/internal/meson.build > > +++ b/include/libcamera/internal/meson.build > > @@ -6,7 +6,7 @@ libcamera_tracepoint_header = custom_target( > > 'tp_header', > > input: ['tracepoints.h.in', tracepoint_files], > > output: 'tracepoints.h', > > - command: [gen_tracepoints_header, '@OUTPUT@', '@INPUT@'], > > + command: [gen_tracepoints_header, include_build_dir, '@OUTPUT@', '@INPUT@'], > > ) > > > > libcamera_internal_headers = files([ > > diff --git a/include/meson.build b/include/meson.build > > index 27ce2f41c534..19b93a7bd753 100644 > > --- a/include/meson.build > > +++ b/include/meson.build > > @@ -1,4 +1,6 @@ > > # SPDX-License-Identifier: CC0-1.0 > > > > +include_build_dir = meson.current_build_dir() > > + > > subdir('android') > > subdir('libcamera') > > diff --git a/utils/tracepoints/gen-tp-header.py b/utils/tracepoints/gen-tp-header.py > > index bbd472d972d0..a454615e4625 100755 > > --- a/utils/tracepoints/gen-tp-header.py > > +++ b/utils/tracepoints/gen-tp-header.py > > @@ -8,22 +8,23 @@ > > > > import datetime > > import jinja2 > > +import pathlib > > import os > > import sys > > > > def main(argv): > > - if len(argv) < 3: > > - print(f'Usage: {argv[0]} output template tp_files...') > > + if len(argv) < 4: > > + print(f'Usage: {argv[0]} include_build_dir output template tp_files...') > > return 1 > > > > - output = argv[1] > > - template = argv[2] > > + output = argv[2] > > + template = argv[3] > > > > year = datetime.datetime.now().year > > - path = output.replace('include/', '', 1) > > + path = pathlib.Path(output).absolute().relative_to(argv[1]) > > > > source = '' > > - for fname in argv[3:]: > > + for fname in argv[4:]: > > source += open(fname, 'r', encoding='utf-8').read() + '\n\n' > > > > template = jinja2.Template(open(template, 'r', encoding='utf-8').read()) > > -- > > 2.38.1 > > > >
Hi > Quoting Paul Elder via libcamera-devel (2022-11-10 03:27:36) > > On Wed, Nov 09, 2022 at 04:36:14PM +0000, Barnabás Pőcze via libcamera-devel wrote: > > > Date: Wed, 09 Nov 2022 16:36:14 +0000 > > > From: Barnabás Pőcze <pobrn@protonmail.com> > > > To: libcamera-devel@lists.libcamera.org > > > Subject: [PATCH v1] libcamera: tracing: fix header generation when built as > > > subproject > > > > > > The previously used > > > > > > path = output.replace('include/', '', 1) > > > > > > logic is not sufficient to correctly determine the proper path > > > when libcamera is built as a subproject. Fix it by using Python's > > > pathlib to calculate the relative path of the output file with > > > respect to the "include" directory of libcamera. > > > > > > Signed-off-by: Barnabás Pőcze <pobrn@protonmail.com> > > > > Reviewed-by: Paul Elder <paul.elder@ideasonboard.com> > > This looks ok to me, and doesn't break the builds for my integration > tests - but I haven't easily tested as a submodule yet. > > However, given that I expect that has been tested by Barnabás, > > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > I am using this with pipewire. For non-subproject builds it should generate the exact same path that was previously generated. I did not elaborate in the commit message but the issue is that when built as a subproject, the value of `path` would be 'subprojects/libcamera/include/libcamera/internal/tracepoints.h'.replace('include/', '', 1) which evaluates to 'subprojects/libcamera/libcamera/internal/tracepoints.h' so the tracepoints.h header file will try to include: #define TRACEPOINT_INCLUDE "subprojects/libcamera/libcamera/internal/tracepoints.h" which will fail. My understanding of the previous logic was that it tried to set `path` to the path of the output file relative to the libcamera include directory. The pathlib.Path(output).absolute().relative_to(argv[1]) expression is supposed to do the same. Regards, Barnabás Pőcze > > > > > --- > > > include/libcamera/internal/meson.build | 2 +- > > > include/meson.build | 2 ++ > > > utils/tracepoints/gen-tp-header.py | 13 +++++++------ > > > 3 files changed, 10 insertions(+), 7 deletions(-) > > > > > > diff --git a/include/libcamera/internal/meson.build b/include/libcamera/internal/meson.build > > > index 7a780d48ee57..f8be86e040c5 100644 > > > --- a/include/libcamera/internal/meson.build > > > +++ b/include/libcamera/internal/meson.build > > > @@ -6,7 +6,7 @@ libcamera_tracepoint_header = custom_target( > > > 'tp_header', > > > input: ['tracepoints.h.in', tracepoint_files], > > > output: 'tracepoints.h', > > > - command: [gen_tracepoints_header, '@OUTPUT@', '@INPUT@'], > > > + command: [gen_tracepoints_header, include_build_dir, '@OUTPUT@', '@INPUT@'], > > > ) > > > > > > libcamera_internal_headers = files([ > > > diff --git a/include/meson.build b/include/meson.build > > > index 27ce2f41c534..19b93a7bd753 100644 > > > --- a/include/meson.build > > > +++ b/include/meson.build > > > @@ -1,4 +1,6 @@ > > > # SPDX-License-Identifier: CC0-1.0 > > > > > > +include_build_dir = meson.current_build_dir() > > > + > > > subdir('android') > > > subdir('libcamera') > > > diff --git a/utils/tracepoints/gen-tp-header.py b/utils/tracepoints/gen-tp-header.py > > > index bbd472d972d0..a454615e4625 100755 > > > --- a/utils/tracepoints/gen-tp-header.py > > > +++ b/utils/tracepoints/gen-tp-header.py > > > @@ -8,22 +8,23 @@ > > > > > > import datetime > > > import jinja2 > > > +import pathlib > > > import os > > > import sys > > > > > > def main(argv): > > > - if len(argv) < 3: > > > - print(f'Usage: {argv[0]} output template tp_files...') > > > + if len(argv) < 4: > > > + print(f'Usage: {argv[0]} include_build_dir output template tp_files...') > > > return 1 > > > > > > - output = argv[1] > > > - template = argv[2] > > > + output = argv[2] > > > + template = argv[3] > > > > > > year = datetime.datetime.now().year > > > - path = output.replace('include/', '', 1) > > > + path = pathlib.Path(output).absolute().relative_to(argv[1]) > > > > > > source = '' > > > - for fname in argv[3:]: > > > + for fname in argv[4:]: > > > source += open(fname, 'r', encoding='utf-8').read() + '\n\n' > > > > > > template = jinja2.Template(open(template, 'r', encoding='utf-8').read()) > > > -- > > > 2.38.1 > > > > > > >
Hi Barnabás, On Mon, Nov 14, 2022 at 04:12:23PM +0000, Barnabás Pőcze via libcamera-devel wrote: > On Mon, 14 Nov 2022 16:12:23 +0000, Kieran Bingham wrote: > > Quoting Paul Elder via libcamera-devel (2022-11-10 03:27:36) > > > On Wed, Nov 09, 2022 at 04:36:14PM +0000, Barnabás Pőcze via libcamera-devel wrote: > > > > Date: Wed, 09 Nov 2022 16:36:14 +0000 > > > > From: Barnabás Pőcze <pobrn@protonmail.com> > > > > To: libcamera-devel@lists.libcamera.org > > > > Subject: [PATCH v1] libcamera: tracing: fix header generation when built as > > > > subproject > > > > > > > > The previously used > > > > > > > > path = output.replace('include/', '', 1) > > > > > > > > logic is not sufficient to correctly determine the proper path > > > > when libcamera is built as a subproject. Fix it by using Python's > > > > pathlib to calculate the relative path of the output file with > > > > respect to the "include" directory of libcamera. > > > > > > > > Signed-off-by: Barnabás Pőcze <pobrn@protonmail.com> > > > > > > Reviewed-by: Paul Elder <paul.elder@ideasonboard.com> > > > > This looks ok to me, and doesn't break the builds for my integration > > tests - but I haven't easily tested as a submodule yet. > > > > However, given that I expect that has been tested by Barnabás, > > > > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > I am using this with pipewire. For non-subproject builds it should generate > the exact same path that was previously generated. > > I did not elaborate in the commit message but the issue is that when built as a > subproject, the value of `path` would be > > 'subprojects/libcamera/include/libcamera/internal/tracepoints.h'.replace('include/', '', 1) > > which evaluates to > > 'subprojects/libcamera/libcamera/internal/tracepoints.h' > > so the tracepoints.h header file will try to include: > > #define TRACEPOINT_INCLUDE "subprojects/libcamera/libcamera/internal/tracepoints.h" > > which will fail. Thank you for the explanation, I was wondering what was wrong with the previous logic. Kieran, could you record this in the commit message when pushing ? > My understanding of the previous logic was that it tried to set `path` to > the path of the output file relative to the libcamera include directory. > > The > > pathlib.Path(output).absolute().relative_to(argv[1]) > > expression is supposed to do the same. > > > > > --- > > > > include/libcamera/internal/meson.build | 2 +- > > > > include/meson.build | 2 ++ > > > > utils/tracepoints/gen-tp-header.py | 13 +++++++------ > > > > 3 files changed, 10 insertions(+), 7 deletions(-) > > > > > > > > diff --git a/include/libcamera/internal/meson.build b/include/libcamera/internal/meson.build > > > > index 7a780d48ee57..f8be86e040c5 100644 > > > > --- a/include/libcamera/internal/meson.build > > > > +++ b/include/libcamera/internal/meson.build > > > > @@ -6,7 +6,7 @@ libcamera_tracepoint_header = custom_target( > > > > 'tp_header', > > > > input: ['tracepoints.h.in', tracepoint_files], > > > > output: 'tracepoints.h', > > > > - command: [gen_tracepoints_header, '@OUTPUT@', '@INPUT@'], > > > > + command: [gen_tracepoints_header, include_build_dir, '@OUTPUT@', '@INPUT@'], > > > > ) > > > > > > > > libcamera_internal_headers = files([ > > > > diff --git a/include/meson.build b/include/meson.build > > > > index 27ce2f41c534..19b93a7bd753 100644 > > > > --- a/include/meson.build > > > > +++ b/include/meson.build > > > > @@ -1,4 +1,6 @@ > > > > # SPDX-License-Identifier: CC0-1.0 > > > > > > > > +include_build_dir = meson.current_build_dir() > > > > + > > > > subdir('android') > > > > subdir('libcamera') > > > > diff --git a/utils/tracepoints/gen-tp-header.py b/utils/tracepoints/gen-tp-header.py > > > > index bbd472d972d0..a454615e4625 100755 > > > > --- a/utils/tracepoints/gen-tp-header.py > > > > +++ b/utils/tracepoints/gen-tp-header.py > > > > @@ -8,22 +8,23 @@ > > > > > > > > import datetime > > > > import jinja2 > > > > +import pathlib > > > > import os > > > > import sys > > > > > > > > def main(argv): > > > > - if len(argv) < 3: > > > > - print(f'Usage: {argv[0]} output template tp_files...') > > > > + if len(argv) < 4: > > > > + print(f'Usage: {argv[0]} include_build_dir output template tp_files...') > > > > return 1 > > > > > > > > - output = argv[1] > > > > - template = argv[2] > > > > + output = argv[2] > > > > + template = argv[3] > > > > > > > > year = datetime.datetime.now().year > > > > - path = output.replace('include/', '', 1) > > > > + path = pathlib.Path(output).absolute().relative_to(argv[1]) > > > > > > > > source = '' > > > > - for fname in argv[3:]: > > > > + for fname in argv[4:]: > > > > source += open(fname, 'r', encoding='utf-8').read() + '\n\n' > > > > > > > > template = jinja2.Template(open(template, 'r', encoding='utf-8').read())
diff --git a/include/libcamera/internal/meson.build b/include/libcamera/internal/meson.build index 7a780d48ee57..f8be86e040c5 100644 --- a/include/libcamera/internal/meson.build +++ b/include/libcamera/internal/meson.build @@ -6,7 +6,7 @@ libcamera_tracepoint_header = custom_target( 'tp_header', input: ['tracepoints.h.in', tracepoint_files], output: 'tracepoints.h', - command: [gen_tracepoints_header, '@OUTPUT@', '@INPUT@'], + command: [gen_tracepoints_header, include_build_dir, '@OUTPUT@', '@INPUT@'], ) libcamera_internal_headers = files([ diff --git a/include/meson.build b/include/meson.build index 27ce2f41c534..19b93a7bd753 100644 --- a/include/meson.build +++ b/include/meson.build @@ -1,4 +1,6 @@ # SPDX-License-Identifier: CC0-1.0 +include_build_dir = meson.current_build_dir() + subdir('android') subdir('libcamera') diff --git a/utils/tracepoints/gen-tp-header.py b/utils/tracepoints/gen-tp-header.py index bbd472d972d0..a454615e4625 100755 --- a/utils/tracepoints/gen-tp-header.py +++ b/utils/tracepoints/gen-tp-header.py @@ -8,22 +8,23 @@ import datetime import jinja2 +import pathlib import os import sys def main(argv): - if len(argv) < 3: - print(f'Usage: {argv[0]} output template tp_files...') + if len(argv) < 4: + print(f'Usage: {argv[0]} include_build_dir output template tp_files...') return 1 - output = argv[1] - template = argv[2] + output = argv[2] + template = argv[3] year = datetime.datetime.now().year - path = output.replace('include/', '', 1) + path = pathlib.Path(output).absolute().relative_to(argv[1]) source = '' - for fname in argv[3:]: + for fname in argv[4:]: source += open(fname, 'r', encoding='utf-8').read() + '\n\n' template = jinja2.Template(open(template, 'r', encoding='utf-8').read())
The previously used path = output.replace('include/', '', 1) logic is not sufficient to correctly determine the proper path when libcamera is built as a subproject. Fix it by using Python's pathlib to calculate the relative path of the output file with respect to the "include" directory of libcamera. Signed-off-by: Barnabás Pőcze <pobrn@protonmail.com> --- include/libcamera/internal/meson.build | 2 +- include/meson.build | 2 ++ utils/tracepoints/gen-tp-header.py | 13 +++++++------ 3 files changed, 10 insertions(+), 7 deletions(-) -- 2.38.1