| Message ID | 20200302163550.11640-1-kieran.bingham@ideasonboard.com |
|---|---|
| State | Rejected |
| Delegated to: | Kieran Bingham |
| Headers | show |
| Series |
|
| Related | show |
Hi Kieran, Thank you for the patch. On Mon, Mar 02, 2020 at 04:35:50PM +0000, Kieran Bingham wrote: > As part of updating controls to support properties, the commit > f870591a9bf5 ("libcamera: properties: Add location property") > inadvertently moved the generated headers out of the libcamera_api > dependency generation. > > This allowed a race condition to occur in builds where objects are > attempted to be built before the API definitions had been generated. > > Move the headers out of the libcamera_sources variable and into the > libcamera_api variable to match the other headers in this location. > > Fixes: f870591a9bf5 ("libcamera: properties: Add location property") > > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > --- > include/libcamera/meson.build | 2 ++ > src/libcamera/meson.build | 1 - > 2 files changed, 2 insertions(+), 1 deletion(-) > > diff --git a/include/libcamera/meson.build b/include/libcamera/meson.build > index f58c02d2cf35..c02e1d250aff 100644 > --- a/include/libcamera/meson.build > +++ b/include/libcamera/meson.build > @@ -43,6 +43,8 @@ foreach header : control_source_files > install_dir : join_paths('include', include_dir)) > endforeach > > +libcamera_api += control_headers > + > gen_header = files('gen-header.sh') > > libcamera_h = custom_target('gen-header', > diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build > index 88658ac563f7..b3a6a1aad659 100644 > --- a/src/libcamera/meson.build > +++ b/src/libcamera/meson.build > @@ -80,7 +80,6 @@ foreach source : control_source_files > command : [gen_controls, '-o', '@OUTPUT@', '@INPUT@']) > endforeach > > -libcamera_sources += control_headers > libcamera_sources += control_sources control_ids_h used to be in both libcamera_api and libcamera_sources. Is the latter not needed ? > > gen_version = join_paths(meson.source_root(), 'utils', 'gen-version.sh')
On 02/03/2020 16:38, Laurent Pinchart wrote: > Hi Kieran, > > Thank you for the patch. > > On Mon, Mar 02, 2020 at 04:35:50PM +0000, Kieran Bingham wrote: >> As part of updating controls to support properties, the commit >> f870591a9bf5 ("libcamera: properties: Add location property") >> inadvertently moved the generated headers out of the libcamera_api >> dependency generation. >> >> This allowed a race condition to occur in builds where objects are >> attempted to be built before the API definitions had been generated. >> >> Move the headers out of the libcamera_sources variable and into the >> libcamera_api variable to match the other headers in this location. >> >> Fixes: f870591a9bf5 ("libcamera: properties: Add location property") >> >> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> >> --- >> include/libcamera/meson.build | 2 ++ >> src/libcamera/meson.build | 1 - >> 2 files changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/include/libcamera/meson.build b/include/libcamera/meson.build >> index f58c02d2cf35..c02e1d250aff 100644 >> --- a/include/libcamera/meson.build >> +++ b/include/libcamera/meson.build >> @@ -43,6 +43,8 @@ foreach header : control_source_files >> install_dir : join_paths('include', include_dir)) >> endforeach >> >> +libcamera_api += control_headers >> + >> gen_header = files('gen-header.sh') >> >> libcamera_h = custom_target('gen-header', >> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build >> index 88658ac563f7..b3a6a1aad659 100644 >> --- a/src/libcamera/meson.build >> +++ b/src/libcamera/meson.build >> @@ -80,7 +80,6 @@ foreach source : control_source_files >> command : [gen_controls, '-o', '@OUTPUT@', '@INPUT@']) >> endforeach >> >> -libcamera_sources += control_headers >> libcamera_sources += control_sources > > control_ids_h used to be in both libcamera_api and libcamera_sources. Is > the latter not needed ? libcamera_api contains all of the public api headers generated in include/libcamera/meson.build. No other header from that list is explicitly added to libcamera_sources, so, no I do not believe these headers should be added to libcamera_sources. I've tested that the headers still get installed (because of being in the libcamera_api variable) ... however I have since hit this race again with the RKISP1 pipeline handler, thus where I believed this was an issue of the headers not being in the libcamera_api was wrong. We likely need to create a separate dependency declaration to make sure those headers are created early on. I'll look at this again later.. but for now this patch isn't a solution. > >> >> gen_version = join_paths(meson.source_root(), 'utils', 'gen-version.sh') >
diff --git a/include/libcamera/meson.build b/include/libcamera/meson.build index f58c02d2cf35..c02e1d250aff 100644 --- a/include/libcamera/meson.build +++ b/include/libcamera/meson.build @@ -43,6 +43,8 @@ foreach header : control_source_files install_dir : join_paths('include', include_dir)) endforeach +libcamera_api += control_headers + gen_header = files('gen-header.sh') libcamera_h = custom_target('gen-header', diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build index 88658ac563f7..b3a6a1aad659 100644 --- a/src/libcamera/meson.build +++ b/src/libcamera/meson.build @@ -80,7 +80,6 @@ foreach source : control_source_files command : [gen_controls, '-o', '@OUTPUT@', '@INPUT@']) endforeach -libcamera_sources += control_headers libcamera_sources += control_sources gen_version = join_paths(meson.source_root(), 'utils', 'gen-version.sh')
As part of updating controls to support properties, the commit f870591a9bf5 ("libcamera: properties: Add location property") inadvertently moved the generated headers out of the libcamera_api dependency generation. This allowed a race condition to occur in builds where objects are attempted to be built before the API definitions had been generated. Move the headers out of the libcamera_sources variable and into the libcamera_api variable to match the other headers in this location. Fixes: f870591a9bf5 ("libcamera: properties: Add location property") Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> --- include/libcamera/meson.build | 2 ++ src/libcamera/meson.build | 1 - 2 files changed, 2 insertions(+), 1 deletion(-)