[igt-dev] [PATCH i-g-t] man/build: Fix dependency handling

[igt-dev] [PATCH i-g-t] man/build: Fix dependency handling

[Arkadiusz Hiler]

build_man is a 'feature' option, which means that we get an opaque
object, so comparing it to 'yes' is always false.

It should have been 'if build_man.enabled()'.

But going even further we can prune the whole check, as rst2man is
defined as follows:

rst2man = find_program('rst2man-3', 'rst2man', required : build_man)

From the patch overhauling those options:

    get_option() on a feature returns opaque object that can be passed as
    a 'required' argument of a dependency. Auto is equivalent to 'required
    : false', enabled is equivalent to 'required : true' and disabled
    introduces new behavior forcing the dependency to be considered not
    found.

This would cause the build to fail if build_man is enabled (aka
'required') and rst2man is not found. So...

if rst2man.found()
  # rst2man found
  # buildu buildu
else
  # rst2man not found, but considered optional because
  # we haven't erred out on find_program
  if build_man.enabled() # cannot be, it's optional!
    error('...')
  endif
endif

We can get rid of the whole else block here.

Cc: Antonio Argenziano <antonio.argenziano@intel.com>
Cc: Petri Latvala <petri.latvala@intel.com>
Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
---
 man/meson.build | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/man/meson.build b/man/meson.build
index 0f0a6dd4..2c1396af 100644
--- a/man/meson.build
+++ b/man/meson.build
@@ -38,10 +38,6 @@ if rst2man.found()
 				install : true,
 				install_dir : join_paths(mandir, 'man1'))
 	endforeach
-	build_info += 'Build man pages: true'
-else
-	if build_man == 'yes'
-		error('Cannot build man pages due to missing dependencies')
-	endif
-	build_info += 'Build man pages: false'
 endif
+
+build_info += 'Build man pages: @0@'.format(rst2man.found())
-- 
2.21.0

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] ✓ Fi.CI.BAT: success for man/build: Fix dependency handling

[Patchwork]

== Series Details ==

Series: man/build: Fix dependency handling
URL   : https://patchwork.freedesktop.org/series/61082/
State : success

== Summary ==

CI Bug Log - changes from IGT_5012 -> IGTPW_3050
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/61082/revisions/1/mbox/

Known issues
------------

  Here are the changes found in IGTPW_3050 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@i915_selftest@live_hangcheck:
    - fi-skl-iommu:       [PASS][1] -> [INCOMPLETE][2] ([fdo#108602] / [fdo#108744])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5012/fi-skl-iommu/igt@i915_selftest@live_hangcheck.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3050/fi-skl-iommu/igt@i915_selftest@live_hangcheck.html

  
#### Possible fixes ####

  * igt@i915_module_load@reload-with-fault-injection:
    - {fi-icl-u3}:        [DMESG-WARN][3] ([fdo#110718]) -> [PASS][4]
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5012/fi-icl-u3/igt@i915_module_load@reload-with-fault-injection.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3050/fi-icl-u3/igt@i915_module_load@reload-with-fault-injection.html

  * igt@i915_selftest@live_contexts:
    - fi-skl-gvtdvm:      [DMESG-FAIL][5] ([fdo#110235]) -> [PASS][6]
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5012/fi-skl-gvtdvm/igt@i915_selftest@live_contexts.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3050/fi-skl-gvtdvm/igt@i915_selftest@live_contexts.html

  * igt@kms_cursor_legacy@basic-flip-after-cursor-legacy:
    - {fi-icl-u3}:        [DMESG-WARN][7] ([fdo#107724]) -> [PASS][8]
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5012/fi-icl-u3/igt@kms_cursor_legacy@basic-flip-after-cursor-legacy.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3050/fi-icl-u3/igt@kms_cursor_legacy@basic-flip-after-cursor-legacy.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#107724]: https://bugs.freedesktop.org/show_bug.cgi?id=107724
  [fdo#108602]: https://bugs.freedesktop.org/show_bug.cgi?id=108602
  [fdo#108744]: https://bugs.freedesktop.org/show_bug.cgi?id=108744
  [fdo#110235]: https://bugs.freedesktop.org/show_bug.cgi?id=110235
  [fdo#110718]: https://bugs.freedesktop.org/show_bug.cgi?id=110718


Participating hosts (44 -> 41)
------------------------------

  Additional (1): fi-hsw-4770r 
  Missing    (4): fi-kbl-soraka fi-ilk-m540 fi-bsw-cyan fi-bdw-samus 


Build changes
-------------

  * IGT: IGT_5012 -> IGTPW_3050

  CI_DRM_6138: a17cee540f7ffd493c7944dea5ea6e2d32fdfdb4 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_3050: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3050/
  IGT_5012: 996b4346b6a7f2bd0919817648a4f7a382e59757 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3050/
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t] man/build: Fix dependency handling

[Ser, Simon]

On Fri, 2019-05-24 at 08:28 +0300, Arkadiusz Hiler wrote:
> build_man is a 'feature' option, which means that we get an opaque
> object, so comparing it to 'yes' is always false.
> 
> It should have been 'if build_man.enabled()'.
> 
> But going even further we can prune the whole check, as rst2man is
> defined as follows:
> 
> rst2man = find_program('rst2man-3', 'rst2man', required : build_man)
> 
> From the patch overhauling those options:
> 
>     get_option() on a feature returns opaque object that can be passed as
>     a 'required' argument of a dependency. Auto is equivalent to 'required
>     : false', enabled is equivalent to 'required : true' and disabled
>     introduces new behavior forcing the dependency to be considered not
>     found.
> 
> This would cause the build to fail if build_man is enabled (aka
> 'required') and rst2man is not found. So...
> 
> if rst2man.found()
>   # rst2man found
>   # buildu buildu

:)

> else
>   # rst2man not found, but considered optional because
>   # we haven't erred out on find_program
>   if build_man.enabled() # cannot be, it's optional!
>     error('...')
>   endif
> endif
> 
> We can get rid of the whole else block here.
> 
> Cc: Antonio Argenziano <antonio.argenziano@intel.com>
> Cc: Petri Latvala <petri.latvala@intel.com>
> Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>

LGTM

Reviewed-by: Simon Ser <simon.ser@intel.com>

> ---
>  man/meson.build | 8 ++------
>  1 file changed, 2 insertions(+), 6 deletions(-)
> 
> diff --git a/man/meson.build b/man/meson.build
> index 0f0a6dd4..2c1396af 100644
> --- a/man/meson.build
> +++ b/man/meson.build
> @@ -38,10 +38,6 @@ if rst2man.found()
>  				install : true,
>  				install_dir : join_paths(mandir, 'man1'))
>  	endforeach
> -	build_info += 'Build man pages: true'
> -else
> -	if build_man == 'yes'
> -		error('Cannot build man pages due to missing dependencies')
> -	endif
> -	build_info += 'Build man pages: false'
>  endif
> +
> +build_info += 'Build man pages: @0@'.format(rst2man.found())
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t] man/build: Fix dependency handling

[Antonio Argenziano]

On 23/05/19 22:28, Arkadiusz Hiler wrote:
> build_man is a 'feature' option, which means that we get an opaque
> object, so comparing it to 'yes' is always false.
> 
> It should have been 'if build_man.enabled()'.
> 
> But going even further we can prune the whole check, as rst2man is
> defined as follows:
> 
> rst2man = find_program('rst2man-3', 'rst2man', required : build_man)
> 
>  From the patch overhauling those options:
> 
>      get_option() on a feature returns opaque object that can be passed as
>      a 'required' argument of a dependency. Auto is equivalent to 'required
>      : false', enabled is equivalent to 'required : true' and disabled
>      introduces new behavior forcing the dependency to be considered not
>      found.
> 
> This would cause the build to fail if build_man is enabled (aka
> 'required') and rst2man is not found. So...
> 
> if rst2man.found()
>    # rst2man found
>    # buildu buildu
> else
>    # rst2man not found, but considered optional because
>    # we haven't erred out on find_program
>    if build_man.enabled() # cannot be, it's optional!
>      error('...')
>    endif
> endif
> 
> We can get rid of the whole else block here.
> 
> Cc: Antonio Argenziano <antonio.argenziano@intel.com>
> Cc: Petri Latvala <petri.latvala@intel.com>
> Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>

Acked-by: Antonio Argenziano <antonio.argenziano@intel.com>

> ---
>   man/meson.build | 8 ++------
>   1 file changed, 2 insertions(+), 6 deletions(-)
> 
> diff --git a/man/meson.build b/man/meson.build
> index 0f0a6dd4..2c1396af 100644
> --- a/man/meson.build
> +++ b/man/meson.build
> @@ -38,10 +38,6 @@ if rst2man.found()
>   				install : true,
>   				install_dir : join_paths(mandir, 'man1'))
>   	endforeach
> -	build_info += 'Build man pages: true'
> -else
> -	if build_man == 'yes'
> -		error('Cannot build man pages due to missing dependencies')
> -	endif
> -	build_info += 'Build man pages: false'
>   endif
> +
> +build_info += 'Build man pages: @0@'.format(rst2man.found())
> 
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev